Skip to content

Comments

fix(outlook): actually send emails instead of creating drafts#1239

Merged
elie222 merged 4 commits intomainfrom
fix/outlook-send-email-actually-sends
Jan 9, 2026
Merged

fix(outlook): actually send emails instead of creating drafts#1239
elie222 merged 4 commits intomainfrom
fix/outlook-send-email-actually-sends

Conversation

@elie222
Copy link
Owner

@elie222 elie222 commented Jan 9, 2026

User description

TLDR

sendEmailWithHtml for Outlook was using the wrong Microsoft Graph endpoint, creating drafts instead of sending emails.

Changes

  • Changed from POST /me/messages (creates draft) to POST /me/sendMail (actually sends)
  • Added saveToSentItems: true to ensure emails appear in Sent Items

Impact

This bug affected:

  • Meeting briefings for Outlook users (emails were sitting as drafts, never sent)
  • AI Send Email action for Outlook users (drafts created instead of sending)

Gmail users were unaffected.

Notes

  • /me/sendMail returns 202 with no body, so we can't get the sent message ID
  • conversationId (thread ID) is preserved since it's set before sending
  • Meeting briefings don't use the return value
  • AI action returns empty messageId but valid threadId

Generated description

Below is a concise technical summary of the changes proposed in this PR:
Updates the sendEmailWithHtml function in apps/web/utils/outlook/mail.ts to correctly send emails for Outlook users by switching the Microsoft Graph API endpoint from /me/messages to /me/sendMail. Ensures sent emails are saved to the Sent Items folder, resolving issues where emails were only created as drafts for features like Meeting Briefings and AI Send Email.

TopicDetails
Fix Outlook Email Corrects the sendEmailWithHtml function to use the proper Microsoft Graph API endpoint (/me/sendMail) for sending emails, ensuring they are delivered instead of being saved as drafts and are stored in the Sent Items folder.
Modified files (1)
  • apps/web/utils/outlook/mail.ts
Latest Contributors(2)
UserCommitDate
elie222assistant-fix-CC-BCC-d...January 01, 2026
cursoragent@cursor.comRefactor-Extract-reply...September 14, 2025
Update Coding Rules Refines coding guidelines in .cursor/rules/notes.mdc and apps/web/CLAUDE.md to emphasize self-documenting code over comments, specifically stating that comments should explain 'why' not 'what'.
Modified files (2)
  • .cursor/rules/notes.mdc
  • apps/web/CLAUDE.md
Latest Contributors(1)
UserCommitDate
elie222feat-add-getActionErro...January 08, 2026
This pull request is reviewed by Baz. Review like a pro on (Baz).

sendEmailWithHtml was using POST /me/messages which only creates a draft.
Changed to use /me/sendMail endpoint which actually delivers the email.

Note: sendMail returns 202 with no body, so messageId is empty but
conversationId (threadId) is preserved.
@vercel
Copy link

vercel bot commented Jan 9, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
inbox-zero Ready Ready Preview Jan 9, 2026 9:41pm

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 9, 2026

📝 Walkthrough

Walkthrough

Switches Outlook send flow to POST /me/sendMail with a generated X-InboxZero-TrackingId header, saves to SentItems, then queries SentItems for the sent message by that header to return id and conversationId. Also tightens comment guidelines in docs to forbid "what" comments.

Changes

Cohort / File(s) Summary
Outlook Mail API Integration
apps/web/utils/outlook/mail.ts
Add internetMessageHeaders to request and SentEmailResult type; generate trackingId (crypto.randomUUID) and add X-InboxZero-TrackingId header; use POST /me/sendMail with saveToSentItems: true; after send, query /me/mailFolders/SentItems/messages filtered by the tracking header to find id and conversationId; return { id: string, conversationId: string } (id may be empty; conversationId falls back to original message).
Commenting Guidelines / Docs
/.cursor/rules/notes.mdc, apps/web/CLAUDE.md
Tighten editorial guidance: forbid comments that merely describe what the code does; require comments to explain why, not what. No runtime changes.

Sequence Diagram(s)

sequenceDiagram
  participant App as App (sendEmailWithHtml)
  participant Graph as Microsoft Graph API
  participant Mailbox as Graph (SentItems)

  App->>Graph: POST /me/sendMail\n(message + internetMessageHeaders including X-InboxZero-TrackingId,\nsaveToSentItems:true)
  Graph-->>App: 202 Accepted
  Note right of App: No sent message id returned
  App->>Mailbox: GET /me/mailFolders/SentItems/messages?\n$filter=internetMessageHeaders/Any(h:h/name eq 'X-InboxZero-TrackingId' and h/value eq '{trackingId}')&$select=id,conversationId&$orderby=sentDateTime desc
  Mailbox-->>App: message list (maybe empty)
  App->>App: pick most recent → return { id: string ("" if not found), conversationId (found or original) }
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰
I seeded a header, tiny and bright,
SentMail hopped off into the night.
I sniffed SentItems, nose to the sky,
Found thread and id — or waved goodbye.
Hooray for tracked threads, snug and tight ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately reflects the main functional change: switching from creating drafts to actually sending emails via the correct Microsoft Graph endpoint.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 1 file

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
apps/web/utils/outlook/mail.ts (1)

73-77: Use an explicit return type instead of type assertion for better type safety.

The function currently uses as Message to cast a partial object to the full Message type from @microsoft/microsoft-graph-types. Since the Outlook API returns 202 Accepted with no response body, only id and conversationId are available. Define an explicit return type to reflect this:

type SendEmailResult = Pick<Message, 'id' | 'conversationId'>;

Then update the function signature:

 export async function sendEmailWithHtml(
   client: OutlookClient,
   body: SendEmailBody,
   logger: Logger,
-) {
+): Promise<SendEmailResult> {

And remove the type assertion:

   return {
     id: "",
     conversationId: message.conversationId,
-  } as Message;
+  };
 }

This eliminates the type assertion while accurately reflecting what the function returns, preventing consumers from accidentally accessing unavailable properties.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ddd7ffd and 9c9dc47.

📒 Files selected for processing (1)
  • apps/web/utils/outlook/mail.ts
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/data-fetching.mdc)

**/*.{ts,tsx}: For API GET requests to server, use the swr package
Use result?.serverError with toastError from @/components/Toast for error handling in async operations

**/*.{ts,tsx}: Use wrapper functions for Gmail message operations (get, list, batch, etc.) from @/utils/gmail/message.ts instead of direct API calls
Use wrapper functions for Gmail thread operations from @/utils/gmail/thread.ts instead of direct API calls
Use wrapper functions for Gmail label operations from @/utils/gmail/label.ts instead of direct API calls

**/*.{ts,tsx}: For early access feature flags, create hooks using the naming convention use[FeatureName]Enabled that return a boolean from useFeatureFlagEnabled("flag-key")
For A/B test variant flags, create hooks using the naming convention use[FeatureName]Variant that define variant types, use useFeatureFlagVariantKey() with type casting, and provide a default "control" fallback
Use kebab-case for PostHog feature flag keys (e.g., inbox-cleaner, pricing-options-2)
Always define types for A/B test variant flags (e.g., type PricingVariant = "control" | "variant-a" | "variant-b") and provide type safety through type casting

**/*.{ts,tsx}: Don't use primitive type aliases or misleading types
Don't use empty type parameters in type aliases and interfaces
Don't use this and super in static contexts
Don't use any or unknown as type constraints
Don't use the TypeScript directive @ts-ignore
Don't use TypeScript enums
Don't export imported variables
Don't add type annotations to variables, parameters, and class properties that are initialized with literal expressions
Don't use TypeScript namespaces
Don't use non-null assertions with the ! postfix operator
Don't use parameter properties in class constructors
Don't use user-defined types
Use as const instead of literal types and type annotations
Use either T[] or Array<T> consistently
Initialize each enum member value explicitly
Use export type for types
Use `impo...

Files:

  • apps/web/utils/outlook/mail.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/prisma-enum-imports.mdc)

Always import Prisma enums from @/generated/prisma/enums instead of @/generated/prisma/client to avoid Next.js bundling errors in client components

Import Prisma using the project's centralized utility: import prisma from '@/utils/prisma'

Files:

  • apps/web/utils/outlook/mail.ts
apps/web/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)

Import specific lodash functions rather than entire lodash library to minimize bundle size (e.g., import groupBy from 'lodash/groupBy')

apps/web/**/*.{ts,tsx}: Use TypeScript with strict null checks
Do not export types/interfaces that are only used within the same file. Export later if needed
Infer types from Zod schemas using z.infer<typeof schema> instead of duplicating as separate interfaces
Centralize types in dedicated type files when shared across multiple files
Use SWR for client-side data fetching with pattern useSWR<GetExampleResponse>("/api/user/example")
Call mutate() after successful mutations or use revalidatePath in server actions for cache invalidation

Files:

  • apps/web/utils/outlook/mail.ts
**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/security.mdc)

**/*.ts: ALL database queries MUST be scoped to the authenticated user/account by including user/account filtering in WHERE clauses to prevent unauthorized data access
Always validate that resources belong to the authenticated user before performing operations, using ownership checks in WHERE clauses or relationships
Always validate all input parameters for type, format, and length before using them in database queries
Use SafeError for error responses to prevent information disclosure. Generic error messages should not reveal internal IDs, logic, or resource ownership details
Only return necessary fields in API responses using Prisma's select option. Never expose sensitive data such as password hashes, private keys, or system flags
Prevent Insecure Direct Object References (IDOR) by validating resource ownership before operations. All findUnique/findFirst calls MUST include ownership filters
Prevent mass assignment vulnerabilities by explicitly whitelisting allowed fields in update operations instead of accepting all user-provided data
Prevent privilege escalation by never allowing users to modify system fields, ownership fields, or admin-only attributes through user input
All findMany queries MUST be scoped to the user's data by including appropriate WHERE filters to prevent returning data from other users
Use Prisma relationships for access control by leveraging nested where clauses (e.g., emailAccount: { id: emailAccountId }) to validate ownership

Files:

  • apps/web/utils/outlook/mail.ts
**/*.{tsx,ts}

📄 CodeRabbit inference engine (.cursor/rules/ui-components.mdc)

**/*.{tsx,ts}: Use Shadcn UI and Tailwind for components and styling
Use next/image package for images
For API GET requests to server, use the swr package with hooks like useSWR to fetch data
For text inputs, use the Input component with registerProps for form integration and error handling

Files:

  • apps/web/utils/outlook/mail.ts
**/*.{tsx,ts,css}

📄 CodeRabbit inference engine (.cursor/rules/ui-components.mdc)

Implement responsive design with Tailwind CSS using a mobile-first approach

Files:

  • apps/web/utils/outlook/mail.ts
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)

**/*.{js,jsx,ts,tsx}: Don't use accessKey attribute on any HTML element
Don't set aria-hidden="true" on focusable elements
Don't add ARIA roles, states, and properties to elements that don't support them
Don't use distracting elements like <marquee> or <blink>
Only use the scope prop on <th> elements
Don't assign non-interactive ARIA roles to interactive HTML elements
Make sure label elements have text content and are associated with an input
Don't assign interactive ARIA roles to non-interactive HTML elements
Don't assign tabIndex to non-interactive HTML elements
Don't use positive integers for tabIndex property
Don't include "image", "picture", or "photo" in img alt prop
Don't use explicit role property that's the same as the implicit/default role
Make static elements with click handlers use a valid role attribute
Always include a title element for SVG elements
Give all elements requiring alt text meaningful information for screen readers
Make sure anchors have content that's accessible to screen readers
Assign tabIndex to non-interactive HTML elements with aria-activedescendant
Include all required ARIA attributes for elements with ARIA roles
Make sure ARIA properties are valid for the element's supported roles
Always include a type attribute for button elements
Make elements with interactive roles and handlers focusable
Give heading elements content that's accessible to screen readers (not hidden with aria-hidden)
Always include a lang attribute on the html element
Always include a title attribute for iframe elements
Accompany onClick with at least one of: onKeyUp, onKeyDown, or onKeyPress
Accompany onMouseOver/onMouseOut with onFocus/onBlur
Include caption tracks for audio and video elements
Use semantic elements instead of role attributes in JSX
Make sure all anchors are valid and navigable
Ensure all ARIA properties (aria-*) are valid
Use valid, non-abstract ARIA roles for elements with ARIA roles
Use valid AR...

Files:

  • apps/web/utils/outlook/mail.ts
!(pages/_document).{jsx,tsx}

📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)

Don't use the next/head module in pages/_document.js on Next.js projects

Files:

  • apps/web/utils/outlook/mail.ts
**/*.{js,ts,jsx,tsx}

📄 CodeRabbit inference engine (.cursor/rules/utilities.mdc)

**/*.{js,ts,jsx,tsx}: Use lodash utilities for common operations (arrays, objects, strings)
Import specific lodash functions to minimize bundle size (e.g., import groupBy from 'lodash/groupBy')

Files:

  • apps/web/utils/outlook/mail.ts
**/{utils,helpers,lib}/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/logging.mdc)

Logger should be passed as a parameter to helper functions instead of creating their own logger instances

Files:

  • apps/web/utils/outlook/mail.ts
apps/web/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (apps/web/CLAUDE.md)

apps/web/**/*.{ts,tsx,js,jsx}: Use @/ path aliases for imports from project root instead of relative paths
Use proper error handling with try/catch blocks
Add helper functions to the bottom of files, not the top
All imports go at the top of files, no mid-file dynamic imports
Use NEXT_PUBLIC_ prefix for client-side environment variables

Files:

  • apps/web/utils/outlook/mail.ts
🧠 Learnings (2)
📚 Learning: 2025-11-25T14:37:22.660Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/gmail-api.mdc:0-0
Timestamp: 2025-11-25T14:37:22.660Z
Learning: Applies to **/*.{ts,tsx} : Use wrapper functions for Gmail message operations (get, list, batch, etc.) from @/utils/gmail/message.ts instead of direct API calls

Applied to files:

  • apps/web/utils/outlook/mail.ts
📚 Learning: 2025-11-25T14:37:22.660Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/gmail-api.mdc:0-0
Timestamp: 2025-11-25T14:37:22.660Z
Learning: Applies to **/*.{ts,tsx} : Use wrapper functions for Gmail thread operations from @/utils/gmail/thread.ts instead of direct API calls

Applied to files:

  • apps/web/utils/outlook/mail.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Baz Reviewer
  • GitHub Check: test
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
apps/web/utils/outlook/mail.ts (1)

62-71: LGTM! Correct endpoint change to send emails.

The switch from /me/messages to /me/sendMail properly addresses the issue where emails were being created as drafts instead of being sent. The addition of saveToSentItems: true ensures sent messages appear in the Sent Items folder.

Comment on lines 62 to 77
// Use sendMail endpoint to actually send the email
// Note: sendMail returns 202 Accepted with no body, so we can't get the sent message ID
await withOutlookRetry(
() =>
client.getClient().api("/me/sendMail").post({
message,
saveToSentItems: true,
}),
logger,
);
return result;

// Return conversationId if available (thread ID persists across send)
return {
id: "",
conversationId: message.conversationId,
} as Message;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switching to POST /me/sendMail returns no message ID, so downstream flows (like reply-tracking) receive empty messageId/threadId and break. Could we create a draft via /me/messages first, then send it?

Prompt for AI Agents:

In apps/web/utils/outlook/mail.ts around lines 62 to 77, the sendEmailWithHtml function
currently uses POST /me/sendMail which returns no id and ignores the supplied
conversationId, breaking downstream consumers that rely on valid messageId/threadId
(e.g. reply-tracking in ReplyTrackerEmails.tsx). Change this to first create a draft by
POSTing the message to /me/messages to obtain the Message (id and conversationId), then
send the draft with POST /me/messages/{id}/send (using withOutlookRetry for both calls).
Return the created Message (with id and conversationId) to callers so downstream
reply-tracking receives a valid thread id. Alternatively, if you must keep /me/sendMail,
immediately query the Sent Items (e.g. search /me/mailFolders/SentItems/messages with
filters on conversationId, subject, InternetMessageId or sentDateTime) to find and
return the created message's id and conversationId. Ensure no empty id is returned and
update error handling accordingly.

Fix in Cursor


Finding types: Logical Bugs Breaking Changes

Heads up!

Your free trial ends in 3 days.
To keep getting your PRs reviewed by Baz, update your team's subscription

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review! I investigated the downstream usage:

  1. ReplyTrackerEmails.tsx - Uses _messageId (underscore prefix = unused), only threadId matters
  2. EmailThread.tsx - Uses messageId to auto-expand the sent message in UI (minor, not critical)

The threadId (conversationId) IS preserved because we set it on the message before calling sendMail. Only messageId is empty.

Given that messageId is either unused or only used for a minor UI feature (auto-expanding), the complexity of querying Sent Items doesn't seem justified. If we find a real use case that needs the messageId, we can add the query at that point.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, that tradeoff makes sense given the current usage—I'll save this behavior and rationale to memory after the PR is merged so future reviews don’t push for the extra Sent Items query unless there’s a stronger use case.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated! Now querying Sent Items after sending to get the actual message ID for UI features.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit 85c90d4 addressed this comment by implementing the alternative solution of querying Sent Items after sending. The code now queries /me/mailFolders/SentItems/messages to retrieve the sent message's id and conversationId, returning actual values instead of an empty string for the message ID. This addresses the original concern about missing message IDs, even though the thread discussion concluded this complexity might not be justified given current usage patterns.

- Query /me/mailFolders/SentItems/messages after sending to get actual ID
- Update coding guidelines: no self-explanatory comments
Comment on lines 71 to 89
const sentMessages: { value: Message[] } = await withOutlookRetry(
() =>
client
.getClient()
.api("/me/mailFolders/SentItems/messages")
.filter(`subject eq '${message.subject.replace(/'/g, "''")}'`)
.orderby("sentDateTime desc")
.top(1)
.select("id,conversationId")
.get(),
logger,
);

const sentMessage = sentMessages.value?.[0];

return {
id: sentMessage?.id || "",
conversationId: sentMessage?.conversationId || message.conversationId,
} as Message;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sendEmailWithHtml now queries /me/mailFolders/SentItems/messages filtered only by subject and returns the most recent hit, but nothing ties that query back to the message we just sent. When two emails share a subject (or when the new message hasn’t yet appeared in SentItems due to Graph’s eventual consistency) the lookup can return the wrong message or none at all, so we hand downstream callers stale/empty messageId/threadId even though the send succeeded. Previously the /me/messages POST response gave the exact sent message, so this regression leaks incorrect IDs and breaks thread-linking behavior for any repeated subjects.


Finding type: Logical Bugs

Heads up!

Your free trial ends in 3 days.
To keep getting your PRs reviewed by Baz, update your team's subscription

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit a41a578 addressed this comment by implementing a unique tracking mechanism. The code now generates a UUID (trackingId) for each email, embeds it as a custom header (X-InboxZero-TrackingId) in the outgoing message, and queries for the sent message using this unique identifier instead of the unreliable subject-based filter. This ensures the query returns the exact message that was just sent, eliminating the issues with duplicate subjects and eventual consistency.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed! Now using a unique UUID tracking header (X-InboxZero-TrackingId) instead of subject-based filtering. The query filters by this specific header value, eliminating both the subject collision and race condition issues.

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 3 files (changes from recent commits).

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="apps/web/utils/outlook/mail.ts">

<violation number="1" location="apps/web/utils/outlook/mail.ts:76">
P2: Subject-based lookup after `sendMail` is unreliable. The email may not be immediately available in SentItems (202 Accepted is async), and multiple emails can share the same subject. Consider adding a small delay or retry logic, and/or filter by additional criteria like `sentDateTime gt <timestamp>` to narrow results. Alternatively, accept that message ID may be unavailable when using `/me/sendMail`.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

client
.getClient()
.api("/me/mailFolders/SentItems/messages")
.filter(`subject eq '${message.subject.replace(/'/g, "''")}'`)
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Subject-based lookup after sendMail is unreliable. The email may not be immediately available in SentItems (202 Accepted is async), and multiple emails can share the same subject. Consider adding a small delay or retry logic, and/or filter by additional criteria like sentDateTime gt <timestamp> to narrow results. Alternatively, accept that message ID may be unavailable when using /me/sendMail.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/utils/outlook/mail.ts, line 76:

<comment>Subject-based lookup after `sendMail` is unreliable. The email may not be immediately available in SentItems (202 Accepted is async), and multiple emails can share the same subject. Consider adding a small delay or retry logic, and/or filter by additional criteria like `sentDateTime gt <timestamp>` to narrow results. Alternatively, accept that message ID may be unavailable when using `/me/sendMail`.</comment>

