Conversation
…eat/outlook-provider
…eat/outlook-provider
adds outlook provider
…mail search prompt with additional guidelines
…o useBilling hook
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis update introduces a robust Microsoft Outlook mail driver using the Microsoft Graph API, enabling comprehensive Outlook mail operations alongside Gmail. Colored icon components for Gmail, Microsoft, and Outlook are added, and provider configuration is refactored to use these icons. Billing logic is streamlined with a direct Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI (Mail App)
participant Server API
participant OutlookMailManager (Microsoft Graph)
participant Microsoft Graph API
User->>UI (Mail App): Initiate Outlook connection or mail operation
UI (Mail App)->>Server API: Request mail data (e.g., list, get, send)
Server API->>OutlookMailManager: Call mail method with auth (incl. userId)
OutlookMailManager->>Microsoft Graph API: Make Graph API call (e.g., /messages)
Microsoft Graph API-->>OutlookMailManager: Return data
OutlookMailManager-->>Server API: Return normalized data
Server API-->>UI (Mail App): Respond with mail data
UI (Mail App)-->>User: Display mail or action result
Poem
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure ✨ 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.
Actionable comments posted: 6
🧹 Nitpick comments (11)
apps/mail/lib/env.ts (1)
21-22: Clarify or remove commented OAuth variables.
TheMICROSOFT_CLIENT_IDandMICROSOFT_CLIENT_SECRETentries are currently commented out. Either enable them for Microsoft OAuth flows or add a brief comment explaining when and why they should be uncommented to avoid confusion.apps/mail/lib/constants.tsx (1)
22-26: Consider uncommenting the Outlook providerSince you've updated the commented Microsoft provider to use the new
OutlookColorcomponent, it seems you're preparing for Outlook support. If Outlook implementation is complete as mentioned in the PR summary, consider uncommenting these lines.- // { - // name: 'Outlook', - // icon: OutlookColor, - // providerId: 'microsoft', - // }, + { + name: 'Outlook', + icon: OutlookColor, + providerId: 'microsoft', + },apps/mail/app/(routes)/settings/connections/page.tsx (1)
95-95: Simplified image alt attributeThe alt attribute was simplified to an empty string. Consider adding a descriptive alt text for accessibility purposes.
- alt="" + alt={`${connection.name}'s profile picture`}apps/mail/components/icons/icons.tsx (2)
18-26: Add<title>element for accessibility compliance
GmailColoris currently the only exported SVG in this file that lacks a<title>child. Screen-reader users will not receive any textual description, causing the icon to be announced as just “image”. Please add a short, localised title (e.g. “Gmail”) to stay consistent with the other icons.+ <title>Gmail</title> ...
28-36: Propagate colour via CSS variables (optional)The new coloured Microsoft logo hard-codes its fill via
var(--icon-color), which will render as monochrome if the CSS variable is left to the default.If the intent is to show the coloured Windows logo (red/green/blue/yellow quadrants), consider either:
- four separate
<rect>elements with explicit brand colours, or- leaving this component as-is but renaming it to
MicrosoftMonoto avoid confusion with a future coloured variant.No change required if monochrome is intended—this is merely to avoid future mix-ups.
apps/mail/components/ui/nav-user.tsx (3)
66-67: UnusedbillingCustomerdestructuringIf
billingCustomeris no longer used in this component it can be safely omitted:- const { openBillingPortal, customer: billingCustomer, isPro } = useBilling(); + const { openBillingPortal, isPro } = useBilling();Keeping the object lean avoids unnecessary re-renders triggered by unrelated billing updates.
43-44: Remove unuseduseRouterimport
useRouteris still imported but no longer referenced after the refactor, which will trip linters/CI.-import { useRouter } from 'next/navigation';
438-445: Avoid double instantiation of<PricingDialog>
PricingDialogis rendered twice: once here (lines 438-445) and once again for the “Get verified” button (~lines 542-548). Mounting two hidden dialogs increases DOM size and state complexity.Consider hoisting a single
<PricingDialog>to the component root and controlling it via shared state:<PricingDialog open={showPricingDialog || showPricing} onOpenChange={(v)=>{ setShowPricingDialog(v); setShowPricing(v); }} />apps/server/src/lib/driver/microsoft.ts (3)
274-304:list()performs N+1 calls – heavy latency & rate-limit riskAfter fetching a page, the code iterates over every message and calls
get()individually, resulting in up tomaxResults + 1requests.Graph can return body/attachments in the first query via
$expandor?$select=bodyPreview,bodywith?$filter.
Example:- let request = this.graphClient.api(`/me/mailFolders/${folderId}/messages`).top(maxResults); + let request = this.graphClient + .api(`/me/mailFolders/${folderId}/messages`) + .top(maxResults) + .expand('attachments') + .select('id,subject,bodyPreview,body,from,toRecipients,ccRecipients,...');This slashes round-trips and keeps within throttling quotas.
1038-1040: Remove strayconsole.log– leaks recipient data in server logsLeft-over debugging statement logs full
toarray (PII) on every send.- console.log(to);
1150-1161: Broaden error handling – Graph errors often exposestatusnotstatusCodeSome Graph SDK errors surface as
error.status(number) or undererror.response.status.
Consider normalising:- (error.statusCode >= 400 && error.statusCode < 500 && error.statusCode !== 429); + (Number(error.statusCode ?? error.status) >= 400 && + Number(error.statusCode ?? error.status) < 500 && + Number(error.statusCode ?? error.status) !== 429);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (20)
apps/mail/app/(auth)/login/login-client.tsx(2 hunks)apps/mail/app/(routes)/settings/connections/page.tsx(1 hunks)apps/mail/components/connection/add.tsx(2 hunks)apps/mail/components/icons/icons.tsx(2 hunks)apps/mail/components/mail/mail.tsx(3 hunks)apps/mail/components/ui/ai-sidebar.tsx(1 hunks)apps/mail/components/ui/nav-user.tsx(3 hunks)apps/mail/hooks/use-billing.ts(3 hunks)apps/mail/lib/constants.tsx(2 hunks)apps/mail/lib/env.ts(1 hunks)apps/mail/package.json(1 hunks)apps/server/package.json(2 hunks)apps/server/src/lib/auth-providers.ts(1 hunks)apps/server/src/lib/auth.ts(1 hunks)apps/server/src/lib/driver/google.ts(1 hunks)apps/server/src/lib/driver/index.ts(1 hunks)apps/server/src/lib/driver/microsoft.ts(1 hunks)apps/server/src/lib/driver/types.ts(1 hunks)apps/server/src/lib/driver/utils.ts(1 hunks)apps/server/src/lib/prompts.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (7)
apps/server/src/lib/auth.ts (1)
packages/db/src/schema.ts (1)
account(40-56)
apps/mail/app/(auth)/login/login-client.tsx (1)
apps/mail/components/icons/icons.tsx (1)
Microsoft(28-36)
apps/server/src/lib/driver/index.ts (2)
apps/server/src/lib/driver/google.ts (1)
GoogleMailManager(22-1049)apps/server/src/lib/driver/microsoft.ts (1)
OutlookMailManager(23-1194)
apps/mail/lib/constants.tsx (1)
apps/mail/components/icons/icons.tsx (1)
GmailColor(18-26)
apps/mail/components/connection/add.tsx (2)
apps/mail/lib/constants.tsx (1)
emailProviders(16-27)apps/mail/lib/auth-client.ts (2)
authClient(6-12)authClient(14-14)
apps/mail/components/mail/mail.tsx (2)
apps/mail/hooks/use-connections.ts (1)
useConnections(4-8)packages/db/src/schema.ts (1)
connection(85-104)
apps/mail/components/ui/ai-sidebar.tsx (1)
apps/mail/hooks/use-billing.ts (1)
useBilling(69-126)
🪛 Biome (1.9.4)
apps/server/src/lib/driver/microsoft.ts
[error] 139-144: This code is unreachable
(lint/correctness/noUnreachable)
🔇 Additional comments (31)
apps/mail/package.json (1)
89-89: Validate the beta dependency version for stability.
Thebetter-authpackage was updated to a pre-release (1.2.8-beta.7). Please confirm that this beta version includes the necessary fixes and features for the new Microsoft support and that it meets your stability requirements. Consider pinning to a stable release if long-term support is needed.apps/server/package.json (1)
19-21: New Microsoft Graph and hono-party dependencies added.
The additions of@microsoft/microsoft-graph-client,@microsoft/microsoft-graph-types, andhono-partyalign with the Outlook mail driver and party integration on the server. Verify there are no version conflicts and that these dependencies satisfy your deployment environment (e.g., Wrangler).Also applies to: 37-37
apps/mail/app/(auth)/login/login-client.tsx (1)
7-7: Implement Microsoft icon in login client.
ImportingMicrosoftand adding thecase 'microsoft'branch ingetProviderIconcorrectly enables the Microsoft login option in the UI.Also applies to: 45-47
apps/server/src/lib/auth.ts (1)
34-38: IncludeuserIdin auth payload.
AddinguserId: account.userIdto theauthobject passed intocreateDriveraligns with the updatedManagerConfigand ensures driver implementations (e.g., OutlookMailManager) receive the necessary context.apps/server/src/lib/driver/types.ts (1)
30-31: Added userId to support Microsoft Outlook integration.The addition of the
userIdfield to theauthobject inManagerConfigproperly supports the new Microsoft Outlook mail driver implementation. This field is crucial for authentication context in the Graph API interactions.apps/server/src/lib/driver/utils.ts (1)
41-41: LGTM: Passing userId to mail drivers.This change appropriately passes the user ID from the active connection to the mail driver, consistent with the updated
ManagerConfigtype requirements. This ensures proper authentication context for both Google and Microsoft mail drivers.apps/server/src/lib/driver/index.ts (1)
2-2: Successfully integrated Microsoft Outlook mail driver.The import and registration of the
OutlookMailManagerclass enables Microsoft Outlook mail operations via the Microsoft Graph API. This change completes the mail provider integration alongside the existing Gmail functionality.Also applies to: 7-7
apps/server/src/lib/auth-providers.ts (2)
62-76: Improved Microsoft provider configuration.The Microsoft provider configuration has been enhanced with:
- Removal of non-null assertions for better type safety
- Additional Graph API permissions for mail management
- Added
disableProfilePhotoflagThese changes align with best practices for Microsoft Graph API integration.
77-77: Setting Microsoft as a required provider.Marking the Microsoft provider as
required: trueindicates it's an essential provider for the application. This will ensure proper error handling if the provider's environment variables aren't configured correctly.apps/mail/lib/constants.tsx (2)
1-1: Great addition of reusable icon components!Importing dedicated icon components is much better than using inline SVG paths. This promotes reusability and consistency across the UI.
18-20: Good brand alignment updateUpdating from "Google" to "Gmail" and using the proper
GmailColorcomponent improves brand consistency and visual coherence.apps/mail/components/ui/ai-sidebar.tsx (1)
386-386: Good simplification by leveraging useBilling's isPro flagThis change appropriately leverages the
isProflag now directly provided by theuseBillinghook, removing redundant computation in the component. This enhances code maintainability and readability.apps/mail/components/mail/mail.tsx (3)
38-38: Good addition of connections hookAdding the
useConnectionshook is appropriate for retrieving the user's email connections, which is needed to determine the active email provider.
328-334: Well-implemented active account logicGood use of
useMemoto efficiently compute the active account based on the session's active connection ID. This avoids unnecessary recalculations on rerenders.
466-468: Good provider-specific UI renderingThe updated condition properly ensures that Gmail-specific UI elements like
CategorySelectare only shown when both:
- The active account is using Google as the provider
- The current folder is 'inbox'
This is critical for multi-provider support and prevents showing Google-specific features in Microsoft Outlook accounts.
apps/server/src/lib/driver/google.ts (1)
1021-1023: Improved type safety in findAttachments methodGreat improvement by replacing
any[]with the more specificgmail_v1.Schema$MessagePart[]type. This enhances type safety and makes the code more maintainable.apps/mail/components/connection/add.tsx (2)
99-126: Improved email provider rendering with component iconsThe refactoring of email provider rendering improves code clarity and maintainability. The change from inline SVG paths to using provider icon components from a centralized location creates a more consistent approach across the application.
117-117: Environment variable access standardizedChanged from using an imported
envobject to directly accessingprocess.env.NEXT_PUBLIC_APP_URL, which aligns with other changes in the codebase for consistent environment variable access.apps/mail/app/(routes)/settings/connections/page.tsx (4)
83-85: Improved provider icon handlingThe new approach of dynamically finding the matching provider by
providerIdand using its icon component is more flexible and maintainable than the previous static approach.
102-102: Consistent icon renderingUsing the dynamically found icon component with a standardized size class (
size-6) provides consistent styling across the application.
152-152: Environment variable access standardizedChanged from using an imported
envobject to directly accessingprocess.env.NEXT_PUBLIC_APP_URL, which aligns with other changes in the codebase.
171-171: Dialog overlay property addedAdded the
showOverlayprop to theDialogContentcomponent, providing explicit control over the overlay behavior.apps/mail/hooks/use-billing.ts (4)
24-54: Enhanced type safety with explicit TypeScript interfacesAdding explicit TypeScript interfaces (
Product,Customer,TrackParams, andBillingHook) improves code readability, maintainability, and provides better IDE support.
67-67: Centralized PRO_PLANS definitionGood practice to define the pro plan identifiers as a constant. This makes it easier to maintain if plan IDs change in the future.
73-78: Optimized isPro calculation with useMemoThe
isProflag is efficiently calculated usinguseMemoto prevent unnecessary recalculations, and the implementation handles edge cases like missing or malformed customer data.
123-123: Centralized pro status accessExporting the
isProflag directly from the hook simplifies component code that needs to check pro status, eliminating the need for redundant calculations in multiple places.apps/server/src/lib/prompts.ts (4)
256-260: Improved prompt format and added current date contextThe prompt format has been standardized, and a dynamic
<current_date>tag has been added using thegetCurrentDateContext()function, which will help the AI assistant provide more time-relevant responses.
268-269: Enhanced Gmail search operatorsThe list of Gmail search operators has been expanded to include
in:anywhereandintext:, which provides more comprehensive search capabilities.
276-278: Clarified default search behaviorAdded a clear explanation about default search behavior for unqualified terms and the use of
intext:for message body searches, which will help users create more accurate search queries.
280-281: Consistent output format instructionThe output format instruction remains unchanged but has been reformatted for consistency with the rest of the prompt.
apps/mail/components/ui/nav-user.tsx (1)
54-56: Good call on renamingrefetch→refetchSessionThe explicit alias improves readability and prevents the common “which refetch am I calling?” problem.
| export const OutlookColor = ({ className }: { className?: string }) => ( | ||
| <svg | ||
| role="img" | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| viewBox="0 0 1831.085 1703.335" | ||
| className={className} | ||
| > | ||
| <path | ||
| fill="#0A2767" | ||
| d="M1831.083,894.25c0.1-14.318-7.298-27.644-19.503-35.131h-0.213l-0.767-0.426l-634.492-375.585 c-2.74-1.851-5.583-3.543-8.517-5.067c-24.498-12.639-53.599-12.639-78.098,0c-2.934,1.525-5.777,3.216-8.517,5.067L446.486,858.693 l-0.766,0.426c-19.392,12.059-25.337,37.556-13.278,56.948c3.553,5.714,8.447,10.474,14.257,13.868l634.492,375.585 c2.749,1.835,5.592,3.527,8.517,5.068c24.498,12.639,53.599,12.639,78.098,0c2.925-1.541,5.767-3.232,8.517-5.068l634.492-375.585 C1823.49,922.545,1831.228,908.923,1831.083,894.25z" | ||
| /> | ||
| <path | ||
| fill="#0364B8" | ||
| d="M520.453,643.477h416.38v381.674h-416.38V643.477z M1745.917,255.5V80.908 c1-43.652-33.552-79.862-77.203-80.908H588.204C544.552,1.046,510,37.256,511,80.908V255.5l638.75,170.333L1745.917,255.5z" | ||
| /> | ||
| <path fill="#0078D4" d="M511,255.5h425.833v383.25H511V255.5z" /> | ||
| <path | ||
| fill="#28A8EA" | ||
| d="M1362.667,255.5H936.833v383.25L1362.667,1022h383.25V638.75L1362.667,255.5z" | ||
| /> | ||
| <path fill="#0078D4" d="M936.833,638.75h425.833V1022H936.833V638.75z" /> | ||
| <path fill="#0364B8" d="M936.833,1022h425.833v383.25H936.833V1022z" /> | ||
| <path fill="#14447D" d="M520.453,1025.151h416.38v346.969h-416.38V1025.151z" /> | ||
| <path fill="#0078D4" d="M1362.667,1022h383.25v383.25h-383.25V1022z" /> | ||
| <linearGradient | ||
| id="SVGID_1_" | ||
| gradientUnits="userSpaceOnUse" | ||
| x1="1128.4584" | ||
| y1="811.0833" | ||
| x2="1128.4584" | ||
| y2="1.9982" | ||
| gradientTransform="matrix(1 0 0 -1 0 1705.3334)" | ||
| > | ||
| <stop offset="0" style={{ stopColor: '#35B8F1' }} /> | ||
| <stop offset="1" style={{ stopColor: '#28A8EA' }} /> | ||
| </linearGradient> | ||
| <path | ||
| fill="url(#SVGID_1_)" | ||
| d="M1811.58,927.593l-0.809,0.426l-634.492,356.848c-2.768,1.703-5.578,3.321-8.517,4.769 c-10.777,5.132-22.481,8.029-34.407,8.517l-34.663-20.27c-2.929-1.47-5.773-3.105-8.517-4.897L447.167,906.003h-0.298 l-21.036-11.753v722.384c0.328,48.196,39.653,87.006,87.849,86.7h1230.914c0.724,0,1.363-0.341,2.129-0.341 c10.18-0.651,20.216-2.745,29.808-6.217c4.145-1.756,8.146-3.835,11.966-6.217c2.853-1.618,7.75-5.152,7.75-5.152 c21.814-16.142,34.726-41.635,34.833-68.772V894.25C1831.068,908.067,1823.616,920.807,1811.58,927.593z" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Duplicate IDs will break when the component is mounted more than once
OutlookColor defines two <linearGradient> elements with static IDs (SVGID_1_, SVGID_2_). If two instances of <OutlookColor/> appear on the same page React will emit duplicate-ID warnings and only the first gradient will be used, yielding incorrect fills on the second icon.
Create unique IDs per render, e.g.:
+import { useId } from 'react';
export const OutlookColor = ({ className }: { className?: string }) => {
+ const grad1 = useId();
+ const grad2 = useId();
return (
<svg
...
- <linearGradient id="SVGID_1_" ...>
+ <linearGradient id={grad1} ...>
...
- <path fill="url(#SVGID_1_)" d="..." />
+ <path fill={`url(#${grad1})`} d="..." />
...
- <linearGradient id="SVGID_2_" ...>
+ <linearGradient id={grad2} ...>
...
- <path fill="url(#SVGID_2_)" d="..." />
+ <path fill={`url(#${grad2})`} d="..." />While touching the file, please add a <title> element, mirroring other icons.
Also applies to: 85-97
🤖 Prompt for AI Agents
In apps/mail/components/icons/icons.tsx between lines 48 and 86, the
OutlookColor component uses static IDs for linearGradient elements, causing
duplicate ID conflicts when multiple instances render. Fix this by generating
unique IDs dynamically for each linearGradient per render, for example by
appending a unique suffix or using a unique ID generator. Also add a <title>
element inside the SVG for accessibility, consistent with other icons. Apply the
same fixes to lines 85-97 where similar issues exist.
| if (connectionId === session?.connectionId) return; | ||
| await setDefaultConnection({ connectionId }); | ||
| refetch(); | ||
| refetchConnections(); | ||
| refetchThreads(); | ||
| refetchLabels(); | ||
| refetchStats(); | ||
| refetchBrainState(); | ||
| refetchBrainLabels(); | ||
| await refetchConnections(); | ||
| refetchSession(); | ||
| // TODO: fix this cache issue, for now this is a quick fix to hard refresh the page | ||
| window.location.href = pathname; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
handleAccountSwitch – brittle full page refresh & missing error handling
-
For a SPA experience prefer
router.refresh()(Next 13+) orrouter.replace(pathname)instead ofwindow.location.href = pathname; this avoids a hard reload and preserves scroll/Focus states. -
setDefaultConnectionisasyncbut its rejection is not caught. A network error would silently explode and the user would stay on the wrong account.
- await setDefaultConnection({ connectionId });
- await refetchConnections();
- refetchSession();
- window.location.href = pathname;
+try {
+ await setDefaultConnection({ connectionId });
+ await Promise.all([refetchConnections(), refetchSession()]);
+ router.refresh(); // or router.replace(pathname)
+} catch (err) {
+ toast.error('Unable to switch account. Please try again.');
+}- After adopting
router.refresh()you can finally drop the now-unusedwindowcall.
📝 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.
| if (connectionId === session?.connectionId) return; | |
| await setDefaultConnection({ connectionId }); | |
| refetch(); | |
| refetchConnections(); | |
| refetchThreads(); | |
| refetchLabels(); | |
| refetchStats(); | |
| refetchBrainState(); | |
| refetchBrainLabels(); | |
| await refetchConnections(); | |
| refetchSession(); | |
| // TODO: fix this cache issue, for now this is a quick fix to hard refresh the page | |
| window.location.href = pathname; | |
| }; | |
| if (connectionId === session?.connectionId) return; | |
| // TODO: fix this cache issue, for now this is a quick fix to hard refresh the page | |
| try { | |
| await setDefaultConnection({ connectionId }); | |
| await Promise.all([refetchConnections(), refetchSession()]); | |
| router.refresh(); // or router.replace(pathname) | |
| } catch (err) { | |
| toast.error('Unable to switch account. Please try again.'); | |
| } | |
| }; |
🤖 Prompt for AI Agents
In apps/mail/components/ui/nav-user.tsx around lines 98 to 104, replace the hard
page reload using window.location.href with router.refresh() or
router.replace(pathname) to maintain SPA behavior and preserve scroll and focus
states. Additionally, wrap the await call to setDefaultConnection in a try-catch
block to handle potential errors gracefully and prevent silent failures. After
implementing router.refresh(), remove the window.location.href line as it will
no longer be needed.
| public getTokens<T>(code: string) { | ||
| return this.withErrorHandler( | ||
| 'getTokens', | ||
| async () => { | ||
| const tokens = { | ||
| accessToken: this.config.auth?.accessToken, | ||
| refreshToken: this.config.auth?.refreshToken, | ||
| }; | ||
| return { tokens } as T; | ||
| }, |
There was a problem hiding this comment.
getTokens is a stub – OAuth flow will fail for fresh log-ins
The method simply echoes existing tokens and ignores the code parameter.
Without exchanging the authorization code for new access/refresh tokens the first-time connection path is broken.
Action items:
- Call your token endpoint (
/tokenor MSAL) withcode,redirect_uri,client_id,client_secret. - Persist
access_token,refresh_token, and their expiry inconfig.auth.
Failing to do so will surface as “invalid_grant” on refresh.
🤖 Prompt for AI Agents
In apps/server/src/lib/driver/microsoft.ts around lines 157 to 166, the
getTokens method currently just returns existing tokens and ignores the
authorization code parameter, which breaks the OAuth flow for new logins. To fix
this, implement the OAuth token exchange by calling the Microsoft token endpoint
with the provided code, redirect_uri, client_id, and client_secret. Then, update
config.auth to persist the new access_token, refresh_token, and their expiry
time. This ensures the first-time login correctly obtains and stores tokens,
preventing "invalid_grant" errors on refresh.
| let photoUrl = ''; | ||
| try { | ||
| // Requires separate fetching logic | ||
| } catch (error: unknown) { | ||
| console.warn( | ||
| 'Could not fetch user photo:', | ||
| error instanceof Error ? error.message : 'Unknown error', | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove empty try/catch – it produces unreachable-code warnings
The try block does not execute any statement, so the associated catch is unreachable (Biome warning 139-144).
Either delete the block or implement the photo-fetch logic.
- let photoUrl = '';
- try {
- // Requires separate fetching logic
- } catch (error: unknown) {
- console.warn(
- 'Could not fetch user photo:',
- error instanceof Error ? error.message : 'Unknown error',
- );
- }
+ // TODO: fetch profile photo or drop support entirely
+ const photoUrl = '';📝 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.
| let photoUrl = ''; | |
| try { | |
| // Requires separate fetching logic | |
| } catch (error: unknown) { | |
| console.warn( | |
| 'Could not fetch user photo:', | |
| error instanceof Error ? error.message : 'Unknown error', | |
| ); | |
| } | |
| // TODO: fetch profile photo or drop support entirely | |
| const photoUrl = ''; |
🧰 Tools
🪛 Biome (1.9.4)
[error] 139-144: This code is unreachable
(lint/correctness/noUnreachable)
🤖 Prompt for AI Agents
In apps/server/src/lib/driver/microsoft.ts between lines 136 and 144, there is
an empty try block with a catch that logs errors, which causes unreachable-code
warnings. To fix this, either remove the entire try/catch block if no
photo-fetch logic is implemented yet, or add the necessary code inside the try
block to fetch the user photo so the catch can handle potential errors properly.
| if (messageIds.length === 0) { | ||
| return; | ||
| } | ||
|
|
||
| const batchRequests = messageIds.map((id, index) => ({ | ||
| id: `${index}`, | ||
| method: 'PATCH', | ||
| url: `/me/messages/${id}`, | ||
| body: { isRead: isRead }, | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| })); | ||
|
|
||
| try { | ||
| await this.graphClient.api('/$batch').post({ requests: batchRequests }); | ||
| } catch (error) { | ||
| console.error('Error during batch update of message read status:', error); | ||
| throw error; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Chunk $batch calls to max 20 requests to avoid hard Graph limit
Microsoft Graph rejects batches larger than 20.
modifyMessageReadStatus may receive any length array, producing a 413/400 error for large inputs.
- try {
- await this.graphClient.api('/$batch').post({ requests: batchRequests });
+ const CHUNK = 20;
+ for (let i = 0; i < batchRequests.length; i += CHUNK) {
+ const slice = batchRequests.slice(i, i + CHUNK);
+ await this.graphClient.api('/$batch').post({ requests: slice });
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/server/src/lib/driver/microsoft.ts around lines 108 to 126, the batch
request to Microsoft Graph can exceed the maximum allowed 20 requests per batch,
causing errors. Modify the code to split the messageIds array into chunks of 20
or fewer, then send multiple batch requests sequentially or in parallel,
ensuring no batch exceeds 20 requests. This will prevent 413/400 errors from the
Graph API.
| const batchRequests = messageIds.map((id, index) => { | ||
| const patchBody = {}; | ||
|
|
||
| if (addItems.length > 0 || removeItems.length > 0) { | ||
| console.warn( | ||
| `Modifying categories (${addItems.join(',')}, ${removeItems.join(',')}) on message ${id} is not fully implemented.`, | ||
| ); | ||
| } | ||
|
|
||
| if (!addItems[0]) { | ||
| console.warn('No addItems'); | ||
| return; | ||
| } | ||
|
|
||
| let moveToFolderId: string | undefined; | ||
| if (addItems.length > 0 && this.getOutlookFolderId(addItems[0])) { | ||
| moveToFolderId = this.getOutlookFolderId(addItems[0]) || addItems[0]; | ||
| console.warn( | ||
| `Attempting to move message ${id} to folder ${moveToFolderId}. This is a move operation, not adding a label.`, | ||
| ); | ||
| return { | ||
| id: `${index}`, | ||
| method: 'POST', | ||
| url: `/me/messages/${id}/move`, | ||
| body: { destinationId: moveToFolderId }, | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| }; | ||
| } | ||
| return { | ||
| id: `${index}`, | ||
| method: 'PATCH', | ||
| url: `/me/messages/${id}`, | ||
| body: patchBody, | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| }; | ||
| }); | ||
|
|
||
| const validBatchRequests = batchRequests | ||
| .filter((req) => typeof req !== 'undefined') | ||
| .filter((req) => Object.keys(req.body).length > 0 || req.method === 'POST'); | ||
|
|
||
| if (validBatchRequests.length === 0) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
patchBody is always empty → PATCH requests are filtered out
modifyMessageLabelsOrFolders builds patchBody but never populates it, and later filters out requests whose body has no keys – effectively disabling category updates.
Consider:
- const patchBody = {};
+ const patchBody: Record<string, unknown> = {};
+ if (addItems.length) patchBody.categories = addItems;
+ if (removeItems.length) patchBody.categoriesToRemove = removeItems;Alternatively, short-circuit and throw NotImplemented until category support is ready to avoid silent no-ops.
📝 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 batchRequests = messageIds.map((id, index) => { | |
| const patchBody = {}; | |
| if (addItems.length > 0 || removeItems.length > 0) { | |
| console.warn( | |
| `Modifying categories (${addItems.join(',')}, ${removeItems.join(',')}) on message ${id} is not fully implemented.`, | |
| ); | |
| } | |
| if (!addItems[0]) { | |
| console.warn('No addItems'); | |
| return; | |
| } | |
| let moveToFolderId: string | undefined; | |
| if (addItems.length > 0 && this.getOutlookFolderId(addItems[0])) { | |
| moveToFolderId = this.getOutlookFolderId(addItems[0]) || addItems[0]; | |
| console.warn( | |
| `Attempting to move message ${id} to folder ${moveToFolderId}. This is a move operation, not adding a label.`, | |
| ); | |
| return { | |
| id: `${index}`, | |
| method: 'POST', | |
| url: `/me/messages/${id}/move`, | |
| body: { destinationId: moveToFolderId }, | |
| headers: { 'Content-Type': 'application/json' }, | |
| }; | |
| } | |
| return { | |
| id: `${index}`, | |
| method: 'PATCH', | |
| url: `/me/messages/${id}`, | |
| body: patchBody, | |
| headers: { 'Content-Type': 'application/json' }, | |
| }; | |
| }); | |
| const validBatchRequests = batchRequests | |
| .filter((req) => typeof req !== 'undefined') | |
| .filter((req) => Object.keys(req.body).length > 0 || req.method === 'POST'); | |
| if (validBatchRequests.length === 0) { | |
| const batchRequests = messageIds.map((id, index) => { | |
| - const patchBody = {}; | |
| + const patchBody: Record<string, unknown> = {}; | |
| + if (addItems.length) patchBody.categories = addItems; | |
| + if (removeItems.length) patchBody.categoriesToRemove = removeItems; | |
| if (addItems.length > 0 || removeItems.length > 0) { | |
| console.warn( | |
| `Modifying categories (${addItems.join(',')}, ${removeItems.join(',')}) on message ${id} is not fully implemented.`, | |
| ); | |
| } | |
| if (!addItems[0]) { | |
| console.warn('No addItems'); | |
| return; | |
| } | |
| let moveToFolderId: string | undefined; | |
| if (addItems.length > 0 && this.getOutlookFolderId(addItems[0])) { | |
| moveToFolderId = this.getOutlookFolderId(addItems[0]) || addItems[0]; | |
| console.warn( | |
| `Attempting to move message ${id} to folder ${moveToFolderId}. This is a move operation, not adding a label.`, | |
| ); | |
| return { | |
| id: `${index}`, | |
| method: 'POST', | |
| url: `/me/messages/${id}/move`, | |
| body: { destinationId: moveToFolderId }, | |
| headers: { 'Content-Type': 'application/json' }, | |
| }; | |
| } | |
| return { | |
| id: `${index}`, | |
| method: 'PATCH', | |
| url: `/me/messages/${id}`, | |
| body: patchBody, | |
| headers: { 'Content-Type': 'application/json' }, | |
| }; | |
| }); | |
| const validBatchRequests = batchRequests | |
| .filter((req) => typeof req !== 'undefined') | |
| .filter((req) => Object.keys(req.body).length > 0 || req.method === 'POST'); | |
| if (validBatchRequests.length === 0) { |
🤖 Prompt for AI Agents
In apps/server/src/lib/driver/microsoft.ts between lines 480 and 521, the
patchBody object used in PATCH requests is never populated, causing these
requests to be filtered out later due to empty bodies and disabling category
updates. To fix this, populate patchBody with the appropriate category
modifications before returning the PATCH request, or alternatively, throw a
NotImplemented error early to avoid silent no-ops until category support is
fully implemented.
3 participants
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores