Dev#3
Conversation
📝 WalkthroughWalkthroughThe changes establish a channels API feature by introducing authentication middleware, database schemas for guild contexts, OpenAPI definitions with error responses, HTTP status codes, REST endpoints for listing and creating channels, and a comprehensive sidebar UI component suite with user authentication integration. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 3❌ Failed checks (2 warnings, 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)
⚔️ Resolve merge conflicts (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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@apps/api/package.json`:
- Around line 14-24: Update the dependency entry for "@hono/zod-openapi" in
package.json from "^1.2.2" to the published latest "^1.2.1"; locate the
dependency key "@hono/zod-openapi" in the dependencies block and change its
version string to "^1.2.1" (keeping the existing zod dependency as-is since
"@hono/zod-openapi@1.2.1" is compatible with "zod@^4.3.6").
In `@apps/api/src/routes/v1/channels/routes.ts`:
- Around line 16-57: Add 400 and 404 responses to the OpenAPI response maps for
both listChannels and createChannel: include HttpStatusCodes.BAD_REQUEST with a
jsonContent/schema (e.g., badRequestSchema) describing "No active guild
selected" and for POST also "validation errors" from the
createChannelRequestSchema, and include HttpStatusCodes.NOT_FOUND with a
jsonContent/schema (e.g., notFoundSchema) describing "Guild not found"; update
the responses object in the createRoute calls for listChannels and createChannel
(referencing listChannels, createChannel, authMiddleware, and
createChannelRequestSchema) so client generation and docs include these cases.
In `@apps/web/src/components/sidebar/guild-bar.tsx`:
- Around line 46-51: The Home/DM area, each GuildIcon, and the Add guild control
are currently rendered as non-interactive <div>s; replace those wrappers with
semantic interactive elements (e.g., <button type="button"> for actions or <a>
for navigation) so they become keyboard-focusable and expose correct
semantics—update the JSX in guild-bar.tsx where the Home/DM group, the GuildIcon
wrapper component, and the Add guild element are defined, keep existing
className styling, add appropriate aria-labels (e.g., "Home", guild name, "Add
guild"), and ensure you do not nest interactive elements inside buttons;
preserve click handlers but move them to the new button/anchor elements.
In `@apps/web/src/components/sidebar/user-bar.tsx`:
- Around line 8-9: The username constant is incorrectly derived from
session?.user.name causing name and username to be identical; change the
username assignment to read session?.user.username instead. Update the variable
declaration for username (in the user-bar component where const name =
session?.user.name ?? "User" and const username = ...) to use
session?.user.username ?? "user" so it falls back correctly when absent, and
keep the name assignment unchanged.
🧹 Nitpick comments (11)
apps/web/src/routes/__root.tsx (1)
16-16: Commented-out devtools with unused lazy import.The
TanStackRouterDevtoolslazy import (Lines 4–10) is still present but the usage is commented out. Consider removing the commented-out JSX or re-enabling it — leaving dead code can cause confusion. The empty<Suspense>wrapper also serves no purpose without children.♻️ Suggested cleanup
- <Suspense>{/*<TanStackRouterDevtools />*/}</Suspense> + <Suspense> + <TanStackRouterDevtools /> + </Suspense>Or, if devtools should stay disabled, remove both the import and the
<Suspense>wrapper entirely.apps/web/src/components/sidebar/channel-list.tsx (1)
19-87: Channel items lack interactive semantics.Similar to the guild bar, channel items (both text and voice) are rendered as plain
<div>elements. When these are wired up to real navigation, ensure they become<button>or<a>elements (or use a router<Link>) for keyboard accessibility and screen-reader support. Fine for scaffolding, just flagging for when mock data is replaced.apps/api/src/lib/helpers/openapi/schemas.ts (2)
4-28:internalServerErrorSchema(Line 30) usesz.boolean()while the new error schemas usez.literal(false)— consider aligning.The new
errorSchemacorrectly constrainssuccesstoz.literal(false), but the pre-existinginternalServerErrorSchemastill usesz.boolean(), allowingsuccess: truefor a 500 response. Consider refactoringinternalServerErrorSchemato also use the sharederrorSchemafor consistency.♻️ Proposed fix
export const internalServerErrorSchema = jsonContent({ - schema: z - .object({ - success: z.boolean(), - message: z.string(), - }) - .openapi({ - example: { - success: false, - message: "Internal server error", - }, - }), + schema: errorSchema.openapi({ + example: { success: false, message: "Internal server error" }, + }), description: "Internal server error", })
2-2: Use@/*path alias for local imports inapps/api/src.Per coding guidelines, compiled packages like
apps/apishould use the@/*path alias (rewritten bytsc-aliasat build time) instead of relative imports.♻️ Proposed fix
-import jsonContent from "./json-content" +import jsonContent from "@/lib/helpers/openapi/json-content"As per coding guidelines: "Use
@/* path alias for imports in compiled packages like apps/api (tsc-alias rewrites to relative .js paths at build time)"packages/db/src/schemas/guilds.ts (2)
48-50:insertGuildSchemadoesn't omit server-managed fields unlikeinsertChannelSchema.
insertChannelSchemacarefully omitsid,createdAt,updatedAt,guildId,ownerId, etc. — butinsertGuildSchemaexposes everything. If this schema will be used for API-facing input validation, consider omitting fields that should be server-managed (e.g.,id,createdAt,ownerId,slug).♻️ Proposed fix
export const insertGuildSchema = createInsertSchema(guild, { name: (s) => s.min(1).max(100), -}) +}).omit({ + id: true, + createdAt: true, + ownerId: true, +})
52-54: Same consideration forupdateGuildSchema— omit immutable fields.Similar to the insert schema, consider omitting fields that shouldn't be user-modifiable (e.g.,
id,createdAt,ownerId), consistent with the pattern inupdateChannelSchema.apps/api/src/lib/helpers/http/status-codes.ts (1)
1-2: Consider using thehttp-status-codesnpm package instead of vendoring.The comment indicates this file was generated from the
prettymuchbryce/http-status-codesrepository. That package is available on npm and provides both status codes and phrases with TypeScript types. Using it directly avoids maintaining a vendored copy and keeps you up-to-date.apps/api/src/routes/v1/channels/handlers.ts (1)
23-30: Consider wrapping the DB insert in a try/catch for constraint violations.If a database constraint fails (e.g., duplicate channel name within a guild), the unhandled exception will bypass your structured
{ success, message }error format. If there's no global error handler producing that shape, clients will get an inconsistent error response.apps/api/src/middleware/auth.ts (1)
38-69: Run the member and guild lookups in parallel.The
memberRecordandguildRecordqueries are independent — both useactiveGuildIdand don't depend on each other. Running them withPromise.allsaves a round-trip on every authenticated request.⚡ Proposed parallel queries
- const memberRecord = await db - .select() - .from(guildMember) - .where( - and( - eq(guildMember.userId, session.user.id), - eq(guildMember.guildId, activeGuildId) - ) - ) - .limit(1) - .then((rows) => rows[0]) - - if (!memberRecord) { - return c.json( - { success: false, message: "You are not a member of this guild" }, - HttpStatusCodes.FORBIDDEN - ) - } - - const guildRecord = await db - .select() - .from(guild) - .where(eq(guild.id, activeGuildId)) - .limit(1) - .then((rows) => rows[0]) - - if (!guildRecord) { - return c.json( - { success: false, message: "Guild not found" }, - HttpStatusCodes.NOT_FOUND - ) - } + const [memberRecord, guildRecord] = await Promise.all([ + db + .select() + .from(guildMember) + .where( + and( + eq(guildMember.userId, session.user.id), + eq(guildMember.guildId, activeGuildId) + ) + ) + .limit(1) + .then((rows) => rows[0]), + db + .select() + .from(guild) + .where(eq(guild.id, activeGuildId)) + .limit(1) + .then((rows) => rows[0]), + ]) + + if (!guildRecord) { + return c.json( + { success: false, message: "Guild not found" }, + HttpStatusCodes.NOT_FOUND + ) + } + + if (!memberRecord) { + return c.json( + { success: false, message: "You are not a member of this guild" }, + HttpStatusCodes.FORBIDDEN + ) + }apps/api/src/lib/types/app-types.ts (1)
10-18: AllVariablesfields are required, which assumes every route runsauthMiddleware.Routes that don't use
authMiddleware(e.g., health check, waitlist) will havec.var.guildtyped asGuildbut actuallyundefinedat runtime. If those routes never access these variables this is fine, but it's a latent type-safety gap. Consider whether unauthenticated routes use a differentAppBindingsvariant or if these fields should be optional.apps/api/src/routes/v1/channels/schema.ts (1)
4-4:channelResponseSchemaappears unused.It's not referenced in
routes.tsorhandlers.ts. If it's intended for future use, a brief comment would help; otherwise consider removing it.
| export const listChannels = createRoute({ | ||
| path: "/channels", | ||
| method: "get", | ||
| summary: "List channels", | ||
| description: "Lists all channels in the user's active guild.", | ||
| tags: ["Channels"], | ||
| middleware: [authMiddleware] as const, | ||
| responses: { | ||
| [HttpStatusCodes.OK]: jsonContent({ | ||
| schema: listChannelsResponseSchema, | ||
| description: "List of channels", | ||
| }), | ||
| [HttpStatusCodes.UNAUTHORIZED]: unauthorizedSchema, | ||
| [HttpStatusCodes.FORBIDDEN]: forbiddenSchema, | ||
| [HttpStatusCodes.INTERNAL_SERVER_ERROR]: internalServerErrorSchema, | ||
| }, | ||
| }) | ||
|
|
||
| export const createChannel = createRoute({ | ||
| path: "/channels", | ||
| method: "post", | ||
| summary: "Create a channel", | ||
| description: | ||
| "Creates a new channel in the user's active guild. Guild ID is derived from the session.", | ||
| tags: ["Channels"], | ||
| middleware: [authMiddleware] as const, | ||
| request: { | ||
| body: jsonContent({ | ||
| schema: createChannelRequestSchema, | ||
| description: "Channel details", | ||
| }), | ||
| }, | ||
| responses: { | ||
| [HttpStatusCodes.CREATED]: jsonContent({ | ||
| schema: createChannelResponseSchema, | ||
| description: "Channel created", | ||
| }), | ||
| [HttpStatusCodes.UNAUTHORIZED]: unauthorizedSchema, | ||
| [HttpStatusCodes.FORBIDDEN]: forbiddenSchema, | ||
| [HttpStatusCodes.INTERNAL_SERVER_ERROR]: internalServerErrorSchema, | ||
| }, | ||
| }) |
There was a problem hiding this comment.
OpenAPI responses are missing 400 Bad Request and 404 Not Found.
Both routes can return 400 (middleware: "No active guild selected"; POST also from zod validation failures) and 404 (middleware: "Guild not found"). Documenting these in the OpenAPI spec ensures accurate client codegen and API docs.
🤖 Prompt for AI Agents
In `@apps/api/src/routes/v1/channels/routes.ts` around lines 16 - 57, Add 400 and
404 responses to the OpenAPI response maps for both listChannels and
createChannel: include HttpStatusCodes.BAD_REQUEST with a jsonContent/schema
(e.g., badRequestSchema) describing "No active guild selected" and for POST also
"validation errors" from the createChannelRequestSchema, and include
HttpStatusCodes.NOT_FOUND with a jsonContent/schema (e.g., notFoundSchema)
describing "Guild not found"; update the responses object in the createRoute
calls for listChannels and createChannel (referencing listChannels,
createChannel, authMiddleware, and createChannelRequestSchema) so client
generation and docs include these cases.
| <div className="group relative flex items-center justify-center px-3 py-1"> | ||
| <div className="absolute left-0 h-0 w-1 rounded-r-full bg-foreground transition-all group-hover:h-5" /> | ||
| <div className="flex size-12 items-center justify-center rounded-[24px] bg-muted text-muted-foreground transition-all hover:rounded-2xl hover:bg-primary hover:text-primary-foreground"> | ||
| <MessageCircle className="size-6" /> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
Interactive elements should be buttons for accessibility.
The Home/DM area (Lines 46–51), guild icons, and the Add guild button (Lines 65–69) are all <div> elements that behave as interactive controls. They are not keyboard-focusable and lack semantic roles, which breaks accessibility for keyboard and screen-reader users.
Wrap each in a <button type="button"> (or anchor if navigating) to get native focus, keyboard activation, and screen-reader semantics.
Example for the Home/DM button
- <div className="group relative flex items-center justify-center px-3 py-1">
- <div className="absolute left-0 h-0 w-1 rounded-r-full bg-foreground transition-all group-hover:h-5" />
- <div className="flex size-12 items-center justify-center rounded-[24px] bg-muted text-muted-foreground transition-all hover:rounded-2xl hover:bg-primary hover:text-primary-foreground">
+ <button type="button" className="group relative flex items-center justify-center px-3 py-1">
+ <div className="absolute left-0 h-0 w-1 rounded-r-full bg-foreground transition-all group-hover:h-5" />
+ <div className="flex size-12 items-center justify-center rounded-[24px] bg-muted text-muted-foreground transition-all hover:rounded-2xl hover:bg-primary hover:text-primary-foreground">
<MessageCircle className="size-6" />
</div>
- </div>
+ </button>Apply similarly to GuildIcon and the Add guild element.
📝 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.
| <div className="group relative flex items-center justify-center px-3 py-1"> | |
| <div className="absolute left-0 h-0 w-1 rounded-r-full bg-foreground transition-all group-hover:h-5" /> | |
| <div className="flex size-12 items-center justify-center rounded-[24px] bg-muted text-muted-foreground transition-all hover:rounded-2xl hover:bg-primary hover:text-primary-foreground"> | |
| <MessageCircle className="size-6" /> | |
| </div> | |
| </div> | |
| <button type="button" className="group relative flex items-center justify-center px-3 py-1"> | |
| <div className="absolute left-0 h-0 w-1 rounded-r-full bg-foreground transition-all group-hover:h-5" /> | |
| <div className="flex size-12 items-center justify-center rounded-[24px] bg-muted text-muted-foreground transition-all hover:rounded-2xl hover:bg-primary hover:text-primary-foreground"> | |
| <MessageCircle className="size-6" /> | |
| </div> | |
| </button> |
🤖 Prompt for AI Agents
In `@apps/web/src/components/sidebar/guild-bar.tsx` around lines 46 - 51, The
Home/DM area, each GuildIcon, and the Add guild control are currently rendered
as non-interactive <div>s; replace those wrappers with semantic interactive
elements (e.g., <button type="button"> for actions or <a> for navigation) so
they become keyboard-focusable and expose correct semantics—update the JSX in
guild-bar.tsx where the Home/DM group, the GuildIcon wrapper component, and the
Add guild element are defined, keep existing className styling, add appropriate
aria-labels (e.g., "Home", guild name, "Add guild"), and ensure you do not nest
interactive elements inside buttons; preserve click handlers but move them to
the new button/anchor elements.
| const name = session?.user.name ?? "User" | ||
| const username = session?.user.name ?? "user" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check what fields are available on the session user object
find . -type f -name "*.ts" -o -name "*.tsx" | xargs grep -l "session?.user\." | head -10Repository: BuckyMcYolo/townhall
Length of output: 110
🏁 Script executed:
# Look for auth schema definition and available user fields
find . -type f -name "*auth*" \( -name "*.ts" -o -name "*.tsx" \) | head -20Repository: BuckyMcYolo/townhall
Length of output: 247
🏁 Script executed:
# Check for username field in auth-related files
rg -n "username" --type=ts --glob="**/auth*"Repository: BuckyMcYolo/townhall
Length of output: 342
🏁 Script executed:
# Examine the specific file mentioned in the review
cat -n apps/web/src/components/sidebar/user-bar.tsxRepository: BuckyMcYolo/townhall
Length of output: 1470
🏁 Script executed:
# Check the auth.ts file to see the auth configuration and what fields are exposed
cat -n packages/auth/src/lib/auth.tsRepository: BuckyMcYolo/townhall
Length of output: 2427
🏁 Script executed:
# Check auth-client.ts to understand the session structure
cat -n packages/auth/src/lib/auth-client.tsRepository: BuckyMcYolo/townhall
Length of output: 881
🏁 Script executed:
# Search for any usage of session?.user.username in the codebase
rg 'session\?\.user\.username' --type=ts --type=tsxRepository: BuckyMcYolo/townhall
Length of output: 91
🏁 Script executed:
# Also check if there are any type definitions showing the session user structure
rg 'user\.' packages/auth/src/lib/ -A2 -B2 | head -60Repository: BuckyMcYolo/townhall
Length of output: 46
🏁 Script executed:
# Search for session.user.username usage without file type issues
rg 'session.*user.*username' | head -20Repository: BuckyMcYolo/townhall
Length of output: 46
🏁 Script executed:
# Check if there are any types or interfaces defining the user structure
rg 'interface.*[Uu]ser|type.*[Uu]ser' packages/auth/src/lib/ -A3Repository: BuckyMcYolo/townhall
Length of output: 275
🏁 Script executed:
# Look at better-auth imports to understand what the username plugin adds
rg 'better-auth' packages/auth/src/lib/auth.ts -A5 -B5Repository: BuckyMcYolo/townhall
Length of output: 494
🏁 Script executed:
# Search for user table schema definition
find . -type f -name "*.ts" | xargs grep -l "user.*schema\|schema.*user" | grep -E "(schema|db)" | head -10Repository: BuckyMcYolo/townhall
Length of output: 46
🏁 Script executed:
# Look for database schema files
find . -path "*/packages/db*" -type f -name "*.ts" | head -20Repository: BuckyMcYolo/townhall
Length of output: 665
🏁 Script executed:
# Check if there's a schema.ts or similar in the db package
find . -name schema.ts -o -name schema.jsRepository: BuckyMcYolo/townhall
Length of output: 148
🏁 Script executed:
# Check the user schema definition
cat -n packages/db/src/schemas/users.tsRepository: BuckyMcYolo/townhall
Length of output: 1680
Use session?.user.username for the username variable instead of session?.user.name.
Both variables currently derive from session?.user.name, so they'll always be identical. The database schema and auth configuration include a separate username field that should be used here.
Suggested fix:
const name = session?.user.name ?? "User"
- const username = session?.user.name ?? "user"
+ const username = session?.user.username ?? "user"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const name = session?.user.name ?? "User" | |
| const username = session?.user.name ?? "user" | |
| const name = session?.user.name ?? "User" | |
| const username = session?.user.username ?? "user" |
🤖 Prompt for AI Agents
In `@apps/web/src/components/sidebar/user-bar.tsx` around lines 8 - 9, The
username constant is incorrectly derived from session?.user.name causing name
and username to be identical; change the username assignment to read
session?.user.username instead. Update the variable declaration for username (in
the user-bar component where const name = session?.user.name ?? "User" and const
username = ...) to use session?.user.username ?? "user" so it falls back
correctly when absent, and keep the name assignment unchanged.
1 participant
Summary by CodeRabbit
Release Notes
New Features
Chores