<file context>
@@ -70,10 +68,24 @@ export async function sendEmailWithHtml(
+      client
+        .getClient()
+        .api("/me/mailFolders/SentItems/messages")
+        .filter(`subject eq '${message.subject.replace(/'/g, "''")}'`)
+        .orderby("sentDateTime desc")
+        .top(1)
</file context>

✅ Addressed in a41a578

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit a41a578 addressed this comment by replacing the unreliable subject-based lookup with a unique tracking ID approach. The code now generates a UUID, adds it as a custom internet message header (X-InboxZero-TrackingId) to the outgoing email, and filters by this unique ID instead of the subject, eliminating both the uniqueness and ambiguity issues raised in the comment.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed by using a unique UUID tracking header instead of subject-based lookup. Each email now has X-InboxZero-TrackingId header with a unique value that we filter by.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback! I've saved this as a new learning to improve future reviews.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In @apps/web/utils/outlook/mail.ts:
- Around line 84-89: The code unsafely casts a partial object to microsoft-graph
Message; create a small explicit type (e.g. SentEmailResult { id: string;
conversationId?: string }) and use that as the function's return type instead of
Message, remove the "as Message" cast, return { id: sentMessage?.id || "",
conversationId: sentMessage?.conversationId || message.conversationId } typed as
SentEmailResult, and update any callers to expect SentEmailResult rather than a
full Message.
- Around line 71-82: The SentItems lookup using withOutlookRetry on
client.getClient().api(...).filter(`subject eq '${message.subject.replace(/'/g,
"''")}'`) is fragile and races with the async send; change the query to include
a sentDateTime window (e.g., sentDateTime ge timestampBeforeSend) and at least
one recipient criterion (recipient email or conversationId if available) instead
of relying solely on subject, and if necessary add a short explicit sleep before
the first retry or extend the retry/backoff so the message has time to appear;
update references to sentMessages and the .filter() call accordingly.
🧹 Nitpick comments (1)
apps/web/utils/outlook/mail.ts (1)

132-140: Consider consistency: replyToEmail doesn't return message details.

The replyToEmail function uses /me/sendMail (line 134) but doesn't attempt to retrieve and return the sent message's id and conversationId like sendEmailWithHtml does (lines 71-89).

If callers of replyToEmail need the sent message ID for tracking or logging purposes, consider applying the same pattern used in sendEmailWithHtml.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9c9dc47 and 85c90d4.

📒 Files selected for processing (3)
  • .cursor/rules/notes.mdc
  • apps/web/CLAUDE.md
  • apps/web/utils/outlook/mail.ts
🧰 Additional context used
📓 Path-based instructions (13)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/data-fetching.mdc)

**/*.{ts,tsx}: For API GET requests to server, use the swr package
Use result?.serverError with toastError from @/components/Toast for error handling in async operations

**/*.{ts,tsx}: Use wrapper functions for Gmail message operations (get, list, batch, etc.) from @/utils/gmail/message.ts instead of direct API calls
Use wrapper functions for Gmail thread operations from @/utils/gmail/thread.ts instead of direct API calls
Use wrapper functions for Gmail label operations from @/utils/gmail/label.ts instead of direct API calls

