fix chat alignment and UI improvements#249
Open
Sarthakxtiwari wants to merge 2 commits into
Open
Conversation
Contributor
|
Someone is attempting to deploy a commit to the mrimmortal09's projects Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (15)
💤 Files with no reviewable changes (6)
✅ Files skipped from review due to trivial changes (8)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThis PR adds standalone MongoDB deployment support to three authentication routes, refactors chat message component layout for alignment and sizing, adjusts the chat page to full-width layout, and updates environment and workspace configuration with functional defaults. ChangesStandalone MongoDB transaction handling and chat UI layout
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/api/auth/reset-password/route.ts (1)
61-68:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winAborting after save rolls back rate-limiting changes, allowing unlimited reset attempts.
When
attempts > MAX_ATTEMPTS, the code savesused = trueto the database, then aborts the transaction. Aborting undoes the save, so:
- The
usedflag never persists- The incremented
attemptscounter is rolled back- Attackers can retry indefinitely
This only affects replica set deployments; standalone mode works correctly because there's no transaction to abort.
To persist the rate-limiting state before returning an error, commit instead of abort:
🔒 Proposed fix
if (resetRequest.attempts > MAX_ATTEMPTS) { resetRequest.used = true await resetRequest.save({ session }) - if (session) await session.abortTransaction() + if (session) await session.commitTransaction() return NextResponse.json( { message: 'Too many attempts. Please request a new reset link.' }, { status: 400 } ) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/api/auth/reset-password/route.ts` around lines 61 - 68, The current branch marks resetRequest.used = true and calls resetRequest.save({ session }) but then aborts the session which rolls back those changes; update the error path in the reset-password handler so that when resetRequest.attempts > MAX_ATTEMPTS you persist the rate-limit state instead of aborting — either call await session.commitTransaction() after save (or save outside the transaction) and then end/cleanup the session, and only then return the NextResponse; ensure you reference the existing resetRequest, attempts, used, save, session, abortTransaction/commitTransaction and MAX_ATTEMPTS symbols when making the change.
🧹 Nitpick comments (1)
app/api/auth/verify-otp/route.ts (1)
8-16: ⚡ Quick winSession setup outside try block; consider extracting shared helper.
Same issue as
reset-password/route.ts: ifstartSession()throws, the error is unhandled. Additionally, the topology detection block is duplicated across all three auth routes.Consider moving the session setup inside the try block and extracting a shared helper:
♻️ Suggested helper in lib/dbConnect.ts
// Add to lib/dbConnect.ts export async function startSessionIfSupported(): Promise<mongoose.ClientSession | null> { const client = mongoose.connection.client as any const isStandalone = client?.topology?.description?.type === 'Single' if (isStandalone) return null const session = await mongoose.startSession() session.startTransaction() return session }Then in routes:
+ import dbConnect, { startSessionIfSupported } from '`@/lib/dbConnect`' - import dbConnect from '`@/lib/dbConnect`' export async function POST(request: NextRequest) { + let session: mongoose.ClientSession | null = null try { await dbConnect() - // eslint-disable-next-line `@typescript-eslint/no-explicit-any` - const client = mongoose.connection.client as any - const isStandalone = client?.topology?.description?.type === 'Single' - const session = isStandalone ? null : await mongoose.startSession() - if (session) { - session.startTransaction() - } + session = await startSessionIfSupported()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/api/auth/verify-otp/route.ts` around lines 8 - 16, Move the MongoDB session setup into the try block and extract the duplicated topology/session logic into a shared helper (e.g., startSessionIfSupported in lib/dbConnect.ts) so startSession() errors are caught; implement startSessionIfSupported to inspect mongoose.connection.client?.topology?.description?.type === 'Single' and return null for standalone or start a session, startTransaction(), and return the session, then replace the duplicated detection + mongoose.startSession() calls in verify-otp/route.ts (and other auth routes) with a call to startSessionIfSupported and ensure the caller ends/aborts/commits the session inside the try/catch/finally where errors are handled.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/api/auth/reset-password/route.ts`:
- Around line 11-18: The session setup (dbConnect(), reading
mongoose.connection.client, computing isStandalone, calling
mongoose.startSession(), and session.startTransaction()) is currently outside
the try block and can throw uncaught errors; move the block that calls
dbConnect(), evaluates client?.topology?.description?.type (isStandalone),
awaits mongoose.startSession(), and starts the transaction
(session.startTransaction()) inside the existing try (or the retry loop) so any
errors from dbConnect() or startSession() are caught and handled consistently
with register-with-otp/route.ts.
In `@components/chat/ChatMessage.tsx`:
- Line 282: The DropdownMenuTrigger in ChatMessage.tsx currently strips the
keyboard focus outline via "outline-none"; restore visible focus by replacing
that with focus-visible utility classes on the same element (the
DropdownMenuTrigger) so keyboard users get a clear indicator — e.g., remove
"outline-none" and add a focus-visible style such as "focus-visible:outline-none
focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2" (or
your design-system equivalent) while keeping existing padding/rounded/hover
classes.
- Around line 253-258: The current rendering in ChatMessage.tsx checks a
hardcoded tombstone string to detect deleted replies; change the conditional to
rely on the structural flag message.replyTo.isDeleted instead. In the JSX that
references message.replyTo.content, first null-check message.replyTo, then if
message.replyTo.isDeleted render 'Deleted message', else if typeof
message.replyTo.content === 'string' render that content, otherwise render the
existing empty-string fallback; update the expression that currently compares to
'🚫 This message was deleted.' to use message.replyTo.isDeleted so the UI is
resilient to backend copy changes.
In `@env.sample`:
- Line 6: Replace the weak NEXTAUTH_SECRET value with a realistic placeholder
showing a long, random secret and add a comment instructing developers to
generate a secure 32-byte key; specifically, update the NEXTAUTH_SECRET entry to
use a 32+ character base64-style placeholder and add a one-line comment (above
or beside NEXTAUTH_SECRET) that tells developers to run a command such as
`openssl rand -base64 32` to generate a proper secret before deployment so the
app does not use a trivially short secret.
- Around line 19-21: The env.sample contains real credentials and PII: revoke
the exposed Google App Password immediately in the account settings, then remove
the actual email and password from the sample file and replace them with safe
placeholders (e.g., set EMAIL_SERVER_USER="your-email@example.com",
EMAIL_SERVER_FROM="Your Name <no-reply@example.com>",
EMAIL_SERVER_PASSWORD="YOUR_APP_PASSWORD_HERE"); ensure EMAIL_SERVER_PASSWORD no
longer contains the space-separated real token and that the sample only contains
dummy values, and add a short comment reminding users to set real secrets in
their local .env or secret manager instead of committing them.
---
Outside diff comments:
In `@app/api/auth/reset-password/route.ts`:
- Around line 61-68: The current branch marks resetRequest.used = true and calls
resetRequest.save({ session }) but then aborts the session which rolls back
those changes; update the error path in the reset-password handler so that when
resetRequest.attempts > MAX_ATTEMPTS you persist the rate-limit state instead of
aborting — either call await session.commitTransaction() after save (or save
outside the transaction) and then end/cleanup the session, and only then return
the NextResponse; ensure you reference the existing resetRequest, attempts,
used, save, session, abortTransaction/commitTransaction and MAX_ATTEMPTS symbols
when making the change.
---
Nitpick comments:
In `@app/api/auth/verify-otp/route.ts`:
- Around line 8-16: Move the MongoDB session setup into the try block and
extract the duplicated topology/session logic into a shared helper (e.g.,
startSessionIfSupported in lib/dbConnect.ts) so startSession() errors are
caught; implement startSessionIfSupported to inspect
mongoose.connection.client?.topology?.description?.type === 'Single' and return
null for standalone or start a session, startTransaction(), and return the
session, then replace the duplicated detection + mongoose.startSession() calls
in verify-otp/route.ts (and other auth routes) with a call to
startSessionIfSupported and ensure the caller ends/aborts/commits the session
inside the try/catch/finally where errors are handled.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 80fd2c6d-003c-4071-bd3e-0a23f9e08f96
📒 Files selected for processing (7)
app/api/auth/register-with-otp/route.tsapp/api/auth/reset-password/route.tsapp/api/auth/verify-otp/route.tsapp/chat/page.tsxcomponents/chat/ChatMessage.tsxenv.samplepnpm-workspace.yaml
Comment on lines
+11
to
+18
| await dbConnect() | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| const client = mongoose.connection.client as any | ||
| const isStandalone = client?.topology?.description?.type === 'Single' | ||
| const session = isStandalone ? null : await mongoose.startSession() | ||
| if (session) { | ||
| session.startTransaction() | ||
| } |
Contributor
There was a problem hiding this comment.
Session setup outside try block is inconsistent and risks unhandled errors.
In register-with-otp/route.ts, the session setup is inside the try block (within the retry loop), ensuring errors from dbConnect() or startSession() are caught and produce a proper 500 response. Here, if either fails, the error propagates unhandled.
Consider moving lines 11-18 inside the try block for consistent error handling.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/api/auth/reset-password/route.ts` around lines 11 - 18, The session setup
(dbConnect(), reading mongoose.connection.client, computing isStandalone,
calling mongoose.startSession(), and session.startTransaction()) is currently
outside the try block and can throw uncaught errors; move the block that calls
dbConnect(), evaluates client?.topology?.description?.type (isStandalone),
awaits mongoose.startSession(), and starts the transaction
(session.startTransaction()) inside the existing try (or the retry loop) so any
errors from dbConnect() or startSession() are caught and handled consistently
with register-with-otp/route.ts.
Comment on lines
+253
to
+258
| {typeof message.replyTo.content === 'string' && | ||
| message.replyTo.content === '🚫 This message was deleted.' | ||
| ? 'Deleted message' | ||
| : typeof message.replyTo.content === 'string' | ||
| ? message.replyTo.content | ||
| : ''} |
Contributor
There was a problem hiding this comment.
Avoid coupling deleted-reply rendering to a hardcoded content string.
Line 254 compares against a literal tombstone message ('🚫 This message was deleted.'). If backend copy changes, the UI silently misclassifies deleted replies. Prefer the structural flag (replyTo.isDeleted) that your SSE update flow already maintains.
Proposed fix
- <div className="truncate opacity-75 max-w-[200px]">
- {typeof message.replyTo.content === 'string' &&
- message.replyTo.content === '🚫 This message was deleted.'
- ? 'Deleted message'
- : typeof message.replyTo.content === 'string'
- ? message.replyTo.content
- : ''}
- </div>
+ <div className="truncate opacity-75 max-w-[200px]">
+ {(message.replyTo as { isDeleted?: boolean }).isDeleted
+ ? 'Deleted message'
+ : typeof message.replyTo.content === 'string'
+ ? message.replyTo.content
+ : ''}
+ </div>📝 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.
Suggested change
| {typeof message.replyTo.content === 'string' && | |
| message.replyTo.content === '🚫 This message was deleted.' | |
| ? 'Deleted message' | |
| : typeof message.replyTo.content === 'string' | |
| ? message.replyTo.content | |
| : ''} | |
| <div className="truncate opacity-75 max-w-[200px]"> | |
| {(message.replyTo as { isDeleted?: boolean }).isDeleted | |
| ? 'Deleted message' | |
| : typeof message.replyTo.content === 'string' | |
| ? message.replyTo.content | |
| : ''} | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/chat/ChatMessage.tsx` around lines 253 - 258, The current
rendering in ChatMessage.tsx checks a hardcoded tombstone string to detect
deleted replies; change the conditional to rely on the structural flag
message.replyTo.isDeleted instead. In the JSX that references
message.replyTo.content, first null-check message.replyTo, then if
message.replyTo.isDeleted render 'Deleted message', else if typeof
message.replyTo.content === 'string' render that content, otherwise render the
existing empty-string fallback; update the expression that currently compares to
'🚫 This message was deleted.' to use message.replyTo.isDeleted so the UI is
resilient to backend copy changes.
| {!message.isDeleted && ( | ||
| <div className="opacity-0 group-hover:opacity-100 transition-opacity flex items-center shrink-0 px-1 gap-1"> | ||
| <DropdownMenu> | ||
| <DropdownMenuTrigger className="p-1 rounded-full hover:bg-muted text-muted-foreground outline-none"> |
Contributor
There was a problem hiding this comment.
Restore visible keyboard focus on the action trigger.
Line 282 removes outline (outline-none) but does not add a focus-visible replacement, which makes keyboard navigation hard to track.
Proposed fix
-<DropdownMenuTrigger className="p-1 rounded-full hover:bg-muted text-muted-foreground outline-none">
+<DropdownMenuTrigger className="p-1 rounded-full hover:bg-muted text-muted-foreground outline-none focus-visible:ring-2 focus-visible:ring-primary/50 focus-visible:ring-offset-2">📝 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.
Suggested change
| <DropdownMenuTrigger className="p-1 rounded-full hover:bg-muted text-muted-foreground outline-none"> | |
| <DropdownMenuTrigger className="p-1 rounded-full hover:bg-muted text-muted-foreground outline-none focus-visible:ring-2 focus-visible:ring-primary/50 focus-visible:ring-offset-2"> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/chat/ChatMessage.tsx` at line 282, The DropdownMenuTrigger in
ChatMessage.tsx currently strips the keyboard focus outline via "outline-none";
restore visible focus by replacing that with focus-visible utility classes on
the same element (the DropdownMenuTrigger) so keyboard users get a clear
indicator — e.g., remove "outline-none" and add a focus-visible style such as
"focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring
focus-visible:ring-offset-2" (or your design-system equivalent) while keeping
existing padding/rounded/hover classes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #{{TODO: add issue number}}.
Description
Live Demo (if any)
Note for Maintainer
Checkout
Summary by CodeRabbit
Style
Bug Fixes
Chores