Dev#7
Conversation
📝 WalkthroughWalkthroughAdds a client-side multi-step OnboardingDialog for guild create/join, a reusable Radix-based Dialog component, tracks onboardingCompleted in auth and DB, integrates the dialog into the authenticated layout, and updates sidebar components with per-item menus and layout tweaks. CORS origin logic simplified. Changes
Sequence DiagramsequenceDiagram
actor User
participant App as App / Router
participant Dialog as OnboardingDialog
participant Auth as Auth Service (authClient)
participant DB as Database
User->>App: Open app (session exists & onboarding not completed)
App->>Dialog: Render OnboardingDialog (open)
User->>Dialog: Select "Create Guild" -> enter name/slug
Dialog->>Auth: authClient.organization.create(name, slug)
Auth->>DB: Insert guild (ownerId set via beforeCreateOrganization)
DB-->>Auth: Guild created
Auth->>DB: Update user.onboardingCompleted = true (non-blocking)
DB-->>Auth: Update confirmed
Auth-->>Dialog: Return success (organization)
Dialog->>App: Navigate to new guild route
App-->>User: Guild view displayed
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/components/sidebar/channel-panel/user-bar.tsx (1)
30-32:⚠️ Potential issue | 🟠 MajorHandle potential errors from
authClient.signOut().If
signOut()rejects, the error is silently swallowed and the user remains logged in with no feedback.🛡️ Proposed fix
- const handleLogout = async () => { - await authClient.signOut() - } + const handleLogout = async () => { + try { + await authClient.signOut() + } catch (error) { + console.error("Sign out failed:", error) + // Surface error to user via toast/notification as appropriate + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/sidebar/channel-panel/user-bar.tsx` around lines 30 - 32, The logout handler handleLogout currently awaits authClient.signOut() without error handling; wrap the call in a try/catch inside handleLogout (and return early on failure) so rejected promises are handled, log the error (or send to your existing logger) and surface user feedback (e.g., show a toast/error banner) when signOut fails, and on success continue with existing post-logout steps (redirect, update auth state, etc.); reference handleLogout and authClient.signOut when making the change.
🧹 Nitpick comments (4)
apps/web/src/components/sidebar/channel-panel/user-bar.tsx (1)
120-123: UseonSelectinstead ofonClickfor consistency with Radix UI conventions.
DropdownMenuItemuses Radix UI's idiomaticonSelecthandler (see line 78 in the same file). UsingonClickworks but is inconsistent with the established pattern.The
variant="destructive"prop is already supported in the custom component and properly styled.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/sidebar/channel-panel/user-bar.tsx` around lines 120 - 123, Replace the onClick usage on the DropdownMenuItem with the Radix-consistent onSelect handler: change the DropdownMenuItem that currently uses onClick={handleLogout} to use onSelect={handleLogout} so it matches other menu items (see DropdownMenuItem and handleLogout in this file) and preserves the variant="destructive" styling and behavior.apps/web/src/components/onboarding/onboarding-dialog.tsx (1)
193-203: Raw<input>instead of the<Input>component.The guild-name field (line 177) uses the styled
<Input>component, but the slug field uses a raw<input>. This creates inconsistent focus/accessibility behavior. Consider using the<Input>component here as well, or at minimum ensuring the raw input picks up the same focus ring and accessibility attributes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/onboarding/onboarding-dialog.tsx` around lines 193 - 203, The slug field uses a raw <input> causing inconsistent styling and focus/accessibility compared to the styled <Input> used for guild-name; replace the raw element with the shared Input component (or wrap it to accept the same props) and wire up the same props: id="guild-slug", value={slug}, onChange={(e) => { setSlugEdited(true); setSlug(toSlug(e.target.value)); }}, disabled={loading} so it inherits the same focus ring and ARIA/accessibility behavior as the guild-name field.packages/ui/src/components/dialog.tsx (1)
109-116: Footer close button renders after children — ordering may surprise consumers.With
sm:flex-row sm:justify-end, the close button will appear to the right ofchildren. Conventionally the dismiss action sits to the left of the primary action. Consider rendering the close button before{children}, or document the expected ordering for consumers.Proposed reorder
> + {showCloseButton && ( + <DialogPrimitive.Close asChild> + <Button variant="outline">Close</Button> + </DialogPrimitive.Close> + )} {children} - {showCloseButton && ( - <DialogPrimitive.Close asChild> - <Button variant="outline">Close</Button> - </DialogPrimitive.Close> - )} </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/dialog.tsx` around lines 109 - 116, The footer currently renders the close button after {children}, which places it to the right under "sm:flex-row sm:justify-end"; change the order so the conditional DialogPrimitive.Close (wrapping <Button variant="outline">Close</Button>) is rendered before {children} (i.e., check showCloseButton and output DialogPrimitive.Close before children) so the dismiss action appears to the left of primary actions; keep the existing conditional, asChild wrapper, and styling classes intact when reordering.apps/web/src/components/sidebar/channel-panel/channel-list.tsx (1)
576-594: Dropdown menu items have no click handlers.
Edit Channel,Copy Channel ID, andDelete Channelare rendered but none haveonClickhandlers, so they're non-functional. If these are intentional placeholders, consider adding a// TODOcomment or disabling the items to signal that they're not yet wired up. The destructive "Delete Channel" item is especially worth calling out — a user clicking it would get no feedback.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/sidebar/channel-panel/channel-list.tsx` around lines 576 - 594, Dropdown menu items rendered with DropdownMenuItem (inside the ChannelList component) lack onClick handlers and therefore are non-functional; add explicit onClick handlers or visibly mark them as TODO/disabled. Create and reference handler functions (e.g., handleEditChannel, handleCopyChannelId, handleDeleteChannel) and attach them to the respective DropdownMenuItem components, ensuring Delete Channel triggers a confirmation flow or is marked destructive and disabled until implemented; alternatively add a clear TODO comment and set a disabled prop on each DropdownMenuItem to indicate they are placeholders.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/components/onboarding/onboarding-dialog.tsx`:
- Around line 54-68: The current success path is non-atomic: after
authClient.organization.create succeeds, a thrown error from
authClient.updateUser would leave a created guild but onboardingCompleted false,
causing the onboarding dialog to reappear and a slug conflict; update the flow
in the onSubmit/handler that calls authClient.organization.create so that you
catch failures from authClient.updateUser separately (wrap the call to
authClient.updateUser in its own try/catch), ensure
queryClient.invalidateQueries(["guilds"]) and navigate({ to: "/$guildSlug",
params: { guildSlug: slug.trim() } }) still run even if updateUser fails, and
surface a non-blocking warning via setError or another UI indicator while still
setting setLoading(false) — reference authClient.organization.create,
authClient.updateUser, queryClient.invalidateQueries, navigate, setError, and
setLoading to locate and change the code.
- Around line 21-29: The toSlug function can produce leading/trailing hyphens
because hyphens survive the character filter and collapsing step; update toSlug
(the function shown) to trim any leading/trailing hyphens after collapsing
spaces/dashes and before enforcing the max length—e.g., run a final replace to
remove /^-+|-+$/g (or equivalent) and then apply slice(0,50) (or apply slice
before final trim if you prefer to ensure length) so the returned slug has no
leading/trailing hyphens and respects the 50-char limit.
In `@apps/web/src/components/sidebar/channel-panel/user-bar.tsx`:
- Around line 51-52: The "Online" label in the UserBar component is hardcoded;
change it to use the real presence value (or stop rendering it) by reading the
presence prop/state (e.g., user.presence, presence.status, or isOnline) inside
the component in apps/web/src/components/sidebar/channel-panel/user-bar.tsx and
conditionally render the text and/or styling based on that value (e.g., show
"Online", "Away", "Offline" or omit the element when presence is unknown);
ensure you handle null/undefined presence with a sensible fallback and update
any relevant prop types or hooks used to supply presence.
- Around line 112-116: The Settings DropdownMenuItem in user-bar.tsx is a no-op
because it lacks an event handler; add a navigation handler to the
DropdownMenuItem (e.g., attach onSelect or onClick) that routes to the settings
page (for Next apps use useRouter().push('/settings') or wrap the item with a
Link) so clicking the Settings item actually navigates to "/settings" (reference
DropdownMenuItem and the Settings icon in user-bar.tsx).
- Around line 126-131: The Settings icon-only button is unlabeled and has no
click action; add an accessible label (aria-label="Open settings" and
title="Open settings") to the button containing the <Settings /> icon and wire
its onClick to the same handler used by the Settings dropdown item (e.g., call
handleOpenSettings / openSettings or reuse the dropdown's openSettings function)
so clicking or activating the icon toggles/opens the settings UI; ensure the
handler name you call matches the existing function in the component (or lift
the dropdown's handler into a shared function) so behavior and accessibility are
consistent.
In `@apps/web/src/components/sidebar/dm-panel/dm-panel.tsx`:
- Around line 31-37: The "Message Requests" button in the DmPanel JSX is
currently a non-functional stub; either wire it to the intended handler or make
its placeholder intent explicit. If you don't have the navigation implemented
yet, add disabled and aria-disabled attributes to the button element (and a
brief TODO comment above it referencing implementMessageRequests or
navigateToMessageRequests) and remove hover styling so it doesn't appear
interactive; alternatively, if a route exists, add an onClick that calls the
existing navigation function (e.g., navigateToMessageRequests or
router.push('/message-requests')) and ensure the handler is imported and typed.
Ensure you update the button JSX (the <button> with <Inbox /> and "Message
Requests") accordingly so the intent is explicit and accessible.
In `@apps/web/src/routes/_authenticated.tsx`:
- Around line 47-54: The onboarding flag defaulting to false will force all
existing users through onboarding; update the logic around showOnboarding (used
to control OnboardingDialog and reading session.user.onboardingCompleted) to
also check for existing guild membership or perform a backfill migration: either
add a migration that sets onboardingCompleted = true for users with existing
guilds, or change showOnboarding to require both onboardingCompleted === false
AND no existing guilds (e.g., derive hasExistingGuilds from the session or a
quick query of the Guild/User relation) so current users with guilds aren’t
shown the dialog.
In `@packages/db/src/schemas/users.ts`:
- Line 28: The new users schema field onboardingCompleted (onboarding_completed)
being NOT NULL DEFAULT false will mark all existing users as not onboarded; add
a migration that, before or immediately after applying the schema change,
updates existing users who already have guild memberships to
onboarding_completed = true (e.g., UPDATE users SET onboarding_completed = TRUE
WHERE EXISTS (SELECT 1 FROM guild_memberships WHERE guild_memberships.user_id =
users.id)); keep the NOT NULL DEFAULT false for new users so only future users
default to false. Ensure the migration references the users table and the guild
membership table used by your app (guild_memberships or equivalent) and is run
as part of the deployment.
---
Outside diff comments:
In `@apps/web/src/components/sidebar/channel-panel/user-bar.tsx`:
- Around line 30-32: The logout handler handleLogout currently awaits
authClient.signOut() without error handling; wrap the call in a try/catch inside
handleLogout (and return early on failure) so rejected promises are handled, log
the error (or send to your existing logger) and surface user feedback (e.g.,
show a toast/error banner) when signOut fails, and on success continue with
existing post-logout steps (redirect, update auth state, etc.); reference
handleLogout and authClient.signOut when making the change.
---
Nitpick comments:
In `@apps/web/src/components/onboarding/onboarding-dialog.tsx`:
- Around line 193-203: The slug field uses a raw <input> causing inconsistent
styling and focus/accessibility compared to the styled <Input> used for
guild-name; replace the raw element with the shared Input component (or wrap it
to accept the same props) and wire up the same props: id="guild-slug",
value={slug}, onChange={(e) => { setSlugEdited(true);
setSlug(toSlug(e.target.value)); }}, disabled={loading} so it inherits the same
focus ring and ARIA/accessibility behavior as the guild-name field.
In `@apps/web/src/components/sidebar/channel-panel/channel-list.tsx`:
- Around line 576-594: Dropdown menu items rendered with DropdownMenuItem
(inside the ChannelList component) lack onClick handlers and therefore are
non-functional; add explicit onClick handlers or visibly mark them as
TODO/disabled. Create and reference handler functions (e.g., handleEditChannel,
handleCopyChannelId, handleDeleteChannel) and attach them to the respective
DropdownMenuItem components, ensuring Delete Channel triggers a confirmation
flow or is marked destructive and disabled until implemented; alternatively add
a clear TODO comment and set a disabled prop on each DropdownMenuItem to
indicate they are placeholders.
In `@apps/web/src/components/sidebar/channel-panel/user-bar.tsx`:
- Around line 120-123: Replace the onClick usage on the DropdownMenuItem with
the Radix-consistent onSelect handler: change the DropdownMenuItem that
currently uses onClick={handleLogout} to use onSelect={handleLogout} so it
matches other menu items (see DropdownMenuItem and handleLogout in this file)
and preserves the variant="destructive" styling and behavior.
In `@packages/ui/src/components/dialog.tsx`:
- Around line 109-116: The footer currently renders the close button after
{children}, which places it to the right under "sm:flex-row sm:justify-end";
change the order so the conditional DialogPrimitive.Close (wrapping <Button
variant="outline">Close</Button>) is rendered before {children} (i.e., check
showCloseButton and output DialogPrimitive.Close before children) so the dismiss
action appears to the left of primary actions; keep the existing conditional,
asChild wrapper, and styling classes intact when reordering.
| <div className="truncate text-[11px] leading-tight text-muted-foreground"> | ||
| Online |
There was a problem hiding this comment.
"Online" status is hardcoded.
The status label is always "Online" regardless of actual user presence. This will mislead users who are away or offline. Either drive this from a real presence value or omit it until presence tracking is implemented.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/components/sidebar/channel-panel/user-bar.tsx` around lines 51 -
52, The "Online" label in the UserBar component is hardcoded; change it to use
the real presence value (or stop rendering it) by reading the presence
prop/state (e.g., user.presence, presence.status, or isOnline) inside the
component in apps/web/src/components/sidebar/channel-panel/user-bar.tsx and
conditionally render the text and/or styling based on that value (e.g., show
"Online", "Away", "Offline" or omit the element when presence is unknown);
ensure you handle null/undefined presence with a sensible fallback and update
any relevant prop types or hooks used to supply presence.
| <DropdownMenuItem> | ||
| <Settings className="h-4 w-4 mr-2" /> | ||
| Settings | ||
| </DropdownMenuItem> | ||
| </DropdownMenuGroup> |
There was a problem hiding this comment.
Settings DropdownMenuItem has no handler — clicking is a no-op.
Neither onSelect nor onClick is attached, so the item produces no navigation or action. Either wire up a handler (e.g., route to /settings) or remove the item until the settings page exists.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/components/sidebar/channel-panel/user-bar.tsx` around lines 112
- 116, The Settings DropdownMenuItem in user-bar.tsx is a no-op because it lacks
an event handler; add a navigation handler to the DropdownMenuItem (e.g., attach
onSelect or onClick) that routes to the settings page (for Next apps use
useRouter().push('/settings') or wrap the item with a Link) so clicking the
Settings item actually navigates to "/settings" (reference DropdownMenuItem and
the Settings icon in user-bar.tsx).
| <button | ||
| type="button" | ||
| className="flex size-8 shrink-0 items-center justify-center rounded-md text-muted-foreground hover:bg-foreground/5 hover:text-foreground" | ||
| > | ||
| <Settings className="size-4" /> | ||
| </button> |
There was a problem hiding this comment.
Icon-only Settings button is inaccessible and non-functional.
Two separate problems:
- Accessibility blocker — the button has no
aria-labelortitle. Screen readers will announce it as an unlabeled button, making it unusable for keyboard/AT users. - No handler — like the Settings item inside the dropdown, clicking this button does nothing.
🛡️ Proposed fix
<button
type="button"
+ aria-label="Settings"
className="flex size-8 shrink-0 items-center justify-center rounded-md text-muted-foreground hover:bg-foreground/5 hover:text-foreground"
+ onClick={() => { /* navigate to /settings */ }}
>
<Settings className="size-4" />
</button>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <button | |
| type="button" | |
| className="flex size-8 shrink-0 items-center justify-center rounded-md text-muted-foreground hover:bg-foreground/5 hover:text-foreground" | |
| > | |
| <Settings className="size-4" /> | |
| </button> | |
| <button | |
| type="button" | |
| aria-label="Settings" | |
| className="flex size-8 shrink-0 items-center justify-center rounded-md text-muted-foreground hover:bg-foreground/5 hover:text-foreground" | |
| onClick={() => { /* navigate to /settings */ }} | |
| > | |
| <Settings className="size-4" /> | |
| </button> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/components/sidebar/channel-panel/user-bar.tsx` around lines 126
- 131, The Settings icon-only button is unlabeled and has no click action; add
an accessible label (aria-label="Open settings" and title="Open settings") to
the button containing the <Settings /> icon and wire its onClick to the same
handler used by the Settings dropdown item (e.g., call handleOpenSettings /
openSettings or reuse the dropdown's openSettings function) so clicking or
activating the icon toggles/opens the settings UI; ensure the handler name you
call matches the existing function in the component (or lift the dropdown's
handler into a shared function) so behavior and accessibility are consistent.
| username: text("username").unique(), | ||
| displayUsername: text("display_username"), | ||
| twoFactorEnabled: boolean("two_factor_enabled").default(false), | ||
| onboardingCompleted: boolean("onboarding_completed").default(false).notNull(), |
There was a problem hiding this comment.
Migration concern: existing users will get onboarding_completed = false.
Adding a NOT NULL DEFAULT false column applies the default to all existing rows. This is the root cause of the issue flagged in _authenticated.tsx — every current user will be shown the onboarding dialog. Consider pairing this schema change with a data migration that sets onboarding_completed = true for users who already have guild memberships.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/db/src/schemas/users.ts` at line 28, The new users schema field
onboardingCompleted (onboarding_completed) being NOT NULL DEFAULT false will
mark all existing users as not onboarded; add a migration that, before or
immediately after applying the schema change, updates existing users who already
have guild memberships to onboarding_completed = true (e.g., UPDATE users SET
onboarding_completed = TRUE WHERE EXISTS (SELECT 1 FROM guild_memberships WHERE
guild_memberships.user_id = users.id)); keep the NOT NULL DEFAULT false for new
users so only future users default to false. Ensure the migration references the
users table and the guild membership table used by your app (guild_memberships
or equivalent) and is run as part of the deployment.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/api/src/app.ts (1)
35-49: Remove stale commented-out code.The old loop-based routing approach is fully superseded by the chained
.route()calls above. This block is dead weight.🧹 Proposed cleanup
-// // Internal routes (not versioned) -// const internalRoutes = [waitlistRouter] as const -// for (const route of internalRoutes) { -// app.route("/", route) -// } -// -// // Versioned public API routes -// const v1Routes = [channelsRouter] as const -// for (const route of v1Routes) { -// app.route("/v1", route) -// } -// -// const allRoutes = [...internalRoutes, ...v1Routes] as const -// -// export type AppType = (typeof allRoutes)[number]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app.ts` around lines 35 - 49, Remove the stale commented-out routing block that defines internalRoutes, v1Routes, the allRoutes spread, and the exported AppType (these refer to waitlistRouter, channelsRouter and used the old loop-based app.route logic); delete the entire commented section so only the active chained app.route() calls remain and no unused commented declarations (internalRoutes, v1Routes, allRoutes, AppType) linger in apps/api/src/app.ts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api/src/app.ts`:
- Around line 15-16: The CORS config in app.ts currently uses origin: "*" and
credentials: false which both weakens security and breaks cookie-based auth;
update the CORS middleware (the object with origin and credentials) to restore
an environment-scoped origin whitelist (use the existing env values previously
used: dev localhost and NEXT_PUBLIC_API_URL or validate request Origin
dynamically) and set credentials: true so cookies are allowed; ensure you do NOT
send Access-Control-Allow-Origin: * when credentials: true by returning the
validated origin value per request and keep client-side using credentials:
"include" to match.
---
Nitpick comments:
In `@apps/api/src/app.ts`:
- Around line 35-49: Remove the stale commented-out routing block that defines
internalRoutes, v1Routes, the allRoutes spread, and the exported AppType (these
refer to waitlistRouter, channelsRouter and used the old loop-based app.route
logic); delete the entire commented section so only the active chained
app.route() calls remain and no unused commented declarations (internalRoutes,
v1Routes, allRoutes, AppType) linger in apps/api/src/app.ts.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
apps/web/src/components/sidebar/channel-panel/user-bar.tsx (1)
12-12:useNavigateis imported and instantiated but unused.
navigateis only referenced inside the commented-out body ofhandleOpenSettings. This is dead code until the settings route is wired up. Consider removing both the import and the call to avoid lint warnings and unnecessary bundle cost.Also applies to: 27-27, 40-43
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/sidebar/channel-panel/user-bar.tsx` at line 12, The import and call to useNavigate are unused: remove the import "useNavigate" and the const navigate = useNavigate() instantiation in user-bar.tsx, and also remove or leave commented-out any references inside the handleOpenSettings function (specifically the commented navigation usage that references navigate) so there is no dead code; check and remove any other unused navigate usages around handleOpenSettings and the related commented block (lines referencing handleOpenSettings, navigate, and the useNavigate import) to satisfy linting and reduce bundle size.apps/api/src/app.ts (1)
35-49: Remove commented-out dead code.This block duplicates the active route-mounting logic above and adds noise. If it's kept for reference, consider moving the rationale into a commit message or ADR instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app.ts` around lines 35 - 49, Remove the commented-out duplicate route mounting block (the commented declarations and loops for internalRoutes, v1Routes, allRoutes and the exported AppType referencing waitlistRouter and channelsRouter) as dead code; simply delete the entire commented section so only the active route setup remains (keep any necessary logic already implemented above) and, if you need to preserve rationale, move it to the commit message or an ADR instead of inline comments.apps/web/src/components/onboarding/onboarding-dialog.tsx (1)
1-1:"use client"directive is unnecessary in a TanStack Router app.TanStack Router doesn't use React Server Components, so this directive is a no-op. It won't cause harm but is misleading about the framework in use.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/onboarding/onboarding-dialog.tsx` at line 1, Remove the unnecessary "use client" directive at the top of the module (the literal "use client" string) — it's a no-op in this TanStack Router app and misleading; simply delete that line from onboarding-dialog.tsx so the file doesn't suggest RSC usage. Ensure no other code depends on client-only semantics before committing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/components/onboarding/onboarding-dialog.tsx`:
- Around line 49-81: The handleCreate function never resets loading on the
success path; move the setLoading(false) into a finally block (or ensure it runs
after invalidateQueries and before/after navigate) so loading is always cleared
whether the create/update/invalidate/navigate steps succeed or fail; update
handleCreate to wrap the try/catch in a try/finally (or add a finally) that
calls setLoading(false) and keep the existing setError calls intact; reference
authClient.organization.create, authClient.updateUser,
queryClient.invalidateQueries, navigate, and setLoading in the change.
In `@apps/web/src/components/sidebar/dm-panel/dm-panel.tsx`:
- Around line 47-52: The Plus button currently rendered with <Plus
className="size-4" /> is interactive but lacks a handler; update that JSX to
match the "Message Requests" disabled-stub pattern by adding the disabled
attribute and aria-disabled="true" to the button element and insert a TODO
comment inside the button explaining the click handler should be implemented
later (e.g., "TODO: implement new DM flow"), keeping the existing className and
semantics intact.
---
Duplicate comments:
In `@apps/api/src/app.ts`:
- Around line 14-17: The current CORS config in app.ts uses origin: (origin) =>
origin with credentials: true which echoes any origin and permits authenticated
requests; change this to validate against a maintained allowlist: create an
array of allowed origins (e.g., allowedOrigins) and replace origin: (origin) =>
origin with a validator function that returns the origin only if it is present
in allowedOrigins (otherwise return false or undefined), ensuring credentials:
true remains effective only for trusted origins; update the validator to handle
undefined/non-browser requests (allow server-to-server) and reference the
cors(...) call in app.ts when implementing the fix.
---
Nitpick comments:
In `@apps/api/src/app.ts`:
- Around line 35-49: Remove the commented-out duplicate route mounting block
(the commented declarations and loops for internalRoutes, v1Routes, allRoutes
and the exported AppType referencing waitlistRouter and channelsRouter) as dead
code; simply delete the entire commented section so only the active route setup
remains (keep any necessary logic already implemented above) and, if you need to
preserve rationale, move it to the commit message or an ADR instead of inline
comments.
In `@apps/web/src/components/onboarding/onboarding-dialog.tsx`:
- Line 1: Remove the unnecessary "use client" directive at the top of the module
(the literal "use client" string) — it's a no-op in this TanStack Router app and
misleading; simply delete that line from onboarding-dialog.tsx so the file
doesn't suggest RSC usage. Ensure no other code depends on client-only semantics
before committing.
In `@apps/web/src/components/sidebar/channel-panel/user-bar.tsx`:
- Line 12: The import and call to useNavigate are unused: remove the import
"useNavigate" and the const navigate = useNavigate() instantiation in
user-bar.tsx, and also remove or leave commented-out any references inside the
handleOpenSettings function (specifically the commented navigation usage that
references navigate) so there is no dead code; check and remove any other unused
navigate usages around handleOpenSettings and the related commented block (lines
referencing handleOpenSettings, navigate, and the useNavigate import) to satisfy
linting and reduce bundle size.
Summary by CodeRabbit
New Features
Chores