**/*.{ts,tsx}: For early access feature flags, create hooks using the naming convention use[FeatureName]Enabled that return a boolean from useFeatureFlagEnabled("flag-key")
For A/B test variant flags, create hooks using the naming convention use[FeatureName]Variant that define variant types, use useFeatureFlagVariantKey() with type casting, and provide a default "control" fallback
Use kebab-case for PostHog feature flag keys (e.g., inbox-cleaner, pricing-options-2)
Always define types for A/B test variant flags (e.g., type PricingVariant = "control" | "variant-a" | "variant-b") and provide type safety through type casting

**/*.{ts,tsx}: Don't use primitive type aliases or misleading types
Don't use empty type parameters in type aliases and interfaces
Don't use this and super in static contexts
Don't use any or unknown as type constraints
Don't use the TypeScript directive @ts-ignore
Don't use TypeScript enums
Don't export imported variables
Don't add type annotations to variables, parameters, and class properties that are initialized with literal expressions
Don't use TypeScript namespaces
Don't use non-null assertions with the ! postfix operator
Don't use parameter properties in class constructors
Don't use user-defined types
Use as const instead of literal types and type annotations
Use either T[] or Array<T> consistently
Initialize each enum member value explicitly
Use export type for types
Use `impo...

Files:

  • apps/web/utils/outlook/mail.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/prisma-enum-imports.mdc)

Always import Prisma enums from @/generated/prisma/enums instead of @/generated/prisma/client to avoid Next.js bundling errors in client components

Import Prisma using the project's centralized utility: import prisma from '@/utils/prisma'

Files:

  • apps/web/utils/outlook/mail.ts
apps/web/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)

Import specific lodash functions rather than entire lodash library to minimize bundle size (e.g., import groupBy from 'lodash/groupBy')

apps/web/**/*.{ts,tsx}: Use TypeScript with strict null checks
Do not export types/interfaces that are only used within the same file. Export later if needed
Infer types from Zod schemas using z.infer<typeof schema> instead of duplicating as separate interfaces
Centralize types in dedicated type files when shared across multiple files
Use SWR for client-side data fetching with pattern useSWR<GetExampleResponse>("/api/user/example")
Call mutate() after successful mutations or use revalidatePath in server actions for cache invalidation

Files:

  • apps/web/utils/outlook/mail.ts
**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/security.mdc)

**/*.ts: ALL database queries MUST be scoped to the authenticated user/account by including user/account filtering in WHERE clauses to prevent unauthorized data access
Always validate that resources belong to the authenticated user before performing operations, using ownership checks in WHERE clauses or relationships
Always validate all input parameters for type, format, and length before using them in database queries
Use SafeError for error responses to prevent information disclosure. Generic error messages should not reveal internal IDs, logic, or resource ownership details
Only return necessary fields in API responses using Prisma's select option. Never expose sensitive data such as password hashes, private keys, or system flags
Prevent Insecure Direct Object References (IDOR) by validating resource ownership before operations. All findUnique/findFirst calls MUST include ownership filters
Prevent mass assignment vulnerabilities by explicitly whitelisting allowed fields in update operations instead of accepting all user-provided data
Prevent privilege escalation by never allowing users to modify system fields, ownership fields, or admin-only attributes through user input
All findMany queries MUST be scoped to the user's data by including appropriate WHERE filters to prevent returning data from other users
Use Prisma relationships for access control by leveraging nested where clauses (e.g., emailAccount: { id: emailAccountId }) to validate ownership

Files:

  • apps/web/utils/outlook/mail.ts
**/*.{tsx,ts}

📄 CodeRabbit inference engine (.cursor/rules/ui-components.mdc)

**/*.{tsx,ts}: Use Shadcn UI and Tailwind for components and styling
Use next/image package for images
For API GET requests to server, use the swr package with hooks like useSWR to fetch data
For text inputs, use the Input component with registerProps for form integration and error handling

Files:

  • apps/web/utils/outlook/mail.ts
**/*.{tsx,ts,css}

📄 CodeRabbit inference engine (.cursor/rules/ui-components.mdc)

Implement responsive design with Tailwind CSS using a mobile-first approach

Files:

  • apps/web/utils/outlook/mail.ts
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)

**/*.{js,jsx,ts,tsx}: Don't use accessKey attribute on any HTML element
Don't set aria-hidden="true" on focusable elements
Don't add ARIA roles, states, and properties to elements that don't support them
Don't use distracting elements like <marquee> or <blink>
Only use the scope prop on <th> elements
Don't assign non-interactive ARIA roles to interactive HTML elements
Make sure label elements have text content and are associated with an input
Don't assign interactive ARIA roles to non-interactive HTML elements
Don't assign tabIndex to non-interactive HTML elements
Don't use positive integers for tabIndex property
Don't include "image", "picture", or "photo" in img alt prop
Don't use explicit role property that's the same as the implicit/default role
Make static elements with click handlers use a valid role attribute
Always include a title element for SVG elements
Give all elements requiring alt text meaningful information for screen readers
Make sure anchors have content that's accessible to screen readers
Assign tabIndex to non-interactive HTML elements with aria-activedescendant
Include all required ARIA attributes for elements with ARIA roles
Make sure ARIA properties are valid for the element's supported roles
Always include a type attribute for button elements
Make elements with interactive roles and handlers focusable
Give heading elements content that's accessible to screen readers (not hidden with aria-hidden)
Always include a lang attribute on the html element
Always include a title attribute for iframe elements
Accompany onClick with at least one of: onKeyUp, onKeyDown, or onKeyPress
Accompany onMouseOver/onMouseOut with onFocus/onBlur
Include caption tracks for audio and video elements
Use semantic elements instead of role attributes in JSX
Make sure all anchors are valid and navigable
Ensure all ARIA properties (aria-*) are valid
Use valid, non-abstract ARIA roles for elements with ARIA roles
Use valid AR...

Files:

  • apps/web/utils/outlook/mail.ts
!(pages/_document).{jsx,tsx}

📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)

Don't use the next/head module in pages/_document.js on Next.js projects

Files:

  • apps/web/utils/outlook/mail.ts
  • apps/web/CLAUDE.md
  • .cursor/rules/notes.mdc
**/*.{js,ts,jsx,tsx}

📄 CodeRabbit inference engine (.cursor/rules/utilities.mdc)

**/*.{js,ts,jsx,tsx}: Use lodash utilities for common operations (arrays, objects, strings)
Import specific lodash functions to minimize bundle size (e.g., import groupBy from 'lodash/groupBy')

Files:

  • apps/web/utils/outlook/mail.ts
**/{utils,helpers,lib}/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/logging.mdc)

Logger should be passed as a parameter to helper functions instead of creating their own logger instances

Files:

  • apps/web/utils/outlook/mail.ts
apps/web/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (apps/web/CLAUDE.md)

apps/web/**/*.{ts,tsx,js,jsx}: Use @/ path aliases for imports from project root instead of relative paths
Use proper error handling with try/catch blocks
Add helper functions to the bottom of files, not the top
All imports go at the top of files, no mid-file dynamic imports
Use NEXT_PUBLIC_ prefix for client-side environment variables

Files:

  • apps/web/utils/outlook/mail.ts
.cursor/rules/**/*.mdc

📄 CodeRabbit inference engine (.cursor/rules/cursor-rules.mdc)

.cursor/rules/**/*.mdc: Always place cursor rule files in PROJECT_ROOT/.cursor/rules/ directory
Use kebab-case for cursor rule filenames
Make cursor rule filenames descriptive of the rule's purpose
Never place cursor rule files in the project root, in subdirectories outside .cursor/rules, or in any other location outside .cursor/rules/
Cursor rules must have a YAML frontmatter with description, optional globs pattern, and alwaysApply boolean flag
Include a main title heading and structured content with step-by-step instructions, code examples, and guidelines in cursor rule files

Files:

  • .cursor/rules/notes.mdc
.cursor/rules/**

📄 CodeRabbit inference engine (.cursor/rules/cursor-rules.mdc)

Always use .mdc extension for cursor rule files

Files:

  • .cursor/rules/notes.mdc
🧠 Learnings (18)
📚 Learning: 2025-11-25T14:37:22.660Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/gmail-api.mdc:0-0
Timestamp: 2025-11-25T14:37:22.660Z
Learning: Applies to **/*.{ts,tsx} : Use wrapper functions for Gmail message operations (get, list, batch, etc.) from @/utils/gmail/message.ts instead of direct API calls

Applied to files:

  • apps/web/utils/outlook/mail.ts
📚 Learning: 2025-11-25T14:37:22.660Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/gmail-api.mdc:0-0
Timestamp: 2025-11-25T14:37:22.660Z
Learning: Applies to **/*.{ts,tsx} : Use wrapper functions for Gmail thread operations from @/utils/gmail/thread.ts instead of direct API calls

Applied to files:

  • apps/web/utils/outlook/mail.ts
📚 Learning: 2026-01-04T00:25:22.311Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/notes.mdc:0-0
Timestamp: 2026-01-04T00:25:22.311Z
Learning: Prefer self-documenting code over comments; use descriptive variable and function names instead of explaining intent with comments

Applied to files:

  • apps/web/CLAUDE.md
  • .cursor/rules/notes.mdc
📚 Learning: 2025-11-25T14:38:07.606Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-11-25T14:38:07.606Z
Learning: Applies to apps/web/utils/ai/**/*.ts : Keep related AI functions in the same file or directory, extract common patterns into utility functions, and document complex AI logic with clear comments

Applied to files:

  • apps/web/CLAUDE.md
  • .cursor/rules/notes.mdc
📚 Learning: 2026-01-08T15:09:16.739Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: apps/web/CLAUDE.md:0-0
Timestamp: 2026-01-08T15:09:16.739Z
Learning: Applies to apps/web/**/*.{ts,tsx,js,jsx} : Use proper error handling with try/catch blocks

Applied to files:

  • apps/web/CLAUDE.md
📚 Learning: 2026-01-08T15:09:16.739Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: apps/web/CLAUDE.md:0-0
Timestamp: 2026-01-08T15:09:16.739Z
Learning: Applies to apps/web/**/*.test.{ts,tsx,js,jsx} : Co-locate test files next to source files (e.g., `utils/example.test.ts`). Only E2E and AI tests go in `__tests__/`

Applied to files:

  • apps/web/CLAUDE.md
  • .cursor/rules/notes.mdc
📚 Learning: 2025-11-25T14:36:33.376Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/cursor-rules.mdc:0-0
Timestamp: 2025-11-25T14:36:33.376Z
Learning: Applies to .cursor/rules/**/*.mdc : Include a main title heading and structured content with step-by-step instructions, code examples, and guidelines in cursor rule files

Applied to files:

  • apps/web/CLAUDE.md
  • .cursor/rules/notes.mdc
📚 Learning: 2025-11-25T14:42:08.869Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/ultracite.mdc:0-0
Timestamp: 2025-11-25T14:42:08.869Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Make sure to use the "use strict" directive in script files

Applied to files:

  • apps/web/CLAUDE.md
  • .cursor/rules/notes.mdc
📚 Learning: 2025-11-25T14:38:07.606Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-11-25T14:38:07.606Z
Learning: Applies to apps/web/utils/ai/**/*.ts : Implement early returns for invalid LLM inputs, use proper error types and logging, implement fallbacks for AI failures, and add retry logic for transient failures using `withRetry`

Applied to files:

  • apps/web/CLAUDE.md
📚 Learning: 2026-01-04T00:25:22.311Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/notes.mdc:0-0
Timestamp: 2026-01-04T00:25:22.311Z
Learning: Applies to **/*.test.{js,jsx,ts,tsx} : Co-locate test files next to source files (e.g., `utils/example.test.ts`). Only E2E and AI tests go in `__tests__/`

Applied to files:

  • apps/web/CLAUDE.md
  • .cursor/rules/notes.mdc
📚 Learning: 2025-11-25T14:38:07.606Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-11-25T14:38:07.606Z
Learning: Applies to apps/web/{utils/ai,utils/llms,__tests__}/**/*.ts : LLM-related code must be organized in specific directories: `apps/web/utils/ai/` for main implementations, `apps/web/utils/llms/` for core utilities and configurations, and `apps/web/__tests__/` for LLM-specific tests

Applied to files:

  • apps/web/CLAUDE.md
  • .cursor/rules/notes.mdc
📚 Learning: 2026-01-08T15:09:16.739Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: apps/web/CLAUDE.md:0-0
Timestamp: 2026-01-08T15:09:16.739Z
Learning: Applies to apps/web/**/*.{ts,tsx,js,jsx} : All imports go at the top of files, no mid-file dynamic imports

Applied to files:

  • apps/web/CLAUDE.md
  • .cursor/rules/notes.mdc
📚 Learning: 2026-01-04T00:25:22.311Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/notes.mdc:0-0
Timestamp: 2026-01-04T00:25:22.311Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : All imports go at the top of files, no mid-file dynamic imports

Applied to files:

  • apps/web/CLAUDE.md
  • .cursor/rules/notes.mdc
📚 Learning: 2025-11-25T14:36:33.376Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/cursor-rules.mdc:0-0
Timestamp: 2025-11-25T14:36:33.376Z
Learning: Applies to .cursor/rules/**/*.mdc : Make cursor rule filenames descriptive of the rule's purpose

Applied to files:

  • .cursor/rules/notes.mdc
📚 Learning: 2025-11-25T14:42:08.869Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/ultracite.mdc:0-0
Timestamp: 2025-11-25T14:42:08.869Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Make sure JSDoc comment lines start with a single asterisk, except for the first one

Applied to files:

  • .cursor/rules/notes.mdc
📚 Learning: 2025-11-25T14:42:08.869Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/ultracite.mdc:0-0
Timestamp: 2025-11-25T14:42:08.869Z
Learning: Applies to **/*.{test,spec}.{js,jsx,ts,tsx} : Don't use disabled tests

Applied to files:

  • .cursor/rules/notes.mdc
📚 Learning: 2025-11-25T14:42:08.869Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/ultracite.mdc:0-0
Timestamp: 2025-11-25T14:42:08.869Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Don't write unreachable code

Applied to files:

  • .cursor/rules/notes.mdc
📚 Learning: 2026-01-01T10:42:29.775Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2026-01-01T10:42:29.775Z
Learning: Applies to **/__tests__/**/*.{ts,tsx} : Place AI tests in the `__tests__` directory and do not run them by default as they use a real LLM

Applied to files:

  • .cursor/rules/notes.mdc
🧬 Code graph analysis (1)
apps/web/utils/outlook/mail.ts (1)
apps/web/utils/outlook/retry.ts (1)
  • withOutlookRetry (17-79)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: cubic · AI code reviewer
  • GitHub Check: Baz Reviewer
  • GitHub Check: test
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (4)
.cursor/rules/notes.mdc (1)

7-7: LGTM - Clear guideline enhancement.

The strengthened commenting guideline clearly articulates the distinction between "what" and "why" comments, reinforcing self-documenting code practices. This aligns well with the existing coding philosophy.

apps/web/utils/outlook/mail.ts (2)

62-69: LGTM - Correct sendMail endpoint usage.

The switch to /me/sendMail with saveToSentItems: true correctly addresses the issue where emails were being created as drafts instead of being sent.


76-76: The subject escaping is correct for OData v4 filters.

The single-quote escaping using '' is the standard and only required escaping for OData string literals according to the OData v4 specification. Backslashes and newlines do not require special escaping within OData filter string literals—only single quotes must be doubled. The implementation is sufficient and correct.

Likely an incorrect or invalid review comment.

apps/web/CLAUDE.md (1)

23-23: LGTM - Consistent documentation update.

The commenting guideline update matches the change in .cursor/rules/notes.mdc, ensuring consistency across development documentation.

- Add X-InboxZero-TrackingId custom header with UUID before sending
- Query Sent Items by this unique header instead of subject
- Fixes race condition and subject collision issues
- Add explicit SentEmailResult return type
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 1 file (changes from recent commits).

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="apps/web/utils/outlook/mail.ts">

<violation number="1" location="apps/web/utils/outlook/mail.ts:85">
P1: Microsoft Graph API does not support filtering by `internetMessageHeaders`. This query will fail with `ErrorInvalidProperty` error. Consider reverting to subject-based filtering (the previous approach), using `singleValueExtendedProperties` instead (which does support filtering), or wrapping this query in a try-catch to gracefully handle the failure.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

.getClient()
.api("/me/mailFolders/SentItems/messages")
.filter(
`internetMessageHeaders/any(h:h/name eq 'X-InboxZero-TrackingId' and h/value eq '${trackingId}')`,
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Microsoft Graph API does not support filtering by internetMessageHeaders. This query will fail with ErrorInvalidProperty error. Consider reverting to subject-based filtering (the previous approach), using singleValueExtendedProperties instead (which does support filtering), or wrapping this query in a try-catch to gracefully handle the failure.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/utils/outlook/mail.ts, line 85:

<comment>Microsoft Graph API does not support filtering by `internetMessageHeaders`. This query will fail with `ErrorInvalidProperty` error. Consider reverting to subject-based filtering (the previous approach), using `singleValueExtendedProperties` instead (which does support filtering), or wrapping this query in a try-catch to gracefully handle the failure.</comment>

<file context>
@@ -73,8 +81,9 @@ export async function sendEmailWithHtml(
-        .filter(`subject eq '${message.subject.replace(/'/g, "''")}'`)
-        .orderby("sentDateTime desc")
+        .filter(
+          `internetMessageHeaders/any(h:h/name eq 'X-InboxZero-TrackingId' and h/value eq '${trackingId}')`,
+        )
         .top(1)
