Feat/UI #1143
Feat/UI #1143
Conversation
# READ CAREFULLY THEN REMOVE Remove bullet points that are not relevant. PLEASE REFRAIN FROM USING AI TO WRITE YOUR CODE AND PR DESCRIPTION. IF YOU DO USE AI TO WRITE YOUR CODE PLEASE PROVIDE A DESCRIPTION AND REVIEW IT CAREFULLY. MAKE SURE YOU UNDERSTAND THE CODE YOU ARE SUBMITTING USING AI. - Pull requests that do not follow these guidelines will be closed without review or comment. - If you use AI to write your PR description your pr will be close without review or comment. - If you are unsure about anything, feel free to ask for clarification. ## Description Please provide a clear description of your changes. --- ## Type of Change Please delete options that are not relevant. - [ ] 🐛 Bug fix (non-breaking change which fixes an issue) - [ ] ✨ New feature (non-breaking change which adds functionality) - [ ] 💥 Breaking change (fix or feature with breaking changes) - [ ] 📝 Documentation update - [ ] 🎨 UI/UX improvement - [ ] 🔒 Security enhancement - [ ] ⚡ Performance improvement ## Areas Affected Please check all that apply: - [ ] Email Integration (Gmail, IMAP, etc.) - [ ] User Interface/Experience - [ ] Authentication/Authorization - [ ] Data Storage/Management - [ ] API Endpoints - [ ] Documentation - [ ] Testing Infrastructure - [ ] Development Workflow - [ ] Deployment/Infrastructure ## Testing Done Describe the tests you've done: - [ ] Unit tests added/updated - [ ] Integration tests added/updated - [ ] Manual testing performed - [ ] Cross-browser testing (if UI changes) - [ ] Mobile responsiveness verified (if UI changes) ## Security Considerations For changes involving data or authentication: - [ ] No sensitive data is exposed - [ ] Authentication checks are in place - [ ] Input validation is implemented - [ ] Rate limiting is considered (if applicable) ## Checklist - [ ] I have read the [CONTRIBUTING](../CONTRIBUTING.md) document - [ ] My code follows the project's style guidelines - [ ] I have performed a self-review of my code - [ ] I have commented my code, particularly in complex areas - [ ] I have updated the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix/feature works - [ ] All tests pass locally - [ ] Any dependent changes are merged and published ## Additional Notes Add any other context about the pull request here. ## Screenshots/Recordings Add screenshots or recordings here if applicable. --- _By submitting this pull request, I confirm that my contribution is made under the terms of the project's license._
# READ CAREFULLY THEN REMOVE Remove bullet points that are not relevant. PLEASE REFRAIN FROM USING AI TO WRITE YOUR CODE AND PR DESCRIPTION. IF YOU DO USE AI TO WRITE YOUR CODE PLEASE PROVIDE A DESCRIPTION AND REVIEW IT CAREFULLY. MAKE SURE YOU UNDERSTAND THE CODE YOU ARE SUBMITTING USING AI. - Pull requests that do not follow these guidelines will be closed without review or comment. - If you use AI to write your PR description your pr will be close without review or comment. - If you are unsure about anything, feel free to ask for clarification. ## Description Please provide a clear description of your changes. --- ## Type of Change Please delete options that are not relevant. - [ ] 🐛 Bug fix (non-breaking change which fixes an issue) - [ ] ✨ New feature (non-breaking change which adds functionality) - [ ] 💥 Breaking change (fix or feature with breaking changes) - [ ] 📝 Documentation update - [ ] 🎨 UI/UX improvement - [ ] 🔒 Security enhancement - [ ] ⚡ Performance improvement ## Areas Affected Please check all that apply: - [ ] Email Integration (Gmail, IMAP, etc.) - [ ] User Interface/Experience - [ ] Authentication/Authorization - [ ] Data Storage/Management - [ ] API Endpoints - [ ] Documentation - [ ] Testing Infrastructure - [ ] Development Workflow - [ ] Deployment/Infrastructure ## Testing Done Describe the tests you've done: - [ ] Unit tests added/updated - [ ] Integration tests added/updated - [ ] Manual testing performed - [ ] Cross-browser testing (if UI changes) - [ ] Mobile responsiveness verified (if UI changes) ## Security Considerations For changes involving data or authentication: - [ ] No sensitive data is exposed - [ ] Authentication checks are in place - [ ] Input validation is implemented - [ ] Rate limiting is considered (if applicable) ## Checklist - [ ] I have read the [CONTRIBUTING](../CONTRIBUTING.md) document - [ ] My code follows the project's style guidelines - [ ] I have performed a self-review of my code - [ ] I have commented my code, particularly in complex areas - [ ] I have updated the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix/feature works - [ ] All tests pass locally - [ ] Any dependent changes are merged and published ## Additional Notes Add any other context about the pull request here. ## Screenshots/Recordings Add screenshots or recordings here if applicable. --- _By submitting this pull request, I confirm that my contribution is made under the terms of the project's license._
# UI Improvements: Standardized Dark Mode Colors and Enhanced AI Chat Component ## Description This PR standardizes dark mode colors across the application by replacing hardcoded color values with semantic color variables. It also enhances the AI Chat component with improved tool response rendering and adds a new voice chat capability. Key changes: - Replaced hardcoded dark background colors (`#1A1A1A`) with the semantic `bg-panelDark` class - Replaced hardcoded text colors with semantic classes like `text-muted-foreground` - Enhanced the AI Chat component with better tool response visualization - Added tooltips to display tool arguments in AI responses - Improved email composer editor focus handling - Added a new Tools enum to better type and organize AI tool capabilities ## Type of Change - [x] 🎨 UI/UX improvement - [x] ⚡ Performance improvement ## Areas Affected - [x] User Interface/Experience ## Testing Done - [x] Manual testing performed - [x] Cross-browser testing (if UI changes) - [x] Mobile responsiveness verified (if UI changes) ## Checklist - [x] I have performed a self-review of my code - [x] My changes generate no new warnings - [x] My code follows the project's style guidelines ## Additional Notes This PR is part of our ongoing effort to standardize the UI color system and improve the dark mode experience. The changes to the AI Chat component make tool responses more informative and easier to understand. _By submitting this pull request, I confirm that my contribution is made under the terms of the project's license._
# READ CAREFULLY THEN REMOVE Remove bullet points that are not relevant. PLEASE REFRAIN FROM USING AI TO WRITE YOUR CODE AND PR DESCRIPTION. IF YOU DO USE AI TO WRITE YOUR CODE PLEASE PROVIDE A DESCRIPTION AND REVIEW IT CAREFULLY. MAKE SURE YOU UNDERSTAND THE CODE YOU ARE SUBMITTING USING AI. - Pull requests that do not follow these guidelines will be closed without review or comment. - If you use AI to write your PR description your pr will be close without review or comment. - If you are unsure about anything, feel free to ask for clarification. ## Description Please provide a clear description of your changes. --- ## Type of Change Please delete options that are not relevant. - [ ] 🐛 Bug fix (non-breaking change which fixes an issue) - [ ] ✨ New feature (non-breaking change which adds functionality) - [ ] 💥 Breaking change (fix or feature with breaking changes) - [ ] 📝 Documentation update - [ ] 🎨 UI/UX improvement - [ ] 🔒 Security enhancement - [ ] ⚡ Performance improvement ## Areas Affected Please check all that apply: - [ ] Email Integration (Gmail, IMAP, etc.) - [ ] User Interface/Experience - [ ] Authentication/Authorization - [ ] Data Storage/Management - [ ] API Endpoints - [ ] Documentation - [ ] Testing Infrastructure - [ ] Development Workflow - [ ] Deployment/Infrastructure ## Testing Done Describe the tests you've done: - [ ] Unit tests added/updated - [ ] Integration tests added/updated - [ ] Manual testing performed - [ ] Cross-browser testing (if UI changes) - [ ] Mobile responsiveness verified (if UI changes) ## Security Considerations For changes involving data or authentication: - [ ] No sensitive data is exposed - [ ] Authentication checks are in place - [ ] Input validation is implemented - [ ] Rate limiting is considered (if applicable) ## Checklist - [ ] I have read the [CONTRIBUTING](../CONTRIBUTING.md) document - [ ] My code follows the project's style guidelines - [ ] I have performed a self-review of my code - [ ] I have commented my code, particularly in complex areas - [ ] I have updated the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix/feature works - [ ] All tests pass locally - [ ] Any dependent changes are merged and published ## Additional Notes Add any other context about the pull request here. ## Screenshots/Recordings Add screenshots or recordings here if applicable. --- _By submitting this pull request, I confirm that my contribution is made under the terms of the project's license._
## Description Improved error handling in the ZeroAgent class by providing more specific error messages when authentication fails. The changes include: 1. Enhanced the error message in the execute method to specify whether the failure is due to missing driver or connectionId 2. Improved the error message in onConnect method to clarify when a token is missing 3. Removed redundant driver check in onChatMessage method as this validation is already handled elsewhere 4. Cleaned up imports by removing unused WSMessage type ## Type of Change - [x] 🐛 Bug fix (non-breaking change which fixes an issue) - [x] 🔒 Security enhancement ## Areas Affected - [x] Authentication/Authorization - [x] API Endpoints ## Testing Done - [x] Manual testing performed ## Security Considerations - [x] Authentication checks are in place ## Checklist - [x] I have performed a self-review of my code - [x] My changes generate no new warnings _By submitting this pull request, I confirm that my contribution is made under the terms of the project's license._
done: - adds new hook `useOptimisticAction` - in mail-lsit - in thread-display - adds undo action to toast (design needs updating) wip: - undo sent email within 10-20 seconds - update toast design/undo button - add shortcut to undo 😎 - safari causing slight issues but it works
# READ CAREFULLY THEN REMOVE Remove bullet points that are not relevant. PLEASE REFRAIN FROM USING AI TO WRITE YOUR CODE AND PR DESCRIPTION. IF YOU DO USE AI TO WRITE YOUR CODE PLEASE PROVIDE A DESCRIPTION AND REVIEW IT CAREFULLY. MAKE SURE YOU UNDERSTAND THE CODE YOU ARE SUBMITTING USING AI. - Pull requests that do not follow these guidelines will be closed without review or comment. - If you use AI to write your PR description your pr will be close without review or comment. - If you are unsure about anything, feel free to ask for clarification. ## Description Please provide a clear description of your changes. --- ## Type of Change Please delete options that are not relevant. - [ ] 🐛 Bug fix (non-breaking change which fixes an issue) - [ ] ✨ New feature (non-breaking change which adds functionality) - [ ] 💥 Breaking change (fix or feature with breaking changes) - [ ] 📝 Documentation update - [ ] 🎨 UI/UX improvement - [ ] 🔒 Security enhancement - [ ] ⚡ Performance improvement ## Areas Affected Please check all that apply: - [ ] Email Integration (Gmail, IMAP, etc.) - [ ] User Interface/Experience - [ ] Authentication/Authorization - [ ] Data Storage/Management - [ ] API Endpoints - [ ] Documentation - [ ] Testing Infrastructure - [ ] Development Workflow - [ ] Deployment/Infrastructure ## Testing Done Describe the tests you've done: - [ ] Unit tests added/updated - [ ] Integration tests added/updated - [ ] Manual testing performed - [ ] Cross-browser testing (if UI changes) - [ ] Mobile responsiveness verified (if UI changes) ## Security Considerations For changes involving data or authentication: - [ ] No sensitive data is exposed - [ ] Authentication checks are in place - [ ] Input validation is implemented - [ ] Rate limiting is considered (if applicable) ## Checklist - [ ] I have read the [CONTRIBUTING](../CONTRIBUTING.md) document - [ ] My code follows the project's style guidelines - [ ] I have performed a self-review of my code - [ ] I have commented my code, particularly in complex areas - [ ] I have updated the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix/feature works - [ ] All tests pass locally - [ ] Any dependent changes are merged and published ## Additional Notes Add any other context about the pull request here. ## Screenshots/Recordings Add screenshots or recordings here if applicable. --- _By submitting this pull request, I confirm that my contribution is made under the terms of the project's license._
# Improve Thread Context Menu and Optimistic Updates This PR refactors the thread context menu and optimistic state handling in the mail application to fix several issues: 1. Simplifies the `ThreadContextMenu` component by removing the redundant `emailId` prop and only using `threadId` 2. Adds support for optimistic marking as unread in the thread context menu 3. Improves the optimistic state handling in the mail list by: - Using a consistent thread ID across components - Properly displaying optimistic read/unread state - Fixing star toggling functionality 4. Enhances the optimistic actions system to invalidate specific queries rather than refetching everything ## Type of Change - [x] 🐛 Bug fix (non-breaking change which fixes an issue) - [x] ⚡ Performance improvement ## Areas Affected - [x] User Interface/Experience - [x] Data Storage/Management ## Testing Done - [x] Manual testing performed ## Checklist - [x] I have performed a self-review of my code - [x] My changes generate no new warnings _By submitting this pull request, I confirm that my contribution is made under the terms of the project's license._
# READ CAREFULLY THEN REMOVE Remove bullet points that are not relevant. PLEASE REFRAIN FROM USING AI TO WRITE YOUR CODE AND PR DESCRIPTION. IF YOU DO USE AI TO WRITE YOUR CODE PLEASE PROVIDE A DESCRIPTION AND REVIEW IT CAREFULLY. MAKE SURE YOU UNDERSTAND THE CODE YOU ARE SUBMITTING USING AI. - Pull requests that do not follow these guidelines will be closed without review or comment. - If you use AI to write your PR description your pr will be close without review or comment. - If you are unsure about anything, feel free to ask for clarification. ## Description Please provide a clear description of your changes. --- ## Type of Change Please delete options that are not relevant. - [ ] 🐛 Bug fix (non-breaking change which fixes an issue) - [ ] ✨ New feature (non-breaking change which adds functionality) - [ ] 💥 Breaking change (fix or feature with breaking changes) - [ ] 📝 Documentation update - [ ] 🎨 UI/UX improvement - [ ] 🔒 Security enhancement - [ ] ⚡ Performance improvement ## Areas Affected Please check all that apply: - [ ] Email Integration (Gmail, IMAP, etc.) - [ ] User Interface/Experience - [ ] Authentication/Authorization - [ ] Data Storage/Management - [ ] API Endpoints - [ ] Documentation - [ ] Testing Infrastructure - [ ] Development Workflow - [ ] Deployment/Infrastructure ## Testing Done Describe the tests you've done: - [ ] Unit tests added/updated - [ ] Integration tests added/updated - [ ] Manual testing performed - [ ] Cross-browser testing (if UI changes) - [ ] Mobile responsiveness verified (if UI changes) ## Security Considerations For changes involving data or authentication: - [ ] No sensitive data is exposed - [ ] Authentication checks are in place - [ ] Input validation is implemented - [ ] Rate limiting is considered (if applicable) ## Checklist - [ ] I have read the [CONTRIBUTING](../CONTRIBUTING.md) document - [ ] My code follows the project's style guidelines - [ ] I have performed a self-review of my code - [ ] I have commented my code, particularly in complex areas - [ ] I have updated the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix/feature works - [ ] All tests pass locally - [ ] Any dependent changes are merged and published ## Additional Notes Add any other context about the pull request here. ## Screenshots/Recordings Add screenshots or recordings here if applicable. --- _By submitting this pull request, I confirm that my contribution is made under the terms of the project's license._
## Description Added support for optimistic toggling of "Important" flag in emails. This enhancement allows users to mark emails as important with immediate UI feedback while the action processes in the background. The implementation follows the same pattern as other optimistic actions like starring and marking as read. Also fixed a few UI issues including removing extra whitespace in the AI chat component and removing debug mode from the Party provider. ## Type of Change - [x] ✨ New feature (non-breaking change which adds functionality) - [x] 🐛 Bug fix (non-breaking change which fixes an issue) - [x] 🎨 UI/UX improvement ## Areas Affected - [x] User Interface/Experience - [x] Email Integration (Gmail, IMAP, etc.) ## Testing Done - [x] Manual testing performed ## Checklist - [x] I have performed a self-review of my code - [x] My changes generate no new warnings - [x] My code follows the project's style guidelines ## Additional Notes The implementation includes: 1. Adding the Important button to the thread list UI 2. Supporting optimistic state updates for the Important flag 3. Fixing the ZeroAgent to use user ID instead of email for better identification 4. Removing debug mode from Party provider to reduce console noise
WalkthroughThis set of changes enhances the mail application's UI and optimistic state handling. It introduces improved optimistic updates for thread flags (starred, important, unread), refines context menu and thread list logic, updates font and animation styling, adds thread-level attachment display, and simplifies authentication in the server chat route. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI
participant OptimisticState
participant Server
User->>UI: Toggle thread "starred", "important", or "unread"
UI->>OptimisticState: Apply optimistic update
OptimisticState-->>UI: Update UI immediately
UI->>Server: Send mutation request
Server-->>UI: Respond with success/failure
UI->>OptimisticState: Reconcile optimistic state with server response
sequenceDiagram
participant ThreadDisplay
participant MailDisplay
participant Attachments
ThreadDisplay->>MailDisplay: Pass threadAttachments (all attachments)
MailDisplay->>Attachments: Render thread-level attachments (if any)
Attachments-->>User: Allow download/view actions
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (16)
💤 Files with no reviewable changes (2)
🧰 Additional context used🧬 Code Graph Analysis (7)apps/mail/components/ui/ai-sidebar.tsx (1)
apps/mail/lib/hotkeys/mail-list-hotkeys.tsx (1)
apps/mail/components/context/thread-context.tsx (2)
apps/mail/components/create/ai-chat.tsx (1)
apps/mail/components/home/HomeContent.tsx (1)
apps/server/src/routes/chat.ts (3)
apps/mail/components/mail/mail-display.tsx (1)
🪛 Biome (1.9.4)apps/mail/components/mail/thread-display.tsx[error] 179-179: Avoid the use of spread ( Spread syntax should be avoided on accumulators (like those in (lint/performance/noAccumulatingSpread) 🔇 Additional comments (33)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
apps/mail/components/mail/mail-display.tsx (1)
67-98:⚠️ Potential issueRemove duplicate utility functions.
The
formatFileSizeandgetFileIconfunctions defined here duplicate the utilities available inapps/mail/lib/utils.ts(lines 148-163). This violates the DRY principle.Replace the local implementations with imports:
-// Add formatFileSize utility function -const formatFileSize = (size: number) => { - const sizeInMB = (size / (1024 * 1024)).toFixed(2); - return sizeInMB === '0.00' ? '' : `${sizeInMB} MB`; -}; - -// Add getFileIcon utility function -const getFileIcon = (filename: string) => { - const extension = filename.split('.').pop()?.toLowerCase(); - - switch (extension) { - case 'pdf': - return <PDF className="fill-[#F43F5E]" />; - case 'jpg': - return <ImageFile />; - case 'jpeg': - return <ImageFile />; - case 'png': - return <ImageFile />; - case 'gif': - return <ImageFile />; - case 'docx': - return <Docx />; - case 'fig': - return <Figma />; - case 'webp': - return <ImageFile />; - default: - return <FileText className="h-4 w-4 text-[#8B5CF6]" />; - } -}; +import { formatFileSize, getFileIcon as getFileIconFromUtils } from '@/lib/utils';Note that the imported
getFileIconuses MIME types while the local version uses filename extensions. You'll need to adapt the usage in theThreadAttachmentscomponent to use MIME types or update the utility function to support both approaches.
🧹 Nitpick comments (6)
apps/mail/tailwind.config.ts (1)
93-94: Remove leading whitespace in font family declaration.The
monafont entry currently includes a leading space beforesans-serif, which may cause inconsistency with other font definitions. It should be:- mona: ['Mona Sans', ' sans-serif'], + mona: ['Mona Sans', 'sans-serif'],apps/mail/app/root.tsx (1)
70-74: Prefer<link>over@importfor Google Fonts and add preconnect.Inlining
@importinside a<style>block can block rendering. Switching to<link rel="stylesheet">with preconnect hints improves performance:- <style> - @import url('https://fonts.googleapis.com/css2?family=Sora:wght@600&display=swap'); - @import url('https://fonts.googleapis.com/css2?family=Mona+Sans:ital,wght@0,500;1,500&display=swap'); - </style> + <link rel="preconnect" href="https://fonts.googleapis.com" /> + <link rel="preconnect" href="https://fonts.gstatic.com" crossOrigin="anonymous" /> + <link + href="https://fonts.googleapis.com/css2?family=Sora:wght@600&family=Mona+Sans:ital,wght@0,500;1,500&display=swap" + rel="stylesheet" + />apps/mail/components/home/HomeContent.tsx (1)
358-395: LGTM: Improved button layout with minor optimization opportunityThe restructuring successfully separates the primary call-to-action from secondary information, creating better visual hierarchy. The ArrowRight icon addition enhances the "Get Started" button's prominence.
Consider extracting the shared animation properties to reduce code duplication:
+ const buttonAnimation = { + initial: { opacity: 0, y: 20 }, + animate: { opacity: 1, y: 0 }, + transition: { duration: 0.5, delay: 0.6 } + }; <div className="mt-12 flex flex-row gap-6"> - <motion.div - initial={{ opacity: 0, y: 20 }} - animate={{ opacity: 1, y: 0 }} - transition={{ duration: 0.5, delay: 0.6 }} - > + <motion.div {...buttonAnimation}> {/* No credit card required button */} </motion.div> - <motion.div - initial={{ opacity: 0, y: 20 }} - animate={{ opacity: 1, y: 0 }} - transition={{ duration: 0.5, delay: 0.6 }} - > + <motion.div {...buttonAnimation}> {/* Get Started button */} </motion.div> </div>apps/mail/components/context/thread-context.tsx (1)
223-229: Simplify the read/unread toggle logic.The current logic can be simplified and the error handling improved.
- if (newReadState) { - optimisticMarkAsRead(targets); - } else if (!newReadState) { - optimisticMarkAsUnread(targets); - } else { - toast.error('Failed to mark as read'); - } + if (newReadState) { + optimisticMarkAsRead(targets); + } else { + optimisticMarkAsUnread(targets); + }The
elsecondition!newReadStatewill never be reached sincenewReadStateis boolean, and the error case is unreachable.apps/mail/components/mail/mail-display.tsx (1)
260-312: Consider reusing existing attachment UI patterns.The
ThreadAttachmentscomponent creates a new UI pattern for attachments, but there's already an attachment rendering pattern in lines 1401-1428. Consider extracting a shared component to maintain UI consistency.Additionally, update the component to use the imported utility functions:
-<span className="text-muted-foreground">{getFileIcon(attachment.filename)}</span> +<span className="text-muted-foreground">{getFileIconFromUtils(attachment.mimeType)}</span>-<span className="text-muted-foreground">{formatFileSize(attachment.size)}</span> +<span className="text-muted-foreground">{formatFileSize(attachment.size)}</span>apps/mail/hooks/use-optimistic-actions.ts (1)
188-217: Clean implementation of mark as unread functionality.The function correctly mirrors
optimisticMarkAsReadwith appropriate changes. Consider extracting the common bulk selection logic into a shared helper to reduce duplication between these similar functions.
🛑 Comments failed to post (4)
apps/server/src/routes/chat.ts (3)
42-42: 🛠️ Refactor suggestion
Improve error logging for security monitoring.
The unauthorized access logging should include more context for security monitoring and debugging purposes.
- console.log('Unauthorized no driver or connectionId', connectionId, this.driver); + console.error('Unauthorized access attempt:', { + connectionId: connectionId || 'missing', + hasDriver: !!this.driver, + agentName: this.name || 'missing', + timestamp: new Date().toISOString(), + userAgent: this.connectionContext?.headers?.['user-agent'] || 'unknown' + });📝 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.console.error('Unauthorized access attempt:', { connectionId: connectionId || 'missing', hasDriver: !!this.driver, agentName: this.name || 'missing', timestamp: new Date().toISOString(), userAgent: this.connectionContext?.headers?.['user-agent'] || 'unknown' });🤖 Prompt for AI Agents
In apps/server/src/routes/chat.ts at line 42, the console.log for unauthorized access lacks sufficient context for security monitoring. Enhance the log message by including additional relevant details such as timestamps, user identifiers, request metadata, or any other contextual information available in the scope to improve traceability and debugging.
83-85:
⚠️ Potential issueMissing authentication in connection handler.
The
onConnectmethod should validate the client's authentication before setting up the agent. Currently, it directly callssetupAuth()without any preliminary authentication checks.Add authentication validation:
async onConnect() { + // Extract and validate authentication from connection context + const authHeader = this.connectionContext?.headers?.authorization; + if (!authHeader) { + throw new Error('Unauthorized - missing authentication'); + } + + // Validate the session/token + const session = await this.validateSession(authHeader); + if (!session) { + throw new Error('Unauthorized - invalid session'); + } + + // Set the authenticated user ID + this.name = session.user.id; + await this.setupAuth(); }📝 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.async onConnect() { // Extract and validate authentication from connection context const authHeader = this.connectionContext?.headers?.authorization; if (!authHeader) { throw new Error('Unauthorized - missing authentication'); } // Validate the session/token const session = await this.validateSession(authHeader); if (!session) { throw new Error('Unauthorized - invalid session'); } // Set the authenticated user ID this.name = session.user.id; await this.setupAuth(); }🤖 Prompt for AI Agents
In apps/server/src/routes/chat.ts around lines 83 to 85, the onConnect method calls setupAuth() directly without validating client authentication first. Modify onConnect to include a preliminary authentication check before calling setupAuth(), ensuring only authenticated clients proceed. Implement the necessary logic to verify the client's credentials or session and handle unauthorized access appropriately.
70-81:
⚠️ Potential issueCritical security issue: Missing authentication validation.
The simplified
setupAuthmethod now trusts the client-providedname(user ID) without any session or token validation. This creates a significant security vulnerability where an attacker could potentially access another user's data by providing their user ID.Consider implementing one of these solutions:
Option 1: Restore session validation
private async setupAuth() { + // Validate session/token before proceeding + const session = await this.validateSession(); + if (!session) { + throw new Error('Unauthorized - invalid session'); + } + if (this.name) { const db = createDb(env.HYPERDRIVE.connectionString); const _connection = await db.query.connection.findFirst({ where: eq(connection.userId, this.name), }); + // Verify the session user matches the requested connection + if (_connection && _connection.userId !== session.user.id) { + throw new Error('Unauthorized - user mismatch'); + } if (_connection) { await this.ctx.storage.put('connectionId', _connection.id); this.driver = connectionToDriver(_connection); } } }Option 2: Extract user ID from validated session
private async setupAuth() { + const session = await this.validateSession(); + if (!session) { + throw new Error('Unauthorized - invalid session'); + } + + const userId = session.user.id; const db = createDb(env.HYPERDRIVE.connectionString); const _connection = await db.query.connection.findFirst({ - where: eq(connection.userId, this.name), + where: eq(connection.userId, userId), }); if (_connection) { await this.ctx.storage.put('connectionId', _connection.id); this.driver = connectionToDriver(_connection); } }📝 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.private async setupAuth() { + // 1) Validate session/token before any lookup + const session = await this.validateSession(); + if (!session) { + throw new Error('Unauthorized - invalid session'); + } + + // 2) Derive userId from the trusted session + const userId = session.user.id; const db = createDb(env.HYPERDRIVE.connectionString); const _connection = await db.query.connection.findFirst({ - where: eq(connection.userId, this.name), + where: eq(connection.userId, userId), }); if (_connection) { await this.ctx.storage.put('connectionId', _connection.id); this.driver = connectionToDriver(_connection); } }🤖 Prompt for AI Agents
In apps/server/src/routes/chat.ts around lines 70 to 81, the setupAuth method currently trusts the client-provided user ID without validating the session or token, creating a security risk. To fix this, implement proper authentication by either restoring session validation to verify the user's identity before querying the database or extract the user ID from a validated session or token instead of using the client-provided name directly. This ensures only authenticated users can access their data.apps/mail/components/mail/thread-display.tsx (1)
174-183:
⚠️ Potential issueFix O(n²) performance issue in attachment aggregation.
The spread operator in the reduce function creates a new array on each iteration, resulting in O(n²) time complexity. This could impact performance with threads containing many messages with attachments.
Use array mutation methods instead:
-const allThreadAttachments = useMemo(() => { - if (!emailData?.messages) return []; - return emailData.messages.reduce<Attachment[]>((acc, message) => { - if (message.attachments && message.attachments.length > 0) { - return [...acc, ...message.attachments]; - } - return acc; - }, []); -}, [emailData?.messages]); +const allThreadAttachments = useMemo(() => { + if (!emailData?.messages) return []; + const attachments: Attachment[] = []; + emailData.messages.forEach((message) => { + if (message.attachments && message.attachments.length > 0) { + attachments.push(...message.attachments); + } + }); + return attachments; +}, [emailData?.messages]);Or use
flatMapfor a more functional approach:-const allThreadAttachments = useMemo(() => { - if (!emailData?.messages) return []; - return emailData.messages.reduce<Attachment[]>((acc, message) => { - if (message.attachments && message.attachments.length > 0) { - return [...acc, ...message.attachments]; - } - return acc; - }, []); -}, [emailData?.messages]); +const allThreadAttachments = useMemo(() => { + if (!emailData?.messages) return []; + return emailData.messages.flatMap(message => message.attachments || []); +}, [emailData?.messages]);🧰 Tools
🪛 Biome (1.9.4)
[error] 179-179: Avoid the use of spread (
...) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
🤖 Prompt for AI Agents
In apps/mail/components/mail/thread-display.tsx around lines 174 to 183, the current useMemo hook uses the spread operator inside a reduce function to aggregate attachments, causing O(n²) complexity due to array copying on each iteration. To fix this, replace the reduce with a method that mutates the accumulator array directly by pushing attachments, or alternatively use the flatMap method on emailData.messages to collect all attachments in a single pass, improving performance to O(n).
BlankParticle
force-pushed
the
staging
branch
5 times, most recently
from
e47318e to
a6ec63a
Compare
READ CAREFULLY THEN REMOVE
Remove bullet points that are not relevant.
PLEASE REFRAIN FROM USING AI TO WRITE YOUR CODE AND PR DESCRIPTION. IF YOU DO USE AI TO WRITE YOUR CODE PLEASE PROVIDE A DESCRIPTION AND REVIEW IT CAREFULLY. MAKE SURE YOU UNDERSTAND THE CODE YOU ARE SUBMITTING USING AI.
Description
-> Added the Sora font family for hero section slogan . Changed the YC badge style . Added proper spacing between different components in hero section
Please provide a clear description of your changes.
Type of Change
Please delete options that are not relevant.
Areas Affected
Please check all that apply:
Testing Done
Describe the tests you've done:
Security Considerations
For changes involving data or authentication:
Checklist
Additional Notes
Add any other context about the pull request here.
Screenshots/Recordings
Add screenshots or recordings here if applicable.
By submitting this pull request, I confirm that my contribution is made under the terms of the project's license.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores