feat: Web Dashboard Shell (Next.js 16, Tailwind v4, Discord OAuth2)#60
feat: Web Dashboard Shell (Next.js 16, Tailwind v4, Discord OAuth2)#60BillChirico merged 83 commits intomainfrom
Conversation
BillChirico
force-pushed
the
feat/web-dashboard-shell
branch
from
67487a6 to
2daf4bf
Compare
|
Claude finished @BillChirico's task in 2m 42s —— View job Claude Code Review: PR #60Status: ✅ Approved Review SummaryCompleted a thorough review of this Web Dashboard Shell PR (83 commits, 66 files, +7063/-86 lines). Key Files Reviewed
Security Assessment
Architecture Assessment
Testing Assessment
VerdictLGTM - This PR implements a solid foundation for the web dashboard with proper security practices, good architecture patterns, and comprehensive testing. |
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a Next.js web dashboard under Changes
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Summary
This is a well-structured foundation for the web dashboard with good test coverage (92% statements). However, there are issues that should be addressed:
🟡 Warnings (Should Fix)
-
Middleware not properly configured (
web/src/proxy.ts)- The
proxy.tsfile exports middleware configuration, but there's nomiddleware.tsfile at the project root. Dashboard routes (/dashboard/*) are not protected by authentication middleware. This is a security concern.
- The
-
Missing token refresh logic (
web/src/lib/auth.ts:35)- Discord tokens expire after 7 days, but there's no refresh logic. Users will need to re-authenticate unnecessarily.
🔵 Nitpicks (Nice to Have)
- Silent error swallowing (
web/src/components/layout/server-selector.tsx:38) - Bot API lacks authentication (
web/src/lib/discord.ts:41) - Missing rate limit handling (
web/src/lib/discord.ts:19) - Optional lockfile glob (
web/Dockerfile:8)
✅ What's Good
- Clean TypeScript code with proper type definitions
- Comprehensive test suite with 80% coverage thresholds
- Proper auth flow with JWT sessions
- Good Dockerfile with multi-stage build and non-root user
- Well-documented env vars in
.env.example - No hardcoded credentials or secrets
- Responsive layout with mobile support
Required Action
Please fix the middleware configuration to ensure dashboard routes are protected. The current implementation allows unauthenticated access to /dashboard/* pages.
There was a problem hiding this comment.
Review Summary
This is a solid foundation for the web dashboard with good architecture, comprehensive tests, and proper authentication flow. However, I've identified several issues that should be addressed:
🟡 Warnings (2)
-
No token refresh logic (
web/src/lib/auth.ts:25-35): The JWT callback storesrefreshTokenandaccessTokenExpiresbut never uses them to refresh expired tokens. This will cause silent failures after the Discord access token expires. -
Dockerfile context documentation (
web/Dockerfile:8): The Dockerfile assumes it's run from within theweb/directory, which may not be obvious in a monorepo setup.
🔵 Nitpicks (3)
-
Missing bot API authentication (
web/src/lib/discord.ts:32-51): The bot API endpoint call has no authentication, which could be a security concern if exposed. -
Guild selection not persisted (
web/src/components/layout/server-selector.tsx:24): The selected guild is lost on page refresh. Consider URL state or localStorage. -
Proxy naming convention (
web/src/proxy.ts:3): The file exportsproxyinstead of the conventionalmiddlewarefor Next.js.
✅ What's Good
- Clean TypeScript with proper type definitions
- Good test coverage (56 tests across 13 files)
- Proper NextAuth.js integration with Discord OAuth2
- Well-structured component hierarchy
- Responsive layout with mobile support
- Secure session handling with JWT strategy
- Good error handling in API routes
- Proper environment variable documentation
Please address the warnings before merging. The nitpicks are optional but would improve the codebase.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Code Review: Approved
I've completed a comprehensive review of PR #60: Web Dashboard Shell.
Summary
This PR introduces a well-architected Next.js 16 web dashboard with Discord OAuth2 authentication. The implementation follows security best practices and includes thorough test coverage.
Security
- ✅ Access tokens are properly kept server-side (JWT only, not exposed to client session)
- ✅ Token refresh errors are handled gracefully with session invalidation
- ✅ NEXTAUTH_SECRET validation rejects weak/placeholder values
- ✅ Open redirect protection on login callback URLs
- ✅ Security headers configured (X-Frame-Options, HSTS, nosniff, Referrer-Policy)
- ✅ AbortController cleanup prevents memory leaks
- ✅ Bot API authentication requires shared secret
Architecture
- ✅ Clean separation of server/client code with
server-onlyimport - ✅ Proper TypeScript types for Discord API and NextAuth
- ✅ Rate limit handling with abort-aware retry logic
- ✅ Guild pagination for users in 200+ servers
- ✅ Graceful degradation when bot API is unavailable
- ✅ Error boundaries at root and dashboard levels
Testing
- ✅ 80% coverage threshold enforced
- ✅ Tests cover auth flows, token refresh, API routes, and components
- ✅ Proper env var isolation to prevent test pollution
Documentation
- ✅ README updated with web dashboard section
- ✅ Environment variables documented
- ✅ Setup instructions provided
- ✅ .env.example includes all required variables
Minor Notes
- 🔵 Left one nitpick comment on the Dockerfile build context documentation
The code is production-ready. Well done! 🎉
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
Verify each finding against the current code and only fix it if needed.
In `@web/next.config.ts`:
- Around line 26-34: The images.remotePatterns entry currently allows any path
on cdn.discordapp.com (images.remotePatterns -> pathname: "/**"); tighten this
by replacing the broad pathname with scoped patterns such as "/avatars/**",
"/icons/**", and "/embed/**" to limit allowed Discord assets, and add additional
remotePatterns entries for GitHub (hostname: "avatars.githubusercontent.com"
and/or "raw.githubusercontent.com") with appropriate pathnames to permit
GitHub-sourced images per project guidelines; update the images.remotePatterns
array (the same symbol) to include these narrowed Discord patterns and the new
GitHub patterns so Next.js Image only allows the intended remote image sources.
In `@web/src/lib/auth.ts`:
- Around line 136-141: The callback currently returns an expired token unchanged
when expiresAt has passed and token.refreshToken is falsy; update the logic in
the function handling token refresh (the branch that currently returns token and
calls refreshDiscordToken(token)) to detect expired tokens without a
refreshToken and set an error flag on the returned token (e.g., token.error =
"RefreshAccessTokenError" or similar) so downstream code can detect stale
credentials and redirect to re-auth; keep using refreshDiscordToken(token as
Record<string, unknown>) when a refreshToken exists but ensure the no-refresh
path mutates/returns the token with the error property set.
- Around line 48-50: The code unsafely casts token.refreshToken to string and
may send "undefined" to Discord; update the refresh flow (where
token.refreshToken is used—e.g., the function handling the refresh request that
builds grant_type: "refresh_token") to first assert the refresh token exists
(check token.refreshToken !== undefined/null/empty) and if missing throw or
return a clear error before constructing the request body, so you never cast or
send an undefined refresh token to Discord.
In `@web/tests/components/layout/header.test.tsx`:
- Around line 60-71: Test is currently asserting the Skeleton via a CSS class
(".animate-pulse"), which is brittle; add a stable test id to the Skeleton
component (e.g., data-testid="skeleton" on the Skeleton render) and update the
header test (the "loading state" spec in header.test.tsx that uses
mockUseSession and renders <Header />) to assert the skeleton via
screen.getByTestId("skeleton") or screen.queryByTestId("skeleton") instead of
container.querySelector(".animate-pulse"); this keeps the test resilient to
styling changes while keeping the same behavior.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
web/next.config.tsweb/src/lib/auth.tsweb/src/lib/discord.server.tsweb/tests/api/guilds.test.tsweb/tests/components/layout/header.test.tsx
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:57:34.920Z
Learning: Applies to src/app/**/* : Use Next.js 16 App Router with `src/app` directory structure
📚 Learning: 2025-11-26T01:57:34.920Z
Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:57:34.920Z
Learning: Applies to next.config.ts : Configure Next.js Image component in `next.config.ts` to allow remote patterns for GitHub images
Applied to files:
web/next.config.ts
📚 Learning: 2025-11-26T01:57:34.920Z
Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:57:34.920Z
Learning: Applies to src/app/blog/**/*.{ts,tsx} : Use `next-mdx-remote/rsc` for server-side markdown rendering with syntax highlighting via `rehype-highlight`
Applied to files:
web/next.config.ts
📚 Learning: 2026-02-16T17:06:27.423Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-16T17:06:27.423Z
Learning: Applies to src/commands/*.js : Enforce Discord timeout maximum of 28 days and slowmode maximum of 6 hours (21600 seconds) in command logic
Applied to files:
web/src/lib/auth.tsweb/src/lib/discord.server.ts
📚 Learning: 2026-02-04T02:20:09.131Z
Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-04T02:20:09.131Z
Learning: Applies to src/**/*.{ts,tsx} : Use `reportError(context, error)` from `src/lib/logger.ts` to report errors to Sentry with context metadata, falling back to console.error if Sentry is disabled
Applied to files:
web/src/lib/auth.ts
📚 Learning: 2026-02-16T17:06:27.423Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-16T17:06:27.423Z
Learning: Applies to src/**/*.js : Always log errors with context before re-throwing
Applied to files:
web/src/lib/auth.ts
📚 Learning: 2026-02-16T17:06:27.423Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-16T17:06:27.423Z
Learning: Applies to src/**/*.js : Always use Winston for logging via `import { info, warn, error } from '../logger.js'`, never use `console.log()`, `console.warn()`, `console.error()`, or any `console.*` methods in src/ files
Applied to files:
web/src/lib/auth.ts
📚 Learning: 2026-02-16T17:06:27.423Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-16T17:06:27.423Z
Learning: Applies to src/index.js : Ensure Discord intents are enabled for MessageContent, GuildMembers, and GuildVoiceStates on the bot client
Applied to files:
web/src/lib/auth.tsweb/src/lib/discord.server.ts
📚 Learning: 2025-11-26T01:57:34.920Z
Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:57:34.920Z
Learning: Applies to src/app/**/* : Use Next.js 16 App Router with `src/app` directory structure
Applied to files:
web/src/lib/auth.ts
🧬 Code graph analysis (3)
web/tests/components/layout/header.test.tsx (1)
web/src/components/layout/header.tsx (1)
Header(20-108)
web/tests/api/guilds.test.ts (1)
web/src/app/api/guilds/route.ts (1)
GET(12-38)
web/src/lib/auth.ts (2)
web/src/lib/logger.ts (1)
logger(19-24)src/logger.js (1)
error(230-232)
⏰ 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: claude-review
- GitHub Check: Cursor Bugbot
- GitHub Check: claude-review
🔇 Additional comments (15)
web/next.config.ts (2)
3-22: Security headers look solid.The TODO properly acknowledges the nonce-based CSP deferral. The four headers (clickjacking, MIME-sniffing, referrer, HSTS) are well-configured with appropriate values for a dashboard application.
35-46: Headers and export are correctly structured.The
async headers()properly applies security headers to all routes via the/(.*)source pattern, and the default export follows Next.js conventions.web/tests/components/layout/header.test.tsx (5)
1-36: Well-structured mock setup.Hoisted
vi.fn()references with per-test overrides viamockReturnValueis the right pattern for testing components with different session states. The authenticated session fixture is clean and reusable.
38-58: LGTM — basic rendering assertions are correct.
clearAllMocksinbeforeEachwith a default authenticated session is a solid baseline.
73-83: LGTM — unauthenticated branch is well-covered.Asserting both the link's presence and its
hrefattribute is thorough.
85-104: Good coverage of the RefreshTokenError flow.The mock places
erroron the session data object, matching the component'ssession?.errorcheck. The guard test (line 100–103) ensuringsignOutis not called in the normal case is a valuable negative assertion.
106-139: Good interaction tests distinguishing both sign-out paths.The dropdown tests correctly verify
callbackUrl: "/"(manual sign-out via dropdown) vs the RefreshTokenError test'scallbackUrl: "/login", covering both code paths in the component. UsinguserEvent.setup()withwaitForis the right approach for async Radix dropdown rendering.web/src/lib/auth.ts (2)
7-14: Solid runtime validation for secrets and credentials.The module-level validation for
NEXTAUTH_SECRETwith length and placeholder checks, plus the Discord credential checks, ensures clear startup failures. TheBOT_API_SECRETwarning is also a good operational safeguard.
44-100: Well-structured token refresh with good error handling.Network errors, non-JSON responses, and response shape validation are all covered. The refresh token rotation logic correctly falls back to the existing token.
web/src/lib/discord.server.ts (4)
19-63: Rate-limit retry with abort-aware sleep is well-implemented.The loop bounds, retry-after parsing with sensible fallback, abort signal handling, and clean exit conditions all look correct.
70-128: Paginated guild fetch with proper safety checks.JSON parse guard,
Array.isArrayvalidation, cursor-based pagination, and abort signal forwarding are all in place. Good implementation.
142-195: DiscriminatedBotGuildResultcleanly separates "unavailable" from "empty".The
availableflag, env var guards, and comprehensive error handling (non-OK response, network error, JSON shape) are solid.
202-235:fetchUserGuildsrejection crashes the entiregetMutualGuildscall.
fetchBotGuildshas a defensive.catch()wrapper insidePromise.all, butfetchUserGuildsdoes not. A transient Discord API failure (network blip, 5xx) will reject thePromise.alland propagate to the caller. This is arguably correct (no user guilds = nothing to show), but it's worth noting the asymmetry is intentional — the caller (GET /api/guilds) does wrap this in a try/catch returning 500.No change needed; the error path is handled one level up.
web/tests/api/guilds.test.ts (2)
1-46: Clean test setup with proper mock hoisting and env var restoration.The
vi.mock()calls are correctly hoisted, the top-level import ofGETis clean, and theafterEachproperly restoresNEXTAUTH_SECRET. Previous review concerns are addressed.
48-127: Good coverage of the main API route paths.The five test cases cover the key scenarios: no token, missing access token, successful response, refresh token error, and Discord API failure. The
RefreshTokenErrortest correctly asserts thatgetMutualGuildsis not called, matching the early-return guard in the route.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- getGuildIconUrl now returns null when no icon hash is provided instead of incorrectly using /embed/avatars/ (which is for user avatars, not guilds). Discord has no default guild icon CDN path; callers already handle null by showing a placeholder Server icon. - Rename local 'guilds' variable to 'fetchedGuilds' in server-selector to avoid shadowing the component-level useState variable. - Update tests to match new null return behavior.
BillChirico
dismissed stale reviews from coderabbitai[bot] and claude[bot]
via
326e610
There was a problem hiding this comment.
Code Review Summary
I reviewed the web dashboard implementation. CI is passing with tests at 92% coverage (above 80% threshold). Most patterns are solid, but I found one critical issue that needs fixing.
🔴 Critical
Docker Compose build context mismatch (docker-compose.yml:35-36, web/Dockerfile:8-12)
The docker-compose.yml sets context: ./web for the web service, but web/Dockerfile explicitly requires the monorepo root as context:
# docker-compose.yml
web:
build:
context: ./web # ❌ Wrong - Dockerfile needs root context
dockerfile: Dockerfile# web/Dockerfile comment
# Build context: Must be the monorepo root (not web/). The Dockerfile expects
# pnpm-workspace.yaml, pnpm-lock.yaml, and package.json at the root levelThe Dockerfile copies package.json, pnpm-lock.yaml, pnpm-workspace.yaml from root and web/package.json from a subdirectory. With context: ./web, these paths won't exist and the build will fail.
Fix: Change docker-compose.yml to:
web:
build:
context: .
dockerfile: web/Dockerfile🟡 Warnings
-
Missing web/.dockerignore - The root
.dockerignoreexcludesweb/entirely (line 50), but the web service needs its own.dockerignorewhen using a separate context. Since the fix above changes context to root, this becomes less critical, but you may still want a web-specific dockerignore for local builds. -
CSP not implemented (
web/next.config.ts:3-4) - The TODO comment acknowledges this, but shipping without Content-Security-Policy leaves the app more vulnerable to XSS. Consider prioritizing the nonce-based CSP implementation mentioned in the comment.
🔵 Nitpicks
- Health endpoint could include version (
web/src/app/api/health/route.ts) - For debugging deployed versions, consider addingversion: process.env.npm_package_versionor a build-time git SHA to the health response.
What's Good (not changing verdict)
- Clean OAuth2 implementation with proper token refresh and error propagation
- Rate limiting with abort signal support in Discord API calls
- 80% coverage thresholds enforced in vitest.config.ts
- Security headers properly configured (HSTS, X-Frame-Options, etc.)
- Logger abstraction allows future migration to structured logging
- Graceful degradation when bot API is unavailable
Please fix the Docker Compose context mismatch and this can be approved.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| ENV HOSTNAME="0.0.0.0" | ||
|
|
||
| HEALTHCHECK --interval=30s --timeout=5s --start-period=10s --retries=3 \ | ||
| CMD wget --no-verbose --tries=1 --spider http://localhost:3000/api/health || exit 1 |
There was a problem hiding this comment.
Dockerfile HEALTHCHECK references nonexistent /api/health route
High Severity
The Dockerfile HEALTHCHECK pings http://localhost:3000/api/health, but no corresponding Next.js API route exists at web/src/app/api/health/route.ts. The only API routes created by this PR are /api/auth/[...nextauth] and /api/guilds. This means the health check will always receive a 404, causing Docker (and Railway) to mark the container as unhealthy after three retries, potentially triggering restart loops in production.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
Verify each finding against the current code and only fix it if needed.
In `@web/next.config.ts`:
- Around line 1-22: Add a Permissions-Policy header to the existing
securityHeaders array in next.config.ts to explicitly disable unused features;
specifically add an object with key "Permissions-Policy" and value "camera=(),
microphone=(), geolocation=()" to the securityHeaders array so the header is
sent alongside X-Frame-Options, X-Content-Type-Options, Referrer-Policy, and
Strict-Transport-Security.
In `@web/src/components/layout/server-selector.tsx`:
- Around line 167-174: getGuildIconUrl returns string | null but Image.src
expects string; update the two places in server-selector.tsx where
getGuildIconUrl is passed to the Image component (the branch using
selectedGuild?.icon and the later branch using guild.icon) to assert the value
is non-null (use a non-null assertion on the getGuildIconUrl call) so TypeScript
treats the return as string for the Image src; keep the existing runtime guards
(selectedGuild?.icon / guild.icon) and only add the non-null assertion to the
getGuildIconUrl invocation.
In `@web/tests/components/layout/header.test.tsx`:
- Around line 83-101: The RefreshTokenError test relies on synchronous flushing
of useEffect; make the "calls signOut when session has RefreshTokenError" spec
more robust by making the test async, rendering <Header /> after setting
mockUseSession to return the error, and wrapping the assertion that mockSignOut
was called with { callbackUrl: "/login" } inside Testing Library's waitFor so
the effect-triggered signOut has time to run; update the test that references
mockUseSession, mockSignOut, and Header accordingly.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (11)
web/Dockerfileweb/next.config.tsweb/src/app/login/page.tsxweb/src/components/layout/header.tsxweb/src/components/layout/server-selector.tsxweb/src/lib/auth.tsweb/src/lib/discord.tsweb/src/proxy.tsweb/tests/app/login.test.tsxweb/tests/components/layout/header.test.tsxweb/tests/lib/discord.test.ts
🧰 Additional context used
🧠 Learnings (19)
📓 Common learnings
Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:57:34.920Z
Learning: Applies to src/app/**/* : Use Next.js 16 App Router with `src/app` directory structure
📚 Learning: 2025-11-26T01:57:34.920Z
Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:57:34.920Z
Learning: Applies to src/app/**/*.{ts,tsx} : Use Server Components by default in `src/app` for data fetching; mark interactive components with `"use client"` directive
Applied to files:
web/src/components/layout/server-selector.tsxweb/src/lib/discord.ts
📚 Learning: 2025-11-26T01:57:34.920Z
Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:57:34.920Z
Learning: Applies to src/app/page.tsx : Separate data fetching (Server Components) from UI interactivity (Client Components), passing data as props from `src/app/page.tsx` to `homepage-client.tsx`
Applied to files:
web/src/components/layout/server-selector.tsxweb/src/app/login/page.tsx
📚 Learning: 2026-02-04T02:20:09.131Z
Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-04T02:20:09.131Z
Learning: Applies to src/**/*.{ts,tsx} : Use `reportError(context, error)` from `src/lib/logger.ts` to report errors to Sentry with context metadata, falling back to console.error if Sentry is disabled
Applied to files:
web/src/components/layout/server-selector.tsxweb/src/lib/auth.tsweb/src/lib/discord.ts
📚 Learning: 2026-02-16T17:06:27.423Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-16T17:06:27.423Z
Learning: Applies to src/**/*.js : Always use Winston for logging via `import { info, warn, error } from '../logger.js'`, never use `console.log()`, `console.warn()`, `console.error()`, or any `console.*` methods in src/ files
Applied to files:
web/src/components/layout/server-selector.tsxweb/src/lib/auth.tsweb/src/lib/discord.ts
📚 Learning: 2025-11-26T01:57:34.920Z
Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:57:34.920Z
Learning: Applies to src/components/**/*.{ts,tsx} : Use `cn()` utility function combining `clsx` and `tailwind-merge` from `src/lib/utils.ts` for conditional class names
Applied to files:
web/src/components/layout/server-selector.tsx
📚 Learning: 2026-02-16T17:06:27.423Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-16T17:06:27.423Z
Learning: Applies to src/index.js : Ensure Discord intents are enabled for MessageContent, GuildMembers, and GuildVoiceStates on the bot client
Applied to files:
web/tests/lib/discord.test.tsweb/src/lib/auth.tsweb/src/lib/discord.ts
📚 Learning: 2025-11-26T01:57:34.920Z
Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:57:34.920Z
Learning: Applies to src/components/**/*.{ts,tsx} : Use `phosphor-icons/react` for Phosphor icons in navigation and social links; use `lucide-react` for Lucide icons in theme toggle and blog components
Applied to files:
web/src/app/login/page.tsx
📚 Learning: 2025-10-10T15:05:26.145Z
Learnt from: CR
Repo: BillChirico/LUA-Obfuscator PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-10T15:05:26.145Z
Learning: Applies to **/*.{jsx,tsx} : Use React hooks for state management in the web UI
Applied to files:
web/src/app/login/page.tsx
📚 Learning: 2025-11-26T01:57:34.920Z
Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:57:34.920Z
Learning: Applies to next.config.ts : Configure Next.js Image component in `next.config.ts` to allow remote patterns for GitHub images
Applied to files:
web/next.config.ts
📚 Learning: 2025-11-26T01:57:34.920Z
Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:57:34.920Z
Learning: Applies to src/app/blog/**/*.{ts,tsx} : Use `next-mdx-remote/rsc` for server-side markdown rendering with syntax highlighting via `rehype-highlight`
Applied to files:
web/next.config.ts
📚 Learning: 2026-02-16T17:06:27.423Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-16T17:06:27.423Z
Learning: Applies to src/commands/*.js : Enforce Discord timeout maximum of 28 days and slowmode maximum of 6 hours (21600 seconds) in command logic
Applied to files:
web/src/lib/auth.tsweb/src/lib/discord.ts
📚 Learning: 2026-02-16T17:06:27.423Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-16T17:06:27.423Z
Learning: Applies to src/**/*.js : Always log errors with context before re-throwing
Applied to files:
web/src/lib/auth.ts
📚 Learning: 2025-11-26T01:57:34.920Z
Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:57:34.920Z
Learning: Applies to src/app/**/* : Use Next.js 16 App Router with `src/app` directory structure
Applied to files:
web/src/lib/auth.tsweb/src/proxy.ts
📚 Learning: 2026-02-04T02:20:09.131Z
Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-04T02:20:09.131Z
Learning: Applies to {package.json,.npmrc,pnpm-lock.yaml} : Use pnpm package manager (pinned to v10.23.0) with `.npmrc` configured for strict peer dependencies and disabled shamefully-hoist
Applied to files:
web/Dockerfile
📚 Learning: 2025-11-26T01:57:34.920Z
Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:57:34.920Z
Learning: Applies to **/*.{ts,tsx,js,jsx,json,css,md} : After changing or editing any files, run the complete validation workflow: `pnpm format && pnpm typecheck && pnpm lint && pnpm build` before committing
Applied to files:
web/Dockerfile
📚 Learning: 2025-10-10T15:05:26.145Z
Learnt from: CR
Repo: BillChirico/LUA-Obfuscator PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-10T15:05:26.145Z
Learning: Applies to package.json : Only add new packages when absolutely necessary or explicitly requested
Applied to files:
web/Dockerfile
📚 Learning: 2025-11-26T01:57:34.920Z
Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:57:34.920Z
Learning: Applies to src/components/**/*.{ts,tsx} : Use `canvas-confetti` package to trigger celebratory confetti animations on Discord link click
Applied to files:
web/src/lib/discord.ts
📚 Learning: 2026-02-16T17:06:27.423Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-16T17:06:27.423Z
Learning: Applies to src/**/*.js : Use `splitMessage()` utility to split messages exceeding Discord's 2000-character limit
Applied to files:
web/src/lib/discord.ts
🧬 Code graph analysis (6)
web/src/components/layout/header.tsx (2)
web/src/lib/auth.ts (1)
session(155-168)web/src/components/layout/mobile-sidebar.tsx (1)
MobileSidebar(14-42)
web/tests/lib/discord.test.ts (2)
web/src/lib/discord.ts (1)
getGuildIconUrl(33-41)web/src/lib/discord.server.ts (4)
fetchWithRateLimit(19-63)fetchUserGuilds(70-128)fetchBotGuilds(142-195)getMutualGuilds(202-235)
web/src/lib/auth.ts (1)
web/src/lib/logger.ts (1)
logger(19-24)
web/tests/app/login.test.tsx (1)
web/src/app/login/page.tsx (1)
LoginPage(87-99)
web/tests/components/layout/header.test.tsx (1)
web/src/components/layout/header.tsx (1)
Header(20-108)
web/src/lib/discord.ts (1)
src/deploy-commands.js (1)
clientId(25-25)
🪛 Trivy (0.69.1)
web/Dockerfile
[info] 1-1: No HEALTHCHECK defined
Add HEALTHCHECK instruction in your Dockerfile
Rule: DS-0026
(IaC/Dockerfile)
⏰ 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: Cursor Bugbot
- GitHub Check: Cursor Bugbot
- GitHub Check: claude-review
- GitHub Check: claude-review
🔇 Additional comments (16)
web/Dockerfile (2)
1-14: LGTM — deps stage is well structured.Proper use of cache mount for pnpm store, frozen lockfile for reproducibility, and root
package.jsonis copied for corepack version pinning.
36-63: Runner stage looks solid.Non-root user, proper
--chown, standalone output layout, HEALTHCHECK targeting/api/health, and correctHOSTNAME=0.0.0.0for container networking. The Trivy hint about missing HEALTHCHECK is a false positive — it's present at Lines 60–61.web/next.config.ts (1)
24-46: LGTM — config is clean and well-scoped.
standaloneoutput aligns with the Dockerfile's multi-stage build, and Discord CDN patterns are properly restricted to/{avatars,icons,embed}/**.web/src/proxy.ts (1)
1-28: LGTM!Clean route protection implementation. Good use of
request.nextUrl.pathname + request.nextUrl.searchfor preserving the full callback URL, proper RefreshTokenError handling, and well-scoped matcher config.web/src/lib/discord.ts (1)
1-41: LGTM!Well-documented permissions breakdown with verification expression. Both utility functions handle edge cases cleanly —
getBotInviteUrlreturnsnullwhen unconfigured, andgetGuildIconUrlcorrectly distinguishes animated vs. static icons and returnsnullfor missing hashes.web/src/components/layout/server-selector.tsx (2)
42-97: Well-structured abort and loading guard pattern.The
loadGuildsfunction properly aborts previous in-flight requests, guards against stalefinallyblocks via controller identity check, handles 401 with a redirect, and validates response shape. This addresses all prior review feedback cleanly.
131-157: Good empty-state UX with invite CTA and fallback messaging.The empty state now clearly communicates "Bill Bot isn't in any of your Discord servers" with an invite button when the client ID is configured, and a helpful fallback message when it's not. This addresses the prior feedback about distinguishing empty states.
web/tests/lib/discord.test.ts (1)
1-533: Thorough and well-structured test suite.Good coverage across all Discord utility functions. The env var cleanup pattern in
beforeEach/afterEachis exception-safe, fake timers are used for rate-limit tests, and URL-based mock dispatching ingetMutualGuildstests is clean. The pagination test forfetchUserGuildsproperly validates theaftercursor parameter.web/src/lib/auth.ts (2)
44-105: Solid token refresh implementation with comprehensive error handling.All prior feedback has been addressed: network errors caught, JSON parsing guarded, response shape validated, missing refresh token handled, and token rotation supported. The guard on lines 45–48 makes the
as stringcast on line 54 redundant but harmless.
119-154: JWT callback handles all token lifecycle states correctly.Good coverage of: initial sign-in with default expiry fallback, early return for non-expired tokens, refresh attempt when expired, and error flag when no refresh token is available. The security comment about token exposure (lines 121–126) is a helpful clarification.
web/src/app/login/page.tsx (1)
15-85: Clean login flow with good security and UX considerations.The
callbackUrlvalidation (lines 22–25) correctly prevents open redirects. TheRefreshTokenErrorcarve-out (lines 29–33) properly defers to the Header's centralized sign-out handler. The loading/spinner logic (line 41) avoids the flash-of-login-form for already-authenticated users.web/tests/app/login.test.tsx (1)
1-107: Good test coverage for the login flow.Tests cover all key paths: rendering, default and custom callbackUrl, privacy note, RefreshTokenError (verifying no signOut race), and authenticated redirect. Mock shapes correctly align with the session structure from auth callbacks.
web/src/components/layout/header.tsx (2)
20-35: Good centralized RefreshTokenError handler with duplicate-prevention guard.The
signingOutref prevents multiple sign-out attempts when the session refetches. Note that the ref is never reset — ifsignOutwere to fail without navigating away, subsequent errors would be silently ignored. In practice,signOutredirects the page so the component unmounts, making this a non-issue.
50-104: Clean three-state rendering for session status.Skeleton for loading, sign-in button for unauthenticated, and avatar dropdown for authenticated — all prior feedback addressed. The Documentation link now points to the GitHub repo instead of the Discord Developer Portal, which is more appropriate for end users.
web/tests/components/layout/header.test.tsx (2)
1-42: Well-structured test setup with properly hoisted mocks.The hoisted
vi.fn()declarations beforevi.mock, followed by theHeaderimport, correctly handle vitest's module-mock hoisting semantics. ThebeforeEachreset and mutablemockUseSessionpattern cleanly addresses all prior review feedback about static mocks.
104-137: Dropdown interaction tests are thorough and correctly structured.Good use of
userEvent.setup()andwaitForfor the async dropdown rendering. Both menu visibility and the sign-out action (with distinctcallbackUrl: "/"vs the error-driven"/login") are properly verified.
| import type { NextConfig } from "next"; | ||
|
|
||
| // TODO: Implement nonce-based CSP as a separate task. | ||
| // script-src 'self' without 'unsafe-inline' breaks Next.js RSC streaming/hydration. | ||
| const securityHeaders = [ | ||
| { | ||
| key: "X-Frame-Options", | ||
| value: "DENY", | ||
| }, | ||
| { | ||
| key: "X-Content-Type-Options", | ||
| value: "nosniff", | ||
| }, | ||
| { | ||
| key: "Referrer-Policy", | ||
| value: "strict-origin-when-cross-origin", | ||
| }, | ||
| { | ||
| key: "Strict-Transport-Security", | ||
| value: "max-age=63072000; includeSubDomains; preload", | ||
| }, | ||
| ]; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Good security header baseline; CSP gap is acknowledged.
The four headers provide solid defense-in-depth. The TODO on Lines 3–4 correctly identifies that a nonce-based CSP is needed for Next.js App Router streaming/hydration and defers it as a separate task. This is the right approach — a broken CSP is worse than a missing one.
Consider also adding a Permissions-Policy header to disable unused browser features (camera, microphone, geolocation, etc.) as an additional hardening measure:
{
key: "Permissions-Policy",
value: "camera=(), microphone=(), geolocation=()",
},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/next.config.ts` around lines 1 - 22, Add a Permissions-Policy header to
the existing securityHeaders array in next.config.ts to explicitly disable
unused features; specifically add an object with key "Permissions-Policy" and
value "camera=(), microphone=(), geolocation=()" to the securityHeaders array so
the header is sent alongside X-Frame-Options, X-Content-Type-Options,
Referrer-Policy, and Strict-Transport-Security.
| {selectedGuild?.icon ? ( | ||
| <Image | ||
| src={getGuildIconUrl(selectedGuild.id, selectedGuild.icon, 64)} | ||
| alt={selectedGuild.name} | ||
| width={20} | ||
| height={20} | ||
| className="rounded-full" | ||
| /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the getGuildIconUrl function definition
rg "getGuildIconUrl" --type ts --type tsx -B 2 -A 10Repository: BillChirico/bills-bot
Length of output: 92
🏁 Script executed:
# Check tsconfig.json for strict null checks
fd "tsconfig" --type f -x cat {}Repository: BillChirico/bills-bot
Length of output: 766
🏁 Script executed:
# Read the server-selector.tsx file to see the actual code at lines 167-174 and 196
wc -l web/src/components/layout/server-selector.tsxRepository: BillChirico/bills-bot
Length of output: 114
🏁 Script executed:
# Look for Image import and usage in server-selector.tsx
rg "from.*Image|import.*Image" web/src/components/layout/server-selector.tsx -A 2Repository: BillChirico/bills-bot
Length of output: 216
🏁 Script executed:
# Find getGuildIconUrl function - try without type filter
rg "getGuildIconUrl" -B 2 -A 10Repository: BillChirico/bills-bot
Length of output: 6768
🏁 Script executed:
# Read the server-selector.tsx file completely
cat -n web/src/components/layout/server-selector.tsxRepository: BillChirico/bills-bot
Length of output: 8573
getGuildIconUrl returns string | null but Image.src requires string.
Although the selectedGuild?.icon and guild.icon guards ensure the return value is a string at runtime, TypeScript's strict mode does not narrow the return type of getGuildIconUrl. This causes a type error at line 169 and line 196. Add a non-null assertion to both calls.
Proposed fix
src={getGuildIconUrl(selectedGuild.id, selectedGuild.icon, 64)!}And similarly on Line 196:
src={getGuildIconUrl(guild.id, guild.icon, 64)!}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/layout/server-selector.tsx` around lines 167 - 174,
getGuildIconUrl returns string | null but Image.src expects string; update the
two places in server-selector.tsx where getGuildIconUrl is passed to the Image
component (the branch using selectedGuild?.icon and the later branch using
guild.icon) to assert the value is non-null (use a non-null assertion on the
getGuildIconUrl call) so TypeScript treats the return as string for the Image
src; keep the existing runtime guards (selectedGuild?.icon / guild.icon) and
only add the non-null assertion to the getGuildIconUrl invocation.
| describe("RefreshTokenError", () => { | ||
| it("calls signOut when session has RefreshTokenError", () => { | ||
| mockUseSession.mockReturnValue({ | ||
| data: { | ||
| user: { id: "123", name: "TestUser" }, | ||
| error: "RefreshTokenError", | ||
| }, | ||
| status: "authenticated", | ||
| }); | ||
|
|
||
| render(<Header />); | ||
|
|
||
| expect(mockSignOut).toHaveBeenCalledWith({ callbackUrl: "/login" }); | ||
| }); | ||
|
|
||
| it("does not call signOut when session has no error", () => { | ||
| render(<Header />); | ||
| expect(mockSignOut).not.toHaveBeenCalled(); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
RefreshTokenError assertion relies on useEffect flushing synchronously within render.
This works because Testing Library's render wraps in act, which flushes effects. However, if this ever becomes flaky, wrap the assertion in waitFor for resilience:
render(<Header />);
- expect(mockSignOut).toHaveBeenCalledWith({ callbackUrl: "/login" });
+ await waitFor(() => {
+ expect(mockSignOut).toHaveBeenCalledWith({ callbackUrl: "/login" });
+ });The it callback would also need to be async. This is a minor robustness suggestion — the current form is valid.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/tests/components/layout/header.test.tsx` around lines 83 - 101, The
RefreshTokenError test relies on synchronous flushing of useEffect; make the
"calls signOut when session has RefreshTokenError" spec more robust by making
the test async, rendering <Header /> after setting mockUseSession to return the
error, and wrapping the assertion that mockSignOut was called with {
callbackUrl: "/login" } inside Testing Library's waitFor so the effect-triggered
signOut has time to run; update the test that references mockUseSession,
mockSignOut, and Header accordingly.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| const response = await fetch("/api/guilds", { signal: controller.signal }); | ||
| if (response.status === 401) { | ||
| // Auth failure — redirect to login instead of showing a misleading retry | ||
| window.location.href = "/login"; |
There was a problem hiding this comment.
Brief incorrect UI flash during 401 redirect
Low Severity
When the /api/guilds fetch returns a 401, loadGuilds sets window.location.href = "/login" and returns early from the try block. However, the finally block still runs and sets loading to false. Since setGuilds was never called, the component re-renders with loading=false, error=false, and guilds=[], briefly displaying the "No mutual servers" empty state (with the "Invite Bill Bot" button) before the browser completes the redirect to /login. The finally block doesn't account for the in-progress redirect scenario.
Additional Locations (1)
| href: "/dashboard/settings", | ||
| icon: Settings, | ||
| }, | ||
| ]; |
There was a problem hiding this comment.
Sidebar links to non-existent dashboard routes show 404
Low Severity
The navigation array defines links to /dashboard/moderation, /dashboard/ai, /dashboard/members, /dashboard/config, and /dashboard/settings, but none of these routes have corresponding page components. Only /dashboard has a page.tsx. Clicking any of these sidebar links results in a Next.js 404 page, which is a confusing experience for users exploring the dashboard. Disabling or visually marking unimplemented links as "coming soon" would avoid this.
2 participants
Summary
Adds the foundation for the bills-bot web dashboard — a Next.js app with Discord OAuth2 authentication, responsive layout, and server management capabilities.
Closes #28
What's Included
Stack
@themeblock, notailwind.config)Features
/api/guildsfetches mutual guilds (user ∩ bot)Infrastructure
web/as workspace member)railway.tomlwith health check).env.exampleand.env.local.exampleTesting
Key Files
web/package.jsonweb/src/app/page.tsxweb/src/app/login/page.tsxweb/src/app/dashboard/web/src/components/layout/web/src/lib/auth.tsweb/src/lib/discord.tsweb/src/proxy.tspnpm-workspace.yamlEnvironment Variables
How to Test