</file context>

✅ Addressed in d9e05f4

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit d9e05f4 addressed this comment by completely removing the problematic internetMessageHeaders filtering code that would have caused the ErrorInvalidProperty error. The code now explicitly acknowledges in comments that "Graph doesn't support filtering by internetMessageHeaders" and gracefully handles this limitation by returning an empty ID instead of attempting the unsupported query.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right - Graph API doesn't support filtering by internetMessageHeaders. Removed the tracking header approach entirely. Now returning empty messageId with preserved conversationId (threadId). Added comments explaining why this is acceptable for Outlook.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback! I've updated an existing learning with this new information.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/web/utils/outlook/mail.ts (1)

43-64: Import randomUUID explicitly for consistency with codebase patterns

This file uses crypto.randomUUID() without an explicit import, whereas other utilities in the codebase (e.g., oauth/state.ts, middleware.ts, safe-action.ts) import from node:crypto explicitly. For consistency, add:

import { randomUUID } from "node:crypto";

Then replace crypto.randomUUID() with randomUUID().

Note: crypto.randomUUID() is available globally in Node.js v15+, and this function runs only in server-side contexts, so runtime compatibility is not a concern.

🤖 Fix all issues with AI agents
In @apps/web/utils/outlook/mail.ts:
- Around line 70-78: The sendMail POST is non-idempotent so wrapping it directly
with withOutlookRetry risks duplicate emails; change the call around
client.getClient().api("/me/sendMail").post(...) to either (A) skip automatic
retries for this operation by calling a non-retrying path or passing a
disable-retry flag into withOutlookRetry when invoking sendMail, or (B)
implement an idempotency check before retrying: generate and set the
X-InboxZero-TrackingId header, attempt the send once, and on retryable failures
query the sender's Sent Items (or /me/messages with an internetMessageHeaders
filter for X-InboxZero-TrackingId) to detect whether the message was already
accepted and only retry if not found; update the code that calls
withOutlookRetry and the send flow to use one of these approaches and ensure
X-InboxZero-TrackingId is consistently applied when checking for duplicates.
- Around line 79-98: The Graph call in the withOutlookRetry block is using a
$filter on internetMessageHeaders which Graph does not support and will fail;
change this to fetch the SentItems messages (e.g., remove the .filter(...) and
keep .top(1) or increase page size) via the
client.getClient().api("/me/mailFolders/SentItems/messages").select(...) call,
then client-side inspect each message's internetMessageHeaders for the
X-InboxZero-TrackingId matching trackingId to pick the sentMessage, or
alternatively switch to using a singleValueExtendedProperty (store the tracking
id as an extended property and filter on that server-side); also ensure the
returned conversationId defaults to "" when missing (currently you return
message.conversationId as fallback — replace that with an empty string fallback)
while keeping references to sentMessages, sentMessage, withOutlookRetry,
client.getClient(), and trackingId to locate the code.
🧹 Nitpick comments (1)
apps/web/utils/outlook/mail.ts (1)

