Repository navigation
fix(frontend): stabilize dashboard server visibility and e2e shell contracts - #169
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
✅ Deploy Preview for regal-bunny-0c8efe ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (22)
📝 WalkthroughWalkthroughThis PR enhances guild authorization and error handling across backend and frontend. Changes include: Discord OAuth payload normalization supporting dual-permission representations, granular error mapping for guild access failures, improved guild context resolution with per-guild error tolerance, new guild load error classification with retry logic, server selector component with distinct error states and auto-selection, and expanded guild state with RBAC fields across the stack. Changes
Sequence DiagramsequenceDiagram
participant User as User/Client
participant Store as Guild Store
participant API as Backend API
participant Discord as Discord API
User->>Store: Navigate/Auth Ready
Store->>Store: Set isLoading = true
Store->>API: GET /api/guilds
API->>Discord: Fetch guilds for user
Discord-->>API: Error (401/403/429/5xx)
API->>API: mapGuildAccessError
API-->>Store: AppError (401/403/502)
Store->>Store: classifyGuildLoadError
Store->>Store: Set guildLoadError (auth/forbidden/upstream)
Store->>Store: Set isLoading = false
User->>Store: View error + Retry/Re-auth CTA
User->>Store: Click Retry
Store->>API: GET /api/guilds (retry)
API->>Discord: Fetch guilds for user
Discord-->>API: OK + Guild List
API->>API: normalizeGuildPayload
API-->>Store: Guilds with botAdded flags
Store->>Store: Auto-select first botAdded guild
Store->>Store: Clear guildLoadError
User->>User: Dashboard initialized with guild
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate 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 |
|
Size Change: +564 B (+0.19%) Total Size: 297 kB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/frontend/tests/e2e/helpers/page-helpers.ts (1)
82-89:⚠️ Potential issue | 🟠 MajorWait for dashboard content, not the always-present nav label.
The sidebar already contains "Dashboard", so this helper can resolve before the main panel has reached the loaded, empty, or error state. Scope the wait to the main content heading/empty state instead; otherwise the auth and server-selection flows here stay racy.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/tests/e2e/helpers/page-helpers.ts` around lines 82 - 89, The waitForDashboard helper currently waits for global text which can match the persistent sidebar; update waitForDashboard to wait for the main panel's content (e.g., scope the selector to the main content container or role="main") rather than global text. Specifically, change the selector used in waitForSelector/waitFor to target the main content heading/empty-state (for example by awaiting a locator under <main> or role="main" that matches /Dashboard|Select a Server|Access denied/i) so the helper only resolves once the central panel has loaded its heading/error/empty states.packages/frontend/src/stores/guildStore.test.ts (1)
61-70:⚠️ Potential issue | 🟡 MinorReset
guildLoadErrorin the shared test setup.This reset block omits the new error field, so any case that sets
guildLoadErrorcan leak that state into the next test and make the suite order-dependent. AddguildLoadError: nullhere so each case starts from the same store shape.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/src/stores/guildStore.test.ts` around lines 61 - 70, The shared test setup for useGuildStore resets many fields but omits the new guildLoadError field, causing state leakage between tests; update the reset call to include guildLoadError: null so useGuildStore.setState({...}) resets that error as well (look for the useGuildStore.setState invocation in guildStore.test.ts and add the guildLoadError property to the object alongside selectedGuild, serverSettings, etc.).
🧹 Nitpick comments (5)
packages/frontend/src/hooks/useGuildSelection.ts (1)
15-41: Collapse the overlappingfetchGuilds()effects.Both effects drive the same network call from closely related auth state, which makes the retry window harder to reason about than it needs to be. Merging the auth-ready load and auth-error retry into one branch would simplify the flow and help bring this hook back toward the repo’s size/complexity target.
As per coding guidelines: `**/*.{ts,tsx}`: Functions must be less than 50 lines with cyclomatic complexity less than 10.♻️ Possible simplification
- useEffect(() => { - if (!isAuthenticated || authLoading) { - return - } - - fetchGuilds() - }, [fetchGuilds, isAuthenticated, authLoading]) - - useEffect(() => { - if (!isAuthenticated || authLoading) { - return - } - - if (hasRetriedAuthReadyFetch.current) { - return - } - - if ( - guildLoadError?.kind !== 'auth' && - guildLoadError?.kind !== 'forbidden' - ) { - return - } - - hasRetriedAuthReadyFetch.current = true - fetchGuilds() - }, [authLoading, fetchGuilds, guildLoadError, isAuthenticated]) + useEffect(() => { + if (!isAuthenticated || authLoading) { + hasRetriedAuthReadyFetch.current = false + return + } + + const shouldRetry = + guildLoadError?.kind === 'auth' || + guildLoadError?.kind === 'forbidden' + + if (shouldRetry && hasRetriedAuthReadyFetch.current) { + return + } + + hasRetriedAuthReadyFetch.current = shouldRetry + void fetchGuilds() + }, [authLoading, fetchGuilds, guildLoadError, isAuthenticated]) - - useEffect(() => { - if (!isAuthenticated) { - hasRetriedAuthReadyFetch.current = false - } - }, [isAuthenticated])Also applies to: 52-56
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/src/hooks/useGuildSelection.ts` around lines 15 - 41, The two useEffect blocks calling fetchGuilds should be merged into a single useEffect to remove overlapping triggers: collapse the logic currently split between the effects that depend on isAuthenticated/authLoading and the retry-on-auth-error flow into one effect that runs when [fetchGuilds, isAuthenticated, authLoading, guildLoadError] change; inside it, early-return if !isAuthenticated || authLoading, then if guildLoadError?.kind is 'auth' or 'forbidden' use hasRetriedAuthReadyFetch.current to guard a single retry (set it to true before calling fetchGuilds), otherwise call fetchGuilds once for the normal auth-ready path—this keeps the same hasRetriedAuthReadyFetch behavior and removes duplicate network calls while keeping conditions tied to useEffect dependencies.packages/backend/tests/unit/services/DiscordOAuthService.test.ts (1)
291-307: Good coverage forpermissions_newfallback.This test verifies that
filterAdminGuildscorrectly usespermissions_newwhenpermissionslacks admin rights. Consider adding a complementary test wherepermissionshas admin rights butpermissions_newis'0'to verify the prioritization logic doesn't regress.🧪 Additional test case suggestion
test('should prioritize permissions_new over permissions', () => { const guilds = [ { ...MOCK_DISCORD_GUILDS[0], permissions: '8', // admin in old field permissions_new: '0', // no admin in new field } as (typeof MOCK_DISCORD_GUILDS)[number] & { permissions_new: string }, ] const result = discordOAuthService.filterAdminGuilds(guilds) // permissions_new takes precedence, so no admin expect(result).toHaveLength(0) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/backend/tests/unit/services/DiscordOAuthService.test.ts` around lines 291 - 307, Add a complementary unit test to ensure filterAdminGuilds prioritizes permissions_new over permissions: create a test case in packages/backend/tests/unit/services/DiscordOAuthService.test.ts that supplies a guild with permissions set to an admin value (e.g., '8') and permissions_new set to '0', call discordOAuthService.filterAdminGuilds with that input, and assert the result excludes the guild (length 0); this verifies the precedence logic in filterAdminGuilds.packages/backend/tests/unit/routes/index.test.ts (1)
114-120: Duplicate assertion for 'automation' module - consider clarifying intent.Lines 115 and 119 both assert
requireGuildModuleAccess.toHaveBeenCalledWith('automation'). If this is intentional because multiple routes now use the 'automation' module (commands/automessages AND features), consider usingtoHaveBeenCalledTimes(2)or adding a brief comment to clarify.♻️ Suggested clarification
expect(requireGuildModuleAccess).toHaveBeenCalledWith('moderation') - expect(requireGuildModuleAccess).toHaveBeenCalledWith('automation') + // automation module used for commands, automessages, AND features routes + expect(requireGuildModuleAccess).toHaveBeenCalledWith('automation') expect(requireGuildModuleAccess).toHaveBeenCalledWith('music') expect(requireGuildModuleAccess).toHaveBeenCalledWith('integrations') expect(requireGuildModuleAccess).toHaveBeenCalledWith('settings') - expect(requireGuildModuleAccess).toHaveBeenCalledWith('automation')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/backend/tests/unit/routes/index.test.ts` around lines 114 - 120, The test contains a duplicate assertion calling requireGuildModuleAccess.toHaveBeenCalledWith('automation') twice; update the assertions in tests/unit/routes/index.test.ts to make intent explicit by either removing the duplicate, replacing the two identical expects with expect(requireGuildModuleAccess).toHaveBeenCalledTimes(2) when two routes legitimately require the 'automation' module, or keep both but add a short comment clarifying that two separate routes use 'automation' (and ensure other related assertions like requireGuildModuleAccess.toHaveBeenCalledWith('settings', 'manage') remain intact).packages/backend/src/services/DiscordOAuthService.ts (1)
23-32: Consider addingretryablemetadata toDiscordApiError.The error class structure is good with
statusCodeandendpoint. Per coding guidelines, consider adding aretryableproperty to help callers determine retry behavior based on status code.♻️ Suggested enhancement
export class DiscordApiError extends Error { constructor( message: string, public readonly statusCode: number, public readonly endpoint: string, ) { super(message) this.name = 'DiscordApiError' } + + get retryable(): boolean { + return this.statusCode >= 500 || this.statusCode === 429 + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/backend/src/services/DiscordOAuthService.ts` around lines 23 - 32, Extend the DiscordApiError class to include a public readonly retryable boolean so callers can decide retry behavior; in the DiscordApiError constructor (class name: DiscordApiError) either accept an optional retryable parameter or derive retryable from the statusCode (e.g., true for 429 and 5xx ranges, false for 4xx other than 429), assign it to this.retryable, and keep the rest of the constructor (message, statusCode, endpoint, name) unchanged so existing throw sites still work.packages/frontend/tests/e2e/dashboard-page.spec.ts (1)
169-186: Test does not verify loading state behavior.The test "shows loading states during server fetch" sets up a delayed API response but doesn't assert that any loading indicator is visible. The test just waits 500ms and completes without verifying the intended behavior.
Consider adding assertions for the loading state:
🧪 Suggested improvement
test('shows loading states during server fetch', async ({ page }) => { let delayed = false await page.route('**/api/guilds', async (route) => { if (!delayed) { delayed = true await new Promise((resolve) => setTimeout(resolve, 1000)) } await route.fulfill({ status: 200, contentType: 'application/json', body: JSON.stringify(MOCK_API_RESPONSES.guildsList), }) }) await navigateToDashboard(page) - await page.waitForTimeout(500) + const loadingIndicator = page + .locator('[class*="skeleton"], [class*="Skeleton"], [class*="loading"]') + .first() + const isVisible = await loadingIndicator + .isVisible({ timeout: 2000 }) + .catch(() => false) + + if (isVisible) { + await expect(loadingIndicator).toBeVisible() + } })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/tests/e2e/dashboard-page.spec.ts` around lines 169 - 186, The test "shows loading states during server fetch" sets up a delayed response via page.route and calls navigateToDashboard but never asserts the loading indicator; update the test to assert loading UI appears while the mocked /api/guilds request is delayed and then disappears after the response is fulfilled. Specifically, using the existing page.route and navigateToDashboard flow, add an assertion (e.g., expect(page.locator('<loading-selector>')).toBeVisible()) during the delay (before route.fulfill completes) and then assert it's gone after the request finishes (e.g., expect(...).toBeHidden()) — reference the test name "shows loading states during server fetch", the navigateToDashboard helper, page.route, and MOCK_API_RESPONSES.guildsList to locate and modify the test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Around line 46-63: Update the unreleased changelog entry by appending " (PR
`#169`)" or a dedicated reference line for PR `#169` to the existing bullet list
that begins with "Dashboard guild authorization now tolerates per-guild context
failures" so the entry cites the originating PR; ensure any breaking changes or
linked issues mentioned in the bullets also include their PR/issue references
and add a short "Refs: PR `#169`" line under the unreleased header for
discoverability.
In `@packages/backend/src/services/GuildAccessService.ts`:
- Around line 162-180: Currently all buildContext failures are swallowed
resulting in a 200 [] when every guild context resolution fails; modify
GuildAccessService so that while you still catch per-guild buildContext errors
for partial degradation, you detect the case where every context is null (e.g.,
track a boolean or collect errors during the guilds.map try/catch) and in that
case throw a mapped error (with a generic, non-sensitive message) instead of
returning an empty list; update the logic around buildContext and the subsequent
isAuthorized filter to rethrow or return an appropriate error response when no
contexts successfully resolved so callers can return a retryable HTTP error.
In `@packages/frontend/src/components/Layout/Sidebar.tsx`:
- Around line 310-315: Replace the hardcoded href '/api/auth/discord' with a
computed backend URL using the same resolver the app uses for API/auth calls
(e.g., build the URL as `${API_ORIGIN}/api/auth/discord` where API_ORIGIN is
sourced from the shared helper or env var like NEXT_PUBLIC_API_ORIGIN); import
that helper via the frontend path alias (e.g., from '@/utils' or '@/config') and
use it in the Sidebar component to construct the anchor href so split-origin
deployments redirect to the backend auth endpoint instead of the frontend host.
In `@packages/frontend/src/hooks/useGuildSelection.test.tsx`:
- Around line 65-76: The test currently preloads guildState.guildLoadError which
only proves the hook sees an auth error at mount; change it to simulate the
auth-error occurring on the first fetch and a successful retry afterwards by
changing the fetchGuilds mock so the first call returns an auth error and the
second call succeeds (e.g., fetchGuilds.mockImplementationOnce(() =>
Promise.reject({ kind: 'auth', ... })) and then mockImplementationOnce(() =>
Promise.resolve(...))) or alternatively trigger guildState.guildLoadError after
the first render to emulate the post-fetch auth transition; ensure you still
renderHook(() => useGuildSelection()) and assert fetchGuilds was called twice
and that the second call happens after the simulated auth-ready transition.
In `@packages/frontend/src/stores/guildStore.ts`:
- Around line 83-121: fetchGuilds currently writes state unconditionally which
allows an older failing request to overwrite a newer successful response; fix
this by implementing a request version/token check: add a numeric requestId
(e.g., currentGuildRequestId) to the guild store state, increment it at the
start of fetchGuilds, capture the id in a local variable, then only perform
set(...) for both the success path and the catch path if the captured id ===
get().currentGuildRequestId; reference fetchGuilds, get(), set(), and
classifyGuildLoadError when gating the writes so superseded requests cannot
clobber the store.
In `@packages/frontend/tests/e2e/fixtures/test-data.ts`:
- Around line 49-51: The guild fixtures in MOCK_GUILDS (used to build
MOCK_API_RESPONSES.guildsList) only set hasBot and not the runtime flag
botAdded, so useGuildSelection's auto-select path (which checks guild.botAdded)
is never exercised; update each relevant guild fixture object (the entries
around the current block and the other two similar blocks) to add botAdded: true
where hasBot: true (and false where appropriate) so the mocked guild list
includes the runtime botAdded flag and E2E selection behaves correctly.
In `@packages/frontend/tests/e2e/helpers/api-helpers.ts`:
- Around line 285-294: The write mock for toggles is only registered for
MOCK_GUILDS[0], so add the per-guild write mock inside the loop that registers
the other guild-scoped mocks: inside the for (const guild of MOCK_GUILDS) loop
call await mockToggleUpdate(page, false, guild.id) (and/or await
mockToggleUpdate(page, true, guild.id) if needed) for each guild, and remove or
adjust the post-loop calls that only target MOCK_GUILDS[0] so every guild gets
its toggle-write mock; locate this change where MOCK_GUILDS and mockToggleUpdate
are used in setupMockApiResponses.
---
Outside diff comments:
In `@packages/frontend/src/stores/guildStore.test.ts`:
- Around line 61-70: The shared test setup for useGuildStore resets many fields
but omits the new guildLoadError field, causing state leakage between tests;
update the reset call to include guildLoadError: null so
useGuildStore.setState({...}) resets that error as well (look for the
useGuildStore.setState invocation in guildStore.test.ts and add the
guildLoadError property to the object alongside selectedGuild, serverSettings,
etc.).
In `@packages/frontend/tests/e2e/helpers/page-helpers.ts`:
- Around line 82-89: The waitForDashboard helper currently waits for global text
which can match the persistent sidebar; update waitForDashboard to wait for the
main panel's content (e.g., scope the selector to the main content container or
role="main") rather than global text. Specifically, change the selector used in
waitForSelector/waitFor to target the main content heading/empty-state (for
example by awaiting a locator under <main> or role="main" that matches
/Dashboard|Select a Server|Access denied/i) so the helper only resolves once the
central panel has loaded its heading/error/empty states.
---
Nitpick comments:
In `@packages/backend/src/services/DiscordOAuthService.ts`:
- Around line 23-32: Extend the DiscordApiError class to include a public
readonly retryable boolean so callers can decide retry behavior; in the
DiscordApiError constructor (class name: DiscordApiError) either accept an
optional retryable parameter or derive retryable from the statusCode (e.g., true
for 429 and 5xx ranges, false for 4xx other than 429), assign it to
this.retryable, and keep the rest of the constructor (message, statusCode,
endpoint, name) unchanged so existing throw sites still work.
In `@packages/backend/tests/unit/routes/index.test.ts`:
- Around line 114-120: The test contains a duplicate assertion calling
requireGuildModuleAccess.toHaveBeenCalledWith('automation') twice; update the
assertions in tests/unit/routes/index.test.ts to make intent explicit by either
removing the duplicate, replacing the two identical expects with
expect(requireGuildModuleAccess).toHaveBeenCalledTimes(2) when two routes
legitimately require the 'automation' module, or keep both but add a short
comment clarifying that two separate routes use 'automation' (and ensure other
related assertions like
requireGuildModuleAccess.toHaveBeenCalledWith('settings', 'manage') remain
intact).
In `@packages/backend/tests/unit/services/DiscordOAuthService.test.ts`:
- Around line 291-307: Add a complementary unit test to ensure filterAdminGuilds
prioritizes permissions_new over permissions: create a test case in
packages/backend/tests/unit/services/DiscordOAuthService.test.ts that supplies a
guild with permissions set to an admin value (e.g., '8') and permissions_new set
to '0', call discordOAuthService.filterAdminGuilds with that input, and assert
the result excludes the guild (length 0); this verifies the precedence logic in
filterAdminGuilds.
In `@packages/frontend/src/hooks/useGuildSelection.ts`:
- Around line 15-41: The two useEffect blocks calling fetchGuilds should be
merged into a single useEffect to remove overlapping triggers: collapse the
logic currently split between the effects that depend on
isAuthenticated/authLoading and the retry-on-auth-error flow into one effect
that runs when [fetchGuilds, isAuthenticated, authLoading, guildLoadError]
change; inside it, early-return if !isAuthenticated || authLoading, then if
guildLoadError?.kind is 'auth' or 'forbidden' use
hasRetriedAuthReadyFetch.current to guard a single retry (set it to true before
calling fetchGuilds), otherwise call fetchGuilds once for the normal auth-ready
path—this keeps the same hasRetriedAuthReadyFetch behavior and removes duplicate
network calls while keeping conditions tied to useEffect dependencies.
In `@packages/frontend/tests/e2e/dashboard-page.spec.ts`:
- Around line 169-186: The test "shows loading states during server fetch" sets
up a delayed response via page.route and calls navigateToDashboard but never
asserts the loading indicator; update the test to assert loading UI appears
while the mocked /api/guilds request is delayed and then disappears after the
response is fulfilled. Specifically, using the existing page.route and
navigateToDashboard flow, add an assertion (e.g.,
expect(page.locator('<loading-selector>')).toBeVisible()) during the delay
(before route.fulfill completes) and then assert it's gone after the request
finishes (e.g., expect(...).toBeHidden()) — reference the test name "shows
loading states during server fetch", the navigateToDashboard helper, page.route,
and MOCK_API_RESPONSES.guildsList to locate and modify the test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 063ed8e3-12f9-417c-9ffe-8051d355b2dc
📒 Files selected for processing (25)
CHANGELOG.mdREADME.mdpackages/backend/src/routes/guilds.tspackages/backend/src/routes/index.tspackages/backend/src/services/DiscordOAuthService.tspackages/backend/src/services/GuildAccessService.tspackages/backend/tests/integration/routes/guilds.test.tspackages/backend/tests/unit/routes/index.test.tspackages/backend/tests/unit/services/DiscordOAuthService.test.tspackages/backend/tests/unit/services/GuildAccessService.test.tspackages/frontend/src/App.authRoutes.test.tsxpackages/frontend/src/App.tsxpackages/frontend/src/components/Layout/Sidebar.test.tsxpackages/frontend/src/components/Layout/Sidebar.tsxpackages/frontend/src/hooks/useGuildSelection.test.tsxpackages/frontend/src/hooks/useGuildSelection.tspackages/frontend/src/stores/guildStore.test.tspackages/frontend/src/stores/guildStore.tspackages/frontend/tests/e2e/dashboard-page.spec.tspackages/frontend/tests/e2e/fixtures/test-data.tspackages/frontend/tests/e2e/helpers/api-helpers.tspackages/frontend/tests/e2e/helpers/page-helpers.tspackages/frontend/tests/e2e/helpers/ui-helpers.tspackages/frontend/tests/e2e/layout-navigation.spec.tspackages/frontend/tests/e2e/servers-page.spec.ts
|
Follow-up update on dashboard stabilization gates (head:
No code-level regression was found in the local verification path for this branch. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
packages/frontend/src/hooks/useGuildSelection.test.tsx (2)
65-82: Consider adding coverage for'forbidden'error kind.The hook also retries on
guildLoadError?.kind === 'forbidden'(per the implementation). Adding a similar test case withkind: 'forbidden'would ensure both retry paths are covered.💡 Additional test case
test('retries once after forbidden error appears after first auth-ready fetch', async () => { const hook = renderHook(() => useGuildSelection()) await waitFor(() => { expect(fetchGuilds).toHaveBeenCalledTimes(1) }) guildState.guildLoadError = { kind: 'forbidden', message: 'Access denied', status: 403, } hook.rerender() await waitFor(() => { expect(fetchGuilds).toHaveBeenCalledTimes(2) }) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/src/hooks/useGuildSelection.test.tsx` around lines 65 - 82, Add a parallel test to cover the 'forbidden' retry path in useGuildSelection: duplicate the existing test that sets guildState.guildLoadError after the initial fetch, but set kind: 'forbidden' (message like "Access denied", status 403), rerender the hook, and assert fetchGuilds is called a second time; reference the same symbols used in the current test (useGuildSelection, fetchGuilds, guildState.guildLoadError, renderHook, waitFor) so both 'auth' and 'forbidden' retry behaviors are validated.
10-21: Type aliases should useTprefix per coding guidelines.The local type aliases
GuildStateandAuthStateshould follow theT{Name}naming convention.♻️ Suggested rename
-type GuildState = { +type TGuildState = { guilds: Array<{ id: string; botAdded: boolean }> selectedGuild: { id: string; botAdded: boolean } | null fetchGuilds: () => Promise<void> selectGuild: (guild: { id: string; botAdded: boolean }) => void guildLoadError: { kind: string; message: string; status?: number } | null } -type AuthState = { +type TAuthState = { isAuthenticated: boolean isLoading: boolean }Also update references in
beforeEach(lines 47-54) to use the renamed types.As per coding guidelines, "
**/*.{ts,tsx}: UseT{Name}naming convention for type aliases and utility types in TypeScript".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/src/hooks/useGuildSelection.test.tsx` around lines 10 - 21, Rename the local type aliases GuildState and AuthState to follow the T{Name} convention (TGuildState and TAuthState), then update all usages and references (including the types used in the beforeEach test setup that currently reference GuildState and AuthState) to use TGuildState and TAuthState so the file consistently follows the coding guideline.packages/backend/tests/unit/services/GuildAccessService.test.ts (1)
226-236: Consider adding a comment explaining the intentional use of plain objects.Lines 227-230 and 239-242 use plain objects instead of
DiscordApiErrorinstances. This appears intentional to test theextractStatusCodefallback paths that check forstatusCodeandstatusproperties on arbitrary error objects. A brief comment would clarify this is deliberate testing of the fallback behavior.📝 Suggested clarifying comment
test('listAuthorizedGuilds maps Discord 403 errors to forbidden AppError', async () => { + // Uses plain object to test extractStatusCode fallback for non-DiscordApiError mockGetUserGuilds.mockRejectedValue({ statusCode: 403, message: 'missing scope', })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/backend/tests/unit/services/GuildAccessService.test.ts` around lines 226 - 236, Add a brief inline comment in the test 'listAuthorizedGuilds maps Discord 403 errors to forbidden AppError' noting that mockGetUserGuilds is intentionally rejecting with plain objects (having statusCode/status) rather than DiscordApiError instances to exercise the extractStatusCode fallback logic; place the comment next to the mockRejectedValue call for mockGetUserGuilds (and mirror it where similar plain-object errors are used) so future readers know this is deliberate test coverage for GuildAccessService.listAuthorizedGuilds and extractStatusCode.packages/backend/src/services/GuildAccessService.ts (2)
91-144: Consider extracting member context resolution to reduce function length.The
buildContextmethod is 54 lines, slightly exceeding the 50-line guideline. The member context resolution logic (lines 113-125) could be extracted into a small private helper to improve readability and stay within limits.♻️ Suggested extraction
+ private async resolveMemberContext( + guildId: string, + userId: string, + hasBot: boolean, + isAdmin: boolean, + ): Promise<{ nickname: string | null; roleIds: string[] }> { + if (!hasBot || isAdmin) { + return { nickname: null, roleIds: [] } + } + + return guildService + .getGuildMemberContext(guildId, userId) + .catch((error) => { + errorLog({ + message: 'Failed to resolve guild member context', + error, + data: { guildId, userId }, + }) + return { nickname: null, roleIds: [] as string[] } + }) + } private async buildContext( guild: DiscordGuild, userId: string, ): Promise<GuildAccessContext> { // ... isAdmin and hasBot resolution ... - const memberContext = - hasBot && !isAdmin - ? await guildService - .getGuildMemberContext(guild.id, userId) - .catch((error) => { - errorLog({ - message: 'Failed to resolve guild member context', - error, - data: { guildId: guild.id, userId }, - }) - return { nickname: null, roleIds: [] as string[] } - }) - : { nickname: null, roleIds: [] as string[] } + const memberContext = await this.resolveMemberContext( + guild.id, + userId, + hasBot, + isAdmin, + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/backend/src/services/GuildAccessService.ts` around lines 91 - 144, The member context resolution inside buildContext should be extracted into a small private helper to shorten buildContext; create a private async method (e.g., resolveMemberContext or getMemberContext) that accepts guild.id, userId, hasBot, and isAdmin and contains the conditional call to guildService.getGuildMemberContext and its .catch fallback (the logic currently building memberContext), then replace the inline block in buildContext with a call to that helper and use its returned { nickname, roleIds } for the rest of buildContext (keep names like buildContext, guildService.getGuildMemberContext, memberContext, and resolveEffectiveAccess unchanged so the rest of the method remains intact).
216-233: Consider error handling forresolveGuildContext.Unlike
listAuthorizedGuilds, theresolveGuildContextmethod doesn't wrapbuildContextin try/catch. If RBAC resolution fails, the error will propagate as-is. This may be intentional (fail-fast for single guild context), but consider whether a mapped error would provide better UX consistency with the list endpoint.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/backend/src/services/GuildAccessService.ts` around lines 216 - 233, resolveGuildContext currently calls buildContext without error handling so RBAC resolution errors propagate; update resolveGuildContext to mirror listAuthorizedGuilds by wrapping the buildContext(guild, session.user.id) call in a try/catch, catch any RBAC/build errors, and either return null or throw a mapped/annotated error for consistent UX; reference the resolveGuildContext, buildContext, listAuthorizedGuilds, and isAuthorized symbols when making this change.packages/frontend/tests/e2e/helpers/api-helpers.ts (1)
111-217: Split these guild-specific mocks into a sibling helper module.This block pushes the file to 295 lines, which is over the repo cap and makes
setupMockApiResponses()harder to scan. Moving the guild-scoped routes into a separate helper would keep this file focused on the shared setup flow. As per coding guidelines "Files must not exceed 250 lines and this is enforced."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/tests/e2e/helpers/api-helpers.ts` around lines 111 - 217, Move the four guild-scoped route helpers (mockGuildMemberContext, mockServerListing, mockModerationStats, mockModerationCases) out of the large helpers file into a new sibling module (e.g., guild-api-helpers.ts), export them from that module, and update imports where they are used (including where setupMockApiResponses composes mocks) so the original file drops below the 250-line limit; ensure each moved function preserves its Page and guildId signatures and any references to MOCK_GUILDS or MOCK_API_RESPONSES remain accessible (re-export or import those fixtures in the new module) and run tests to confirm no breakages.packages/frontend/src/components/Layout/Sidebar.tsx (1)
149-336: ExtractServerSelectorfromSidebar.tsx.Lines 149-336 add a second sizeable component to a file that is already far past the repo’s size cap. Moving
ServerSelectorandgetGuildLoadMessageinto their ownpackages/frontend/src/components/Layout/module will keep the shell focused; make the extracted propsReadonlywhile you’re there.As per coding guidelines, "
**/*.{ts,tsx,js,jsx}: Files must not exceed 250 lines and this is enforced", "packages/frontend/src/{components,pages}/**/*.{ts,tsx}: Use React functional components and hooks; keep components small and focused", and "packages/frontend/src/components/**/*.{ts,tsx}: Organize UI components inpackages/frontend/src/components/directory`."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/src/components/Layout/Sidebar.tsx` around lines 149 - 336, Extract the ServerSelector component and the helper getGuildLoadMessage into a new file under packages/frontend/src/components/Layout/ (keep names ServerSelector and getGuildLoadMessage) and import any used UI primitives (Avatar, AvatarImage, AvatarFallback, ChevronDown, AnimatePresence, motion, ScrollArea, AlertTriangle, Sparkles, cn, api) from the original file; update Sidebar.tsx to import ServerSelector from the new module. In the new file, export ServerSelector and make its ServerSelectorProps type readonly by declaring it as Readonly or by marking each prop with readonly (reference the existing ServerSelectorProps interface name) so props are immutable. Ensure the new module preserves the exact behavior and JSX structure, exports default or named export consistent with Sidebar import, and update imports/usages accordingly.packages/frontend/src/stores/guildStore.ts (1)
85-137: SplitfetchGuildsinto smaller helpers.This action now owns request versioning, selection reconciliation, auto-selection, and failure reset. Extracting the selection resolution / final-state writes into small helpers will make branches like the one above easier to reason about and bring the function back toward the repo limit.
As per coding guidelines, "
**/*.{ts,tsx}: Functions must be less than 50 lines with cyclomatic complexity less than 10`."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/src/stores/guildStore.ts` around lines 85 - 137, fetchGuilds is doing too many tasks; extract selection reconciliation and error/reset writes into small helper functions to reduce complexity. Create two helpers (e.g., resolveSelectedGuild(guilds, currentSelectedGuildId) which returns {selectedGuild, selectedGuildId} and handleGuildLoadFailure(error, requestId) which performs the set(...) reset and uses classifyGuildLoadError), then refactor fetchGuilds to call api.guilds.list(), use resolveSelectedGuild to compute nextSelectedGuild and call set(...) with the final success state, and on catch call handleGuildLoadFailure(error, requestId); keep existing requestId/version checks and call get().selectGuild(firstWithBot) only when needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/frontend/src/stores/guildStore.ts`:
- Around line 105-120: When a refresh results in no selected guild and no
fallback `botAdded` guild, clear all guild-scoped state instead of only
`selectedGuild`/`selectedGuildId`: update the block in guildStore (the set(...)
call and the subsequent branch after `if (nextSelectedGuild)`) to also reset
`memberContext`, `memberContextLoading`, `serverSettings`, and `serverListing`
to their initial/empty values; additionally add a regression test that simulates
"selected guild removed + no bot-added guild remains" and asserts those four
fields are cleared (use the same store helpers/methods used elsewhere and the
`get().selectGuild` flow to reproduce the condition).
---
Nitpick comments:
In `@packages/backend/src/services/GuildAccessService.ts`:
- Around line 91-144: The member context resolution inside buildContext should
be extracted into a small private helper to shorten buildContext; create a
private async method (e.g., resolveMemberContext or getMemberContext) that
accepts guild.id, userId, hasBot, and isAdmin and contains the conditional call
to guildService.getGuildMemberContext and its .catch fallback (the logic
currently building memberContext), then replace the inline block in buildContext
with a call to that helper and use its returned { nickname, roleIds } for the
rest of buildContext (keep names like buildContext,
guildService.getGuildMemberContext, memberContext, and resolveEffectiveAccess
unchanged so the rest of the method remains intact).
- Around line 216-233: resolveGuildContext currently calls buildContext without
error handling so RBAC resolution errors propagate; update resolveGuildContext
to mirror listAuthorizedGuilds by wrapping the buildContext(guild,
session.user.id) call in a try/catch, catch any RBAC/build errors, and either
return null or throw a mapped/annotated error for consistent UX; reference the
resolveGuildContext, buildContext, listAuthorizedGuilds, and isAuthorized
symbols when making this change.
In `@packages/backend/tests/unit/services/GuildAccessService.test.ts`:
- Around line 226-236: Add a brief inline comment in the test
'listAuthorizedGuilds maps Discord 403 errors to forbidden AppError' noting that
mockGetUserGuilds is intentionally rejecting with plain objects (having
statusCode/status) rather than DiscordApiError instances to exercise the
extractStatusCode fallback logic; place the comment next to the
mockRejectedValue call for mockGetUserGuilds (and mirror it where similar
plain-object errors are used) so future readers know this is deliberate test
coverage for GuildAccessService.listAuthorizedGuilds and extractStatusCode.
In `@packages/frontend/src/components/Layout/Sidebar.tsx`:
- Around line 149-336: Extract the ServerSelector component and the helper
getGuildLoadMessage into a new file under
packages/frontend/src/components/Layout/ (keep names ServerSelector and
getGuildLoadMessage) and import any used UI primitives (Avatar, AvatarImage,
AvatarFallback, ChevronDown, AnimatePresence, motion, ScrollArea, AlertTriangle,
Sparkles, cn, api) from the original file; update Sidebar.tsx to import
ServerSelector from the new module. In the new file, export ServerSelector and
make its ServerSelectorProps type readonly by declaring it as Readonly or by
marking each prop with readonly (reference the existing ServerSelectorProps
interface name) so props are immutable. Ensure the new module preserves the
exact behavior and JSX structure, exports default or named export consistent
with Sidebar import, and update imports/usages accordingly.
In `@packages/frontend/src/hooks/useGuildSelection.test.tsx`:
- Around line 65-82: Add a parallel test to cover the 'forbidden' retry path in
useGuildSelection: duplicate the existing test that sets
guildState.guildLoadError after the initial fetch, but set kind: 'forbidden'
(message like "Access denied", status 403), rerender the hook, and assert
fetchGuilds is called a second time; reference the same symbols used in the
current test (useGuildSelection, fetchGuilds, guildState.guildLoadError,
renderHook, waitFor) so both 'auth' and 'forbidden' retry behaviors are
validated.
- Around line 10-21: Rename the local type aliases GuildState and AuthState to
follow the T{Name} convention (TGuildState and TAuthState), then update all
usages and references (including the types used in the beforeEach test setup
that currently reference GuildState and AuthState) to use TGuildState and
TAuthState so the file consistently follows the coding guideline.
In `@packages/frontend/src/stores/guildStore.ts`:
- Around line 85-137: fetchGuilds is doing too many tasks; extract selection
reconciliation and error/reset writes into small helper functions to reduce
complexity. Create two helpers (e.g., resolveSelectedGuild(guilds,
currentSelectedGuildId) which returns {selectedGuild, selectedGuildId} and
handleGuildLoadFailure(error, requestId) which performs the set(...) reset and
uses classifyGuildLoadError), then refactor fetchGuilds to call
api.guilds.list(), use resolveSelectedGuild to compute nextSelectedGuild and
call set(...) with the final success state, and on catch call
handleGuildLoadFailure(error, requestId); keep existing requestId/version checks
and call get().selectGuild(firstWithBot) only when needed.
In `@packages/frontend/tests/e2e/helpers/api-helpers.ts`:
- Around line 111-217: Move the four guild-scoped route helpers
(mockGuildMemberContext, mockServerListing, mockModerationStats,
mockModerationCases) out of the large helpers file into a new sibling module
(e.g., guild-api-helpers.ts), export them from that module, and update imports
where they are used (including where setupMockApiResponses composes mocks) so
the original file drops below the 250-line limit; ensure each moved function
preserves its Page and guildId signatures and any references to MOCK_GUILDS or
MOCK_API_RESPONSES remain accessible (re-export or import those fixtures in the
new module) and run tests to confirm no breakages.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: de1d136b-eb0c-4e55-935a-4e9e6779a8f7
📒 Files selected for processing (10)
CHANGELOG.mdpackages/backend/src/services/GuildAccessService.tspackages/backend/tests/unit/services/GuildAccessService.test.tspackages/frontend/src/components/Layout/Sidebar.test.tsxpackages/frontend/src/components/Layout/Sidebar.tsxpackages/frontend/src/hooks/useGuildSelection.test.tsxpackages/frontend/src/stores/guildStore.test.tspackages/frontend/src/stores/guildStore.tspackages/frontend/tests/e2e/fixtures/test-data.tspackages/frontend/tests/e2e/helpers/api-helpers.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/frontend/tests/e2e/fixtures/test-data.ts
- CHANGELOG.md
📜 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: SonarCloud Scan
- GitHub Check: Quality Gates
🧰 Additional context used
📓 Path-based instructions (29)
**/*.{js,jsx,ts,tsx,vue,html}
📄 CodeRabbit inference engine (.cursor/rules/accessibility-openness.mdc)
Provide accessible UI components using semantic HTML and ARIA attributes where necessary
Files:
packages/frontend/src/hooks/useGuildSelection.test.tsxpackages/frontend/src/stores/guildStore.tspackages/backend/tests/unit/services/GuildAccessService.test.tspackages/frontend/src/components/Layout/Sidebar.tsxpackages/frontend/tests/e2e/helpers/api-helpers.tspackages/backend/src/services/GuildAccessService.tspackages/frontend/src/stores/guildStore.test.tspackages/frontend/src/components/Layout/Sidebar.test.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/dependency-injection.mdc)
**/*.{ts,tsx,js,jsx}: Prefer constructor injection for classes that require dependencies
Avoid global mutable singletons unless necessary
Use explicit interfaces for external dependencies to make testing easier
**/*.{ts,tsx,js,jsx}: Include required references in PRs/code for non-trivial logic: TypeScript (official docs), MDN (JavaScript reference), and official docs for any runtime/framework/libraries used (e.g., Node.js, React) as applicable.
Before assuming behavior of an API, include the doc link and a ≤25-word quote when the change relies on it.
**/*.{ts,tsx,js,jsx}: Prefer named exports for clear usage and easier refactors in TypeScript/JavaScript
Keep import order consistent: external first, then internal modules
Remove dead code and unused imports
**/*.{ts,tsx,js,jsx}: Use PascalCase naming convention for React/UI components
Use camelCase naming convention for variables and functions
Use UPPER_SNAKE_CASE naming convention for constants
Maintain consistent import grouping and ordering within the project, keeping third-party imports separate from local imports
For external data sources (HTTP, database), always validate and sanitize input using type guards or schema validators
**/*.{ts,tsx,js,jsx}: Use Prettier with no semicolons, single quotes, 4-space indent, 80 character width
Files must not exceed 250 lines and this is enforcedImplement TypeScript typecheck and linter in CI quality checks
**/*.{ts,tsx,js,jsx}: Use TypeScript for enhanced type safety
Implement error handling and error logging
Avoid commenting code unless extremely necessary - code should explain itself with descriptive names
Leave NO todos, placeholders or missing pieces in the code
Variables and functions must use camelCase
Constants must use UPPER_SNAKE_CASE
Use arrow functions for methods and computed properties
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements
Maintain consistent import grouping/order: external imports first, then...
Files:
packages/frontend/src/hooks/useGuildSelection.test.tsxpackages/frontend/src/stores/guildStore.tspackages/backend/tests/unit/services/GuildAccessService.test.tspackages/frontend/src/components/Layout/Sidebar.tsxpackages/frontend/tests/e2e/helpers/api-helpers.tspackages/backend/src/services/GuildAccessService.tspackages/frontend/src/stores/guildStore.test.tspackages/frontend/src/components/Layout/Sidebar.test.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.{js,jsx,ts,tsx}: Never throw strings. ThrowError(or typed subclasses) with descriptive messages
Include causal error ascausewhen available for better debugging
Define clear, stable error codes (e.g.,ERR_AUTH_EXPIRED,ERR_NETWORK_TIMEOUT)
Provide optional metadata (e.g.,details,retryable,status,correlationId) in error objects
Use domain error classes per area (e.g.,AuthenticationError,ValidationError,NetworkError)
Log errors with structure (message, code, stack, cause, correlationId, user context where appropriate)
MarkretryablevsnonRetryableerrors where helpful for operations
Set timeouts and handle aborts/cancellations; avoid dangling requests in API/network code
Implement backoff for transient failures; avoid infinite retries
Map HTTP status → domain errors; 4xx vs 5xx behave differently (e.g., retry for 5xx/network)
**/*.{js,jsx,ts,tsx}: Use functional components with hooks in React/React Native. Avoid class components.
Keep components focused on a single responsibility; extract complex logic into custom hooks.
Keep state local when possible. Use Context/Zustand/Redux only when necessary for state management.
If props or state traverse more than 3 levels, consider using context or a feature-scoped store instead of prop drilling.
Use performance optimization techniques:React.memo,useMemo,useCallback,Suspense(web), and virtualization for long lists; avoid unnecessary re-renders.
Web accessibility: use semantic HTML, labels, focus management, keyboard navigation, andaria-*attributes as needed.
React Native accessibility: use accessibility props (accessible,accessibilityLabel), proper roles and labels.
Identify and extract repetitive UI components proactively tocomponents/with clear props and minimal coupling.
Web styles: prefer co-located styles or design system tokens; avoid global style leakage.
React Native styles: preferStyleSheet.create, design tokens, and theme providers; avoid in...
Files:
packages/frontend/src/hooks/useGuildSelection.test.tsxpackages/frontend/src/stores/guildStore.tspackages/backend/tests/unit/services/GuildAccessService.test.tspackages/frontend/src/components/Layout/Sidebar.tsxpackages/frontend/tests/e2e/helpers/api-helpers.tspackages/backend/src/services/GuildAccessService.tspackages/frontend/src/stores/guildStore.test.tspackages/frontend/src/components/Layout/Sidebar.test.tsx
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.{jsx,tsx}: Use toasts/snackbars for transient errors; avoid blocking modals for non-critical issues in React/React Native UI
Debounce/suppress duplicate toasts to prevent spam
Provide retry/refresh actions when meaningful (e.g., network failure) in error UI
Use error boundaries for render-time exceptions; show fallback UI in React
Respect accessibility: toasts should be announced (aria-live on web; accessibility hints on React Native)
Files:
packages/frontend/src/hooks/useGuildSelection.test.tsxpackages/frontend/src/components/Layout/Sidebar.tsxpackages/frontend/src/components/Layout/Sidebar.test.tsx
**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/frontend.mdc)
**/*.{test,spec}.{js,jsx,ts,tsx}: Test behavior, not implementation. Prefer Testing Library utilities for testing React/React Native components.
For React Native tests: mock native modules and test component interactions and accessibility labels.
Files:
packages/frontend/src/hooks/useGuildSelection.test.tsxpackages/backend/tests/unit/services/GuildAccessService.test.tspackages/frontend/src/stores/guildStore.test.tspackages/frontend/src/components/Layout/Sidebar.test.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/pattern.mdc)
Introduce interfaces at module boundaries to enable testing and substitutions
**/*.{ts,tsx}: Avoid usinganytype in TypeScript. If unavoidable, useunknownwith type guards and justify with a code comment
Preferinterfacefor defining public object shapes in TypeScript, usetypefor unions and utility types
Use TypeScript utility types such asPartial,Pick,Omit,Readonly, andRecordwhen appropriate
UseI{Name}naming convention for interfaces in TypeScript
UseT{Name}naming convention for type aliases and utility types in TypeScript
**/*.{ts,tsx}: Functions must be less than 50 lines with cyclomatic complexity less than 10
Do not useanytypes - ESLint enforces this at error level
**/*.{ts,tsx}: Prefer types over interfaces for most cases
Don't ever useany- type safety always
Avoid enums; use const objects instead
For complex types, create a separate file to declare them and import them
Avoid usinganytype; if unavoidable, useunknownwith type guards and justify with code comment
Preferinterfacefor public API shapes; usetypefor unions and utility types
Use TypeScript utility types (Partial, Pick, Omit, Readonly, Record)
Files:
packages/frontend/src/hooks/useGuildSelection.test.tsxpackages/frontend/src/stores/guildStore.tspackages/backend/tests/unit/services/GuildAccessService.test.tspackages/frontend/src/components/Layout/Sidebar.tsxpackages/frontend/tests/e2e/helpers/api-helpers.tspackages/backend/src/services/GuildAccessService.tspackages/frontend/src/stores/guildStore.test.tspackages/frontend/src/components/Layout/Sidebar.test.tsx
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/pattern.mdc)
**/*.{test,spec}.{ts,tsx,js,jsx}: Test behavior, not implementation details
Prefer unit tests for core logic; add integration tests at meaningful boundaries
Files:
packages/frontend/src/hooks/useGuildSelection.test.tsxpackages/backend/tests/unit/services/GuildAccessService.test.tspackages/frontend/src/stores/guildStore.test.tspackages/frontend/src/components/Layout/Sidebar.test.tsx
**/*.{test,spec}.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/testing-quality.mdc)
**/*.{test,spec}.{js,ts,jsx,tsx}: Use Jest + a React testing library for unit and component tests as applicable
Test behavior, not implementation details
Files:
packages/frontend/src/hooks/useGuildSelection.test.tsxpackages/backend/tests/unit/services/GuildAccessService.test.tspackages/frontend/src/stores/guildStore.test.tspackages/frontend/src/components/Layout/Sidebar.test.tsx
packages/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
packages/frontend/src/**/*.{ts,tsx}: Frontend errors are created by Axios interceptor and should be of typeApiErrorwith status and details from backend
Frontend uses path alias@/mapped tosrc/- use this alias for all imports from the src directoryDo not depend on
@lucky/sharedpackage in frontend code; make API calls to backend via configured base URL (env)
Files:
packages/frontend/src/hooks/useGuildSelection.test.tsxpackages/frontend/src/stores/guildStore.tspackages/frontend/src/components/Layout/Sidebar.tsxpackages/frontend/src/stores/guildStore.test.tspackages/frontend/src/components/Layout/Sidebar.test.tsx
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/documentation.mdc)
**/*.{js,ts,tsx,jsx}: Minimize comments in code; explain the 'why' when non-obvious, let code express the 'what' through clear naming
Document trade-offs briefly when deviating from ideal patterns
**/*.{js,ts,tsx,jsx}: Store secrets, ports, and hosts in environment variables (.env,.env.example) and never hardcode them
Avoid redundant or decorative AI comments; code should be self-explanatory and only commented when logic is non-obvious; prefer refactoring over lengthy comments
Files:
packages/frontend/src/hooks/useGuildSelection.test.tsxpackages/frontend/src/stores/guildStore.tspackages/backend/tests/unit/services/GuildAccessService.test.tspackages/frontend/src/components/Layout/Sidebar.tsxpackages/frontend/tests/e2e/helpers/api-helpers.tspackages/backend/src/services/GuildAccessService.tspackages/frontend/src/stores/guildStore.test.tspackages/frontend/src/components/Layout/Sidebar.test.tsx
packages/frontend/**
📄 CodeRabbit inference engine (.cursor/rules/lucky-project.mdc)
The
frontendpackage uses React with Vite and must not depend on the shared package
Files:
packages/frontend/src/hooks/useGuildSelection.test.tsxpackages/frontend/src/stores/guildStore.tspackages/frontend/src/components/Layout/Sidebar.tsxpackages/frontend/tests/e2e/helpers/api-helpers.tspackages/frontend/src/stores/guildStore.test.tspackages/frontend/src/components/Layout/Sidebar.test.tsx
**/*.{spec,test}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/quality.mdc)
**/*.{spec,test}.{ts,tsx,js,jsx}: Use Jest for unit and integration tests
Test behavior, not implementation details
Run unit, integration tests, and coverage report in CI quality checks
Files:
packages/frontend/src/hooks/useGuildSelection.test.tsxpackages/backend/tests/unit/services/GuildAccessService.test.tspackages/frontend/src/stores/guildStore.test.tspackages/frontend/src/components/Layout/Sidebar.test.tsx
**/use[A-Z]*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
Use camelCase for composables (e.g.,
useAuthState.ts)
Files:
packages/frontend/src/hooks/useGuildSelection.test.tsx
packages/frontend/src/{stores,services}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/lucky-frontend.mdc)
Store application state in
packages/frontend/src/stores/and API calls inpackages/frontend/src/services/api.ts
Files:
packages/frontend/src/stores/guildStore.tspackages/frontend/src/stores/guildStore.test.ts
**/*.{js,mjs,ts,mts}
📄 CodeRabbit inference engine (.cursor/rules/lucky-project.mdc)
Use Node.js version ≥22 with ESM (ECMAScript modules) only; no CommonJS
Files:
packages/frontend/src/stores/guildStore.tspackages/backend/tests/unit/services/GuildAccessService.test.tspackages/frontend/tests/e2e/helpers/api-helpers.tspackages/backend/src/services/GuildAccessService.tspackages/frontend/src/stores/guildStore.test.ts
packages/backend/tests/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/lucky-backend-api.mdc)
Organize tests in
packages/backend/tests/with unit tests underunit/and integration tests underintegration/, following existing patterns with fixtures and setup
Files:
packages/backend/tests/unit/services/GuildAccessService.test.ts
packages/backend/**
📄 CodeRabbit inference engine (.cursor/rules/lucky-project.mdc)
The
backendpackage depends onsharedand contains Express API with auth and guild routes
Files:
packages/backend/tests/unit/services/GuildAccessService.test.tspackages/backend/src/services/GuildAccessService.ts
{packages/*/tests/**/*.test.{js,ts},tests/**/*.test.{js,ts}}
📄 CodeRabbit inference engine (.cursor/rules/lucky-project.mdc)
Add or adjust unit and integration tests when changing behavior; follow existing patterns in
packages/*/testsand roottests/directories
Files:
packages/backend/tests/unit/services/GuildAccessService.test.ts
packages/backend/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/subagent-backend.mdc)
packages/backend/**/*.ts: Apply.cursor/rules/lucky-backend-api.mdcfor structure and conventions when acting as backend specialist
Use.cursor/skills/backend-express/SKILL.mdfor Express routes, middleware, and services when acting as backend specialist
Use@lucky/sharedfor config and DB/Redis when needed in backend code
Files:
packages/backend/tests/unit/services/GuildAccessService.test.tspackages/backend/src/services/GuildAccessService.ts
packages/backend/tests/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/subagent-backend.mdc)
Follow existing patterns for unit and integration tests in
packages/backend/tests/
Files:
packages/backend/tests/unit/services/GuildAccessService.test.ts
**/[A-Z]*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
Components must use PascalCase naming
Files:
packages/backend/tests/unit/services/GuildAccessService.test.tspackages/frontend/src/components/Layout/Sidebar.tsxpackages/backend/src/services/GuildAccessService.tspackages/frontend/src/components/Layout/Sidebar.test.tsx
packages/frontend/src/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/lucky-frontend.mdc)
Organize UI components in
packages/frontend/src/components/directory
Files:
packages/frontend/src/components/Layout/Sidebar.tsxpackages/frontend/src/components/Layout/Sidebar.test.tsx
packages/frontend/src/{components,pages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/lucky-frontend.mdc)
Use React functional components and hooks; keep components small and focused
Files:
packages/frontend/src/components/Layout/Sidebar.tsxpackages/frontend/src/components/Layout/Sidebar.test.tsx
packages/frontend/src/{components,pages}/**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/lucky-frontend.mdc)
Follow existing styling approach (e.g., Tailwind if present); avoid inline styles for layout and theming
Files:
packages/frontend/src/components/Layout/Sidebar.tsxpackages/frontend/src/components/Layout/Sidebar.test.tsx
packages/frontend/tests/**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/lucky-frontend.mdc)
Write tests in
packages/frontend/tests/using existing test patterns (e.g., Playwright for e2e if configured)
Files:
packages/frontend/tests/e2e/helpers/api-helpers.ts
packages/backend/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
packages/backend/src/**/*.ts: Validation must use Zod schemas inbackend/src/schemas/and be applied viavalidateBody,validateParams, orvalidateQuery
Do not reassignreq.queryin Express middleware - it is read-only in Express 5
Files:
packages/backend/src/services/GuildAccessService.ts
packages/backend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/lucky-backend-api.mdc)
packages/backend/src/**/*.{ts,tsx}: Use shared config and env from@lucky/sharedwhen needed; avoid duplicating env parsing in backend code
Keep tokens and secrets in environment variables only; never hardcode or expose in code
Files:
packages/backend/src/services/GuildAccessService.ts
packages/backend/src/{services,middleware}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/lucky-backend-api.mdc)
packages/backend/src/{services,middleware}/**/*.{ts,tsx}: Implement Discord OAuth for authentication in backend services
Use SessionService in middleware for session handling; keep session and auth logic centralized
Files:
packages/backend/src/services/GuildAccessService.ts
packages/backend/src/services/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/subagent-backend.mdc)
Place services in
services/directory
Files:
packages/backend/src/services/GuildAccessService.ts
🧠 Learnings (18)
📓 Common learnings
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-backend-api.mdc:0-0
Timestamp: 2026-03-09T20:20:38.694Z
Learning: Applies to packages/backend/src/{services,middleware}/**/*.{ts,tsx} : Implement Discord OAuth for authentication in backend services
📚 Learning: 2026-03-09T20:21:08.612Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-project.mdc:0-0
Timestamp: 2026-03-09T20:21:08.612Z
Learning: Applies to {packages/*/tests/**/*.test.{js,ts},tests/**/*.test.{js,ts}} : Add or adjust unit and integration tests when changing behavior; follow existing patterns in `packages/*/tests` and root `tests/` directories
Applied to files:
packages/frontend/src/hooks/useGuildSelection.test.tsxpackages/backend/tests/unit/services/GuildAccessService.test.tspackages/frontend/tests/e2e/helpers/api-helpers.tspackages/frontend/src/stores/guildStore.test.tspackages/frontend/src/components/Layout/Sidebar.test.tsx
📚 Learning: 2026-03-09T20:21:31.459Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/quality.mdc:0-0
Timestamp: 2026-03-09T20:21:31.459Z
Learning: Applies to tests/**/*.test.{ts,tsx,js,jsx} : Add integration tests where appropriate
Applied to files:
packages/frontend/src/hooks/useGuildSelection.test.tsxpackages/frontend/src/components/Layout/Sidebar.test.tsx
📚 Learning: 2026-03-09T20:22:47.453Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-09T20:22:47.453Z
Learning: For unit tests and Jest ESM mocks, use the `testing-lucky` skill
Applied to files:
packages/frontend/src/hooks/useGuildSelection.test.tsxpackages/frontend/tests/e2e/helpers/api-helpers.tspackages/frontend/src/stores/guildStore.test.ts
📚 Learning: 2026-03-09T20:20:56.356Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-frontend.mdc:0-0
Timestamp: 2026-03-09T20:20:56.356Z
Learning: Applies to packages/frontend/tests/**/*.{ts,tsx,js} : Write tests in `packages/frontend/tests/` using existing test patterns (e.g., Playwright for e2e if configured)
Applied to files:
packages/frontend/src/hooks/useGuildSelection.test.tsx
📚 Learning: 2026-03-09T20:20:38.694Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-backend-api.mdc:0-0
Timestamp: 2026-03-09T20:20:38.694Z
Learning: Applies to packages/backend/tests/**/*.{ts,tsx} : Organize tests in `packages/backend/tests/` with unit tests under `unit/` and integration tests under `integration/`, following existing patterns with fixtures and setup
Applied to files:
packages/frontend/src/hooks/useGuildSelection.test.tsxpackages/frontend/src/components/Layout/Sidebar.test.tsx
📚 Learning: 2026-03-09T20:21:58.991Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/subagent-frontend.mdc:0-0
Timestamp: 2026-03-09T20:21:58.991Z
Learning: Write unit and integration tests in `packages/frontend/tests`; use Playwright for E2E tests when changing user flows
Applied to files:
packages/frontend/src/hooks/useGuildSelection.test.tsx
📚 Learning: 2026-03-09T20:21:38.098Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/subagent-backend.mdc:0-0
Timestamp: 2026-03-09T20:21:38.098Z
Learning: Applies to packages/backend/tests/**/*.ts : Follow existing patterns for unit and integration tests in `packages/backend/tests/`
Applied to files:
packages/frontend/src/hooks/useGuildSelection.test.tsxpackages/backend/tests/unit/services/GuildAccessService.test.tspackages/frontend/tests/e2e/helpers/api-helpers.ts
📚 Learning: 2026-03-09T20:20:38.694Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-backend-api.mdc:0-0
Timestamp: 2026-03-09T20:20:38.694Z
Learning: Applies to packages/backend/src/{services,middleware}/**/*.{ts,tsx} : Implement Discord OAuth for authentication in backend services
Applied to files:
packages/backend/tests/unit/services/GuildAccessService.test.tspackages/frontend/src/components/Layout/Sidebar.tsxpackages/backend/src/services/GuildAccessService.ts
📚 Learning: 2026-03-09T20:20:56.356Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-frontend.mdc:0-0
Timestamp: 2026-03-09T20:20:56.356Z
Learning: Applies to packages/frontend/src/**/*.{ts,tsx} : Do not depend on `lucky/shared` package in frontend code; make API calls to backend via configured base URL (env)
Applied to files:
packages/frontend/src/components/Layout/Sidebar.tsx
📚 Learning: 2026-03-09T20:21:58.991Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/subagent-frontend.mdc:0-0
Timestamp: 2026-03-09T20:21:58.991Z
Learning: Keep frontend code scoped to `packages/frontend`; communicate with backend via `services/api.ts` using the configured env base URL; do not access shared database or Redis directly
Applied to files:
packages/frontend/src/components/Layout/Sidebar.tsx
📚 Learning: 2026-03-09T20:20:38.694Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-backend-api.mdc:0-0
Timestamp: 2026-03-09T20:20:38.694Z
Learning: Applies to packages/backend/src/**/*.{ts,tsx} : Keep tokens and secrets in environment variables only; never hardcode or expose in code
Applied to files:
packages/frontend/src/components/Layout/Sidebar.tsx
📚 Learning: 2026-03-09T20:20:23.892Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-09T20:20:23.892Z
Learning: Applies to packages/frontend/src/**/*.{ts,tsx} : Frontend uses path alias `@/` mapped to `src/` - use this alias for all imports from the src directory
Applied to files:
packages/frontend/src/components/Layout/Sidebar.tsx
📚 Learning: 2026-03-09T20:20:56.356Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-frontend.mdc:0-0
Timestamp: 2026-03-09T20:20:56.356Z
Learning: Applies to packages/frontend/src/{components,pages}/**/*.{ts,tsx} : Use React functional components and hooks; keep components small and focused
Applied to files:
packages/frontend/src/components/Layout/Sidebar.tsx
📚 Learning: 2026-03-09T20:20:38.694Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-backend-api.mdc:0-0
Timestamp: 2026-03-09T20:20:38.694Z
Learning: Applies to packages/backend/src/{routes,middleware}/**/*.{ts,tsx} : Return consistent JSON error responses with appropriate HTTP status codes; do not expose stack traces or secrets in responses
Applied to files:
packages/backend/src/services/GuildAccessService.ts
📚 Learning: 2026-03-09T20:20:47.877Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-discord-bot.mdc:0-0
Timestamp: 2026-03-09T20:20:47.877Z
Learning: Applies to packages/bot/**/*.{ts,tsx} : Use services from `lucky/shared` (DatabaseService, Redis client) for database and cache access; do not instantiate Prisma or Redis directly in the bot package
Applied to files:
packages/backend/src/services/GuildAccessService.ts
📚 Learning: 2026-03-09T20:20:47.877Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-discord-bot.mdc:0-0
Timestamp: 2026-03-09T20:20:47.877Z
Learning: Applies to packages/bot/**/*.{ts,tsx} : Do not duplicate queue or player state outside Discord Player; use shared services from `lucky/shared` for persistent data like track history and session information
Applied to files:
packages/backend/src/services/GuildAccessService.ts
📚 Learning: 2026-03-09T20:20:56.356Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-frontend.mdc:0-0
Timestamp: 2026-03-09T20:20:56.356Z
Learning: Applies to packages/frontend/src/{stores,services}/**/*.{ts,tsx} : Store application state in `packages/frontend/src/stores/` and API calls in `packages/frontend/src/services/api.ts`
Applied to files:
packages/frontend/src/stores/guildStore.test.ts
🪛 GitHub Check: SonarCloud Code Analysis
packages/frontend/src/components/Layout/Sidebar.tsx
[warning] 173-182: Mark the props of the component as read-only.
🔇 Additional comments (12)
packages/frontend/src/hooks/useGuildSelection.test.tsx (2)
65-82: LGTM – retry test now correctly exercises the auth-error transition.The test properly simulates the sequence: initial fetch completes → auth error appears → retry triggers on rerender. This addresses the previous concern about preloading the error before the first render.
84-113: Auto-selection tests look correct.Both positive and negative cases align with the hook's guild selection logic and properly test user-facing behavior.
packages/backend/tests/unit/services/GuildAccessService.test.ts (3)
300-330: LGTM!Good test coverage for the member context fallback scenario. The assertion inside the mock implementation effectively verifies that the empty
roleIdsfallback was passed toresolveEffectiveAccess.
332-353: LGTM!Good defensive test coverage for the data integrity check when enriched guilds don't match authorized contexts.
11-16: No issues found. The test correctly mocksDiscordApiErrorwithMockDiscordApiErrorat line 33 viajest.mock, which patches the import at line 65. When line 217 callsnew DiscordApiError(401, 'invalid token'), it invokes the mock class with the correct parameter order for that mock's signature. The mock's simplified constructor is intentional for testing and differs appropriately from the production class.packages/backend/src/services/GuildAccessService.ts (3)
34-55: LGTM!The
extractStatusCodehelper properly handlesDiscordApiErrorinstances first, then falls back to duck-typing for arbitrary error objects withstatusCodeorstatusproperties. Good use ofunknowntype with proper type guards.
57-89: LGTM!Robust error mapping that translates Discord API errors to appropriate
AppErrorinstances with user-friendly messages. The 502 status for rate limiting (429) and server errors (5xx) correctly represents upstream unavailability. As per coding guidelines, errors include clear messages without exposing internal details.
177-185: LGTM - Addresses past review feedback.The all-context-failure detection at lines 180-185 correctly surfaces a retryable 502 error when there are guilds but no contexts could be resolved, instead of silently returning an empty list. This addresses the previously raised concern about distinguishing "no accessible servers" from "service unavailable."
packages/frontend/tests/e2e/helpers/api-helpers.ts (4)
111-144: Guild-scoped/memock looks solid.Pulling
effectiveAccessandcanManageRbacfromMOCK_GUILDSkeeps the permission context tied to the selected guild instead of a single canned response.
146-168: Nice touch making/listingreflect the selected guild.Reusing the current guild name here keeps server-shell assertions aligned with the guild under test without needing a separate listing fixture per server.
170-217: The deterministic moderation stubs are a good addition.Mocking both stats and cases here should keep these specs from drifting into unrelated network traffic while the shell loads.
285-293: Good catch registering toggle writes for every guild.Putting
mockToggleUpdate(page, false, guild.id)inside theMOCK_GUILDSloop closes the hole where a non-first guild could fall through to the network.
25fd56e to
70fede7
Compare
|
Addressed the remaining guild-store stale-state concern in commit
Validated locally:
|
Superseded by rebase and follow-up fixes on newer head (4833247), including resolved review threads and regression coverage.
|
PR is now merge-ready on code/review gates, but merge remains blocked under no-admin-bypass policy. Latest merge attempt: Result:
Current state snapshot:
No admin bypass used. This PR cannot be merged until repository auto-merge is enabled or admin merge is explicitly allowed. |
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/frontend/tests/e2e/servers-page.spec.ts (1)
164-177:⚠️ Potential issue | 🟡 MinorTest does not assert error handling behavior.
This test mocks a 500 error but only calls
waitForTimeout(2000)without asserting that an error message, retry button, or fallback UI is displayed. Consider adding an assertion for the expected error state.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/tests/e2e/servers-page.spec.ts` around lines 164 - 177, The test "handles error when API fails" currently mocks a 500 response but never asserts the UI reaction; update the test to verify the expected error state after calling navigateToServers and waitForServerList by asserting presence of the error message or retry/fallback UI (e.g., check a visible error banner text, a "Retry" button, or that the server list locator is empty). Use the existing helpers (navigateToServers, waitForServerList) then add assertions via page.locator(...) and expect(...).toBeVisible()/toHaveText()/toHaveCount(0) to validate the error handling behavior.
🧹 Nitpick comments (5)
packages/frontend/src/components/Layout/Sidebar.test.tsx (1)
253-342: Consider covering the new upstream/502 state here too.These additions lock down
auth,forbidden, andnetwork, but the retryable upstream classification introduced in this PR is still untested in the sidebar. OneguildLoadError: { kind: 'upstream', status: 502, ... }case would keep that UI contract from drifting.Based on learnings, "Add or adjust unit and integration tests when changing behavior; follow existing patterns in
packages/*/testsand roottests/directories."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/src/components/Layout/Sidebar.test.tsx` around lines 253 - 342, Add a test in Sidebar.test.tsx that covers the new upstream/502 failure case by calling mockGuildStoreState({ guilds: [], selectedGuild: null, selectedGuildId: null, guildLoadError: { kind: 'upstream', status: 502, message: 'Bad gateway' } } as Partial<ReturnType<typeof useGuildStore>>), then renderSidebar(), open the server menu (screen.getByRole('button', { name: /select a server/i })), and assert the upstream retry guidance is shown (expect screen.getByText(...upstream message...).toBeInTheDocument()), that a Retry button is present (expect screen.getByRole('button', { name: 'Retry' })).toBeInTheDocument()), that there is no Re-authenticate link (expect screen.queryByRole('link', { name: 'Re-authenticate' })).not.toBeInTheDocument()), and finally simulate clicking Retry and assert mockFetchGuilds was called (expect(mockFetchGuilds).toHaveBeenCalledTimes(1)); use existing patterns from the tests that use mockGuildStoreState, renderSidebar, userEvent.setup(), and mockFetchGuilds to match style.packages/frontend/src/components/Layout/Sidebar.tsx (2)
162-171: Mark props interface as readonly for immutability.Per static analysis, marking the props as read-only prevents accidental mutation and aligns with React best practices.
♻️ Proposed fix
-interface ServerSelectorProps { - guilds: Guild[] - selectedGuild: Guild | null - guildLoadError: GuildLoadErrorState | null - isLoading: boolean - serverDropdownOpen: boolean - setServerDropdownOpen: Dispatch<SetStateAction<boolean>> - fetchGuilds: () => Promise<void> - selectGuild: (guild: Guild | null) => void -} +interface ServerSelectorProps { + readonly guilds: Guild[] + readonly selectedGuild: Guild | null + readonly guildLoadError: GuildLoadErrorState | null + readonly isLoading: boolean + readonly serverDropdownOpen: boolean + readonly setServerDropdownOpen: Dispatch<SetStateAction<boolean>> + readonly fetchGuilds: () => Promise<void> + readonly selectGuild: (guild: Guild | null) => void +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/src/components/Layout/Sidebar.tsx` around lines 162 - 171, Make the ServerSelectorProps interface fully immutable by marking each property readonly: change guilds: Guild[] to readonly guilds: ReadonlyArray<Guild> (or readonly Guild[]), and prefix every other property (selectedGuild, guildLoadError, isLoading, serverDropdownOpen, setServerDropdownOpen, fetchGuilds, selectGuild) with readonly so the interface ServerSelectorProps cannot be mutated; update the ServerSelectorProps declaration in Sidebar.tsx accordingly.
149-160: Consider explicitly handling the 'upstream' error kind.The
getGuildLoadMessageswitch handlesauth,forbidden, andnetworkbut falls through to the default forupstream. While the default returnsguildLoadError.messagewhich works, an explicit case improves clarity and ensures the message is appropriate.💡 Optional: Add explicit upstream case
function getGuildLoadMessage(guildLoadError: GuildLoadErrorState): string { switch (guildLoadError.kind) { case 'auth': return 'Your session expired. Please sign in again.' case 'forbidden': return 'Discord access is missing required scope.' case 'network': return 'Network connection failed. Check connectivity and retry.' + case 'upstream': + return 'Discord is temporarily unavailable. Please retry.' default: return guildLoadError.message } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/src/components/Layout/Sidebar.tsx` around lines 149 - 160, Update the getGuildLoadMessage function to explicitly handle the 'upstream' kind of GuildLoadErrorState instead of relying on the default branch: add a case 'upstream' in the switch inside getGuildLoadMessage that returns a clear, user-facing message (for example, "An upstream service returned an error. Please try again later.") so the intent is explicit and the message can be tailored separately from other error kinds.packages/frontend/tests/e2e/layout-navigation.spec.ts (1)
77-88: Consider using a more resilient selector for the server dropdown.The text-based selector
text=Select a serveris fragile—if the placeholder text changes or gets localized, this test will break. Using an accessibility-oriented selector (e.g.,aria-label,role, ordata-testid) would be more robust.💡 Optional: Use a test-id or role-based selector
test('server selector dropdown in sidebar', async ({ page }) => { await navigateToDashboard(page) - const serverSelector = page.locator('text=Select a server').first() + const serverSelector = page.locator('[data-testid="server-selector"]').first() const isVisible = await serverSelector .isVisible({ timeout: 3000 }) .catch(() => false)This requires adding
data-testid="server-selector"to theServerSelectorcomponent's trigger button.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/tests/e2e/layout-navigation.spec.ts` around lines 77 - 88, The test "server selector dropdown in sidebar" uses a fragile text selector; update the ServerSelector component to add a stable test-specific attribute (e.g., add data-testid="server-selector" to the trigger button or set an explicit aria-label/role), then change the test to locate the element via that stable selector (e.g., locator('[data-testid="server-selector"]') or getByRole('button', { name: /server selector/i }) ) instead of 'text=Select a server' so the test is resilient to copy changes or localization.packages/frontend/tests/e2e/servers-page.spec.ts (1)
96-97: AvoidwaitForTimeoutin favor of explicit wait conditions.
waitForTimeout(1000)is a flaky anti-pattern in Playwright tests. PreferwaitForURLorexpect(page).toHaveURL()for navigation assertions.♻️ Proposed fix
- await page.waitForTimeout(1000) - expect(page.url()).not.toContain('/servers') + await expect(page).not.toHaveURL(/\/servers/)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/tests/e2e/servers-page.spec.ts` around lines 96 - 97, The test uses a flaky sleep via page.waitForTimeout(1000); replace this with an explicit navigation assertion using Playwright's APIs (e.g., page.waitForURL or expect(page).toHaveURL) to assert the page is not on '/servers'; locate the call to page.waitForTimeout in the test (servers-page.spec.ts) and change the logic to wait for or assert the URL explicitly using page.waitForURL or expect(page).toHaveURL() combined with not.toContain('/servers').
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/backend/src/services/GuildAccessService.ts`:
- Around line 102-125: The current handlers for
guildService.hasBotInGuild(guild.id) and
guildService.getGuildMemberContext(guild.id, userId) swallow errors and return
fallback values which cause isAuthorized() to treat transient failures as “no
access”; instead, after calling errorLog(...), re-throw the caught error so the
failure bubbles (so per-guild failures can be skipped and all-guild failures
surface as upstream errors). Specifically, in the try/catch around hasBotInGuild
and in the .catch handler on getGuildMemberContext, remove the fallback-return
behavior and throw the error after logging (retain the errorLog calls and error
data), ensuring any caller logic that distinguishes per-guild vs global failures
can handle the exception.
In `@packages/frontend/src/stores/guildStore.ts`:
- Around line 155-175: The async calls in the guild-selection block
(fetchMemberContext, api.guilds.getSettings, api.guilds.getListing) need
request-id gating like fetchGuilds to avoid stale-data overwrites: generate a
new requestId when a guild is selected, store it in the store state (e.g.,
currentRequestId), and before applying any .then/set(...) verify the response's
requestId matches state; if not, ignore the result. Replace empty .catch(() =>
{}) handlers with structured logging using the project logger (include message,
error.code, error.stack, correlationId/requestId and guild.id) and set
serverSettings/serverListing to null only when the requestId still matches.
Ensure symbols referenced: fetchMemberContext, getSettings, getListing,
fetchGuilds, serverSettings, serverListing, currentRequestId.
In `@packages/frontend/tests/e2e/dashboard-page.spec.ts`:
- Line 2: The import statement in dashboard-page.spec.ts currently brings in
setupMockApiResponses and mockGuildsList but mockGuildsList is unused; edit the
import to remove mockGuildsList so only setupMockApiResponses is imported
(update the import that references setupMockApiResponses, mockGuildsList
accordingly).
In `@packages/frontend/tests/e2e/helpers/page-helpers.ts`:
- Around line 68-71: Replace the generic text-based waits that use
page.waitForSelector('text=/servers|Server|No servers/i') with page-scoped
selectors: for the servers list target
section[aria-labelledby="servers-heading"] (use that in the page.waitForSelector
call), for the dashboard scope waits use a selector scoped under main (e.g.,
main [role="region"] or a stats tile/section heading that uniquely appears on
the dashboard) and for the features page wait for a specific features page
element (a known feature list/container or heading) instead of generic page
text; update the calls to page.waitForSelector and keep the existing timeout
parameter to avoid flakiness.
In `@packages/frontend/tests/e2e/servers-page.spec.ts`:
- Around line 154-161: The current pattern using isEmptyVisible = await
emptyState.isVisible({ timeout: 2000 }).catch(() => false) then conditional
expect allows the test to pass without verifying anything; replace it with a
deterministic assertion: either assert the empty state unconditionally (use
expect(emptyState).toBeVisible with an appropriate timeout) or explicitly assert
that one of two known locators is visible (e.g., emptyState OR the server list
locator) so the test fails if neither appears; update references to emptyState
and isEmptyVisible (and the alternative server list locator) accordingly.
- Around line 2-7: The import statement in servers-page.spec.ts includes unused
symbols mockGuildsList and mockAuthStatus; remove those unused imports so only
the actually used helpers (e.g., setupMockApiResponses and mockInviteUrl) are
imported. Locate the import block that currently lists setupMockApiResponses,
mockGuildsList, mockAuthStatus, mockInviteUrl and delete mockGuildsList and
mockAuthStatus from that list to satisfy static analysis.
---
Outside diff comments:
In `@packages/frontend/tests/e2e/servers-page.spec.ts`:
- Around line 164-177: The test "handles error when API fails" currently mocks a
500 response but never asserts the UI reaction; update the test to verify the
expected error state after calling navigateToServers and waitForServerList by
asserting presence of the error message or retry/fallback UI (e.g., check a
visible error banner text, a "Retry" button, or that the server list locator is
empty). Use the existing helpers (navigateToServers, waitForServerList) then add
assertions via page.locator(...) and
expect(...).toBeVisible()/toHaveText()/toHaveCount(0) to validate the error
handling behavior.
---
Nitpick comments:
In `@packages/frontend/src/components/Layout/Sidebar.test.tsx`:
- Around line 253-342: Add a test in Sidebar.test.tsx that covers the new
upstream/502 failure case by calling mockGuildStoreState({ guilds: [],
selectedGuild: null, selectedGuildId: null, guildLoadError: { kind: 'upstream',
status: 502, message: 'Bad gateway' } } as Partial<ReturnType<typeof
useGuildStore>>), then renderSidebar(), open the server menu
(screen.getByRole('button', { name: /select a server/i })), and assert the
upstream retry guidance is shown (expect screen.getByText(...upstream
message...).toBeInTheDocument()), that a Retry button is present (expect
screen.getByRole('button', { name: 'Retry' })).toBeInTheDocument()), that there
is no Re-authenticate link (expect screen.queryByRole('link', { name:
'Re-authenticate' })).not.toBeInTheDocument()), and finally simulate clicking
Retry and assert mockFetchGuilds was called
(expect(mockFetchGuilds).toHaveBeenCalledTimes(1)); use existing patterns from
the tests that use mockGuildStoreState, renderSidebar, userEvent.setup(), and
mockFetchGuilds to match style.
In `@packages/frontend/src/components/Layout/Sidebar.tsx`:
- Around line 162-171: Make the ServerSelectorProps interface fully immutable by
marking each property readonly: change guilds: Guild[] to readonly guilds:
ReadonlyArray<Guild> (or readonly Guild[]), and prefix every other property
(selectedGuild, guildLoadError, isLoading, serverDropdownOpen,
setServerDropdownOpen, fetchGuilds, selectGuild) with readonly so the interface
ServerSelectorProps cannot be mutated; update the ServerSelectorProps
declaration in Sidebar.tsx accordingly.
- Around line 149-160: Update the getGuildLoadMessage function to explicitly
handle the 'upstream' kind of GuildLoadErrorState instead of relying on the
default branch: add a case 'upstream' in the switch inside getGuildLoadMessage
that returns a clear, user-facing message (for example, "An upstream service
returned an error. Please try again later.") so the intent is explicit and the
message can be tailored separately from other error kinds.
In `@packages/frontend/tests/e2e/layout-navigation.spec.ts`:
- Around line 77-88: The test "server selector dropdown in sidebar" uses a
fragile text selector; update the ServerSelector component to add a stable
test-specific attribute (e.g., add data-testid="server-selector" to the trigger
button or set an explicit aria-label/role), then change the test to locate the
element via that stable selector (e.g.,
locator('[data-testid="server-selector"]') or getByRole('button', { name:
/server selector/i }) ) instead of 'text=Select a server' so the test is
resilient to copy changes or localization.
In `@packages/frontend/tests/e2e/servers-page.spec.ts`:
- Around line 96-97: The test uses a flaky sleep via page.waitForTimeout(1000);
replace this with an explicit navigation assertion using Playwright's APIs
(e.g., page.waitForURL or expect(page).toHaveURL) to assert the page is not on
'/servers'; locate the call to page.waitForTimeout in the test
(servers-page.spec.ts) and change the logic to wait for or assert the URL
explicitly using page.waitForURL or expect(page).toHaveURL() combined with
not.toContain('/servers').
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 867cf276-9475-4d6d-a171-45617a1ce2db
📒 Files selected for processing (22)
CHANGELOG.mdREADME.mdpackages/backend/src/routes/guilds.tspackages/backend/src/services/DiscordOAuthService.tspackages/backend/src/services/GuildAccessService.tspackages/backend/tests/unit/services/DiscordOAuthService.test.tspackages/backend/tests/unit/services/GuildAccessService.test.tspackages/frontend/src/App.authRoutes.test.tsxpackages/frontend/src/App.tsxpackages/frontend/src/components/Layout/Sidebar.test.tsxpackages/frontend/src/components/Layout/Sidebar.tsxpackages/frontend/src/hooks/useGuildSelection.test.tsxpackages/frontend/src/hooks/useGuildSelection.tspackages/frontend/src/stores/guildStore.test.tspackages/frontend/src/stores/guildStore.tspackages/frontend/tests/e2e/dashboard-page.spec.tspackages/frontend/tests/e2e/fixtures/test-data.tspackages/frontend/tests/e2e/helpers/api-helpers.tspackages/frontend/tests/e2e/helpers/page-helpers.tspackages/frontend/tests/e2e/helpers/ui-helpers.tspackages/frontend/tests/e2e/layout-navigation.spec.tspackages/frontend/tests/e2e/servers-page.spec.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/backend/tests/unit/services/DiscordOAuthService.test.ts
- README.md
- packages/frontend/src/hooks/useGuildSelection.test.tsx
- packages/frontend/src/App.authRoutes.test.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: SonarCloud Scan
🧰 Additional context used
📓 Path-based instructions (37)
**/*.{js,jsx,ts,tsx,vue,html}
📄 CodeRabbit inference engine (.cursor/rules/accessibility-openness.mdc)
Provide accessible UI components using semantic HTML and ARIA attributes where necessary
Files:
packages/frontend/tests/e2e/helpers/ui-helpers.tspackages/frontend/src/hooks/useGuildSelection.tspackages/frontend/tests/e2e/layout-navigation.spec.tspackages/backend/src/routes/guilds.tspackages/backend/tests/unit/services/GuildAccessService.test.tspackages/backend/src/services/DiscordOAuthService.tspackages/frontend/tests/e2e/helpers/page-helpers.tspackages/frontend/src/stores/guildStore.tspackages/backend/src/services/GuildAccessService.tspackages/frontend/src/stores/guildStore.test.tspackages/frontend/src/components/Layout/Sidebar.tsxpackages/frontend/tests/e2e/helpers/api-helpers.tspackages/frontend/tests/e2e/servers-page.spec.tspackages/frontend/src/components/Layout/Sidebar.test.tsxpackages/frontend/tests/e2e/fixtures/test-data.tspackages/frontend/src/App.tsxpackages/frontend/tests/e2e/dashboard-page.spec.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/dependency-injection.mdc)
**/*.{ts,tsx,js,jsx}: Prefer constructor injection for classes that require dependencies
Avoid global mutable singletons unless necessary
Use explicit interfaces for external dependencies to make testing easier
**/*.{ts,tsx,js,jsx}: Include required references in PRs/code for non-trivial logic: TypeScript (official docs), MDN (JavaScript reference), and official docs for any runtime/framework/libraries used (e.g., Node.js, React) as applicable.
Before assuming behavior of an API, include the doc link and a ≤25-word quote when the change relies on it.
**/*.{ts,tsx,js,jsx}: Prefer named exports for clear usage and easier refactors in TypeScript/JavaScript
Keep import order consistent: external first, then internal modules
Remove dead code and unused imports
**/*.{ts,tsx,js,jsx}: Use PascalCase naming convention for React/UI components
Use camelCase naming convention for variables and functions
Use UPPER_SNAKE_CASE naming convention for constants
Maintain consistent import grouping and ordering within the project, keeping third-party imports separate from local imports
For external data sources (HTTP, database), always validate and sanitize input using type guards or schema validators
**/*.{ts,tsx,js,jsx}: Use Prettier with no semicolons, single quotes, 4-space indent, 80 character width
Files must not exceed 250 lines and this is enforcedImplement TypeScript typecheck and linter in CI quality checks
**/*.{ts,tsx,js,jsx}: Use TypeScript for enhanced type safety
Implement error handling and error logging
Avoid commenting code unless extremely necessary - code should explain itself with descriptive names
Leave NO todos, placeholders or missing pieces in the code
Variables and functions must use camelCase
Constants must use UPPER_SNAKE_CASE
Use arrow functions for methods and computed properties
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements
Maintain consistent import grouping/order: external imports first, then...
Files:
packages/frontend/tests/e2e/helpers/ui-helpers.tspackages/frontend/src/hooks/useGuildSelection.tspackages/frontend/tests/e2e/layout-navigation.spec.tspackages/backend/src/routes/guilds.tspackages/backend/tests/unit/services/GuildAccessService.test.tspackages/backend/src/services/DiscordOAuthService.tspackages/frontend/tests/e2e/helpers/page-helpers.tspackages/frontend/src/stores/guildStore.tspackages/backend/src/services/GuildAccessService.tspackages/frontend/src/stores/guildStore.test.tspackages/frontend/src/components/Layout/Sidebar.tsxpackages/frontend/tests/e2e/helpers/api-helpers.tspackages/frontend/tests/e2e/servers-page.spec.tspackages/frontend/src/components/Layout/Sidebar.test.tsxpackages/frontend/tests/e2e/fixtures/test-data.tspackages/frontend/src/App.tsxpackages/frontend/tests/e2e/dashboard-page.spec.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.{js,jsx,ts,tsx}: Never throw strings. ThrowError(or typed subclasses) with descriptive messages
Include causal error ascausewhen available for better debugging
Define clear, stable error codes (e.g.,ERR_AUTH_EXPIRED,ERR_NETWORK_TIMEOUT)
Provide optional metadata (e.g.,details,retryable,status,correlationId) in error objects
Use domain error classes per area (e.g.,AuthenticationError,ValidationError,NetworkError)
Log errors with structure (message, code, stack, cause, correlationId, user context where appropriate)
MarkretryablevsnonRetryableerrors where helpful for operations
Set timeouts and handle aborts/cancellations; avoid dangling requests in API/network code
Implement backoff for transient failures; avoid infinite retries
Map HTTP status → domain errors; 4xx vs 5xx behave differently (e.g., retry for 5xx/network)
**/*.{js,jsx,ts,tsx}: Use functional components with hooks in React/React Native. Avoid class components.
Keep components focused on a single responsibility; extract complex logic into custom hooks.
Keep state local when possible. Use Context/Zustand/Redux only when necessary for state management.
If props or state traverse more than 3 levels, consider using context or a feature-scoped store instead of prop drilling.
Use performance optimization techniques:React.memo,useMemo,useCallback,Suspense(web), and virtualization for long lists; avoid unnecessary re-renders.
Web accessibility: use semantic HTML, labels, focus management, keyboard navigation, andaria-*attributes as needed.
React Native accessibility: use accessibility props (accessible,accessibilityLabel), proper roles and labels.
Identify and extract repetitive UI components proactively tocomponents/with clear props and minimal coupling.
Web styles: prefer co-located styles or design system tokens; avoid global style leakage.
React Native styles: preferStyleSheet.create, design tokens, and theme providers; avoid in...
Files:
packages/frontend/tests/e2e/helpers/ui-helpers.tspackages/frontend/src/hooks/useGuildSelection.tspackages/frontend/tests/e2e/layout-navigation.spec.tspackages/backend/src/routes/guilds.tspackages/backend/tests/unit/services/GuildAccessService.test.tspackages/backend/src/services/DiscordOAuthService.tspackages/frontend/tests/e2e/helpers/page-helpers.tspackages/frontend/src/stores/guildStore.tspackages/backend/src/services/GuildAccessService.tspackages/frontend/src/stores/guildStore.test.tspackages/frontend/src/components/Layout/Sidebar.tsxpackages/frontend/tests/e2e/helpers/api-helpers.tspackages/frontend/tests/e2e/servers-page.spec.tspackages/frontend/src/components/Layout/Sidebar.test.tsxpackages/frontend/tests/e2e/fixtures/test-data.tspackages/frontend/src/App.tsxpackages/frontend/tests/e2e/dashboard-page.spec.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/pattern.mdc)
Introduce interfaces at module boundaries to enable testing and substitutions
**/*.{ts,tsx}: Avoid usinganytype in TypeScript. If unavoidable, useunknownwith type guards and justify with a code comment
Preferinterfacefor defining public object shapes in TypeScript, usetypefor unions and utility types
Use TypeScript utility types such asPartial,Pick,Omit,Readonly, andRecordwhen appropriate
UseI{Name}naming convention for interfaces in TypeScript
UseT{Name}naming convention for type aliases and utility types in TypeScript
**/*.{ts,tsx}: Functions must be less than 50 lines with cyclomatic complexity less than 10
Do not useanytypes - ESLint enforces this at error level
**/*.{ts,tsx}: Prefer types over interfaces for most cases
Don't ever useany- type safety always
Avoid enums; use const objects instead
For complex types, create a separate file to declare them and import them
Avoid usinganytype; if unavoidable, useunknownwith type guards and justify with code comment
Preferinterfacefor public API shapes; usetypefor unions and utility types
Use TypeScript utility types (Partial, Pick, Omit, Readonly, Record)
Files:
packages/frontend/tests/e2e/helpers/ui-helpers.tspackages/frontend/src/hooks/useGuildSelection.tspackages/frontend/tests/e2e/layout-navigation.spec.tspackages/backend/src/routes/guilds.tspackages/backend/tests/unit/services/GuildAccessService.test.tspackages/backend/src/services/DiscordOAuthService.tspackages/frontend/tests/e2e/helpers/page-helpers.tspackages/frontend/src/stores/guildStore.tspackages/backend/src/services/GuildAccessService.tspackages/frontend/src/stores/guildStore.test.tspackages/frontend/src/components/Layout/Sidebar.tsxpackages/frontend/tests/e2e/helpers/api-helpers.tspackages/frontend/tests/e2e/servers-page.spec.tspackages/frontend/src/components/Layout/Sidebar.test.tsxpackages/frontend/tests/e2e/fixtures/test-data.tspackages/frontend/src/App.tsxpackages/frontend/tests/e2e/dashboard-page.spec.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/documentation.mdc)
**/*.{js,ts,tsx,jsx}: Minimize comments in code; explain the 'why' when non-obvious, let code express the 'what' through clear naming
Document trade-offs briefly when deviating from ideal patterns
**/*.{js,ts,tsx,jsx}: Store secrets, ports, and hosts in environment variables (.env,.env.example) and never hardcode them
Avoid redundant or decorative AI comments; code should be self-explanatory and only commented when logic is non-obvious; prefer refactoring over lengthy comments
Files:
packages/frontend/tests/e2e/helpers/ui-helpers.tspackages/frontend/src/hooks/useGuildSelection.tspackages/frontend/tests/e2e/layout-navigation.spec.tspackages/backend/src/routes/guilds.tspackages/backend/tests/unit/services/GuildAccessService.test.tspackages/backend/src/services/DiscordOAuthService.tspackages/frontend/tests/e2e/helpers/page-helpers.tspackages/frontend/src/stores/guildStore.tspackages/backend/src/services/GuildAccessService.tspackages/frontend/src/stores/guildStore.test.tspackages/frontend/src/components/Layout/Sidebar.tsxpackages/frontend/tests/e2e/helpers/api-helpers.tspackages/frontend/tests/e2e/servers-page.spec.tspackages/frontend/src/components/Layout/Sidebar.test.tsxpackages/frontend/tests/e2e/fixtures/test-data.tspackages/frontend/src/App.tsxpackages/frontend/tests/e2e/dashboard-page.spec.ts
packages/frontend/tests/**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/lucky-frontend.mdc)
Write tests in
packages/frontend/tests/using existing test patterns (e.g., Playwright for e2e if configured)
Files:
packages/frontend/tests/e2e/helpers/ui-helpers.tspackages/frontend/tests/e2e/layout-navigation.spec.tspackages/frontend/tests/e2e/helpers/page-helpers.tspackages/frontend/tests/e2e/helpers/api-helpers.tspackages/frontend/tests/e2e/servers-page.spec.tspackages/frontend/tests/e2e/fixtures/test-data.tspackages/frontend/tests/e2e/dashboard-page.spec.ts
packages/frontend/**
📄 CodeRabbit inference engine (.cursor/rules/lucky-project.mdc)
The
frontendpackage uses React with Vite and must not depend on the shared package
Files:
packages/frontend/tests/e2e/helpers/ui-helpers.tspackages/frontend/src/hooks/useGuildSelection.tspackages/frontend/tests/e2e/layout-navigation.spec.tspackages/frontend/tests/e2e/helpers/page-helpers.tspackages/frontend/src/stores/guildStore.tspackages/frontend/src/stores/guildStore.test.tspackages/frontend/src/components/Layout/Sidebar.tsxpackages/frontend/tests/e2e/helpers/api-helpers.tspackages/frontend/tests/e2e/servers-page.spec.tspackages/frontend/src/components/Layout/Sidebar.test.tsxpackages/frontend/tests/e2e/fixtures/test-data.tspackages/frontend/src/App.tsxpackages/frontend/tests/e2e/dashboard-page.spec.ts
**/*.{js,mjs,ts,mts}
📄 CodeRabbit inference engine (.cursor/rules/lucky-project.mdc)
Use Node.js version ≥22 with ESM (ECMAScript modules) only; no CommonJS
Files:
packages/frontend/tests/e2e/helpers/ui-helpers.tspackages/frontend/src/hooks/useGuildSelection.tspackages/frontend/tests/e2e/layout-navigation.spec.tspackages/backend/src/routes/guilds.tspackages/backend/tests/unit/services/GuildAccessService.test.tspackages/backend/src/services/DiscordOAuthService.tspackages/frontend/tests/e2e/helpers/page-helpers.tspackages/frontend/src/stores/guildStore.tspackages/backend/src/services/GuildAccessService.tspackages/frontend/src/stores/guildStore.test.tspackages/frontend/tests/e2e/helpers/api-helpers.tspackages/frontend/tests/e2e/servers-page.spec.tspackages/frontend/tests/e2e/fixtures/test-data.tspackages/frontend/tests/e2e/dashboard-page.spec.ts
packages/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
packages/frontend/src/**/*.{ts,tsx}: Frontend errors are created by Axios interceptor and should be of typeApiErrorwith status and details from backend
Frontend uses path alias@/mapped tosrc/- use this alias for all imports from the src directoryDo not depend on
@lucky/sharedpackage in frontend code; make API calls to backend via configured base URL (env)
Files:
packages/frontend/src/hooks/useGuildSelection.tspackages/frontend/src/stores/guildStore.tspackages/frontend/src/stores/guildStore.test.tspackages/frontend/src/components/Layout/Sidebar.tsxpackages/frontend/src/components/Layout/Sidebar.test.tsxpackages/frontend/src/App.tsx
**/use[A-Z]*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
Use camelCase for composables (e.g.,
useAuthState.ts)
Files:
packages/frontend/src/hooks/useGuildSelection.ts
**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/frontend.mdc)
**/*.{test,spec}.{js,jsx,ts,tsx}: Test behavior, not implementation. Prefer Testing Library utilities for testing React/React Native components.
For React Native tests: mock native modules and test component interactions and accessibility labels.
Files:
packages/frontend/tests/e2e/layout-navigation.spec.tspackages/backend/tests/unit/services/GuildAccessService.test.tspackages/frontend/src/stores/guildStore.test.tspackages/frontend/tests/e2e/servers-page.spec.tspackages/frontend/src/components/Layout/Sidebar.test.tsxpackages/frontend/tests/e2e/dashboard-page.spec.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/pattern.mdc)
**/*.{test,spec}.{ts,tsx,js,jsx}: Test behavior, not implementation details
Prefer unit tests for core logic; add integration tests at meaningful boundaries
Files:
packages/frontend/tests/e2e/layout-navigation.spec.tspackages/backend/tests/unit/services/GuildAccessService.test.tspackages/frontend/src/stores/guildStore.test.tspackages/frontend/tests/e2e/servers-page.spec.tspackages/frontend/src/components/Layout/Sidebar.test.tsxpackages/frontend/tests/e2e/dashboard-page.spec.ts
**/*.{test,spec}.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/testing-quality.mdc)
**/*.{test,spec}.{js,ts,jsx,tsx}: Use Jest + a React testing library for unit and component tests as applicable
Test behavior, not implementation details
Files:
packages/frontend/tests/e2e/layout-navigation.spec.tspackages/backend/tests/unit/services/GuildAccessService.test.tspackages/frontend/src/stores/guildStore.test.tspackages/frontend/tests/e2e/servers-page.spec.tspackages/frontend/src/components/Layout/Sidebar.test.tsxpackages/frontend/tests/e2e/dashboard-page.spec.ts
**/*.{spec,test}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/quality.mdc)
**/*.{spec,test}.{ts,tsx,js,jsx}: Use Jest for unit and integration tests
Test behavior, not implementation details
Run unit, integration tests, and coverage report in CI quality checks
Files:
packages/frontend/tests/e2e/layout-navigation.spec.tspackages/backend/tests/unit/services/GuildAccessService.test.tspackages/frontend/src/stores/guildStore.test.tspackages/frontend/tests/e2e/servers-page.spec.tspackages/frontend/src/components/Layout/Sidebar.test.tsxpackages/frontend/tests/e2e/dashboard-page.spec.ts
**/*.spec.ts
📄 CodeRabbit inference engine (.cursor/rules/quality.mdc)
Unit tests must use naming convention
*.spec.ts
Files:
packages/frontend/tests/e2e/layout-navigation.spec.tspackages/frontend/tests/e2e/servers-page.spec.tspackages/frontend/tests/e2e/dashboard-page.spec.ts
packages/backend/src/routes/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
packages/backend/src/routes/**/*.ts: Backend route handlers must useasyncHandlerwrapper and throwAppError.xxx()instead of manual try/catch blocks
Rate limiting: useapiLimiter(100/min),authLimiter(20/15min), orwriteLimiter(30/min) as appropriatePlace routes under
packages/backend/src/routes/directory
Files:
packages/backend/src/routes/guilds.ts
packages/backend/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
packages/backend/src/**/*.ts: Validation must use Zod schemas inbackend/src/schemas/and be applied viavalidateBody,validateParams, orvalidateQuery
Do not reassignreq.queryin Express middleware - it is read-only in Express 5
Files:
packages/backend/src/routes/guilds.tspackages/backend/src/services/DiscordOAuthService.tspackages/backend/src/services/GuildAccessService.ts
packages/backend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/lucky-backend-api.mdc)
packages/backend/src/**/*.{ts,tsx}: Use shared config and env from@lucky/sharedwhen needed; avoid duplicating env parsing in backend code
Keep tokens and secrets in environment variables only; never hardcode or expose in code
Files:
packages/backend/src/routes/guilds.tspackages/backend/src/services/DiscordOAuthService.tspackages/backend/src/services/GuildAccessService.ts
packages/backend/src/{routes,middleware}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/lucky-backend-api.mdc)
Return consistent JSON error responses with appropriate HTTP status codes; do not expose stack traces or secrets in responses
Files:
packages/backend/src/routes/guilds.ts
packages/backend/src/routes/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/lucky-backend-api.mdc)
Structure routes in
packages/backend/src/routes/directory with separate files for auth, guilds, toggles, and index routes
Files:
packages/backend/src/routes/guilds.ts
packages/backend/**
📄 CodeRabbit inference engine (.cursor/rules/lucky-project.mdc)
The
backendpackage depends onsharedand contains Express API with auth and guild routes
Files:
packages/backend/src/routes/guilds.tspackages/backend/tests/unit/services/GuildAccessService.test.tspackages/backend/src/services/DiscordOAuthService.tspackages/backend/src/services/GuildAccessService.ts
packages/backend/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/subagent-backend.mdc)
packages/backend/**/*.ts: Apply.cursor/rules/lucky-backend-api.mdcfor structure and conventions when acting as backend specialist
Use.cursor/skills/backend-express/SKILL.mdfor Express routes, middleware, and services when acting as backend specialist
Use@lucky/sharedfor config and DB/Redis when needed in backend code
Files:
packages/backend/src/routes/guilds.tspackages/backend/tests/unit/services/GuildAccessService.test.tspackages/backend/src/services/DiscordOAuthService.tspackages/backend/src/services/GuildAccessService.ts
{CHANGELOG.md,README.md}
📄 CodeRabbit inference engine (.cursor/rules/agent-rules.mdc)
ALWAYS update CHANGELOG.md and README.md as changes are made.
Files:
CHANGELOG.md
CHANGELOG.md
📄 CodeRabbit inference engine (.cursor/rules/templates-examples.mdc)
CHANGELOG.md must be updated with all changes in pull requests
Always update CHANGELOG.md with all code changes
Update CHANGELOG.md with all changes, include breaking changes documentation, and reference issues and PRs
Files:
CHANGELOG.md
{CHANGELOG.md,docs/**}
📄 CodeRabbit inference engine (.cursor/rules/lucky-project.mdc)
Update
CHANGELOG.mdand relevantdocs/files when behavior or setup changes
Files:
CHANGELOG.md
packages/backend/tests/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/lucky-backend-api.mdc)
Organize tests in
packages/backend/tests/with unit tests underunit/and integration tests underintegration/, following existing patterns with fixtures and setup
Files:
packages/backend/tests/unit/services/GuildAccessService.test.ts
{packages/*/tests/**/*.test.{js,ts},tests/**/*.test.{js,ts}}
📄 CodeRabbit inference engine (.cursor/rules/lucky-project.mdc)
Add or adjust unit and integration tests when changing behavior; follow existing patterns in
packages/*/testsand roottests/directories
Files:
packages/backend/tests/unit/services/GuildAccessService.test.ts
packages/backend/tests/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/subagent-backend.mdc)
Follow existing patterns for unit and integration tests in
packages/backend/tests/
Files:
packages/backend/tests/unit/services/GuildAccessService.test.ts
**/[A-Z]*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
Components must use PascalCase naming
Files:
packages/backend/tests/unit/services/GuildAccessService.test.tspackages/backend/src/services/DiscordOAuthService.tspackages/backend/src/services/GuildAccessService.tspackages/frontend/src/components/Layout/Sidebar.tsxpackages/frontend/src/components/Layout/Sidebar.test.tsxpackages/frontend/src/App.tsx
packages/backend/src/{services,middleware}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/lucky-backend-api.mdc)
packages/backend/src/{services,middleware}/**/*.{ts,tsx}: Implement Discord OAuth for authentication in backend services
Use SessionService in middleware for session handling; keep session and auth logic centralized
Files:
packages/backend/src/services/DiscordOAuthService.tspackages/backend/src/services/GuildAccessService.ts
packages/backend/src/services/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/subagent-backend.mdc)
Place services in
services/directory
Files:
packages/backend/src/services/DiscordOAuthService.tspackages/backend/src/services/GuildAccessService.ts
packages/frontend/src/{stores,services}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/lucky-frontend.mdc)
Store application state in
packages/frontend/src/stores/and API calls inpackages/frontend/src/services/api.ts
Files:
packages/frontend/src/stores/guildStore.tspackages/frontend/src/stores/guildStore.test.ts
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.{jsx,tsx}: Use toasts/snackbars for transient errors; avoid blocking modals for non-critical issues in React/React Native UI
Debounce/suppress duplicate toasts to prevent spam
Provide retry/refresh actions when meaningful (e.g., network failure) in error UI
Use error boundaries for render-time exceptions; show fallback UI in React
Respect accessibility: toasts should be announced (aria-live on web; accessibility hints on React Native)
Files:
packages/frontend/src/components/Layout/Sidebar.tsxpackages/frontend/src/components/Layout/Sidebar.test.tsxpackages/frontend/src/App.tsx
packages/frontend/src/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/lucky-frontend.mdc)
Organize UI components in
packages/frontend/src/components/directory
Files:
packages/frontend/src/components/Layout/Sidebar.tsxpackages/frontend/src/components/Layout/Sidebar.test.tsx
packages/frontend/src/{components,pages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/lucky-frontend.mdc)
Use React functional components and hooks; keep components small and focused
Files:
packages/frontend/src/components/Layout/Sidebar.tsxpackages/frontend/src/components/Layout/Sidebar.test.tsx
packages/frontend/src/{components,pages}/**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/lucky-frontend.mdc)
Follow existing styling approach (e.g., Tailwind if present); avoid inline styles for layout and theming
Files:
packages/frontend/src/components/Layout/Sidebar.tsxpackages/frontend/src/components/Layout/Sidebar.test.tsx
packages/frontend/src/{main,App}.tsx
📄 CodeRabbit inference engine (.cursor/rules/lucky-frontend.mdc)
Entry point for the Lucky Frontend React app is
packages/frontend/src/main.tsxwhich connects toApp.tsx
Files:
packages/frontend/src/App.tsx
🧠 Learnings (26)
📓 Common learnings
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-backend-api.mdc:0-0
Timestamp: 2026-03-09T20:20:38.694Z
Learning: Applies to packages/backend/src/{services,middleware}/**/*.{ts,tsx} : Implement Discord OAuth for authentication in backend services
📚 Learning: 2026-03-09T20:21:08.612Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-project.mdc:0-0
Timestamp: 2026-03-09T20:21:08.612Z
Learning: Applies to {packages/*/tests/**/*.test.{js,ts},tests/**/*.test.{js,ts}} : Add or adjust unit and integration tests when changing behavior; follow existing patterns in `packages/*/tests` and root `tests/` directories
Applied to files:
packages/frontend/tests/e2e/layout-navigation.spec.tspackages/backend/tests/unit/services/GuildAccessService.test.tspackages/frontend/src/stores/guildStore.test.tspackages/frontend/tests/e2e/helpers/api-helpers.tspackages/frontend/tests/e2e/servers-page.spec.tspackages/frontend/src/components/Layout/Sidebar.test.tsxpackages/frontend/tests/e2e/fixtures/test-data.tspackages/frontend/tests/e2e/dashboard-page.spec.ts
📚 Learning: 2026-03-09T20:21:31.459Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/quality.mdc:0-0
Timestamp: 2026-03-09T20:21:31.459Z
Learning: Applies to **/*.{spec,test}.{ts,tsx,js,jsx} : Test behavior, not implementation details
Applied to files:
packages/frontend/tests/e2e/layout-navigation.spec.tspackages/frontend/tests/e2e/servers-page.spec.tspackages/frontend/tests/e2e/dashboard-page.spec.ts
📚 Learning: 2026-03-09T20:21:58.991Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/subagent-frontend.mdc:0-0
Timestamp: 2026-03-09T20:21:58.991Z
Learning: Write unit and integration tests in `packages/frontend/tests`; use Playwright for E2E tests when changing user flows
Applied to files:
packages/frontend/tests/e2e/layout-navigation.spec.tspackages/frontend/tests/e2e/servers-page.spec.tspackages/frontend/tests/e2e/dashboard-page.spec.ts
📚 Learning: 2026-03-09T20:20:38.694Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-backend-api.mdc:0-0
Timestamp: 2026-03-09T20:20:38.694Z
Learning: Applies to packages/backend/src/{routes,middleware}/**/*.{ts,tsx} : Return consistent JSON error responses with appropriate HTTP status codes; do not expose stack traces or secrets in responses
Applied to files:
packages/backend/src/routes/guilds.tspackages/backend/src/services/GuildAccessService.ts
📚 Learning: 2026-03-09T20:20:38.694Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-backend-api.mdc:0-0
Timestamp: 2026-03-09T20:20:38.694Z
Learning: Applies to packages/backend/src/routes/**/*.{ts,tsx} : Structure routes in `packages/backend/src/routes/` directory with separate files for auth, guilds, toggles, and index routes
Applied to files:
packages/backend/src/routes/guilds.tspackages/frontend/tests/e2e/helpers/api-helpers.tspackages/frontend/src/App.tsx
📚 Learning: 2026-03-09T20:20:23.892Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-09T20:20:23.892Z
Learning: Applies to packages/backend/src/routes/**/*.ts : Backend route handlers must use `asyncHandler` wrapper and throw `AppError.xxx()` instead of manual try/catch blocks
Applied to files:
packages/backend/src/routes/guilds.ts
📚 Learning: 2026-03-09T20:21:08.612Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-project.mdc:0-0
Timestamp: 2026-03-09T20:21:08.612Z
Learning: Applies to packages/backend/** : The `backend` package depends on `shared` and contains Express API with auth and guild routes
Applied to files:
packages/backend/src/routes/guilds.ts
📚 Learning: 2026-03-09T20:20:38.694Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-backend-api.mdc:0-0
Timestamp: 2026-03-09T20:20:38.694Z
Learning: Applies to packages/backend/src/{services,middleware}/**/*.{ts,tsx} : Implement Discord OAuth for authentication in backend services
Applied to files:
packages/backend/src/routes/guilds.tspackages/backend/tests/unit/services/GuildAccessService.test.tspackages/backend/src/services/DiscordOAuthService.tspackages/backend/src/services/GuildAccessService.tspackages/frontend/src/components/Layout/Sidebar.tsx
📚 Learning: 2026-03-09T20:21:38.098Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/subagent-backend.mdc:0-0
Timestamp: 2026-03-09T20:21:38.098Z
Learning: Applies to packages/backend/tests/**/*.ts : Follow existing patterns for unit and integration tests in `packages/backend/tests/`
Applied to files:
packages/backend/tests/unit/services/GuildAccessService.test.tspackages/frontend/tests/e2e/helpers/api-helpers.tspackages/frontend/tests/e2e/servers-page.spec.tspackages/frontend/tests/e2e/dashboard-page.spec.ts
📚 Learning: 2026-03-09T20:20:56.356Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-frontend.mdc:0-0
Timestamp: 2026-03-09T20:20:56.356Z
Learning: Applies to packages/frontend/src/{stores,services}/**/*.{ts,tsx} : Store application state in `packages/frontend/src/stores/` and API calls in `packages/frontend/src/services/api.ts`
Applied to files:
packages/frontend/src/stores/guildStore.tspackages/frontend/src/stores/guildStore.test.tspackages/frontend/src/App.tsx
📚 Learning: 2026-03-09T20:20:47.877Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-discord-bot.mdc:0-0
Timestamp: 2026-03-09T20:20:47.877Z
Learning: Applies to packages/bot/**/*.{ts,tsx} : Use services from `lucky/shared` (DatabaseService, Redis client) for database and cache access; do not instantiate Prisma or Redis directly in the bot package
Applied to files:
packages/backend/src/services/GuildAccessService.ts
📚 Learning: 2026-03-09T20:20:47.877Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-discord-bot.mdc:0-0
Timestamp: 2026-03-09T20:20:47.877Z
Learning: Applies to packages/bot/**/*.{ts,tsx} : Do not duplicate queue or player state outside Discord Player; use shared services from `lucky/shared` for persistent data like track history and session information
Applied to files:
packages/backend/src/services/GuildAccessService.ts
📚 Learning: 2026-03-09T20:22:47.453Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-09T20:22:47.453Z
Learning: For unit tests and Jest ESM mocks, use the `testing-lucky` skill
Applied to files:
packages/frontend/src/stores/guildStore.test.tspackages/frontend/tests/e2e/servers-page.spec.tspackages/frontend/tests/e2e/dashboard-page.spec.ts
📚 Learning: 2026-03-09T20:20:56.356Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-frontend.mdc:0-0
Timestamp: 2026-03-09T20:20:56.356Z
Learning: Applies to packages/frontend/src/**/*.{ts,tsx} : Do not depend on `lucky/shared` package in frontend code; make API calls to backend via configured base URL (env)
Applied to files:
packages/frontend/src/components/Layout/Sidebar.tsxpackages/frontend/src/App.tsx
📚 Learning: 2026-03-09T20:21:58.991Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/subagent-frontend.mdc:0-0
Timestamp: 2026-03-09T20:21:58.991Z
Learning: Keep frontend code scoped to `packages/frontend`; communicate with backend via `services/api.ts` using the configured env base URL; do not access shared database or Redis directly
Applied to files:
packages/frontend/src/components/Layout/Sidebar.tsx
📚 Learning: 2026-03-09T20:20:38.694Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-backend-api.mdc:0-0
Timestamp: 2026-03-09T20:20:38.694Z
Learning: Applies to packages/backend/src/**/*.{ts,tsx} : Keep tokens and secrets in environment variables only; never hardcode or expose in code
Applied to files:
packages/frontend/src/components/Layout/Sidebar.tsx
📚 Learning: 2026-03-09T20:20:23.892Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-09T20:20:23.892Z
Learning: Applies to packages/frontend/src/**/*.{ts,tsx} : Frontend uses path alias `@/` mapped to `src/` - use this alias for all imports from the src directory
Applied to files:
packages/frontend/src/components/Layout/Sidebar.tsx
📚 Learning: 2026-03-09T20:20:56.356Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-frontend.mdc:0-0
Timestamp: 2026-03-09T20:20:56.356Z
Learning: Applies to packages/frontend/src/{components,pages}/**/*.{ts,tsx} : Use React functional components and hooks; keep components small and focused
Applied to files:
packages/frontend/src/components/Layout/Sidebar.tsxpackages/frontend/src/App.tsx
📚 Learning: 2026-03-09T20:20:38.694Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-backend-api.mdc:0-0
Timestamp: 2026-03-09T20:20:38.694Z
Learning: Applies to packages/backend/tests/**/*.{ts,tsx} : Organize tests in `packages/backend/tests/` with unit tests under `unit/` and integration tests under `integration/`, following existing patterns with fixtures and setup
Applied to files:
packages/frontend/tests/e2e/helpers/api-helpers.tspackages/frontend/tests/e2e/servers-page.spec.tspackages/frontend/tests/e2e/dashboard-page.spec.ts
📚 Learning: 2026-03-09T20:21:31.459Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/quality.mdc:0-0
Timestamp: 2026-03-09T20:21:31.459Z
Learning: Applies to tests/**/*.test.{ts,tsx,js,jsx} : Add integration tests where appropriate
Applied to files:
packages/frontend/tests/e2e/servers-page.spec.tspackages/frontend/src/components/Layout/Sidebar.test.tsx
📚 Learning: 2026-03-09T20:20:56.356Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-frontend.mdc:0-0
Timestamp: 2026-03-09T20:20:56.356Z
Learning: Applies to packages/frontend/tests/**/*.{ts,tsx,js} : Write tests in `packages/frontend/tests/` using existing test patterns (e.g., Playwright for e2e if configured)
Applied to files:
packages/frontend/tests/e2e/servers-page.spec.tspackages/frontend/tests/e2e/dashboard-page.spec.ts
📚 Learning: 2026-03-09T20:20:56.356Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-frontend.mdc:0-0
Timestamp: 2026-03-09T20:20:56.356Z
Learning: Applies to packages/frontend/src/pages/**/*.{ts,tsx} : Organize pages in `packages/frontend/src/pages/` directory (e.g., Login, Dashboard, Config, Features, ServersPage)
Applied to files:
packages/frontend/src/App.tsx
📚 Learning: 2026-03-09T20:20:38.694Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-backend-api.mdc:0-0
Timestamp: 2026-03-09T20:20:38.694Z
Learning: Applies to packages/backend/src/{server,index}.{ts,tsx} : Backend entry point is `packages/backend/src/server.ts` which imports from `index.ts`
Applied to files:
packages/frontend/src/App.tsx
📚 Learning: 2026-03-09T20:21:38.098Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/subagent-backend.mdc:0-0
Timestamp: 2026-03-09T20:21:38.098Z
Learning: Applies to packages/backend/src/routes/**/*.ts : Place routes under `packages/backend/src/routes/` directory
Applied to files:
packages/frontend/src/App.tsx
📚 Learning: 2026-03-09T20:20:56.356Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-frontend.mdc:0-0
Timestamp: 2026-03-09T20:20:56.356Z
Learning: Applies to packages/frontend/src/{main,App}.tsx : Entry point for the Lucky Frontend React app is `packages/frontend/src/main.tsx` which connects to `App.tsx`
Applied to files:
packages/frontend/src/App.tsx
🪛 Biome (2.4.6)
packages/frontend/tests/e2e/fixtures/test-data.ts
[error] 51-51: This property is later overwritten by an object member with the same name.
(lint/suspicious/noDuplicateObjectKeys)
[error] 52-52: This property is later overwritten by an object member with the same name.
(lint/suspicious/noDuplicateObjectKeys)
[error] 73-73: This property is later overwritten by an object member with the same name.
(lint/suspicious/noDuplicateObjectKeys)
[error] 74-74: This property is later overwritten by an object member with the same name.
(lint/suspicious/noDuplicateObjectKeys)
[error] 96-96: This property is later overwritten by an object member with the same name.
(lint/suspicious/noDuplicateObjectKeys)
[error] 97-97: This property is later overwritten by an object member with the same name.
(lint/suspicious/noDuplicateObjectKeys)
🪛 GitHub Actions: CI/CD Pipeline
packages/frontend/tests/e2e/fixtures/test-data.ts
[error] 54-54: ESLint: no-dupe-keys - Duplicate key 'effectiveAccess'.
🪛 GitHub Check: SonarCloud Code Analysis
packages/frontend/src/components/Layout/Sidebar.tsx
[warning] 173-182: Mark the props of the component as read-only.
packages/frontend/tests/e2e/servers-page.spec.ts
[warning] 5-5: Remove this unused import of 'mockAuthStatus'.
[warning] 4-4: Remove this unused import of 'mockGuildsList'.
packages/frontend/tests/e2e/dashboard-page.spec.ts
[warning] 2-2: Remove this unused import of 'mockGuildsList'.
|
Merge currently blocked by two independent gates (no admin bypass used).\n\nEvidence snapshot (2026-03-11 America/Sao_Paulo):\n- PR is mergeable and checks are green, but base branch policy still reports .\n- Active ruleset on is with , , and constraints.\n- Review decision is currently (latest CodeRabbit review), with unresolved threads that must be addressed before merge.\n\nCurrent status: auto-merge request is enabled, but policy + review blockers still prevent merge. |
|
Superseding previous malformed automation note (shell escaping issue). Merge is currently blocked by two independent gates (no admin bypass used):
Current status: auto-merge request is enabled, but policy + review blockers still prevent merge. |
d62e103 to
5235810
Compare
|
|
…ntracts (#169) * fix(dashboard): recover guild visibility and harden access flow * fix(frontend): stabilize server selection and route access behavior * test(e2e): align dashboard servers navigation specs with rbac shell * test(frontend): reduce duplicated rbac fixtures in auth routes spec * fix(pr169): raise sonar coverage and simplify sidebar complexity * fix(pr169): close remaining dashboard review gates * fix(pr169): clear stale guild state when active guild is removed * test(pr169): fix duplicate e2e fixture keys after rebase * fix(pr169): resolve actionable review blockers



Summary
/serversalways accessible for authenticated users (not module-gated)botAddedguild and keep no selection when none have LuckyCommits
fix(frontend): stabilize server selection and route access behaviortest(e2e): align dashboard servers navigation specs with rbac shellVerification
npm run test --workspace=packages/frontend -- src/stores/guildStore.test.ts src/hooks/useGuildSelection.test.tsx src/App.authRoutes.test.tsx src/components/Layout/Sidebar.test.tsx src/pages/ServersPage.test.tsx src/pages/DashboardOverview.test.tsxCI=1 npm run test:e2e --workspace=packages/frontend -- tests/e2e/dashboard-page.spec.ts tests/e2e/servers-page.spec.ts tests/e2e/layout-navigation.spec.tsnpm run type:checknpm run lintnpm run buildAcceptance Mapping
/serversnow renders for authenticated users without module denialSummary by CodeRabbit
Release Notes
Bug Fixes
New Features
Improvements