Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR replaces bitmask-based Discord permission checks with a dashboard role system (viewer, moderator, admin, owner), adds utilities and middleware to compute/enforce roles, updates backend and web routes/hooks/components to use role-based authorization, enriches guild responses with access role, and adjusts tests to the new semantics. Changes
Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip CodeRabbit can use your project's `biome` configuration to improve the quality of JS/TS/CSS/JSON code reviews.Add a configuration file to your project to customize how CodeRabbit runs |
|
| Filename | Overview |
|---|---|
| src/api/utils/dashboardRoles.js | New utility that maps Discord permission bitfields + owner flag to a four-tier role (viewer/moderator/admin/owner); well-tested and logically sound for all current Discord permission values. |
| web/src/lib/dashboard-roles.ts | TypeScript mirror of the backend role utility; replaces the previous BigInt-based administrator check with Number arithmetic, which is a theoretical regression for future high-order Discord permission bits. |
| src/api/routes/guilds.js | Replaces ad-hoc permission flag checks with the new role-based middleware; GET /:id, /:id/stats, and /:id/analytics are intentionally downgraded from admin to viewer, expanding data access to any guild member. |
| web/src/lib/bot-api-proxy.ts | Replaces authorizeGuildAdmin with the new authorizeGuildRole(minRole) pattern; the deprecated shim for authorizeGuildAdmin is well-named but does not guard against botPresent: false guilds under bot API degradation. |
| web/src/app/api/guilds/[guildId]/role/route.ts | New endpoint returning the user's dashboard role for a guild; correctly handles auth and timeout, but does not check guild.botPresent which causes incorrect role responses when the bot API is unavailable. |
| web/src/components/layout/sidebar.tsx | Significant rework to show role-filtered navigation using useGuildRole; the `else if (loading |
| tests/api/utils/dashboardRoles.test.js | New test suite with comprehensive coverage of getDashboardRole and hasMinimumRole, including edge cases for owner flag, NaN permissions, and string inputs. |
| web/src/app/api/moderation/cases/route.ts | Authorization downgraded from admin to moderator; the inline JSDoc comment on line 18 still says "admin authorization" and should be updated. |
Sequence Diagram
sequenceDiagram
participant User as Browser (User)
participant Sidebar as Sidebar Component
participant RoleAPI as GET /api/guilds/:guildId/role
participant Discord as Discord OAuth API
participant BotAPI as Bot API
participant GuildRoute as GET /api/v1/guilds/:id
User->>Sidebar: mount (guildId selected)
Sidebar->>RoleAPI: fetch /api/guilds/:guildId/role
RoleAPI->>Discord: getMutualGuilds (OAuth token)
Discord-->>RoleAPI: user guilds list
RoleAPI->>RoleAPI: getDashboardRole(permissions, owner)
RoleAPI-->>Sidebar: { role: "viewer"|"moderator"|"admin"|"owner" }
Sidebar->>Sidebar: filter nav by hasMinimumRole(role, item.minRole)
Sidebar-->>User: render visible nav items
User->>GuildRoute: GET /api/v1/guilds/:id (e.g. stats)
GuildRoute->>GuildRoute: requireRole('viewer') middleware
GuildRoute->>Discord: getOAuthGuildDashboardRole
Discord-->>GuildRoute: guild permissions
GuildRoute->>GuildRoute: getDashboardRole → userRole
GuildRoute->>GuildRoute: hasMinimumRole(userRole, 'viewer')
alt authorized
GuildRoute-->>User: 200 guild stats
else insufficient role
GuildRoute-->>User: 403 Forbidden
end
Comments Outside Diff (1)
-
web/src/app/api/guilds/[guildId]/role/route.ts, line 625-631 (link)Role returned for guilds where bot is absent when bot API is degraded
getMutualGuildshas a documented graceful-degradation path: whenBOT_API_URLis unset or the bot API is unreachable, it returns all of the user's Discord guilds withbotPresent: falserather than filtering to only those the bot has joined (seediscord.server.tslines 215–220).This endpoint does not check
guild.botPresentbefore returning a role, so under degraded conditions a user can successfullyGET /api/guilds/<any-guild-they-are-in>/roleand receive a validDashboardRole. The sidebar hook treats a successful200as authoritative, so it will display role-filtered navigation items for a guild the bot isn't actually serving. Every subsequent bot-API call from those nav items will then fail.A one-line guard surfaces the issue immediately rather than letting the user get partway through the dashboard:
const guild = mutualGuilds.find((g) => g.id === guildId); if (!guild || !guild.botPresent) { return NextResponse.json({ error: 'You do not have access to this guild' }, { status: 403 }); }
(The same guard is worth adding to
authorizeGuildRoleinbot-api-proxy.tsfor consistency.)
Prompt To Fix All With AI
This is a comment left during a code review.
Path: web/src/app/api/moderation/cases/route.ts
Line: 18
Comment:
**Stale JSDoc comment**
The inline comment still says "admin authorization" but the endpoint now enforces `moderator` authorization. This will mislead future readers.
```suggestion
* Requires guildId query param and moderator authorization.
```
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: 52-59
Comment:
**Redundant `else if` branch — dead code**
When `role` is `null` the code falls into either the `else if (loading || error)` path **or** the `else` path, and both assign the same value. The condition on `else if` can never produce a different outcome, making it dead code that can confuse maintainers into thinking `loading`/`error` produce distinct behaviour.
```suggestion
if (role !== null) {
visibleNav = navigation.filter((item) => hasMinimumRole(role, item.minRole));
} else {
visibleNav = viewerNavigation;
}
```
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/lib/dashboard-roles.ts
Line: 34-38
Comment:
**`Number()` replaces `BigInt()` for permission bitfield checks**
The previous `hasAdministratorPermission` in `bot-api-proxy.ts` intentionally used `BigInt` to avoid floating-point precision loss on large Discord permission strings:
```ts
// old (removed)
return (BigInt(permissions) & ADMINISTRATOR_PERMISSION) === ADMINISTRATOR_PERMISSION;
```
The new `getDashboardRole` converts to `Number` first. Discord's OAuth permission field is a decimal string that can represent a 64-bit integer. All *current* permission flags occupy bits 0–46 (max value ≈ 2^47), well within JavaScript's 53-bit safe-integer range, so this is safe today. However, Discord has been steadily adding new high-order permission bits — if any flag ever lands at bit 53 or above, the `Number()` conversion will silently corrupt the bitfield and misclassify users.
Consider retaining BigInt for the bitfield arithmetic:
```ts
export function getDashboardRole(permissions: string | number, owner = false): DashboardRole {
if (owner) return 'owner';
let perm: bigint;
try {
perm = BigInt(typeof permissions === 'number' ? Math.trunc(permissions) : permissions);
} catch {
return 'viewer';
}
if ((perm & BigInt(PermissionBits.Administrator)) !== 0n) return 'admin';
if ((perm & BigInt(PermissionBits.ManageGuild)) !== 0n) return 'admin';
const moderatorFlags =
BigInt(PermissionBits.ManageMessages) |
BigInt(PermissionBits.KickMembers) |
BigInt(PermissionBits.BanMembers);
if ((perm & moderatorFlags) !== 0n) return 'moderator';
return 'viewer';
}
```
The backend `dashboardRoles.js` already imports `PermissionFlagsBits` as a BigInt from discord.js; applying the same treatment to the frontend mirror keeps both implementations consistent and future-proof.
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/[guildId]/role/route.ts
Line: 625-631
Comment:
**Role returned for guilds where bot is absent when bot API is degraded**
`getMutualGuilds` has a documented graceful-degradation path: when `BOT_API_URL` is unset or the bot API is unreachable, it returns **all** of the user's Discord guilds with `botPresent: false` rather than filtering to only those the bot has joined (see `discord.server.ts` lines 215–220).
This endpoint does not check `guild.botPresent` before returning a role, so under degraded conditions a user can successfully `GET /api/guilds/<any-guild-they-are-in>/role` and receive a valid `DashboardRole`. The sidebar hook treats a successful `200` as authoritative, so it will display role-filtered navigation items for a guild the bot isn't actually serving. Every subsequent bot-API call from those nav items will then fail.
A one-line guard surfaces the issue immediately rather than letting the user get partway through the dashboard:
```ts
const guild = mutualGuilds.find((g) => g.id === guildId);
if (!guild || !guild.botPresent) {
return NextResponse.json({ error: 'You do not have access to this guild' }, { status: 403 });
}
```
(The same guard is worth adding to `authorizeGuildRole` in `bot-api-proxy.ts` for consistency.)
How can I resolve this? If you propose a fix, please make it concise.Last reviewed commit: 35f3de4
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR iterates on issue #124 by introducing dashboard role tiers (viewer/moderator/admin/owner) derived from Discord guild permissions, then using those roles to widen read-only access (viewer) and allow moderation access (moderator) across backend + Next.js API routes and UI navigation.
Changes:
- Add shared “dashboard role” resolution utilities (backend + web) and update authorization to enforce minimum required role instead of “admin-only”.
- Add a guild role API endpoint + client hook, and filter the dashboard sidebar nav based on the resolved role.
- Update/extend backend tests around guild access metadata and role-based endpoint authorization.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| web/src/lib/dashboard-roles.ts | Introduces frontend role mapping + comparison helper. |
| web/src/lib/bot-api-proxy.ts | Replaces admin-only auth helper with authorizeGuildRole and adds deprecated wrapper for admin. |
| web/src/hooks/use-guild-role.ts | Adds client hook to fetch the user’s role for the selected guild. |
| web/src/components/layout/sidebar.tsx | Filters nav items by role and displays the current role in the sidebar. |
| web/src/app/api/temp-roles/route.ts | Lowers required auth from admin to moderator via authorizeGuildRole. |
| web/src/app/api/temp-roles/[id]/route.ts | Lowers required auth from admin to moderator via authorizeGuildRole. |
| web/src/app/api/moderation/user/[userId]/history/route.ts | Lowers required auth from admin to moderator via authorizeGuildRole. |
| web/src/app/api/moderation/stats/route.ts | Lowers required auth from admin to moderator via authorizeGuildRole. |
| web/src/app/api/moderation/cases/route.ts | Lowers required auth from admin to moderator via authorizeGuildRole. |
| web/src/app/api/moderation/cases/[id]/route.ts | Lowers required auth from admin to moderator via authorizeGuildRole. |
| web/src/app/api/guilds/[guildId]/role/route.ts | Adds endpoint to return the current user’s dashboard role for a guild. |
| web/src/app/api/guilds/[guildId]/analytics/route.ts | Lowers required auth from admin to viewer via authorizeGuildRole. |
| tests/api/utils/dashboardRoles.test.js | Adds unit tests for backend dashboard role mapping and comparisons. |
| tests/api/routes/guilds.test.js | Updates guild route expectations for new access mapping and adds role-based access tests. |
| src/api/utils/dashboardRoles.js | Adds backend role resolution + hierarchy helpers (PermissionFlagsBits-based). |
| src/api/routes/guilds.js | Adds requireRole(minRole) middleware and updates some endpoints to viewer access. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
web/src/lib/dashboard-roles.ts
Outdated
| */ | ||
| export function hasMinimumRole(userRole: DashboardRole, requiredRole: DashboardRole): boolean { | ||
| const a = DASHBOARD_ROLE_ORDER[userRole]; | ||
| const b = DASHBOARD_ROLE_ORDER[requiredRole]; |
There was a problem hiding this comment.
hasMinimumRole doesn’t mirror the backend behavior for unknown/invalid roles. In src/api/utils/dashboardRoles.js, the function explicitly returns false when either role isn’t in the map; here it relies on undefined >= ... coercion and the TS types prevent passing non-DashboardRole values even though runtime data can be untrusted. Consider aligning with the backend by checking a === undefined || b === undefined and returning false (and optionally widening the param types to accept string).
| */ | |
| export function hasMinimumRole(userRole: DashboardRole, requiredRole: DashboardRole): boolean { | |
| const a = DASHBOARD_ROLE_ORDER[userRole]; | |
| const b = DASHBOARD_ROLE_ORDER[requiredRole]; | |
| * | |
| * Unknown or invalid roles (i.e., not present in {@link DASHBOARD_ROLE_ORDER}) | |
| * are treated as insufficient privileges and return {@code false}, mirroring | |
| * the backend implementation in src/api/utils/dashboardRoles.js. | |
| * | |
| * @param userRole - The user's dashboard role or role identifier string. | |
| * @param requiredRole - The minimum dashboard role or role identifier string required. | |
| * @returns {@code true} if {@link userRole} is greater than or equal to | |
| * {@link requiredRole} in {@link DASHBOARD_ROLE_ORDER}, otherwise {@code false}. | |
| */ | |
| export function hasMinimumRole( | |
| userRole: DashboardRole | string, | |
| requiredRole: DashboardRole | string, | |
| ): boolean { | |
| const a = DASHBOARD_ROLE_ORDER[userRole as DashboardRole]; | |
| const b = DASHBOARD_ROLE_ORDER[requiredRole as DashboardRole]; | |
| // Explicitly handle unknown/invalid roles to mirror backend behavior. | |
| if (a === undefined || b === undefined) { | |
| return false; | |
| } |
| return; | ||
| } | ||
|
|
||
| let cancelled = false; |
There was a problem hiding this comment.
When a new guildId is set, the hook sets loading true but doesn’t reset role, so components can briefly show the previous guild’s role (e.g., the sidebar footer) until the fetch completes. Consider setting role to null when starting a new fetch (or when guildId changes) to avoid stale UI.
| let cancelled = false; | |
| let cancelled = false; | |
| // Reset the role when a new guildId is set to avoid showing a stale role | |
| // from the previous guild while the new role is loading. | |
| setRole(null); |
| const visibleNav = loading || role === null | ||
| ? navigation | ||
| : navigation.filter((item) => hasMinimumRole(role, item.minRole)); |
There was a problem hiding this comment.
visibleNav shows the full navigation while the role is loading (loading || role === null ? navigation : ...). This can cause a flash of admin-only links for users who ultimately won’t have access, which conflicts with the goal of hiding unauthorized nav items. Consider rendering a skeleton/placeholder while loading, or defaulting to the lowest-privilege set until the role is known.
| {role && ( | ||
| <div className="border-t px-3 py-3"> | ||
| <p className="text-xs text-muted-foreground">Your role</p> | ||
| <p className="mt-0.5 text-sm font-medium capitalize">{role}</p> | ||
| </div> |
There was a problem hiding this comment.
The role footer renders whenever role is truthy, even during a new guild role fetch. Because useGuildRole can retain the previous role until the request resolves, this footer can display a stale role. Consider gating this on !loading && role (or ensuring the hook clears role when starting a fetch).
| it('should allow viewer role to GET /:id and GET /:id/stats', async () => { | ||
| vi.stubEnv('SESSION_SECRET', 'jwt-test-secret'); | ||
| const token = createOAuthToken(); | ||
| mockFetchGuilds([{ id: 'guild1', name: 'Test', permissions: '0' }]); | ||
|
|
||
| const resInfo = await request(app) | ||
| .get('/api/v1/guilds/guild1') | ||
| .set('Authorization', `Bearer ${token}`); | ||
| expect(resInfo.status).toBe(200); | ||
| expect(resInfo.body.id).toBe('guild1'); | ||
|
|
||
| mockFetchGuilds([{ id: 'guild1', name: 'Test', permissions: '0' }]); | ||
| const resStats = await request(app) | ||
| .get('/api/v1/guilds/guild1/stats') | ||
| .set('Authorization', `Bearer ${token}`); | ||
| expect(resStats.status).toBe(200); | ||
| }); |
There was a problem hiding this comment.
This test calls GET /api/v1/guilds/guild1/stats but doesn’t mock mockPool.query for the two DB queries in the stats handler. With mockPool.query as a bare vi.fn(), the route will hit the catch block and return 500, causing this test to fail. Add two mockResolvedValueOnce(...) responses (or a default mockResolvedValue) before making the /stats request.
| const VALID_ROLES = ['viewer', 'moderator', 'admin', 'owner']; | ||
|
|
||
| /** | ||
| * Return Express middleware that enforces a guild-level permission for OAuth users. | ||
| * | ||
| * The middleware bypasses checks for API-secret requests and for configured bot owners. | ||
| * For OAuth-authenticated requests it calls `permissionCheck(user, guildId)` and: | ||
| * - responds 403 with `errorMessage` when the check resolves to `false`, | ||
| * - responds 502 when the permission verification throws, | ||
| * - otherwise allows the request to continue. | ||
| * Unknown or missing auth methods receive a 401 response. | ||
| * Return Express middleware that requires the OAuth user to have at least the given dashboard role. | ||
| * API-secret requests and bot owners bypass the check. 403 when role is insufficient, 502 on Discord errors. | ||
| * | ||
| * @param {(user: Object, guildId: string) => Promise<boolean>} permissionCheck - Function that returns `true` if the provided user has the required permission in the specified guild, `false` otherwise. | ||
| * @param {string} errorMessage - Message to include in the 403 response when permission is denied. | ||
| * @returns {import('express').RequestHandler} Express middleware enforcing the permission. | ||
| * @param {'viewer'|'moderator'|'admin'|'owner'} minRole - Minimum required dashboard role. | ||
| * @returns {import('express').RequestHandler} | ||
| */ | ||
| function requireGuildPermission(permissionCheck, errorMessage) { | ||
| export function requireRole(minRole) { | ||
| if (!VALID_ROLES.includes(minRole)) { | ||
| throw new Error(`requireRole: invalid minRole "${minRole}"`); | ||
| } | ||
| const errorMessage = `You do not have ${minRole} access to this guild`; |
There was a problem hiding this comment.
VALID_ROLES duplicates the role list already defined/exported in src/api/utils/dashboardRoles.js (ROLES). Reusing the shared constant would reduce the chance of these drifting out of sync as roles evolve.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
web/src/app/api/moderation/stats/route.ts (1)
14-18:⚠️ Potential issue | 🟡 MinorStale documentation: comment says "admin" but code requires "moderator".
The JSDoc comment at line 18 states "Requires guildId query param and admin authorization" but the authorization check at line 25 uses
'moderator'. Update the comment to reflect the actual requirement.📝 Suggested 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 - 18, Update the JSDoc block above the GET /api/moderation/stats handler to match the actual authorization check: replace the phrase "admin authorization" with "moderator authorization" (the authorization check uses 'moderator' in this route). Ensure the comment still mentions the guildId query param and that it proxies to bot API GET /api/v1/moderation/stats so it accurately reflects the behavior in this file and the authorization requirement used by the route.web/src/app/api/moderation/cases/route.ts (1)
15-18:⚠️ Potential issue | 🟡 MinorStale documentation: comment says "admin" but code requires "moderator".
The JSDoc comment at line 18 states "admin authorization" but line 26 uses
authorizeGuildRole(request, guildId, 'moderator', ...). Update to reflect the actual requirement.📝 Suggested 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 - 18, Update the JSDoc above the GET /api/moderation/cases handler to reflect the actual required role: replace "admin authorization" with "moderator authorization" (the code calls authorizeGuildRole(request, guildId, 'moderator', ...)). Ensure the comment text matches the behavior in authorizeGuildRole so docs and implementation stay consistent.
🤖 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/utils/dashboardRoles.js`:
- Around line 21-22: The JSDoc for ROLES declares a union type but the variable
is an array; update the JSDoc to reflect an array of those literals (e.g. use a
tuple/array type such as ("viewer"|"moderator"|"admin"|"owner")[] or
Array<"viewer"|"moderator"|"admin"|"owner">) so IDEs and type checkers
understand ROLES is an array; locate the ROLES constant in dashboardRoles.js and
replace the existing `@type` annotation with the array form referencing the same
literal names.
In `@web/src/components/layout/sidebar.tsx`:
- Around line 54-58: Destructure the error from useGuildRole and implement
fail-closed behavior: replace the current visibleNav logic so that you handle
loading, error, and success explicitly — keep the existing behavior while
loading, but when error is true only expose non-privileged items (e.g.,
navigation.filter(item => !item.minRole)), and when you have a role use
navigation.filter(item => hasMinimumRole(role, item.minRole)); update the
useGuildRole destructuring and the visibleNav computation (references:
useGuildRole, visibleNav, navigation, hasMinimumRole).
In `@web/src/hooks/use-guild-role.ts`:
- Around line 31-34: When starting a new fetch in the useGuildRole hook (on
guildId change) reset the cached role so the previous role isn't shown while
loading: inside the fetch-start block where cancelled is set and
setLoading(true)/setError(false) are called, also call setRole(undefined) (or
null depending on your state type) to clear the previous role; update any
variable names accordingly (e.g., setRole in use-guild-role.ts) so the UI shows
an empty state during the new guild's load.
- Around line 41-43: The hook useGuildRole currently returns early when
data?.role is missing which silently leaves role null; instead, detect when the
fetch succeeds but data?.role is undefined and, provided cancelled is false,
call the hook's error handling (e.g., setError with a descriptive Error like
"Missing role in API response") and ensure any loading state is cleared rather
than silently returning; update the .then handler that checks cancelled and
data?.role to setError(...) (and clear loading) when data exists but data.role
is missing, otherwise continue to call setRole(data.role).
---
Outside diff comments:
In `@web/src/app/api/moderation/cases/route.ts`:
- Around line 15-18: Update the JSDoc above the GET /api/moderation/cases
handler to reflect the actual required role: replace "admin authorization" with
"moderator authorization" (the code calls authorizeGuildRole(request, guildId,
'moderator', ...)). Ensure the comment text matches the behavior in
authorizeGuildRole so docs and implementation stay consistent.
In `@web/src/app/api/moderation/stats/route.ts`:
- Around line 14-18: Update the JSDoc block above the GET /api/moderation/stats
handler to match the actual authorization check: replace the phrase "admin
authorization" with "moderator authorization" (the authorization check uses
'moderator' in this route). Ensure the comment still mentions the guildId query
param and that it proxies to bot API GET /api/v1/moderation/stats so it
accurately reflects the behavior in this file and the authorization requirement
used by the route.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ce09acfd-d12b-4316-99a4-ca81516df8b6
📒 Files selected for processing (16)
src/api/routes/guilds.jssrc/api/utils/dashboardRoles.jstests/api/routes/guilds.test.jstests/api/utils/dashboardRoles.test.jsweb/src/app/api/guilds/[guildId]/analytics/route.tsweb/src/app/api/guilds/[guildId]/role/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/app/api/temp-roles/[id]/route.tsweb/src/app/api/temp-roles/route.tsweb/src/components/layout/sidebar.tsxweb/src/hooks/use-guild-role.tsweb/src/lib/bot-api-proxy.tsweb/src/lib/dashboard-roles.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 (6)
{src,web}/**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Winston logger from
src/logger.js, NEVER useconsole.*
Files:
web/src/app/api/guilds/[guildId]/role/route.tsweb/src/app/api/moderation/user/[userId]/history/route.tsweb/src/lib/dashboard-roles.tsweb/src/app/api/moderation/stats/route.tsweb/src/app/api/temp-roles/[id]/route.tsweb/src/hooks/use-guild-role.tssrc/api/routes/guilds.jsweb/src/app/api/temp-roles/route.tsweb/src/components/layout/sidebar.tsxweb/src/app/api/guilds/[guildId]/analytics/route.tsweb/src/app/api/moderation/cases/[id]/route.tsweb/src/lib/bot-api-proxy.tssrc/api/utils/dashboardRoles.jsweb/src/app/api/moderation/cases/route.ts
web/**/*.{ts,tsx,jsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
Use Next.js 16 App Router for web dashboard, with Discord OAuth2 authentication, dark/light theme support, and mobile-responsive design
Files:
web/src/app/api/guilds/[guildId]/role/route.tsweb/src/app/api/moderation/user/[userId]/history/route.tsweb/src/lib/dashboard-roles.tsweb/src/app/api/moderation/stats/route.tsweb/src/app/api/temp-roles/[id]/route.tsweb/src/hooks/use-guild-role.tsweb/src/app/api/temp-roles/route.tsweb/src/components/layout/sidebar.tsxweb/src/app/api/guilds/[guildId]/analytics/route.tsweb/src/app/api/moderation/cases/[id]/route.tsweb/src/lib/bot-api-proxy.tsweb/src/app/api/moderation/cases/route.ts
tests/**/*.{js,ts}
📄 CodeRabbit inference engine (AGENTS.md)
Maintain 80% test coverage threshold — Never lower the coverage requirement
Files:
tests/api/utils/dashboardRoles.test.jstests/api/routes/guilds.test.js
src/**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{js,ts,jsx,tsx}: Use ESM only — Useimport/export, no CommonJS
Use single quotes for strings — No double quotes except in JSON
Always require semicolons at end of statements
Use 2-space indent, enforced by Biome
Always use async/await for asynchronous operations and promise handling
Files:
src/api/routes/guilds.jssrc/api/utils/dashboardRoles.js
src/**/*.{js,ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use parameterized SQL queries — Never use string interpolation in database queries
Files:
src/api/routes/guilds.jssrc/api/utils/dashboardRoles.js
src/api/routes/**/*.{js,ts}
📄 CodeRabbit inference engine (AGENTS.md)
src/api/routes/**/*.{js,ts}: All API routes must use auth middleware if handling sensitive data and be mounted insrc/api/server.js
API endpoints must include tests intests/api/directory
Files:
src/api/routes/guilds.js
🧠 Learnings (4)
📚 Learning: 2026-03-02T21:23:59.512Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-02T21:23:59.512Z
Learning: Applies to web/**/*.{ts,tsx,jsx,js} : Use Next.js 16 App Router for web dashboard, with Discord OAuth2 authentication, dark/light theme support, and mobile-responsive design
Applied to files:
web/src/app/api/guilds/[guildId]/role/route.tsweb/src/hooks/use-guild-role.tssrc/api/routes/guilds.jsweb/src/components/layout/sidebar.tsx
📚 Learning: 2026-03-02T21:23:59.512Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-02T21:23:59.512Z
Learning: Applies to src/modules/**/*.{js,ts} : Use Discord cache utilities — `src/utils/discordCache.js` for channels/roles/members, `src/utils/reputationCache.js` for leaderboard/rank data
Applied to files:
web/src/lib/dashboard-roles.tsweb/src/hooks/use-guild-role.tssrc/api/routes/guilds.jssrc/api/utils/dashboardRoles.js
📚 Learning: 2026-03-02T21:23:59.512Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-02T21:23:59.512Z
Learning: Applies to src/{commands,index}.{js,ts} : Use safe Discord message methods — Use `safeReply()`, `safeSend()`, or `safeEditReply()` instead of direct Discord.js methods
Applied to files:
src/api/routes/guilds.js
📚 Learning: 2026-03-02T21:23:59.512Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-02T21:23:59.512Z
Learning: Create modules in `src/modules/` for new features, add config section to `config.json`, update `SAFE_CONFIG_KEYS`, create slash command if needed, add database migration if needed, write tests, and update dashboard UI if configurable
Applied to files:
src/api/utils/dashboardRoles.js
🔇 Additional comments (24)
src/api/utils/dashboardRoles.js (2)
31-52: LGTM!The
getDashboardRolefunction correctly maps Discord permissions to dashboard roles. The permission bit values (Administrator=0x8, ManageGuild=0x20, ManageMessages=0x2000, KickMembers=0x2, BanMembers=0x4, ViewChannel=0x400) match the test expectations and the web-sidePermissionBitsconstants. TheNumber()conversion handles discord.js BigIntPermissionFlagsBitscorrectly for bitwise operations.
61-66: LGTM!The
hasMinimumRolefunction correctly implements role hierarchy comparison and defensively returnsfalsefor unknown/undefined roles, which aligns with the PR objective of making role comparison stricter.src/api/routes/guilds.js (4)
219-252: LGTM!The
requireRolemiddleware factory is well-implemented:
- Validates
minRoleat initialization time (fail-fast)- Correctly bypasses checks for
api-secretauth and bot owners- Uses
hasMinimumRolefor hierarchical role comparison- Returns 403 with clear error message when role is insufficient
- Returns 502 on Discord API errors (appropriate for upstream failures)
254-258: LGTM!Clean refactor of
requireGuildAdminandrequireGuildModeratoras thin wrappers aroundrequireRole. This eliminates code duplication and ensures consistent authorization behavior.
758-758: Consistent role enforcement for analytics endpoints.Both
GET /:id/statsandGET /:id/analyticsnow userequireRole('viewer'), which aligns with the PR objective that stats/analytics are viewable by any authenticated guild member. This is consistent with the design where sensitive operations (config, moderation) require higher roles.Also applies to: 870-870
477-477: The test suite explicitly confirms this is intentional—the test at line 418 verifies that viewers can accessGET /:id. The endpoint returns basic guild information including a channel list, while the dedicatedGET /:id/channelsendpoint (which requiresrequireGuildAdmin) provides admin-only access. This design appears sound: viewers can see what channels exist in a guild as part of basic info, but cannot perform channel management operations.web/src/app/api/temp-roles/route.ts (1)
11-11: LGTM!Authorization correctly updated to use
authorizeGuildRolewith'moderator'role for both GET and POST handlers. This aligns with:
- The PR objective to allow moderators to manage temp roles
- The backend
requireGuildModeratormiddleware on the upstream/temp-rolesroutesThe dual enforcement ensures consistent access control across the web proxy and bot API layers.
Also applies to: 32-32, 66-66
web/src/app/api/moderation/stats/route.ts (1)
25-26: LGTM!Authorization correctly updated to require moderator role, consistent with other moderation endpoints in this PR.
web/src/app/api/moderation/cases/[id]/route.ts (1)
4-4: LGTM!Authorization correctly updated to
authorizeGuildRolewith'moderator'role. Consistent with the PR's role-based access control model for moderation endpoints.Also applies to: 28-29
web/src/app/api/moderation/user/[userId]/history/route.ts (1)
4-4: LGTM!Authorization correctly updated to require moderator role for viewing user moderation history. The
authorizeGuildRolefunction properly validates the user's access token, fetches mutual guilds, and checks that the user has at least the required minimum role.Also applies to: 28-29
web/src/app/api/moderation/cases/route.ts (1)
26-27: LGTM!Authorization correctly updated to moderator role.
web/src/app/api/temp-roles/[id]/route.ts (1)
11-11: LGTM!Authorization correctly updated to require moderator role for DELETE operations. This maintains consistency with GET and POST handlers in the sibling
route.tsfile, ensuring uniform access control for all temp-role operations.Also applies to: 36-37
tests/api/utils/dashboardRoles.test.js (1)
1-83: Good test coverage for dashboard role utilities.The test suite comprehensively covers
getDashboardRole,hasMinimumRole, andDASHBOARD_ROLE_ORDERwith appropriate edge cases including string permissions, NaN handling, and owner override.Consider adding a test for
hasMinimumRolewith invalid/undefined roles, since the backend implementation (src/api/utils/dashboardRoles.js lines 63-65) returnsfalsefor undefined roles:💡 Suggested additional test
it('returns false for undefined or invalid roles', () => { expect(hasMinimumRole(undefined, 'viewer')).toBe(false); expect(hasMinimumRole('viewer', undefined)).toBe(false); expect(hasMinimumRole('invalid', 'viewer')).toBe(false); });[approve_code_changes, suggest_optional_refactor]
web/src/lib/dashboard-roles.ts (2)
51-54: Missing undefined guard mentioned in PR description.The PR description states that
hasMinimumRole"treats invalid/unknown roles as insufficient (returns false if a === undefined || b === undefined)". However, this TypeScript implementation lacks that explicit guard, unlike the backend version atsrc/api/utils/dashboardRoles.js:63-65.While TypeScript enforces
DashboardRoleat compile time, runtime values from API responses or other sources could be invalid. For consistency with the backend and the stated PR behavior, add the explicit guard:🛡️ Proposed fix
export function hasMinimumRole(userRole: DashboardRole, requiredRole: DashboardRole): boolean { const a = DASHBOARD_ROLE_ORDER[userRole]; const b = DASHBOARD_ROLE_ORDER[requiredRole]; + if (a === undefined || b === undefined) return false; return a >= b; }[raise_minor_issue, inconsistent_summary]
1-46: Well-structured role resolution module.The implementation correctly mirrors the backend logic with proper priority ordering (owner → admin → moderator → viewer) and handles edge cases like NaN permissions. The use of
as constfor type safety is appropriate.web/src/components/layout/sidebar.tsx (2)
26-44: Good implementation of role-based navigation filtering.The typed navigation array with
minRolerequirements and the use ofhasMinimumRolefor filtering provides a clean, maintainable approach to role-based UI visibility.
88-93: User role display is a helpful UX addition.Displaying the current role to users provides transparency about their access level.
tests/api/routes/guilds.test.js (2)
417-451: Good addition of dashboard role-based access tests.The new test block effectively validates the role hierarchy for API access:
- Viewer role accessing
GET /:idandGET /:id/stats- Moderator role accessing
GET /:id/moderationThe mock setup correctly handles multiple sequential requests with separate
mockFetchGuildscalls.
223-234: Test expectations correctly updated for role-based semantics.The test changes properly align with the new dashboard role mappings:
MANAGE_GUILD (0x20)→adminaccessMANAGE_MESSAGES (0x2000)→moderatoraccess- No special permissions →
vieweraccess- Moderator denied on admin endpoints
Also applies to: 259-268, 291-302, 356-368
web/src/app/api/guilds/[guildId]/role/route.ts (1)
18-53: Well-implemented role endpoint.The route correctly:
- Validates guildId presence and token authentication
- Handles token expiration with appropriate 401 response
- Uses timeout for external Discord API calls
- Returns proper 403 for guild access denial
- Computes role using shared
getDashboardRoleutilityThe implementation is consistent with the authorization pattern in
bot-api-proxy.ts.web/src/app/api/guilds/[guildId]/analytics/route.ts (1)
4-4: Authorization correctly updated to viewer role.The change from
authorizeGuildAdmintoauthorizeGuildRole(request, guildId, 'viewer', ...)aligns with the PR objectives and is consistent with the sidebar configuration where "Performance" hasminRole: 'viewer'.Also applies to: 21-21
web/src/hooks/use-guild-role.ts (1)
1-58: Good implementation of role-fetching hook.The hook correctly handles:
- Null guildId early return
- Cancellation on unmount/dependency change
- Loading and error states
- URL encoding of guildId
web/src/lib/bot-api-proxy.ts (2)
21-65: Well-designed role-based authorization function.The
authorizeGuildRolefunction cleanly encapsulates:
- Token validation with proper 401 responses
- Discord API timeout handling
- Guild membership verification
- Role hierarchy enforcement using
hasMinimumRoleError messages appropriately include the required role (e.g., "You do not have moderator access to this guild").
67-77: Good deprecation pattern forauthorizeGuildAdmin.Delegating to
authorizeGuildRolewith'admin'maintains backward compatibility while encouraging migration to the new role-based API. The@deprecatedJSDoc tag provides clear guidance.
| /** @type {'viewer'|'moderator'|'admin'|'owner'} */ | ||
| const ROLES = ['viewer', 'moderator', 'admin', 'owner']; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
JSDoc type annotation doesn't match actual value.
The JSDoc declares @type {'viewer'|'moderator'|'admin'|'owner'} (a union type), but ROLES is assigned an array. This could confuse IDE tooling and developers.
📝 Suggested fix
-/** `@type` {'viewer'|'moderator'|'admin'|'owner'} */
-const ROLES = ['viewer', 'moderator', 'admin', 'owner'];
+/** `@type` {readonly ['viewer', 'moderator', 'admin', 'owner']} */
+const ROLES = /** `@type` {const} */ (['viewer', 'moderator', 'admin', 'owner']);📝 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.
| /** @type {'viewer'|'moderator'|'admin'|'owner'} */ | |
| const ROLES = ['viewer', 'moderator', 'admin', 'owner']; | |
| /** `@type` {readonly ['viewer', 'moderator', 'admin', 'owner']} */ | |
| const ROLES = /** `@type` {const} */ (['viewer', 'moderator', 'admin', 'owner']); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/api/utils/dashboardRoles.js` around lines 21 - 22, The JSDoc for ROLES
declares a union type but the variable is an array; update the JSDoc to reflect
an array of those literals (e.g. use a tuple/array type such as
("viewer"|"moderator"|"admin"|"owner")[] or
Array<"viewer"|"moderator"|"admin"|"owner">) so IDEs and type checkers
understand ROLES is an array; locate the ROLES constant in dashboardRoles.js and
replace the existing `@type` annotation with the array form referencing the same
literal names.
| .then((data) => { | ||
| if (cancelled || !data?.role) return; | ||
| setRole(data.role); |
There was a problem hiding this comment.
Silent failure when API response lacks role field.
If the API returns a 200 response but data.role is missing or undefined, the hook silently continues without setting the role or error state. This leaves role as null even though the request "succeeded".
Consider treating a missing role as an error:
🛡️ Proposed fix
.then((data) => {
- if (cancelled || !data?.role) return;
- setRole(data.role);
+ if (cancelled) return;
+ if (!data?.role) {
+ setError(true);
+ return;
+ }
+ setRole(data.role);
})📝 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.
| .then((data) => { | |
| if (cancelled || !data?.role) return; | |
| setRole(data.role); | |
| .then((data) => { | |
| if (cancelled) return; | |
| if (!data?.role) { | |
| setError(true); | |
| return; | |
| } | |
| setRole(data.role); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/hooks/use-guild-role.ts` around lines 41 - 43, The hook useGuildRole
currently returns early when data?.role is missing which silently leaves role
null; instead, detect when the fetch succeeds but data?.role is undefined and,
provided cancelled is false, call the hook's error handling (e.g., setError with
a descriptive Error like "Missing role in API response") and ensure any loading
state is cleared rather than silently returning; update the .then handler that
checks cancelled and data?.role to setError(...) (and clear loading) when data
exists but data.role is missing, otherwise continue to call setRole(data.role).
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 762-763: Tests are missing coverage for the new
requireRole('moderator') auth boundary on the GET /:id/members/:userId/cases
endpoint; add API tests under tests/api/ that assert a non-moderator user gets
403, assert a moderator user gets 200 (or reuse existing happy-path test), and
assert admin/owner roles are still allowed (200). Create fixtures or mock auth
tokens for roles (regular user, moderator, admin/owner) and call the route
handler used in members.js (the GET /:id/members/:userId/cases endpoint) to
verify status codes and any expected response shape; ensure tests reference the
same middleware stack (requireRole('moderator'), validateGuild) so they cover
the boundary.
In `@web/src/hooks/use-moderation-cases.ts`:
- Around line 63-75: The code calls await res.json() before checking res.ok so a
malformed/empty body can throw and skip the 403 fallback; wrap the JSON parse in
a try/catch (or only parse when content-type indicates JSON) so you set payload
to null on parse failure, then run the existing if (!res.ok) block using payload
to extract fromPayload and the res.status === 403 fallback; update the logic
around the payload variable used in useModerationCases (payload, res,
fromPayload) to handle parse errors gracefully.
In `@web/src/types/discord.ts`:
- Around line 16-17: Replace this duplicate union by importing and re-exporting
the canonical dashboard role type instead of redefining GuildAccessRole; locate
the original dashboard role type (the canonical type used elsewhere), import it
into this module, then update the local GuildAccessRole to reference or
re-export that imported symbol so additions/renames stay synchronized with the
dashboard type.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 176a5da5-7a57-4732-87f9-e0f075b9ed3a
📒 Files selected for processing (13)
src/api/routes/members.jstests/api/routes/guilds.test.jsweb/src/app/api/guilds/[guildId]/members/[userId]/cases/route.tsweb/src/app/api/guilds/route.tsweb/src/app/api/performance/route.tsweb/src/app/api/temp-roles/[id]/route.tsweb/src/app/api/temp-roles/route.tsweb/src/components/layout/sidebar.tsxweb/src/hooks/use-moderation-cases.tsweb/src/hooks/use-moderation-stats.tsweb/src/hooks/use-user-history.tsweb/src/lib/dashboard-roles.tsweb/src/types/discord.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). (1)
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (6)
{src,web}/**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Winston logger from
src/logger.js, NEVER useconsole.*
Files:
web/src/app/api/performance/route.tsweb/src/types/discord.tsweb/src/lib/dashboard-roles.tsweb/src/app/api/guilds/[guildId]/members/[userId]/cases/route.tsweb/src/app/api/temp-roles/[id]/route.tsweb/src/hooks/use-user-history.tsweb/src/hooks/use-moderation-cases.tsweb/src/components/layout/sidebar.tsxweb/src/app/api/guilds/route.tsweb/src/hooks/use-moderation-stats.tssrc/api/routes/members.jsweb/src/app/api/temp-roles/route.ts
web/**/*.{ts,tsx,jsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
Use Next.js 16 App Router for web dashboard, with Discord OAuth2 authentication, dark/light theme support, and mobile-responsive design
Files:
web/src/app/api/performance/route.tsweb/src/types/discord.tsweb/src/lib/dashboard-roles.tsweb/src/app/api/guilds/[guildId]/members/[userId]/cases/route.tsweb/src/app/api/temp-roles/[id]/route.tsweb/src/hooks/use-user-history.tsweb/src/hooks/use-moderation-cases.tsweb/src/components/layout/sidebar.tsxweb/src/app/api/guilds/route.tsweb/src/hooks/use-moderation-stats.tsweb/src/app/api/temp-roles/route.ts
tests/**/*.{js,ts}
📄 CodeRabbit inference engine (AGENTS.md)
Maintain 80% test coverage threshold — Never lower the coverage requirement
Files:
tests/api/routes/guilds.test.js
src/**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{js,ts,jsx,tsx}: Use ESM only — Useimport/export, no CommonJS
Use single quotes for strings — No double quotes except in JSON
Always require semicolons at end of statements
Use 2-space indent, enforced by Biome
Always use async/await for asynchronous operations and promise handling
Files:
src/api/routes/members.js
src/**/*.{js,ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use parameterized SQL queries — Never use string interpolation in database queries
Files:
src/api/routes/members.js
src/api/routes/**/*.{js,ts}
📄 CodeRabbit inference engine (AGENTS.md)
src/api/routes/**/*.{js,ts}: All API routes must use auth middleware if handling sensitive data and be mounted insrc/api/server.js
API endpoints must include tests intests/api/directory
Files:
src/api/routes/members.js
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-02T21:23:59.512Z
Learning: Applies to web/**/*.{ts,tsx,jsx,js} : Use Next.js 16 App Router for web dashboard, with Discord OAuth2 authentication, dark/light theme support, and mobile-responsive design
📚 Learning: 2026-03-02T21:23:59.512Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-02T21:23:59.512Z
Learning: Applies to web/**/*.{ts,tsx,jsx,js} : Use Next.js 16 App Router for web dashboard, with Discord OAuth2 authentication, dark/light theme support, and mobile-responsive design
Applied to files:
web/src/lib/dashboard-roles.tsweb/src/components/layout/sidebar.tsxweb/src/app/api/guilds/route.ts
📚 Learning: 2026-03-02T21:23:59.512Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-02T21:23:59.512Z
Learning: Applies to src/modules/**/*.{js,ts} : Use Discord cache utilities — `src/utils/discordCache.js` for channels/roles/members, `src/utils/reputationCache.js` for leaderboard/rank data
Applied to files:
web/src/lib/dashboard-roles.tsweb/src/app/api/guilds/route.tssrc/api/routes/members.js
📚 Learning: 2026-03-02T21:23:59.512Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-02T21:23:59.512Z
Learning: Applies to src/api/routes/**/*.{js,ts} : All API routes must use auth middleware if handling sensitive data and be mounted in `src/api/server.js`
Applied to files:
src/api/routes/members.js
🔇 Additional comments (11)
web/src/app/api/performance/route.ts (1)
8-11: Verify that bot-wide performance data should be accessible to all authenticated users.The documentation states that any authenticated user can access bot-wide performance metrics. Performance data often contains sensitive operational information (CPU usage, memory, response times, etc.) that is typically restricted to admin or owner roles.
Please confirm whether this access level is intentional and aligns with your security requirements.
web/src/hooks/use-moderation-stats.ts (1)
45-57: Same JSON-parse fallback bug as inuse-moderation-cases.This block has the same unguarded
res.json()behavior, so empty/non-JSON 403s can skip the intended permission message.web/src/app/api/temp-roles/route.ts (1)
32-33: Role gating looks correctly scoped for read vs write actions.GET requiring
moderatorand POST requiringadminis a sensible minimum-role split.Also applies to: 66-67
web/src/hooks/use-user-history.ts (1)
59-71: Same JSON-parse fallback bug as inuse-moderation-cases.This block repeats the same pattern where
res.json()can throw before the 403 fallback branch is evaluated.web/src/app/api/guilds/route.ts (1)
28-32: Good response enrichment for role-aware clients.Adding
accessat the API boundary simplifies downstream authorization/UI logic.web/src/app/api/temp-roles/[id]/route.ts (1)
36-37: Admin gate for revoke operation is correct.Using
authorizeGuildRole(..., 'admin', ...)here preserves strict control for destructive actions.web/src/app/api/guilds/[guildId]/members/[userId]/cases/route.ts (1)
30-31: Moderator role gate change is correct for this endpoint.The authorization update cleanly aligns this route with moderator-level case access while preserving centralized role enforcement.
tests/api/routes/guilds.test.js (1)
223-235: Role-based test coverage improvements look solid.These additions/updates cover key boundaries (viewer/moderator/admin) and permission-bit mappings, which materially improves confidence in the RBAC migration.
Also applies to: 259-268, 291-302, 356-368, 417-454
web/src/components/layout/sidebar.tsx (2)
26-44: Role requirements embedded in nav metadata are a good change.Defining
minRoleon each item makes authorization intent explicit and keeps rendering logic maintainable.
56-60: Error-awarevisibleNavgating is a good hardening step.The branch now accounts for role-load failures and avoids blindly relying on null-role state.
web/src/lib/dashboard-roles.ts (1)
32-46: Role resolution and comparison utilities are implemented cleanly.The precedence rules are explicit, and the invalid-role guard in
hasMinimumRoleprovides a safer default.Also applies to: 52-57
| requireRole('moderator'), | ||
| validateGuild, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# First, verify whether tests cover GET /:id/members/:userId/cases auth semantics.
if [ -d tests/api ]; then
echo "=== Searching for tests covering member-cases endpoint and moderator auth ==="
rg -n -C2 "/:id/members/:userId/cases|members/.*/cases|requireRole\('moderator'\)|You do not have moderator access to this guild" tests/api || echo "No matches found"
else
echo "tests/api directory not found"
fi
echo ""
echo "=== Checking src/api/routes/members.js context around lines 762-763 ==="
if [ -f "src/api/routes/members.js" ]; then
sed -n '750,775p' src/api/routes/members.js
else
echo "File not found"
fiRepository: VolvoxLLC/volvox-bot
Length of output: 3444
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Searching for auth boundary tests for the cases endpoint ==="
rg -n -A 10 "GET /api/v1/guilds/:id/members/:userId/cases" tests/api/routes/members.test.js | head -100
echo ""
echo "=== Looking for unauthed/forbidden tests for this endpoint ==="
rg -n -B 5 -A 5 "should.*forbidden|should.*unauthorized|should.*deny|requireRole\('moderator'\)" tests/api/routes/members.test.js | grep -A 5 -B 5 "cases" || echo "No explicit forbidden tests found"
echo ""
echo "=== Checking what authed() helper does ==="
rg -n -B 2 -A 10 "function authed|const authed|authed.*=" tests/api/routes/members.test.js | head -30Repository: VolvoxLLC/volvox-bot
Length of output: 1655
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Looking for role-based auth tests in the file ==="
rg -n "viewer|admin|owner|role" tests/api/routes/members.test.js | head -20
echo ""
echo "=== Searching for tests that check 401/403 responses ==="
rg -n "401|403|Forbidden|Unauthorized" tests/api/routes/members.test.js | head -20
echo ""
echo "=== Checking if there are unauthed() tests ==="
rg -n "unauthed|without.*auth|\.get\(" tests/api/routes/members.test.js | grep -v "authed" | head -20Repository: VolvoxLLC/volvox-bot
Length of output: 1290
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Full test suite for GET /:id/members/:userId/cases endpoint ==="
rg -n -A 100 "describe\('GET /api/v1/guilds/:id/members/:userId/cases" tests/api/routes/members.test.js | head -150
echo ""
echo "=== Check if there's a test for unauthorized access to cases ==="
rg -n -B 5 -A 10 "should.*401|should.*403|should.*forbidden|should.*unauthorized" tests/api/routes/members.test.js | grep -B 5 -A 10 -i "case"Repository: VolvoxLLC/volvox-bot
Length of output: 4070
Add tests for moderator access boundary on the cases endpoint.
The new requireRole('moderator') middleware on GET /:id/members/:userId/cases lacks auth boundary coverage. Current tests only verify the happy path. Add tests for:
- Non-moderator user receives 403
- Moderator user receives 200 (already partially covered)
- Admin/owner still allowed access
Per coding guidelines, API endpoints must include tests in tests/api/ directory.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/api/routes/members.js` around lines 762 - 763, Tests are missing coverage
for the new requireRole('moderator') auth boundary on the GET
/:id/members/:userId/cases endpoint; add API tests under tests/api/ that assert
a non-moderator user gets 403, assert a moderator user gets 200 (or reuse
existing happy-path test), and assert admin/owner roles are still allowed (200).
Create fixtures or mock auth tokens for roles (regular user, moderator,
admin/owner) and call the route handler used in members.js (the GET
/:id/members/:userId/cases endpoint) to verify status codes and any expected
response shape; ensure tests reference the same middleware stack
(requireRole('moderator'), validateGuild) so they cover the boundary.
| const payload: unknown = await res.json(); | ||
| if (!res.ok) { | ||
| const msg = | ||
| const fromPayload = | ||
| typeof payload === 'object' && | ||
| payload !== null && | ||
| 'error' in payload && | ||
| typeof (payload as Record<string, unknown>).error === 'string' | ||
| ? (payload as Record<string, string>).error | ||
| : 'Failed to fetch cases'; | ||
| : null; | ||
| const msg = | ||
| fromPayload ?? | ||
| (res.status === 403 ? "You don't have permission to view this in this server." : 'Failed to fetch cases'); | ||
| throw new Error(msg); |
There was a problem hiding this comment.
Guard JSON parsing so 403 fallback actually applies on empty/non-JSON errors.
await res.json() can throw before !res.ok runs, so your new 403 fallback path is skipped for malformed/empty bodies.
Suggested fix
- const payload: unknown = await res.json();
+ let payload: unknown = null;
+ try {
+ payload = await res.json();
+ } catch {
+ if (res.ok) throw new Error('Failed to fetch cases');
+ }
if (!res.ok) {
const fromPayload =
typeof payload === 'object' &&
payload !== null &&
'error' in payload &&
typeof (payload as Record<string, unknown>).error === 'string'
? (payload as Record<string, string>).error
: null;
const msg =
fromPayload ??
(res.status === 403 ? "You don't have permission to view this in this server." : 'Failed to fetch cases');
throw new Error(msg);
}📝 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 payload: unknown = await res.json(); | |
| if (!res.ok) { | |
| const msg = | |
| const fromPayload = | |
| typeof payload === 'object' && | |
| payload !== null && | |
| 'error' in payload && | |
| typeof (payload as Record<string, unknown>).error === 'string' | |
| ? (payload as Record<string, string>).error | |
| : 'Failed to fetch cases'; | |
| : null; | |
| const msg = | |
| fromPayload ?? | |
| (res.status === 403 ? "You don't have permission to view this in this server." : 'Failed to fetch cases'); | |
| throw new Error(msg); | |
| let payload: unknown = null; | |
| try { | |
| payload = await res.json(); | |
| } catch { | |
| if (res.ok) throw new Error('Failed to fetch cases'); | |
| } | |
| if (!res.ok) { | |
| const fromPayload = | |
| typeof payload === 'object' && | |
| payload !== null && | |
| 'error' in payload && | |
| typeof (payload as Record<string, unknown>).error === 'string' | |
| ? (payload as Record<string, string>).error | |
| : null; | |
| const msg = | |
| fromPayload ?? | |
| (res.status === 403 ? "You don't have permission to view this in this server." : 'Failed to fetch cases'); | |
| throw new Error(msg); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/hooks/use-moderation-cases.ts` around lines 63 - 75, The code calls
await res.json() before checking res.ok so a malformed/empty body can throw and
skip the 403 fallback; wrap the JSON parse in a try/catch (or only parse when
content-type indicates JSON) so you set payload to null on parse failure, then
run the existing if (!res.ok) block using payload to extract fromPayload and the
res.status === 403 fallback; update the logic around the payload variable used
in useModerationCases (payload, res, fromPayload) to handle parse errors
gracefully.
Additional Comments (1)
This effect has an empty dependency array The comment labels it Remove it to avoid confusion: |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 24 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let cancelled = false; | ||
| setLoading(true); | ||
| setError(false); | ||
|
|
||
| fetch(`/api/guilds/${encodeURIComponent(guildId)}/role`) | ||
| .then((res) => { | ||
| if (cancelled) return null; | ||
| if (!res.ok) throw new Error(res.statusText); | ||
| return res.json() as Promise<GuildRoleResponse>; | ||
| }) | ||
| .then((data) => { | ||
| if (cancelled || !data?.role) return; | ||
| setRole(data.role); | ||
| }) |
There was a problem hiding this comment.
role is not cleared when starting a new fetch for a different guildId, so the UI can briefly show a stale role (and stale nav filtering) from the previous guild. Reset role (e.g., setRole(null)) when guildId changes / before starting the fetch, and consider also clearing it on fetch errors for the current guild.
| const visibleNav = loading || (role === null && !error) | ||
| ? navigation | ||
| : role !== null | ||
| ? navigation.filter((item) => hasMinimumRole(role, item.minRole)) | ||
| : navigation.filter((item) => item.minRole === 'viewer'); | ||
|
|
There was a problem hiding this comment.
This nested-ternary makes the navigation visibility rules hard to read/modify, and it currently shows all navigation items while loading is true (which can cause a brief “flash” of admin-only items). Refactor to a small if/else block (or helper function) that (a) keeps logic readable and (b) avoids rendering restricted items during loading (e.g., default to viewer-only until the role is known, or render a loading state).
| const visibleNav = loading || (role === null && !error) | |
| ? navigation | |
| : role !== null | |
| ? navigation.filter((item) => hasMinimumRole(role, item.minRole)) | |
| : navigation.filter((item) => item.minRole === 'viewer'); | |
| let visibleNav: typeof navigation; | |
| // Determine which navigation items should be visible based on loading state and user role. | |
| // While loading or when the role cannot be determined, default to viewer-only items to | |
| // avoid briefly showing admin-only links. | |
| if (loading) { | |
| visibleNav = navigation.filter((item) => item.minRole === 'viewer'); | |
| } else if (role !== null) { | |
| visibleNav = navigation.filter((item) => hasMinimumRole(role, item.minRole)); | |
| } else { | |
| // Role is null (e.g., error fetching role); fall back to the safest viewer-only set. | |
| visibleNav = navigation.filter((item) => item.minRole === 'viewer'); | |
| } |
| { params }: { params: Promise<{ guildId: string }> }, | ||
| ): Promise<NextResponse<{ role: DashboardRole } | { error: string }>> { | ||
| const { guildId } = await params; |
There was a problem hiding this comment.
In Next.js route handlers, params is typically a plain object, not a Promise. Typing it as Promise<...> is misleading and can confuse future refactors; prefer { params: { guildId: string } } and remove the await.
| { params }: { params: Promise<{ guildId: string }> }, | |
| ): Promise<NextResponse<{ role: DashboardRole } | { error: string }>> { | |
| const { guildId } = await params; | |
| { params }: { params: { guildId: string } }, | |
| ): Promise<NextResponse<{ role: DashboardRole } | { error: string }>> { | |
| const { guildId } = params; |
web/src/types/discord.ts
Outdated
| /** Dashboard role for this guild (viewer, moderator, admin, owner). Set by GET /api/guilds. */ | ||
| export type GuildAccessRole = 'viewer' | 'moderator' | 'admin' | 'owner'; | ||
|
|
||
| export interface MutualGuild extends DiscordGuild { | ||
| botPresent: boolean; | ||
| /** User's dashboard role in this guild; present when loaded from GET /api/guilds */ | ||
| access?: GuildAccessRole; |
There was a problem hiding this comment.
GuildAccessRole duplicates DashboardRole (same string union) and risks drifting over time. Consider reusing a single exported type (e.g., import/re-export DashboardRole here, or define the union once and reuse it in both places) so the client and authorization logic stay aligned.
web/src/lib/dashboard-roles.ts
Outdated
| * Check if the user has at least the required dashboard role. | ||
| * Returns false if either role is invalid or not in the hierarchy. |
There was a problem hiding this comment.
The docstring claims it returns false for “invalid/unknown roles”, but the function signature only accepts DashboardRole, so “invalid roles” can’t be passed without casts. Either (a) broaden the accepted types to string (keeping the runtime guard meaningful), or (b) adjust the docstring to reflect the actual typed contract.
| * Check if the user has at least the required dashboard role. | |
| * Returns false if either role is invalid or not in the hierarchy. | |
| * Determines whether a user's dashboard role meets or exceeds the required dashboard role. | |
| * | |
| * @param userRole The user's current dashboard role. | |
| * @param requiredRole The minimum dashboard role required to satisfy the check. | |
| * @returns `true` if {@link userRole} is at least {@link requiredRole} in the hierarchy; otherwise `false`. | |
| * If a non-standard value is forced into this function (e.g., via casts), it is treated as not | |
| * satisfying the requirement and will result in `false`. |
|
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
web/src/hooks/use-guild-role.ts (1)
42-44:⚠️ Potential issue | 🟡 MinorMissing role in successful response is silently ignored.
When the API returns a 200 response but
data.roleis undefined (e.g., malformed response), the hook silently returns without settingerror: true. This leaves the state as{ role: null, loading: false, error: false }, making it impossible for consumers to distinguish between "loading" and "failed".Per the API contract in
web/src/app/api/guilds/[guildId]/role/route.ts, success responses always includerole, but defensive handling would improve robustness.🛡️ Proposed fix
.then((data) => { - if (cancelled || !data?.role) return; - setRole(data.role); + if (cancelled) return; + if (!data?.role) { + setError(true); + return; + } + setRole(data.role); })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/hooks/use-guild-role.ts` around lines 42 - 44, In the .then handler of the fetch in useGuildRole (the promise chain where you call setRole), add defensive handling for a 200 response that lacks data.role: when (!data?.role) setError(true) and ensure setLoading(false) is called (and avoid early return that leaves loading/error unchanged); update the logic around the existing setRole(data.role) call so valid responses setRole and clear error, while malformed success responses setError(true) and setLoading(false) so consumers can distinguish failure from loading.
🤖 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/layout/sidebar.tsx`:
- Around line 58-66: The conditional assigning visibleNav is redundant: both the
loading||error branch and the else branch set visibleNav to viewerNavigation;
simplify by replacing the if/else chain with a single check — if role !== null
then set visibleNav = navigation.filter(item => hasMinimumRole(role,
item.minRole)) otherwise set visibleNav = viewerNavigation; update references to
visibleNav, navigation, role, loading, error, viewerNavigation, and
hasMinimumRole accordingly and remove the unused loading||error branch.
In `@web/src/hooks/use-guild-role.ts`:
- Around line 6-8: Update the GuildRoleResponse type to reflect the API's
possible error shape (e.g., make it a union of { role: DashboardRole } and {
error: string }) and then narrow/guard the parsed response where it's used in
use-guild-role.ts (the fetch/response parsing and the block that currently
throws on non-OK) so callers check for response.error before accessing
response.role; reference the GuildRoleResponse and DashboardRole types and the
fetch/response-parsing logic to locate where to apply the type change and
runtime check.
In `@web/src/lib/dashboard-roles.ts`:
- Around line 54-59: In hasMinimumRole, remove the redundant type assertions "as
keyof typeof DASHBOARD_ROLE_ORDER" when indexing DASHBOARD_ROLE_ORDER with
userRole and requiredRole (both already typed as DashboardRole); simply use
DASHBOARD_ROLE_ORDER[userRole] and DASHBOARD_ROLE_ORDER[requiredRole], keeping
the existing undefined checks and the final comparison (a >= b) intact to
preserve runtime safety.
---
Duplicate comments:
In `@web/src/hooks/use-guild-role.ts`:
- Around line 42-44: In the .then handler of the fetch in useGuildRole (the
promise chain where you call setRole), add defensive handling for a 200 response
that lacks data.role: when (!data?.role) setError(true) and ensure
setLoading(false) is called (and avoid early return that leaves loading/error
unchanged); update the logic around the existing setRole(data.role) call so
valid responses setRole and clear error, while malformed success responses
setError(true) and setLoading(false) so consumers can distinguish failure from
loading.
🪄 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: c509971a-5c77-4ae2-93b3-136779354317
📒 Files selected for processing (4)
web/src/components/layout/sidebar.tsxweb/src/hooks/use-guild-role.tsweb/src/lib/dashboard-roles.tsweb/src/types/discord.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 (3)
**/*.{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/types/discord.tsweb/src/hooks/use-guild-role.tsweb/src/lib/dashboard-roles.tsweb/src/components/layout/sidebar.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/types/discord.tsweb/src/hooks/use-guild-role.tsweb/src/lib/dashboard-roles.tsweb/src/components/layout/sidebar.tsx
**/*.{js,ts,tsx,mjs}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,tsx,mjs}: Use ESM syntax (import/export) — CommonJS is not allowed
Use single quotes for strings — double quotes only allowed in JSON files
Always include semicolons at end of statements
Use 2-space indentation for all code
Use Winston logger fromsrc/logger.js— never useconsole.*methods
Files:
web/src/types/discord.tsweb/src/hooks/use-guild-role.tsweb/src/lib/dashboard-roles.tsweb/src/components/layout/sidebar.tsx
🧠 Learnings (12)
📓 Common learnings
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T20:36:29.483Z
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-10T20:36:29.483Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T20:36:29.483Z
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/types/discord.tsweb/src/hooks/use-guild-role.tsweb/src/components/layout/sidebar.tsx
📚 Learning: 2026-03-10T20:36:29.483Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T20:36:29.483Z
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/types/discord.tsweb/src/lib/dashboard-roles.tsweb/src/components/layout/sidebar.tsx
📚 Learning: 2026-03-10T20:36:29.483Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T20:36:29.483Z
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/src/types/discord.ts
📚 Learning: 2026-03-12T02:03:52.689Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-12T02:03:52.689Z
Learning: Applies to web/src/app/**/*.{ts,tsx} : Use `DashboardTitleSync` component and `getDashboardDocumentTitle()` for client-side navigation title updates in the dashboard
Applied to files:
web/src/hooks/use-guild-role.tsweb/src/lib/dashboard-roles.tsweb/src/components/layout/sidebar.tsx
📚 Learning: 2026-03-10T20:36:29.483Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T20:36:29.483Z
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/lib/dashboard-roles.tsweb/src/components/layout/sidebar.tsx
📚 Learning: 2026-03-12T02:03:36.476Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.476Z
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/src/lib/dashboard-roles.tsweb/src/components/layout/sidebar.tsx
📚 Learning: 2026-03-10T20:36:29.483Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T20:36:29.483Z
Learning: Applies to web/src/app/**/*.{ts,tsx} : Apply static metadata to server-rendered dashboard entry pages and use title template format for root app metadata
Applied to files:
web/src/lib/dashboard-roles.ts
📚 Learning: 2026-03-12T02:03:52.689Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-12T02:03:52.689Z
Learning: Applies to web/src/app/**/*.{ts,tsx} : Export `metadata` using `createPageMetadata()` from `web/src/lib/page-titles.ts` in SSR entry points for dashboard pages
Applied to files:
web/src/lib/dashboard-roles.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/src/lib/dashboard-roles.tsweb/src/components/layout/sidebar.tsx
📚 Learning: 2026-03-12T02:03:52.689Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-12T02:03:52.689Z
Learning: Add a matcher entry to `dashboardTitleMatchers` in `web/src/lib/page-titles.ts` for every new dashboard route
Applied to files:
web/src/lib/dashboard-roles.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/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
🪛 GitHub Check: SonarCloud Code Analysis
web/src/lib/dashboard-roles.ts
[warning] 56-56: This assertion is unnecessary since it does not change the type of the expression.
[warning] 55-55: This assertion is unnecessary since it does not change the type of the expression.
🔇 Additional comments (5)
web/src/lib/dashboard-roles.ts (1)
32-46: LGTM!The role resolution logic correctly maps Discord permissions to dashboard roles with proper precedence (owner → admin → moderator → viewer). The bitwise checks and fallback behavior are sound.
web/src/types/discord.ts (1)
1-1: LGTM!Good approach using
DashboardRoleas the canonical source forGuildAccessRole. This keeps role definitions synchronized and avoids the duplication flagged in previous reviews.Also applies to: 18-24
web/src/hooks/use-guild-role.ts (1)
31-34: LGTM on state reset and cleanup.Good job addressing the previous review feedback—resetting
roletonullon line 32 whenguildIdchanges prevents stale role display, and the cancellation flag properly prevents state updates after unmount.Also applies to: 56-58
web/src/components/layout/sidebar.tsx (2)
55-56: LGTM on fail-closed behavior.Good implementation of fail-closed authorization—when
erroris true or role is unavailable, the sidebar correctly restricts navigation to viewer-only items rather than exposing privileged UI elements. This addresses the previous review feedback.Also applies to: 60-66
96-101: LGTM!Nice UX touch displaying the user's role at the bottom of the sidebar. The conditional rendering ensures it only appears when the role is successfully loaded.
| let visibleNav: typeof navigation; | ||
|
|
||
| if (role !== null) { | ||
| visibleNav = navigation.filter((item) => hasMinimumRole(role, item.minRole)); | ||
| } else if (loading || error) { | ||
| visibleNav = viewerNavigation; | ||
| } else { | ||
| visibleNav = viewerNavigation; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Simplify redundant conditional branches.
Both the loading || error branch and the else branch return viewerNavigation. This can be simplified.
♻️ Proposed simplification
let visibleNav: typeof navigation;
- if (role !== null) {
+ if (role) {
visibleNav = navigation.filter((item) => hasMinimumRole(role, item.minRole));
- } else if (loading || error) {
- visibleNav = viewerNavigation;
} else {
visibleNav = viewerNavigation;
}📝 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.
| let visibleNav: typeof navigation; | |
| if (role !== null) { | |
| visibleNav = navigation.filter((item) => hasMinimumRole(role, item.minRole)); | |
| } else if (loading || error) { | |
| visibleNav = viewerNavigation; | |
| } else { | |
| visibleNav = viewerNavigation; | |
| } | |
| let visibleNav: typeof navigation; | |
| if (role) { | |
| visibleNav = navigation.filter((item) => hasMinimumRole(role, item.minRole)); | |
| } else { | |
| visibleNav = viewerNavigation; | |
| } |
🤖 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 58 - 66, The conditional
assigning visibleNav is redundant: both the loading||error branch and the else
branch set visibleNav to viewerNavigation; simplify by replacing the if/else
chain with a single check — if role !== null then set visibleNav =
navigation.filter(item => hasMinimumRole(role, item.minRole)) otherwise set
visibleNav = viewerNavigation; update references to visibleNav, navigation,
role, loading, error, viewerNavigation, and hasMinimumRole accordingly and
remove the unused loading||error branch.
| interface GuildRoleResponse { | ||
| role: DashboardRole; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Response type doesn't account for API error shape.
The GuildRoleResponse interface assumes success, but the API can return { error: string } on failure. While non-OK responses throw on line 39, typing the response more accurately could help catch issues if the API behavior changes.
♻️ Optional: more precise response typing
-interface GuildRoleResponse {
- role: DashboardRole;
-}
+interface GuildRoleResponse {
+ role?: DashboardRole;
+ error?: string;
+}📝 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.
| interface GuildRoleResponse { | |
| role: DashboardRole; | |
| } | |
| interface GuildRoleResponse { | |
| role?: DashboardRole; | |
| error?: string; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/hooks/use-guild-role.ts` around lines 6 - 8, Update the
GuildRoleResponse type to reflect the API's possible error shape (e.g., make it
a union of { role: DashboardRole } and { error: string }) and then narrow/guard
the parsed response where it's used in use-guild-role.ts (the fetch/response
parsing and the block that currently throws on non-OK) so callers check for
response.error before accessing response.role; reference the GuildRoleResponse
and DashboardRole types and the fetch/response-parsing logic to locate where to
apply the type change and runtime check.
| export function hasMinimumRole(userRole: DashboardRole, requiredRole: DashboardRole): boolean { | ||
| const a = DASHBOARD_ROLE_ORDER[userRole as keyof typeof DASHBOARD_ROLE_ORDER]; | ||
| const b = DASHBOARD_ROLE_ORDER[requiredRole as keyof typeof DASHBOARD_ROLE_ORDER]; | ||
| if (a === undefined || b === undefined) return false; | ||
| return a >= b; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Remove redundant type assertions flagged by static analysis.
The as keyof typeof DASHBOARD_ROLE_ORDER casts on lines 55-56 are unnecessary since userRole and requiredRole are already typed as DashboardRole, which is defined as keyof typeof DASHBOARD_ROLE_ORDER. The runtime safety for invalid values is already handled by the undefined check on line 57.
♻️ Proposed fix
export function hasMinimumRole(userRole: DashboardRole, requiredRole: DashboardRole): boolean {
- const a = DASHBOARD_ROLE_ORDER[userRole as keyof typeof DASHBOARD_ROLE_ORDER];
- const b = DASHBOARD_ROLE_ORDER[requiredRole as keyof typeof DASHBOARD_ROLE_ORDER];
+ const a = DASHBOARD_ROLE_ORDER[userRole];
+ const b = DASHBOARD_ROLE_ORDER[requiredRole];
if (a === undefined || b === undefined) return false;
return a >= b;
}📝 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.
| export function hasMinimumRole(userRole: DashboardRole, requiredRole: DashboardRole): boolean { | |
| const a = DASHBOARD_ROLE_ORDER[userRole as keyof typeof DASHBOARD_ROLE_ORDER]; | |
| const b = DASHBOARD_ROLE_ORDER[requiredRole as keyof typeof DASHBOARD_ROLE_ORDER]; | |
| if (a === undefined || b === undefined) return false; | |
| return a >= b; | |
| } | |
| export function hasMinimumRole(userRole: DashboardRole, requiredRole: DashboardRole): boolean { | |
| const a = DASHBOARD_ROLE_ORDER[userRole]; | |
| const b = DASHBOARD_ROLE_ORDER[requiredRole]; | |
| if (a === undefined || b === undefined) return false; | |
| return a >= b; | |
| } |
🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis
[warning] 56-56: This assertion is unnecessary since it does not change the type of the expression.
[warning] 55-55: This assertion is unnecessary since it does not change the type of the expression.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/lib/dashboard-roles.ts` around lines 54 - 59, In hasMinimumRole,
remove the redundant type assertions "as keyof typeof DASHBOARD_ROLE_ORDER" when
indexing DASHBOARD_ROLE_ORDER with userRole and requiredRole (both already typed
as DashboardRole); simply use DASHBOARD_ROLE_ORDER[userRole] and
DASHBOARD_ROLE_ORDER[requiredRole], keeping the existing undefined checks and
the final comparison (a >= b) intact to preserve runtime safety.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 24 changed files in this pull request and generated 3 comments.
You can also share your feedback on Copilot code review. Take the survey.
| function isOAuthGuildModerator(user, guildId) { | ||
| return hasOAuthGuildPermission(user, guildId, ADMINISTRATOR_FLAG | MANAGE_GUILD_FLAG); | ||
| } | ||
| const VALID_ROLES = ['viewer', 'moderator', 'admin', 'owner']; |
There was a problem hiding this comment.
VALID_ROLES duplicates the canonical role list already defined in src/api/utils/dashboardRoles.js (exported as ROLES). Keeping two separate sources of truth makes it easy for the middleware validation to drift from the actual role hierarchy. Consider importing and reusing the shared constant (or deriving from DASHBOARD_ROLE_ORDER) instead of hardcoding the array here.
| let cancelled = false; | ||
| setRole(null); | ||
| setLoading(true); | ||
| setError(false); | ||
|
|
||
| fetch(`/api/guilds/${encodeURIComponent(guildId)}/role`) | ||
| .then((res) => { | ||
| if (cancelled) return null; | ||
| if (!res.ok) throw new Error(res.statusText); | ||
| return res.json() as Promise<GuildRoleResponse>; | ||
| }) | ||
| .then((data) => { | ||
| if (cancelled || !data?.role) return; | ||
| setRole(data.role); | ||
| }) | ||
| .catch(() => { | ||
| if (!cancelled) { | ||
| setError(true); | ||
| setRole(null); | ||
| } | ||
| }) | ||
| .finally(() => { | ||
| if (!cancelled) setLoading(false); | ||
| }); | ||
|
|
||
| return () => { | ||
| cancelled = true; |
There was a problem hiding this comment.
This hook’s fetch doesn’t follow the cancellation / caching pattern used by the other dashboard hooks (AbortController + cache: 'no-store'). Without an abort signal, quick guild switching can leave an in-flight request that resolves late and races UI state; without no-store, the browser may reuse a cached role response after a permission change. Consider adding an AbortController (aborting on guildId change/unmount) and setting cache: 'no-store' on the request.
| let cancelled = false; | |
| setRole(null); | |
| setLoading(true); | |
| setError(false); | |
| fetch(`/api/guilds/${encodeURIComponent(guildId)}/role`) | |
| .then((res) => { | |
| if (cancelled) return null; | |
| if (!res.ok) throw new Error(res.statusText); | |
| return res.json() as Promise<GuildRoleResponse>; | |
| }) | |
| .then((data) => { | |
| if (cancelled || !data?.role) return; | |
| setRole(data.role); | |
| }) | |
| .catch(() => { | |
| if (!cancelled) { | |
| setError(true); | |
| setRole(null); | |
| } | |
| }) | |
| .finally(() => { | |
| if (!cancelled) setLoading(false); | |
| }); | |
| return () => { | |
| cancelled = true; | |
| const abortController = new AbortController(); | |
| let cancelled = false; | |
| setRole(null); | |
| setLoading(true); | |
| setError(false); | |
| fetch(`/api/guilds/${encodeURIComponent(guildId)}/role`, { | |
| signal: abortController.signal, | |
| cache: 'no-store', | |
| }) | |
| .then((res) => { | |
| if (cancelled) return null; | |
| if (!res.ok) { | |
| throw new Error(res.statusText); | |
| } | |
| return res.json() as Promise<GuildRoleResponse>; | |
| }) | |
| .then((data) => { | |
| if (cancelled || !data?.role) return; | |
| setRole(data.role); | |
| }) | |
| .catch((error: unknown) => { | |
| if (cancelled || abortController.signal.aborted) { | |
| return; | |
| } | |
| // Swallow abort errors; mark other failures. | |
| setError(true); | |
| setRole(null); | |
| }) | |
| .finally(() => { | |
| if (!cancelled && !abortController.signal.aborted) { | |
| setLoading(false); | |
| } | |
| }); | |
| return () => { | |
| cancelled = true; | |
| abortController.abort(); |
| } else if (loading || error) { | ||
| visibleNav = viewerNavigation; | ||
| } else { | ||
| visibleNav = viewerNavigation; | ||
| } |
There was a problem hiding this comment.
The else if (loading || error) and final else branches assign the same value (viewerNavigation). This can be simplified to a single else branch (or computed with a conditional expression) to reduce redundant control flow and make the role-loading logic easier to read.
Describe your changes
Please describe your changes in detail.
web/src/app/api/guilds/route.ts – After getMutualGuilds(), each guild is mapped to include access: getDashboardRole(g.permissions, g.owner) so the client gets a role per guild without an extra request.
web/src/types/discord.ts – MutualGuild now has optional access?: GuildAccessRole and type GuildAccessRole.
src/api/routes/members.js – Importing requireRole and protecting GET /:id/members/:userId/cases with requireRole('moderator') instead of requireGuildAdmin.
web/src/app/api/guilds/[guildId]/members/[userId]/cases/route.ts – Using authorizeGuildRole(request, guildId, 'moderator', LOG_PREFIX) instead of authorizeGuildAdmin.
web/src/lib/dashboard-roles.ts – hasMinimumRole now treats invalid/unknown roles as insufficient: if a === undefined || b === undefined it returns false.
web/src/hooks/use-moderation-stats.ts, use-moderation-cases.ts, use-user-history.ts – For non-OK responses without an error in the payload, a 403 now uses the message: "You don't have permission to view this in this server." instead of a generic “Failed to fetch…” message.
web/src/app/api/performance/route.ts – Comment added that the Performance API is bot-wide (not guild-scoped) and that “viewer” means any authenticated user.
tests/api/routes/guilds.test.js – Viewer test for GET /:id/stats now mocks mockPool.query (two results for the stats queries) so the test no longer gets a 500. All 87 tests pass (dashboard roles + guilds)
Issue ticket number and link
Web dashboard: role-based access control — moderator vs admin permissions #124
What type of PR is this? (check all applicable)