Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions apps/web/__tests__/e2e/flows/helpers/email.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ export async function sendTestReply(options: {
threadId,
headerMessageId: originalMessage.headers["message-id"] || "",
references: originalMessage.headers.references,
messageId: originalMessageId, // Needed for Outlook's createReply API
},
});

Expand Down
29 changes: 17 additions & 12 deletions apps/web/__tests__/e2e/flows/outbound-tracking.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -230,15 +230,21 @@ describe.skipIf(!shouldRunFlowTests())("Outbound Message Tracking", () => {
where: {
emailAccountId: outlook.id,
threadId: receivedMessage.threadId,
resolved: false,
},
});

logStep("ThreadTracker before reply", {
id: trackerBeforeReply?.id,
exists: !!trackerBeforeReply,
resolved: trackerBeforeReply?.resolved,
type: trackerBeforeReply?.type,
});

// Store the tracker ID to verify it gets resolved
const originalTrackerId = trackerBeforeReply?.id;
expect(originalTrackerId).toBeDefined();

// ========================================
// Send reply
// ========================================
Expand All @@ -260,21 +266,20 @@ describe.skipIf(!shouldRunFlowTests())("Outbound Message Tracking", () => {
// Wait for outbound processing to mark tracker as resolved
await new Promise((resolve) => setTimeout(resolve, 10_000));

// Verify the thread is now marked as "replied to"
const threadTracker = await prisma.threadTracker.findFirst({
where: {
emailAccountId: outlook.id,
threadId: receivedMessage.threadId,
},
// Verify the ORIGINAL tracker is now resolved
// Note: A new AWAITING_REPLY tracker may be created, so we must check
// the specific tracker that existed before the reply
const resolvedTracker = await prisma.threadTracker.findUnique({
where: { id: originalTrackerId! },
});

// Thread tracker should exist and be marked as resolved after reply
expect(threadTracker).toBeDefined();
expect(threadTracker?.resolved).toBe(true);
expect(resolvedTracker).toBeDefined();
expect(resolvedTracker?.resolved).toBe(true);

logStep("Reply tracking found", {
resolved: threadTracker?.resolved,
type: threadTracker?.type,
logStep("Original tracker now resolved", {
id: resolvedTracker?.id,
resolved: resolvedTracker?.resolved,
type: resolvedTracker?.type,
});
},
TIMEOUTS.FULL_CYCLE,
Expand Down
70 changes: 70 additions & 0 deletions apps/web/utils/email/threading.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import { describe, it, expect } from "vitest";
import { buildThreadingHeaders } from "./threading";

describe("buildThreadingHeaders", () => {
it("returns empty strings when headerMessageId is empty", () => {
const result = buildThreadingHeaders({ headerMessageId: "" });
expect(result).toEqual({ inReplyTo: "", references: "" });
});

it("returns empty strings when headerMessageId is falsy", () => {
const result = buildThreadingHeaders({
headerMessageId: undefined as unknown as string,
});
expect(result).toEqual({ inReplyTo: "", references: "" });
});

it("uses headerMessageId for both fields when no references provided", () => {
const messageId = "<abc123@example.com>";
const result = buildThreadingHeaders({ headerMessageId: messageId });

expect(result).toEqual({
inReplyTo: messageId,
references: messageId,
});
});

it("appends headerMessageId to existing references (RFC 5322)", () => {
const messageId = "<msg3@example.com>";
const existingRefs = "<msg1@example.com> <msg2@example.com>";

const result = buildThreadingHeaders({
headerMessageId: messageId,
references: existingRefs,
});

expect(result).toEqual({
inReplyTo: messageId,
references: "<msg1@example.com> <msg2@example.com> <msg3@example.com>",
});
});

it("handles references with trailing whitespace", () => {
const messageId = "<msg2@example.com>";
const existingRefs = "<msg1@example.com> "; // trailing spaces

const result = buildThreadingHeaders({
headerMessageId: messageId,
references: existingRefs,
});

// .trim() should clean up the result
expect(result.references).toBe(
"<msg1@example.com> <msg2@example.com>".trim(),
);
});

it("handles empty string references", () => {
const messageId = "<abc@example.com>";
const result = buildThreadingHeaders({
headerMessageId: messageId,
references: "",
});

// Empty string is falsy, so should use headerMessageId only
expect(result).toEqual({
inReplyTo: messageId,
references: messageId,
});
});
});
20 changes: 20 additions & 0 deletions apps/web/utils/email/threading.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/**
* Build RFC 5322 compliant email threading headers.
* References = parent's References + parent's Message-ID
* https://datatracker.ietf.org/doc/html/rfc5322#appendix-A.2
*/
export function buildThreadingHeaders(options: {
headerMessageId: string;
references?: string;
}): { inReplyTo: string; references: string } {
if (!options.headerMessageId) {
return { inReplyTo: "", references: "" };
}

return {
inReplyTo: options.headerMessageId,
references: options.references
? `${options.references} ${options.headerMessageId}`.trim()
: options.headerMessageId,
};
}
1 change: 1 addition & 0 deletions apps/web/utils/email/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ export interface EmailProvider {
threadId: string;
headerMessageId: string;
references?: string;
messageId?: string; // Platform-specific message ID (Graph ID for Outlook)
};
to: string;
cc?: string;
Expand Down
10 changes: 6 additions & 4 deletions apps/web/utils/gmail/mail.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
mergeAndDedupeRecipients,
} from "@/utils/email/reply-all";
import { formatReplySubject } from "@/utils/email/subject";
import { buildThreadingHeaders } from "@/utils/email/threading";
import { ensureEmailSendingEnabled } from "@/utils/mail";

const logger = createScopedLogger("gmail/mail");
Expand All @@ -31,6 +32,7 @@ export const sendEmailBody = z.object({
threadId: z.string(),
headerMessageId: z.string(), // this is different to the gmail message id and looks something like <123...abc@mail.example.com>
references: z.string().optional(), // for threading
messageId: z.string().optional(), // platform-specific message ID (Graph ID for Outlook)
})
.optional(),
to: z.string(),
Expand Down Expand Up @@ -91,10 +93,10 @@ const createRawMailMessage = async (
],
attachments,
// https://datatracker.ietf.org/doc/html/rfc2822#appendix-A.2
references: replyToEmail
? `${replyToEmail.references || ""} ${replyToEmail.headerMessageId}`.trim()
: "",
inReplyTo: replyToEmail ? replyToEmail.headerMessageId : "",
...buildThreadingHeaders({
headerMessageId: replyToEmail?.headerMessageId || "",
references: replyToEmail?.references,
}),
headers: {
"X-Mailer": "Inbox Zero Web",
},
Expand Down
132 changes: 95 additions & 37 deletions apps/web/utils/outlook/mail.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import {
buildReplyAllRecipients,
mergeAndDedupeRecipients,
} from "@/utils/email/reply-all";
import { formatReplySubject } from "@/utils/email/subject";
import { withOutlookRetry } from "@/utils/outlook/retry";
import { extractEmailAddress, extractNameFromEmail } from "@/utils/email";
import { ensureEmailSendingEnabled } from "@/utils/mail";
Expand All @@ -26,8 +25,6 @@ interface OutlookMessageRequest {
ccRecipients?: { emailAddress: { address: string } }[];
bccRecipients?: { emailAddress: { address: string } }[];
replyTo?: { emailAddress: { address: string } }[];
conversationId?: string;
isDraft?: boolean;
}

type SentEmailResult = Pick<Message, "id" | "conversationId">;
Expand All @@ -39,6 +36,13 @@ export async function sendEmailWithHtml(
): Promise<SentEmailResult> {
ensureEmailSendingEnabled();

// For replies with a message ID, use createReply for proper threading
// Microsoft Graph's sendMail doesn't support In-Reply-To/References headers
if (body.replyToEmail?.messageId) {
return sendReplyUsingCreateReply(client, body, logger);
}

// For new emails (no reply context), use sendMail
const message: OutlookMessageRequest = {
subject: body.subject,
body: {
Expand All @@ -57,10 +61,6 @@ export async function sendEmailWithHtml(
: {}),
};

if (body.replyToEmail?.threadId) {
message.conversationId = body.replyToEmail.threadId;
}

await withOutlookRetry(
() =>
client.getClient().api("/me/sendMail").post({
Expand All @@ -70,13 +70,10 @@ export async function sendEmailWithHtml(
logger,
);

// /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.
// sendMail returns 202 with no body, so we can't get the sent message ID
return {
id: "",
conversationId: message.conversationId,
conversationId: body.replyToEmail?.threadId,
};
Comment on lines +73 to 77
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Jan 11, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# First, let's look at the file structure and understand the context
cd apps/web || exit 1

# Find the sendEmailWithHtml function to understand the context better
echo "=== Content of utils/outlook/mail.ts around lines 73-77 ==="
sed -n '60,85p' utils/outlook/mail.ts

# Find all calls to sendEmailWithHtml
echo -e "\n=== Callers of sendEmailWithHtml ==="
rg -n 'sendEmailWithHtml' --type ts -g '!node_modules'

# Find imports of this function
echo -e "\n=== Imports of sendEmailWithHtml ==="
rg -n "from.*outlook/mail|sendEmailWithHtml" --type ts -g '!node_modules' | grep -v '.test'

Repository: elie222/inbox-zero

Length of output: 3265


🏁 Script executed:

#!/bin/bash
# Now let's examine the specific usage of the returned ID and conversationId
cd apps/web || exit 1

# Look for patterns where the returned object properties are accessed
echo "=== Usage patterns of sendEmailWithHtml result ==="
rg -B5 -A10 'sendEmailWithHtml' --type ts -g '!node_modules' -g '!**\.test\.ts' | head -100

Repository: elie222/inbox-zero

Length of output: 5015


🏁 Script executed:

#!/bin/bash
cd apps/web || exit 1

# Check how the result is used in microsoft.ts
echo "=== Usage in utils/email/microsoft.ts (line 566) ==="
sed -n '560,580p' utils/email/microsoft.ts

# Check how the result is used in actions/mail.ts
echo -e "\n=== Usage in utils/actions/mail.ts (line 234) ==="
sed -n '225,245p' utils/actions/mail.ts

# Check meeting-briefs usage
echo -e "\n=== Usage in utils/meeting-briefs/send-briefing.ts ==="
sed -n '105,125p' utils/meeting-briefs/send-briefing.ts

# Check the SentEmailResult type and where it's used
echo -e "\n=== SentEmailResult type definition ==="
rg -n "SentEmailResult|type.*Result" utils/outlook/mail.ts -B2 -A2

Repository: elie222/inbox-zero

Length of output: 2117


🏁 Script executed:

#!/bin/bash
cd apps/web || exit 1

# Check what happens with the result variable in actions/mail.ts
echo "=== Full context of result usage in actions/mail.ts ==="
sed -n '230,260p' utils/actions/mail.ts

# Check if there are any tests that verify the return value
echo -e "\n=== Tests for sendEmailWithHtml ==="
rg -n "sendEmailWithHtml" __tests__ --type ts -A5 | head -60

Repository: elie222/inbox-zero

Length of output: 1735


🏁 Script executed:

#!/bin/bash
cd apps/web || exit 1

# Check the full sendEmailWithHtml implementation in microsoft.ts
echo "=== Microsoft provider sendEmailWithHtml method ==="
sed -n '548,575p' utils/email/microsoft.ts

# Check if the result ID is used anywhere in the provider
echo -e "\n=== Check if messageId/id from result is stored or tracked ==="
rg -n "result\.id|messageId|conversationId" utils/email/microsoft.ts -B2 -A2 | head -80

Repository: elie222/inbox-zero

Length of output: 3288


🏁 Script executed:

#!/bin/bash
cd apps/web || exit 1

# Check the return type in email/types.ts to understand what's expected
echo "=== EmailProvider interface return type ==="
sed -n '120,135p' utils/email/types.ts

# Check if messageId/threadId from the action response is stored or used
echo -e "\n=== Check action usage of returned messageId/threadId ==="
rg -n "sendEmailBody|sendEmail" utils/actions/mail.ts -B2 -A15 | grep -A 15 "\.action"

# Check if there are database inserts using the messageId
echo -e "\n=== Database operations with messageId ==="
rg -n "prisma.*messageId|messageId.*prisma" apps/web --type ts -g '!node_modules' | head -20

Repository: elie222/inbox-zero

Length of output: 1424


🏁 Script executed:

#!/bin/bash
cd apps/web || exit 1

# Check Gmail implementation to see what it returns for comparison
echo "=== Gmail sendEmailWithHtml return value ==="
sed -n '108,140p' utils/gmail/mail.ts

# Check how the Gmail implementation handles return values
echo -e "\n=== Gmail sendEmailWithHtml full implementation ==="
sed -n '108,145p' utils/gmail/mail.ts

Repository: elie222/inbox-zero

Length of output: 2104


🏁 Script executed:

#!/bin/bash
cd apps/web || exit 1

# Check the full EmailProvider interface to see return type
echo "=== Full EmailProvider.sendEmailWithHtml signature ==="
sed -n '120,145p' utils/email/types.ts

# Check if messageId from the action is persisted or used
echo -e "\n=== Check if returned messageId is stored in DB ==="
rg -n "sendEmailAction|sendEmail.*action" utils --type ts -A 30 | grep -A 30 "sendEmailAction"

# Check what happens with the action result
echo -e "\n=== Check client-side action usage ==="
rg -n "sendEmailAction" --type ts --type tsx -g '!node_modules' -B2 -A10 | head -80

Repository: elie222/inbox-zero

Length of output: 1909


🏁 Script executed:

#!/bin/bash
cd apps/web || exit 1

# Check the Gmail wrapper in the microsoft provider to see how it handles the result
echo "=== Google provider wrapper for sendEmailWithHtml ==="
sed -n '685,715p' utils/email/google.ts

# Check if there are any tests that validate messageId is not empty
echo -e "\n=== Tests checking messageId values ==="
rg -n "messageId.*empty|messageId.*null|messageId.*undefined" __tests__ --type ts -B2 -A2

Repository: elie222/inbox-zero

Length of output: 831


🏁 Script executed:

#!/bin/bash
cd apps/web || exit 1

# Check if there are any features that actually depend on the messageId from send operation
echo "=== Check features using messageId from sendEmail ==="
rg -n "messageId.*threadId|threadId.*messageId" utils/actions --type ts -B5 -A10

# Check if messageId is used for any persistence or tracking after send
echo -e "\n=== Check if sent messageId is used in persistence ==="
rg -n "result\.messageId|result\.threadId" utils/actions --type ts -B3 -A5

# Check the return type expectation
echo -e "\n=== EmailProvider return type contract ==="
sed -n '125,150p' utils/email/types.ts

Repository: elie222/inbox-zero

Length of output: 2342


🏁 Script executed:

#!/bin/bash
cd apps/web || exit 1

# Verify the exact behavior when Outlook returns empty ID
echo "=== Trace the empty ID through all layers ==="

# Step 1: Outlook function returns empty string
echo "Step 1 - Outlook returns:"
sed -n '73,77p' utils/outlook/mail.ts

# Step 2: Microsoft provider transforms it
echo -e "\nStep 2 - Microsoft provider transforms to:"
sed -n '566,572p' utils/email/microsoft.ts

# Step 3: Action returns it
echo -e "\nStep 3 - Action returns:"
sed -n '234,241p' utils/actions/mail.ts

# Check if empty strings are acceptable in the return type
echo -e "\n=== Are empty messageId/threadId acceptable? ==="
sed -n '125,150p' utils/email/types.ts

Repository: elie222/inbox-zero

Length of output: 1414


🏁 Script executed:

#!/bin/bash
cd apps/web || exit 1

# Check if there's any special handling for empty messageId/threadId in callers
echo "=== Search for empty ID checks in codebase ==="
rg -n "messageId.*==.*\"\"|\"\"|messageId.*===.*\"\"" --type ts --type tsx -g '!node_modules' | head -20

# Check if sendEmailAction is used client-side and how the result is handled
echo -e "\n=== Client-side usage of sendEmailAction ==="
rg -n "sendEmailAction|sendEmail.*action" --type ts --type tsx -g '!node_modules' -g '!.test' -B2 -A8 | head -100

Repository: elie222/inbox-zero

Length of output: 227


🏁 Script executed:

#!/bin/bash
cd apps/web || exit 1

# Check if the reply flow (which uses createReply and gets real IDs) is the intended path
echo "=== Check replyToEmail implementation ==="
sed -n '88,120p' utils/outlook/mail.ts

# Check the sendReplyUsingCreateReply function to see if it returns real IDs
echo -e "\n=== sendReplyUsingCreateReply function ==="
sed -n '343,380p' utils/outlook/mail.ts

Repository: elie222/inbox-zero

Length of output: 2061


🏁 Script executed:

#!/bin/bash
cd apps/web || exit 1

# Check the full sendReplyUsingCreateReply to see how it handles the return
echo "=== Full sendReplyUsingCreateReply implementation ==="
sed -n '340,420p' utils/outlook/mail.ts

# Check if there's any code that actually uses the returned messageId from sendEmail
echo -e "\n=== Check if returned messageId is ever used ==="
rg -n "\.messageId|\.threadId" utils/actions/mail.ts -B3 -A3

# Verify the interface contract one more time to be certain about expectations
echo -e "\n=== Confirm the actual return requirement ==="
grep -A 3 "Promise<{" utils/email/types.ts | grep -A 3 "sendEmailWithHtml"

Repository: elie222/inbox-zero

Length of output: 1712


🏁 Script executed:

#!/bin/bash
cd apps/web || exit 1

# Check if Outlook implementation is aware of and accepting this limitation
echo "=== Comment context in Outlook mail ==="
sed -n '30,40p' utils/outlook/mail.ts

# Verify Gmail also returns valid IDs
echo -e "\n=== Verify Gmail implementation can return real IDs ==="
sed -n '108,145p' utils/gmail/mail.ts | tail -20

Repository: elie222/inbox-zero

Length of output: 1063


Address the interface contract violation for Outlook's empty messageId/threadId returns.

The Outlook sendEmailWithHtml function returns empty string for messageId and threadId because Microsoft Graph's sendMail endpoint returns 202 with no body. This violates the interface contract that specifies both as non-empty strings. While the returned values aren't currently used for persistence, this inconsistency with Gmail's implementation could cause silent failures in future code that depends on these IDs.

Consider either:

  • Making messageId and threadId optional in the return type to reflect Outlook's limitations
  • Or exploring alternative approaches (e.g., polling the sent items folder to retrieve the sent message ID after send completes)

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Addressed - returning empty id (Graph limitation) while preserving conversationId/threadId. The interface allows empty strings which is acceptable for Outlook.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Commit b141df2 addressed this comment by improving the handling of Outlook's empty ID limitation. The code now attempts to preserve conversationId by using body.replyToEmail?.threadId instead of returning undefined, and fixes bugs where invalid draft IDs were being returned after send operations. While the interface contract still allows empty strings (rather than making fields optional as suggested), the implementation is now more correct and better documented with explanatory comments about Graph API limitations.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Skipped: comment is from another GitHub bot.

Comment thread
elie222 marked this conversation as resolved.
}

Expand All @@ -95,40 +92,48 @@ export async function replyToEmail(
reply: string,
logger: Logger,
) {
ensureEmailSendingEnabled();

const { html } = createOutlookReplyContent({
textContent: reply,
message,
});

// Only replying to the original sender
const replyMessage = {
subject: formatReplySubject(message.headers.subject),
body: {
contentType: "html",
content: html,
},
toRecipients: [
{
emailAddress: {
address: message.headers["reply-to"] || message.headers.from,
},
},
],
conversationId: message.threadId,
};

ensureEmailSendingEnabled();
// Use createReply to create a properly threaded draft
// Microsoft Graph's sendMail doesn't support setting In-Reply-To/References headers
// Only createReply/createReplyAll endpoints ensure proper threading
const replyDraft: Message = await withOutlookRetry(
() =>
client.getClient().api(`/me/messages/${message.id}/createReply`).post({}),
logger,
);

// Send the email immediately using the sendMail endpoint
const result = await withOutlookRetry(
// Update the draft with our content
await withOutlookRetry(
() =>
client.getClient().api("/me/sendMail").post({
message: replyMessage,
saveToSentItems: true,
}),
client
.getClient()
.api(`/me/messages/${replyDraft.id}`)
.patch({
body: {
contentType: "html",
content: html,
},
}),
logger,
);
return result;

// Send the draft
await withOutlookRetry(
() => client.getClient().api(`/me/messages/${replyDraft.id}/send`).post({}),
logger,
);

// Draft ID is no longer valid after /send; Graph doesn't return sent message ID
return {
id: "",
conversationId: replyDraft.conversationId,
};
}

export async function forwardEmail(
Expand Down Expand Up @@ -333,3 +338,56 @@ function convertTextToHtmlParagraphs(text?: string | null): string {

return `<html><body>${htmlContent}</body></html>`;
}

async function sendReplyUsingCreateReply(
client: OutlookClient,
body: SendEmailBody,
logger: Logger,
): Promise<SentEmailResult> {
const originalMessageId = body.replyToEmail!.messageId!;

// Use createReply to create a properly threaded draft
const replyDraft: Message = await withOutlookRetry(
() =>
client
.getClient()
.api(`/me/messages/${originalMessageId}/createReply`)
.post({}),
logger,
);

// Update the draft with our content and recipients
await withOutlookRetry(
() =>
client
.getClient()
.api(`/me/messages/${replyDraft.id}`)
.patch({
subject: body.subject,
body: {
contentType: "html",
content: body.messageHtml,
},
toRecipients: [{ emailAddress: { address: body.to } }],
...(body.cc
? { ccRecipients: [{ emailAddress: { address: body.cc } }] }
: {}),
...(body.bcc
? { bccRecipients: [{ emailAddress: { address: body.bcc } }] }
: {}),
}),
logger,
);

// Send the draft
await withOutlookRetry(
() => client.getClient().api(`/me/messages/${replyDraft.id}/send`).post({}),
logger,
);

// Draft ID is no longer valid after /send; Graph doesn't return sent message ID
return {
id: "",
conversationId: replyDraft.conversationId,
};
}
Loading