19-35: Tighten SentEmailResult / internetMessageHeaders typing to match actual contract

Pick<Message, "id" | "conversationId"> keeps those fields optional, while callers likely treat them as usable identifiers (esp. conversationId as threadId). Consider making the return type explicit strings (even if empty) and (optionally) reusing Graph’s header type if available to avoid drifting from the API surface.

Proposed adjustment (stronger, clearer contract)
-interface OutlookMessageRequest {
+interface OutlookMessageRequest {
   subject: string;
   body: {
     contentType: string;
     content: string;
   };
   toRecipients: { emailAddress: { address: string } }[];
   ccRecipients?: { emailAddress: { address: string } }[];
   bccRecipients?: { emailAddress: { address: string } }[];
   replyTo?: { emailAddress: { address: string } }[];
   conversationId?: string;
   isDraft?: boolean;
   internetMessageHeaders?: { name: string; value: string }[];
 }
 
-type SentEmailResult = Pick<Message, "id" | "conversationId">;
+type SentEmailResult = { id: string; conversationId: string };
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 85c90d4 and a41a578.

📒 Files selected for processing (1)
  • apps/web/utils/outlook/mail.ts
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/data-fetching.mdc)

**/*.{ts,tsx}: For API GET requests to server, use the swr package
Use result?.serverError with toastError from @/components/Toast for error handling in async operations

**/*.{ts,tsx}: Use wrapper functions for Gmail message operations (get, list, batch, etc.) from @/utils/gmail/message.ts instead of direct API calls
Use wrapper functions for Gmail thread operations from @/utils/gmail/thread.ts instead of direct API calls
Use wrapper functions for Gmail label operations from @/utils/gmail/label.ts instead of direct API calls

