Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
elie222 merged 4 commits into from
Jan 9, 2026
+20 −6
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9c9dc47
fix(outlook): actually send emails instead of creating drafts
elie222 Jan 9, 2026
85c90d4
fix: query Sent Items for message ID, remove self-explanatory comments
elie222 Jan 9, 2026
a41a578
fix: use unique tracking header to reliably get sent message ID
elie222 Jan 9, 2026
d9e05f4
fix: remove broken internetMessageHeaders filter, accept empty messageId
elie222 Jan 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .cursor/rules/notes.mdc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ globs:
alwaysApply: true
---
- Do not try and run the project via `dev` or `build` command unless I explicitly ask you to.
- Prefer self-documenting code over comments; use descriptive variable and function names instead of explaining intent with comments.
- Prefer self-documenting code over comments; use descriptive variable and function names instead of explaining intent with comments. Never add comments that just describe what the code does - code should explain itself. Only add comments for "why" not "what".
- Add helper functions to the bottom of files, not the top!
- All imports go at the top of files, no mid-file dynamic imports.
- Co-locate test files next to source files (e.g., `utils/example.test.ts`). Only E2E and AI tests go in `__tests__/`.
Expand Down
2 changes: 1 addition & 1 deletion apps/web/CLAUDE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
- Use proper error handling with try/catch blocks
- Format code with Prettier
- Consult .cursor/rules for environment variable management
- Prefer self-documenting code over comments; use descriptive variable and function names instead of explaining intent with comments.
- Prefer self-documenting code over comments; use descriptive variable and function names instead of explaining intent with comments. Never add comments that just describe what the code does - code should explain itself. Only add comments for "why" not "what".
- Add helper functions to the bottom of files, not the top!
- All imports go at the top of files, no mid-file dynamic imports.
- Co-locate test files next to source files (e.g., `utils/example.test.ts`). Only E2E and AI tests go in `__tests__/`.
Expand Down
22 changes: 18 additions & 4 deletions apps/web/utils/outlook/mail.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,13 @@ interface OutlookMessageRequest {
isDraft?: boolean;
}

type SentEmailResult = Pick<Message, "id" | "conversationId">;

export async function sendEmailWithHtml(
client: OutlookClient,
body: SendEmailBody,
logger: Logger,
) {
): Promise<SentEmailResult> {
ensureEmailSendingEnabled();

const message: OutlookMessageRequest = {
Expand All @@ -59,11 +61,23 @@ export async function sendEmailWithHtml(
message.conversationId = body.replyToEmail.threadId;
}

const result: Message = await withOutlookRetry(
() => client.getClient().api("/me/messages").post(message),
await withOutlookRetry(
() =>
client.getClient().api("/me/sendMail").post({
message,
saveToSentItems: true,
}),
logger,
);
return result;

// /me/sendMail returns 202 with no body, so we can't get the sent message ID.
// Graph doesn't support filtering by internetMessageHeaders, so we can't query for it.
// conversationId (threadId) is preserved - that's what matters for reply tracking.
// Empty id means auto-expand won't work in EmailThread, but we don't show that for Outlook.
return {
id: "",
conversationId: message.conversationId,
};
Comment on lines +64 to +80
Copy link
Copy Markdown
Contributor

@baz-reviewer baz-reviewer bot Jan 9, 2026

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
Copy Markdown
Contributor

@baz-reviewer baz-reviewer bot Jan 9, 2026

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
Copy Markdown
Owner Author

@elie222 elie222 Jan 9, 2026

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
Copy Markdown
Contributor

@baz-reviewer baz-reviewer bot Jan 9, 2026

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.

}

export async function sendEmailWithPlainText(
Expand Down
Loading