**/*.{ts,tsx}: For early access feature flags, create hooks using the naming convention use[FeatureName]Enabled that return a boolean from useFeatureFlagEnabled("flag-key")
For A/B test variant flags, create hooks using the naming convention use[FeatureName]Variant that define variant types, use useFeatureFlagVariantKey() with type casting, and provide a default "control" fallback
Use kebab-case for PostHog feature flag keys (e.g., inbox-cleaner, pricing-options-2)
Always define types for A/B test variant flags (e.g., type PricingVariant = "control" | "variant-a" | "variant-b") and provide type safety through type casting

**/*.{ts,tsx}: Don't use primitive type aliases or misleading types
Don't use empty type parameters in type aliases and interfaces
Don't use this and super in static contexts
Don't use any or unknown as type constraints
Don't use the TypeScript directive @ts-ignore
Don't use TypeScript enums
Don't export imported variables
Don't add type annotations to variables, parameters, and class properties that are initialized with literal expressions
Don't use TypeScript namespaces
Don't use non-null assertions with the ! postfix operator
Don't use parameter properties in class constructors
Don't use user-defined types
Use as const instead of literal types and type annotations
Use either T[] or Array<T> consistently
Initialize each enum member value explicitly
Use export type for types
Use `impo...

Files:

  • apps/web/utils/outlook/mail.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/prisma-enum-imports.mdc)

Always import Prisma enums from @/generated/prisma/enums instead of @/generated/prisma/client to avoid Next.js bundling errors in client components

Import Prisma using the project's centralized utility: import prisma from '@/utils/prisma'

Files:

  • apps/web/utils/outlook/mail.ts
apps/web/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)

Import specific lodash functions rather than entire lodash library to minimize bundle size (e.g., import groupBy from 'lodash/groupBy')

apps/web/**/*.{ts,tsx}: Use TypeScript with strict null checks
Do not export types/interfaces that are only used within the same file. Export later if needed
Infer types from Zod schemas using z.infer<typeof schema> instead of duplicating as separate interfaces
Centralize types in dedicated type files when shared across multiple files
Use SWR for client-side data fetching with pattern useSWR<GetExampleResponse>("/api/user/example")
Call mutate() after successful mutations or use revalidatePath in server actions for cache invalidation

Files:

  • apps/web/utils/outlook/mail.ts
**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/security.mdc)

**/*.ts: ALL database queries MUST be scoped to the authenticated user/account by including user/account filtering in WHERE clauses to prevent unauthorized data access
Always validate that resources belong to the authenticated user before performing operations, using ownership checks in WHERE clauses or relationships
Always validate all input parameters for type, format, and length before using them in database queries
Use SafeError for error responses to prevent information disclosure. Generic error messages should not reveal internal IDs, logic, or resource ownership details
Only return necessary fields in API responses using Prisma's select option. Never expose sensitive data such as password hashes, private keys, or system flags
Prevent Insecure Direct Object References (IDOR) by validating resource ownership before operations. All findUnique/findFirst calls MUST include ownership filters
Prevent mass assignment vulnerabilities by explicitly whitelisting allowed fields in update operations instead of accepting all user-provided data
Prevent privilege escalation by never allowing users to modify system fields, ownership fields, or admin-only attributes through user input
All findMany queries MUST be scoped to the user's data by including appropriate WHERE filters to prevent returning data from other users
Use Prisma relationships for access control by leveraging nested where clauses (e.g., emailAccount: { id: emailAccountId }) to validate ownership

Files:

  • apps/web/utils/outlook/mail.ts
**/*.{tsx,ts}

📄 CodeRabbit inference engine (.cursor/rules/ui-components.mdc)

**/*.{tsx,ts}: Use Shadcn UI and Tailwind for components and styling
Use next/image package for images
For API GET requests to server, use the swr package with hooks like useSWR to fetch data
For text inputs, use the Input component with registerProps for form integration and error handling

Files:

  • apps/web/utils/outlook/mail.ts
**/*.{tsx,ts,css}

📄 CodeRabbit inference engine (.cursor/rules/ui-components.mdc)

Implement responsive design with Tailwind CSS using a mobile-first approach

Files:

  • apps/web/utils/outlook/mail.ts
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)

**/*.{js,jsx,ts,tsx}: Don't use accessKey attribute on any HTML element
Don't set aria-hidden="true" on focusable elements
Don't add ARIA roles, states, and properties to elements that don't support them
Don't use distracting elements like <marquee> or <blink>
Only use the scope prop on <th> elements
Don't assign non-interactive ARIA roles to interactive HTML elements
Make sure label elements have text content and are associated with an input
Don't assign interactive ARIA roles to non-interactive HTML elements
Don't assign tabIndex to non-interactive HTML elements
Don't use positive integers for tabIndex property
Don't include "image", "picture", or "photo" in img alt prop
Don't use explicit role property that's the same as the implicit/default role
Make static elements with click handlers use a valid role attribute
Always include a title element for SVG elements
Give all elements requiring alt text meaningful information for screen readers
Make sure anchors have content that's accessible to screen readers
Assign tabIndex to non-interactive HTML elements with aria-activedescendant
Include all required ARIA attributes for elements with ARIA roles
Make sure ARIA properties are valid for the element's supported roles
Always include a type attribute for button elements
Make elements with interactive roles and handlers focusable
Give heading elements content that's accessible to screen readers (not hidden with aria-hidden)
Always include a lang attribute on the html element
Always include a title attribute for iframe elements
Accompany onClick with at least one of: onKeyUp, onKeyDown, or onKeyPress
Accompany onMouseOver/onMouseOut with onFocus/onBlur
Include caption tracks for audio and video elements
Use semantic elements instead of role attributes in JSX
Make sure all anchors are valid and navigable
Ensure all ARIA properties (aria-*) are valid
Use valid, non-abstract ARIA roles for elements with ARIA roles
Use valid AR...

Files:

  • apps/web/utils/outlook/mail.ts
!(pages/_document).{jsx,tsx}

📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)

Don't use the next/head module in pages/_document.js on Next.js projects

Files:

  • apps/web/utils/outlook/mail.ts
**/*.{js,ts,jsx,tsx}

📄 CodeRabbit inference engine (.cursor/rules/utilities.mdc)

**/*.{js,ts,jsx,tsx}: Use lodash utilities for common operations (arrays, objects, strings)
Import specific lodash functions to minimize bundle size (e.g., import groupBy from 'lodash/groupBy')

Files:

  • apps/web/utils/outlook/mail.ts
**/{utils,helpers,lib}/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/logging.mdc)

Logger should be passed as a parameter to helper functions instead of creating their own logger instances

Files:

  • apps/web/utils/outlook/mail.ts
apps/web/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (apps/web/CLAUDE.md)

apps/web/**/*.{ts,tsx,js,jsx}: Use @/ path aliases for imports from project root instead of relative paths
Use proper error handling with try/catch blocks
Add helper functions to the bottom of files, not the top
All imports go at the top of files, no mid-file dynamic imports
Use NEXT_PUBLIC_ prefix for client-side environment variables

Files:

  • apps/web/utils/outlook/mail.ts
🧠 Learnings (4)
📚 Learning: 2025-11-25T14:37:22.660Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/gmail-api.mdc:0-0
Timestamp: 2025-11-25T14:37:22.660Z
Learning: Applies to **/*.{ts,tsx} : Use wrapper functions for Gmail message operations (get, list, batch, etc.) from @/utils/gmail/message.ts instead of direct API calls

Applied to files:

  • apps/web/utils/outlook/mail.ts
📚 Learning: 2025-11-25T14:37:22.660Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/gmail-api.mdc:0-0
Timestamp: 2025-11-25T14:37:22.660Z
Learning: Applies to **/*.{ts,tsx} : Use wrapper functions for Gmail thread operations from @/utils/gmail/thread.ts instead of direct API calls

Applied to files:

  • apps/web/utils/outlook/mail.ts
📚 Learning: 2025-11-25T14:38:07.606Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-11-25T14:38:07.606Z
Learning: Applies to apps/web/utils/ai/**/*.ts : Implement early returns for invalid LLM inputs, use proper error types and logging, implement fallbacks for AI failures, and add retry logic for transient failures using `withRetry`

Applied to files:

  • apps/web/utils/outlook/mail.ts
📚 Learning: 2025-07-17T04:19:57.099Z
Learnt from: edulelis
Repo: elie222/inbox-zero PR: 576
File: packages/resend/emails/digest.tsx:78-83
Timestamp: 2025-07-17T04:19:57.099Z
Learning: In packages/resend/emails/digest.tsx, the DigestEmailProps type uses `[key: string]: DigestItem[] | undefined | string | Date | undefined` instead of intersection types like `& Record<string, DigestItem[] | undefined>` due to implementation constraints. This was the initial implementation approach and cannot be changed to more restrictive typing.

Applied to files:

  • apps/web/utils/outlook/mail.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: Baz Reviewer
  • GitHub Check: cubic · AI code reviewer
  • GitHub Check: test
  • GitHub Check: Analyze (javascript-typescript)

Comment on lines 70 to 98
await withOutlookRetry(
() =>
client.getClient().api("/me/sendMail").post({
message,
saveToSentItems: true,
}),
logger,
);
return result;

const sentMessages: { value: Message[] } = await withOutlookRetry(
() =>
client
.getClient()
.api("/me/mailFolders/SentItems/messages")
.filter(
`internetMessageHeaders/any(h:h/name eq 'X-InboxZero-TrackingId' and h/value eq '${trackingId}')`,
)
.top(1)
.select("id,conversationId")
.get(),
logger,
);

const sentMessage = sentMessages.value?.[0];

return {
id: sentMessage?.id || "",
conversationId: sentMessage?.conversationId || message.conversationId,
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new flow queries SentItems immediately after sending, but that folder is eventually consistent—without retries, we might return empty IDs to clients. Could we add polling with exponential backoff here?

Prompt for AI Agents:

In apps/web/utils/outlook/mail.ts around lines 70 to 98, the sendEmailWithHtml flow now
posts the message and immediately queries /me/mailFolders/SentItems/messages (the code
between the post and the sentMessages lookup). This is brittle because SentItems is
eventually consistent; refactor by replacing the single fetch of SentItems with a
polling loop that retries the same withOutlookRetry call using exponential backoff (e.g.
delays like 500ms, 1s, 2s, 4s) for a fixed number of attempts or total timeout (e.g.
30s). Ensure each attempt is logged, stop early when a message is found, and if still
not found after retries log a warning and return the fallback result (empty id but
preserve conversationId) so behavior is explicit. Keep the rest of the method (posting
the mail, message composition, and return shape) unchanged.

Fix in Cursor


Finding type: Breaking Changes

Heads up!

Your free trial ends in 3 days.
To keep getting your PRs reviewed by Baz, update your team's subscription

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit d9e05f4 addressed this comment by removing the problematic SentItems query entirely rather than adding polling with exponential backoff. The code now returns an empty id immediately and preserves only the conversationId, eliminating the eventual consistency issue altogether. The comments explain that Graph API doesn't support the necessary filtering and that conversationId is sufficient for reply tracking.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the SentItems query entirely since Graph doesn't support filtering by internetMessageHeaders. Now returning empty messageId with preserved conversationId (which is what matters for reply tracking).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, that makes sense given Graph’s limitations—returning an empty messageId while preserving conversationId for reply tracking is a good adjustment. I’ll save this behavior to memory after the PR is merged.

Graph API doesn't support filtering by internetMessageHeaders (returns ErrorInvalidProperty).
Simplified to return empty id with preserved conversationId (threadId).
Added comments explaining why and that this is acceptable for Outlook.
@elie222 elie222 merged commit fc2af3f into main Jan 9, 2026
50 checks passed
@elie222 elie222 deleted the fix/outlook-send-email-actually-sends branch January 9, 2026 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant