Conversation
Co-authored-by: eliesteinbock <eliesteinbock@gmail.com>
|
Cursor Agent can help with this pull request. Just |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
|
|
Caution Review failedThe pull request is closed. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a reply‑all utility and tests, and threads it through Gmail and Outlook draft flows and provider interfaces so draft creation computes To/CC using buildReplyAllRecipients and formatCcList; draftEmail signatures and call sites now accept a userEmail parameter. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as Composer Trigger
participant Mail as Draft Flow
participant Util as buildReplyAllRecipients
participant Format as formatCcList
participant Provider as Email Provider
UI->>Mail: draftEmail(originalEmail, args, userEmail)
Mail->>Util: buildReplyAllRecipients(originalEmail.headers, args.to, userEmail)
Note over Util: Determine To (override → Reply‑To → From)\nBuild CC = trimmed, deduped union of original To & CC\nExclude primary recipient and current user
Util-->>Mail: { to, cc[] }
Mail->>Format: formatCcList(cc[])
Format-->>Mail: ccString | undefined
Mail->>Provider: client.draftEmail(..., { to, cc: ccString }, userEmail)
Provider-->>Mail: { draftId }
Mail-->>UI: draft created
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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. Comment |
Co-authored-by: eliesteinbock <eliesteinbock@gmail.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
apps/web/utils/outlook/mail.ts (1)
199-207: Draft To uses reply‑all primary; same normalization caveatMirror the
toRecipientsnormalization in drafts as well.- address: recipients.to, + address: recipients.to.trim(),
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/web/utils/email/reply-all.test.ts(1 hunks)apps/web/utils/email/reply-all.ts(1 hunks)apps/web/utils/gmail/mail.ts(3 hunks)apps/web/utils/outlook/mail.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (12)
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/web/CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TypeScript with strict null checks
Path aliases: Use@/for imports from project root
Use proper error handling with try/catch blocks
Format code with Prettier
Leverage TypeScript inference for better DX
Files:
apps/web/utils/email/reply-all.tsapps/web/utils/email/reply-all.test.tsapps/web/utils/gmail/mail.tsapps/web/utils/outlook/mail.ts
!{.cursor/rules/*.mdc}
📄 CodeRabbit inference engine (.cursor/rules/cursor-rules.mdc)
Never place rule files in the project root, in subdirectories outside .cursor/rules, or in any other location
Files:
apps/web/utils/email/reply-all.tsapps/web/utils/email/reply-all.test.tsapps/web/utils/gmail/mail.tsapps/web/utils/outlook/mail.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/form-handling.mdc)
**/*.ts: The same validation should be done in the server action too
Define validation schemas using Zod
Files:
apps/web/utils/email/reply-all.tsapps/web/utils/email/reply-all.test.tsapps/web/utils/gmail/mail.tsapps/web/utils/outlook/mail.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/logging.mdc)
**/*.{ts,tsx}: UsecreateScopedLoggerfor logging in backend TypeScript files
Typically add the logger initialization at the top of the file when usingcreateScopedLogger
Only use.with()on a logger instance within a specific function, not for a global loggerImport Prisma in the project using
import prisma from "@/utils/prisma";
**/*.{ts,tsx}: Don't use TypeScript enums.
Don't use TypeScript const enum.
Don't use the TypeScript directive @ts-ignore.
Don't use primitive type aliases or misleading types.
Don't use empty type parameters in type aliases and interfaces.
Don't use any or unknown as type constraints.
Don't use implicit any type on variable declarations.
Don't let variables evolve into any type through reassignments.
Don't use non-null assertions with the ! postfix operator.
Don't misuse the non-null assertion operator (!) in TypeScript files.
Don't use user-defined types.
Use as const instead of literal types and type annotations.
Use export type for types.
Use import type for types.
Don't declare empty interfaces.
Don't merge interfaces and classes unsafely.
Don't use overload signatures that aren't next to each other.
Use the namespace keyword instead of the module keyword to declare TypeScript namespaces.
Don't use TypeScript namespaces.
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 parameter properties in class constructors.
Use either T[] or Array consistently.
Initialize each enum member value explicitly.
Make sure all enum members are literal values.
Files:
apps/web/utils/email/reply-all.tsapps/web/utils/email/reply-all.test.tsapps/web/utils/gmail/mail.tsapps/web/utils/outlook/mail.ts
apps/web/utils/**
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Create utility functions in
utils/folder for reusable logic
Files:
apps/web/utils/email/reply-all.tsapps/web/utils/email/reply-all.test.tsapps/web/utils/gmail/mail.tsapps/web/utils/outlook/mail.ts
apps/web/utils/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
apps/web/utils/**/*.ts: Use lodash utilities for common operations (arrays, objects, strings)
Import specific lodash functions to minimize bundle size
Files:
apps/web/utils/email/reply-all.tsapps/web/utils/email/reply-all.test.tsapps/web/utils/gmail/mail.tsapps/web/utils/outlook/mail.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{js,jsx,ts,tsx}: Don't useelements in Next.js projects.
Don't use elements in Next.js projects.
Don't use namespace imports.
Don't access namespace imports dynamically.
Don't use global eval().
Don't use console.
Don't use debugger.
Don't use var.
Don't use with statements in non-strict contexts.
Don't use the arguments object.
Don't use consecutive spaces in regular expression literals.
Don't use the comma operator.
Don't use unnecessary boolean casts.
Don't use unnecessary callbacks with flatMap.
Use for...of statements instead of Array.forEach.
Don't create classes that only have static members (like a static namespace).
Don't use this and super in static contexts.
Don't use unnecessary catch clauses.
Don't use unnecessary constructors.
Don't use unnecessary continue statements.
Don't export empty modules that don't change anything.
Don't use unnecessary escape sequences in regular expression literals.
Don't use unnecessary labels.
Don't use unnecessary nested block statements.
Don't rename imports, exports, and destructured assignments to the same name.
Don't use unnecessary string or template literal concatenation.
Don't use String.raw in template literals when there are no escape sequences.
Don't use useless case statements in switch statements.
Don't use ternary operators when simpler alternatives exist.
Don't use useless this aliasing.
Don't initialize variables to undefined.
Don't use the void operators (they're not familiar).
Use arrow functions instead of function expressions.
Use Date.now() to get milliseconds since the Unix Epoch.
Use .flatMap() instead of map().flat() when possible.
Use literal property access instead of computed property access.
Don't use parseInt() or Number.parseInt() when binary, octal, or hexadecimal literals work.
Use concise optional chaining instead of chained logical expressions.
Use regular expression literals instead of the RegExp constructor when possible.
Don't use number literal object member names th...
Files:
apps/web/utils/email/reply-all.tsapps/web/utils/email/reply-all.test.tsapps/web/utils/gmail/mail.tsapps/web/utils/outlook/mail.ts
!pages/_document.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
!pages/_document.{js,jsx,ts,tsx}: Don't import next/document outside of pages/_document.jsx in Next.js projects.
Don't import next/document outside of pages/_document.jsx in Next.js projects.
Files:
apps/web/utils/email/reply-all.tsapps/web/utils/email/reply-all.test.tsapps/web/utils/gmail/mail.tsapps/web/utils/outlook/mail.ts
**/*.test.{ts,js}
📄 CodeRabbit inference engine (.cursor/rules/security.mdc)
Include security tests in your test suites to verify authentication, authorization, and error handling.
Files:
apps/web/utils/email/reply-all.test.ts
**/*.test.{ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/testing.mdc)
**/*.test.{ts,js,tsx,jsx}: Tests are colocated next to the tested file (e.g.,dir/format.tsanddir/format.test.ts)
Usevi.mock("server-only", () => ({}));to mock theserver-onlymodule in tests
Mock@/utils/prismain tests usingvi.mock("@/utils/prisma")and use the provided prisma mock
Mock external dependencies in tests
Clean up mocks between tests
Do not mock the Logger
Files:
apps/web/utils/email/reply-all.test.ts
**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{test,spec}.{js,jsx,ts,tsx}: Don't use export or module.exports in test files.
Don't use focused tests.
Don't use disabled tests.
Make sure the assertion function, like expect, is placed inside an it() function call.
Don't nest describe() blocks too deeply in test files.
Don't use focused tests.
Don't use disabled tests.
Don't use export or module.exports in test files.
Files:
apps/web/utils/email/reply-all.test.ts
apps/web/utils/gmail/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/gmail-api.mdc)
Keep provider-specific implementation details isolated in the appropriate utils subfolder (e.g., 'apps/web/utils/gmail/')
Files:
apps/web/utils/gmail/mail.ts
🧠 Learnings (1)
📚 Learning: 2025-08-31T12:17:19.419Z
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/llm-test.mdc:0-0
Timestamp: 2025-08-31T12:17:19.419Z
Learning: Applies to apps/web/__tests__/**/*.test.ts : Include standard cases: happy path, error handling, edge cases (empty/null), different user configs, and various input formats
Applied to files:
apps/web/utils/email/reply-all.test.ts
🧬 Code graph analysis (4)
apps/web/utils/email/reply-all.ts (1)
apps/web/utils/types.ts (1)
ParsedMessageHeaders(102-114)
apps/web/utils/email/reply-all.test.ts (2)
apps/web/utils/types.ts (1)
ParsedMessageHeaders(102-114)apps/web/utils/email/reply-all.ts (2)
buildReplyAllRecipients(16-50)formatCcList(56-58)
apps/web/utils/gmail/mail.ts (1)
apps/web/utils/email/reply-all.ts (2)
buildReplyAllRecipients(16-50)formatCcList(56-58)
apps/web/utils/outlook/mail.ts (1)
apps/web/utils/email/reply-all.ts (1)
buildReplyAllRecipients(16-50)
⏰ 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). (1)
- GitHub Check: cubic · AI code reviewer
🔇 Additional comments (7)
apps/web/utils/gmail/mail.ts (2)
20-20: Good abstraction: shared reply‑all utility importCentralizing recipient logic via
@/utils/email/reply-allis the right direction.
149-156: Ensure reply‑all returns bare emails and removes self from CC
- apps/web/utils/email.ts exports extractEmailAddress (normalizes "Name a@b.com"); confirm buildReplyAllRecipients calls it (or otherwise strips display names) so Gmail and Outlook paths get bare addresses before creating the draft.
- Exclude the current user's address from recipients.cc (use session/account email) so the user isn't self‑CC'd.
apps/web/utils/outlook/mail.ts (2)
9-9: Good reuse of shared reply‑all utilityConsistent with the Gmail path.
185-192: Drafts CC conversion mirrors reply; looks goodConversion and conditional inclusion of
ccRecipientsare correct.apps/web/utils/email/reply-all.test.ts (1)
160-176: formatCcList tests LGTMBehavior matches the Gmail composer expectations (undefined when empty).
apps/web/utils/email/reply-all.ts (2)
56-58: LGTMReturning
undefinedfor empty CC aligns with callers.
21-44: Normalize addresses for dedupe — reuse repo parseEmail helperUse the existing parseEmail helper (used in apps/web/components/EmailCell.tsx) to extract the bare email, then lowercase that value before computing replyTo and populating ccSet in apps/web/utils/email/reply-all.ts to avoid duplicates and strip display names; if parseEmail doesn't return a plain address string, call its address/value field and .toLowerCase(). Consider an RFC parser (e.g. email-addresses) only if parseEmail is insufficient.
| describe("buildReplyAllRecipients", () => { | ||
| it("should handle simple reply-all with TO and CC", () => { | ||
| const headers: ParsedMessageHeaders = { | ||
| from: "sender@example.com", | ||
| to: "user@company.com, colleague@company.com", | ||
| cc: "manager@company.com", | ||
| subject: "Test", | ||
| date: "2024-01-01", | ||
| }; | ||
|
|
||
| const result = buildReplyAllRecipients(headers); | ||
|
|
||
| expect(result.to).toBe("sender@example.com"); | ||
| expect(result.cc).toContain("manager@company.com"); | ||
| expect(result.cc).toContain("user@company.com"); | ||
| expect(result.cc).toContain("colleague@company.com"); | ||
| expect(result.cc).toHaveLength(3); | ||
| }); | ||
|
|
||
| it("should use reply-to header when available", () => { | ||
| const headers: ParsedMessageHeaders = { | ||
| from: "sender@example.com", | ||
| "reply-to": "noreply@example.com", | ||
| to: "user@company.com", | ||
| subject: "Test", | ||
| date: "2024-01-01", | ||
| }; | ||
|
|
||
| const result = buildReplyAllRecipients(headers); | ||
|
|
||
| expect(result.to).toBe("noreply@example.com"); | ||
| expect(result.cc).toContain("user@company.com"); | ||
| expect(result.cc).not.toContain("noreply@example.com"); | ||
| }); | ||
|
|
||
| it("should handle no CC recipients", () => { | ||
| const headers: ParsedMessageHeaders = { | ||
| from: "sender@example.com", | ||
| to: "user@company.com, colleague@company.com", | ||
| subject: "Test", | ||
| date: "2024-01-01", | ||
| }; | ||
|
|
||
| const result = buildReplyAllRecipients(headers); | ||
|
|
||
| expect(result.to).toBe("sender@example.com"); | ||
| expect(result.cc).toContain("user@company.com"); | ||
| expect(result.cc).toContain("colleague@company.com"); | ||
| expect(result.cc).toHaveLength(2); | ||
| }); | ||
|
|
||
| it("should handle single recipient (no CC needed)", () => { | ||
| const headers: ParsedMessageHeaders = { | ||
| from: "sender@example.com", | ||
| to: "sender@example.com", | ||
| subject: "Test", | ||
| date: "2024-01-01", | ||
| }; | ||
|
|
||
| const result = buildReplyAllRecipients(headers); | ||
|
|
||
| expect(result.to).toBe("sender@example.com"); | ||
| expect(result.cc).toHaveLength(0); | ||
| }); | ||
|
|
||
| it("should remove duplicates from CC list", () => { | ||
| const headers: ParsedMessageHeaders = { | ||
| from: "sender@example.com", | ||
| to: "user@company.com, colleague@company.com", | ||
| cc: "colleague@company.com, manager@company.com", | ||
| subject: "Test", | ||
| date: "2024-01-01", | ||
| }; | ||
|
|
||
| const result = buildReplyAllRecipients(headers); | ||
|
|
||
| expect(result.to).toBe("sender@example.com"); | ||
| expect(result.cc).toContain("colleague@company.com"); | ||
| expect(result.cc).toContain("manager@company.com"); | ||
| expect(result.cc).toContain("user@company.com"); | ||
| expect(result.cc).toHaveLength(3); | ||
| }); | ||
|
|
||
| it("should handle override TO parameter", () => { | ||
| const headers: ParsedMessageHeaders = { | ||
| from: "sender@example.com", | ||
| to: "user@company.com", | ||
| cc: "manager@company.com", | ||
| subject: "Test", | ||
| date: "2024-01-01", | ||
| }; | ||
|
|
||
| const result = buildReplyAllRecipients(headers, "override@example.com"); | ||
|
|
||
| expect(result.to).toBe("override@example.com"); | ||
| expect(result.cc).toContain("user@company.com"); | ||
| expect(result.cc).toContain("manager@company.com"); | ||
| expect(result.cc).not.toContain("override@example.com"); | ||
| }); | ||
|
|
||
| it("should handle addresses with extra spaces", () => { | ||
| const headers: ParsedMessageHeaders = { | ||
| from: "sender@example.com", | ||
| to: " user@company.com , colleague@company.com ", | ||
| cc: " manager@company.com ", | ||
| subject: "Test", | ||
| date: "2024-01-01", | ||
| }; | ||
|
|
||
| const result = buildReplyAllRecipients(headers); | ||
|
|
||
| expect(result.to).toBe("sender@example.com"); | ||
| expect(result.cc).toContain("user@company.com"); | ||
| expect(result.cc).toContain("colleague@company.com"); | ||
| expect(result.cc).toContain("manager@company.com"); | ||
| expect(result.cc).toHaveLength(3); | ||
| }); | ||
|
|
||
| it("should filter out empty addresses", () => { | ||
| const headers: ParsedMessageHeaders = { | ||
| from: "sender@example.com", | ||
| to: "user@company.com, , colleague@company.com", | ||
| cc: ", manager@company.com, ", | ||
| subject: "Test", | ||
| date: "2024-01-01", | ||
| }; | ||
|
|
||
| const result = buildReplyAllRecipients(headers); | ||
|
|
||
| expect(result.to).toBe("sender@example.com"); | ||
| expect(result.cc).toContain("user@company.com"); | ||
| expect(result.cc).toContain("colleague@company.com"); | ||
| expect(result.cc).toContain("manager@company.com"); | ||
| expect(result.cc).toHaveLength(3); | ||
| }); | ||
|
|
||
| it("should exclude the reply-to address from CC", () => { | ||
| const headers: ParsedMessageHeaders = { | ||
| from: "sender@example.com", | ||
| to: "sender@example.com, user@company.com", | ||
| cc: "manager@company.com", | ||
| subject: "Test", | ||
| date: "2024-01-01", | ||
| }; | ||
|
|
||
| const result = buildReplyAllRecipients(headers); | ||
|
|
||
| expect(result.to).toBe("sender@example.com"); | ||
| expect(result.cc).not.toContain("sender@example.com"); | ||
| expect(result.cc).toContain("user@company.com"); | ||
| expect(result.cc).toContain("manager@company.com"); | ||
| expect(result.cc).toHaveLength(2); | ||
| }); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Solid coverage of reply‑all scenarios
Good assertions on dedupe, trimming, override, and excluding the primary recipient. Add cases for:
- Display name forms:
"User <user@x.com>". - Case-insensitive dedupe:
"USER@x.com"vs"user@x.com". - Multiple Reply‑To addresses (edge RFC case).
🏁 Script executed:
#!/bin/bash
# If you add normalization, add tests guarding it
rg -n 'describe\\("buildReplyAllRecipients"' -nLength of output: 97
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== rg buildReplyAllRecipients =="
rg -n --hidden -S "buildReplyAllRecipients" || true
echo
echo "== rg for reply-all.test.ts =="
rg -n --hidden -S "reply-all.test.ts" || true
echo
TEST_FILE="apps/web/utils/email/reply-all.test.ts"
if [[ -f "$TEST_FILE" ]]; then
echo "== Printing $TEST_FILE =="
sed -n '1,240p' "$TEST_FILE"
else
echo "== $TEST_FILE not found =="
fi
echo
echo "== Search for display-name style addresses (e.g., Name <a@b.com>) =="
rg -n --hidden -S "<[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+>" || true
echo
echo "== Search for 'reply-to' occurrences =="
rg -n --hidden -S "reply-to|replyTo|Reply-To|ReplyTo" || trueLength of output: 16310
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== reply-all.ts =="
sed -n '1,240p' apps/web/utils/email/reply-all.ts || true
echo
echo "== email.ts (helpers) =="
sed -n '1,320p' apps/web/utils/email.ts || true
echo
echo "== email.test.ts (helpers tests) =="
sed -n '1,320p' apps/web/utils/email.test.ts || true
echo
echo "== types.ts (headers) =="
sed -n '1,240p' apps/web/utils/types.ts || true
echo
echo "== Search for extractEmailAddress usages =="
rg -n --hidden -S "extractEmailAddress|extractNameFromEmail|buildReplyAllRecipients" || trueLength of output: 29571
Parse & normalize addresses in buildReplyAllRecipients; add tests for display-name, case-insensitive dedupe, and multi Reply‑To
buildReplyAllRecipients currently splits/trims raw header tokens and compares them verbatim — it doesn't parse "Name a@b.com", normalize case/dots, or handle a Reply‑To mailbox list.
- Tests to add (apps/web/utils/email/reply-all.test.ts): a display-name address (e.g., "User user@x.com") in to/cc; case-insensitive duplicate addresses ("USER@x.com" vs "user@x.com"); and a multi-address Reply‑To (e.g., "a@x.com, b@x.com") asserting expected to/cc behavior.
- Code changes (apps/web/utils/email/reply-all.ts): parse addresses with extractEmailAddress and compare using normalizeEmailAddress (helpers in apps/web/utils/email.ts); normalize overrideTo as well. If Reply‑To is a mailbox-list, pick the first valid mailbox for the reply target and exclude all reply‑to mailboxes from CC.
- Verify downstream callers (gmail/outlook mail and formatCcList) still receive the expected shapes.
🤖 Prompt for AI Agents
In apps/web/utils/email/reply-all.test.ts around lines 5 to 157, the tests and
implementation assume raw token splitting but the code doesn't
parse/display-name addresses, perform case-insensitive dedupe, or treat Reply‑To
as a mailbox-list; update tests to include: a display-name address like "User
<user@x.com>" in to/cc, case-variant duplicates ("USER@x.com" vs "user@x.com"),
and a multi-address Reply‑To ("a@x.com, b@x.com") asserting that the first valid
Reply‑To becomes the to target and all reply‑to mailboxes are excluded from CC;
then in apps/web/utils/email/reply-all.ts parse addresses using
extractEmailAddress, normalize all addresses with normalizeEmailAddress
(including overrideTo), treat "reply-to" as a mailbox-list and pick the first
valid mailbox for the reply target while excluding every reply-to mailbox from
the final CC, and ensure downstream callers (gmail/outlook mail and
formatCcList) still receive the same shapes.
apps/web/utils/outlook/mail.ts
Outdated
| // Use reply-all logic to build recipients | ||
| const recipients = buildReplyAllRecipients(message.headers); | ||
|
|
||
| // Convert CC addresses to Outlook format | ||
| const ccRecipients = recipients.cc.map((addr) => ({ | ||
| emailAddress: { address: addr }, | ||
| })); | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
CC mapping is fine; ensure values are bare emails
recipients.cc.map(addr => ({ emailAddress: { address: addr } })) assumes addr is an email only. Outlook Graph rejects display‑name formats here. Please ensure upstream normalization or sanitize here.
- const ccRecipients = recipients.cc.map((addr) => ({
- emailAddress: { address: addr },
- }));
+ const toOutlook = (addr: string) => addr.trim(); // assumes already bare email
+ const ccRecipients = recipients.cc.map((addr) => ({
+ emailAddress: { address: toOutlook(addr) },
+ }));🏁 Script executed:
#!/bin/bash
# Spot any headers storing display names that could flow here
rg -nC2 -e 'headers\.(to|cc|from|reply-to).*<' --type=tsLength of output: 618
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== outlook/mail.ts (first 240 lines) =="
if [ -f apps/web/utils/outlook/mail.ts ]; then
nl -ba apps/web/utils/outlook/mail.ts | sed -n '1,240p'
else
echo "MISSING: apps/web/utils/outlook/mail.ts"
fi
echo
echo "== locate buildReplyAllRecipients occurrences =="
rg -n --hidden -S "buildReplyAllRecipients" || true
echo
echo "== show files that mention buildReplyAllRecipients (first 240 lines) =="
for f in $(rg -n --hidden -S "buildReplyAllRecipients" --files); do
echo "----- $f -----"
sed -n '1,240p' "$f" || true
done
echo
echo "== search for display-name style uses of headers (angle-bracket) =="
rg -n --hidden -S "headers\.(to|cc|from|reply-to).*<" || true
echo
echo "== search for direct usages/assignments of message.headers.to|cc|from|reply-to =="
rg -n --hidden -S "message\.headers\.(to|cc|from|reply-to)" || true
echo
echo "== find formatFromEmailWithName / formatToEmailWithName definitions/usages =="
rg -n --hidden -S "formatFromEmailWithName|formatToEmailWithName" -C3 || true
echo
echo "== search for helpers that parse/strip display names (parseAddress|extractEmail|emailOnly|stripDisplayName) =="
rg -n --hidden -S "parseAddress|extractEmail|emailOnly|stripDisplayName|onlyEmail|addressOnly" -C3 || true
echo
echo "== search for places mapping recipients.cc or .to =="
rg -n --hidden -S "recipients\.(cc|to)\b|\.cc\.map\(|\.to\.map\(" -C3 || trueLength of output: 80
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== apps/web/utils/outlook/mail.ts (first 240 lines) =="
if [ -f apps/web/utils/outlook/mail.ts ]; then
sed -n '1,240p' apps/web/utils/outlook/mail.ts || true
else
echo "MISSING: apps/web/utils/outlook/mail.ts"
fi
echo
echo "== occurrences of buildReplyAllRecipients =="
rg -n --hidden -S "buildReplyAllRecipients" -C3 || true
echo
echo "== occurrences of recipients.cc/.to mapping =="
rg -n --hidden -S "recipients\.(cc|to)\b|\.cc\.map\(|\.to\.map\(" -C3 || true
echo
echo "== direct usages of message.headers.(to|cc|from|reply-to) =="
rg -n --hidden -S "message\.headers\.(to|cc|from|reply-to)" -C3 || true
echo
echo "== formatFromEmailWithName / formatToEmailWithName uses/definitions =="
rg -n --hidden -S "formatFromEmailWithName|formatToEmailWithName" -C3 || true
echo
echo "== helpers that might parse/strip display names =="
rg -n --hidden -S "extractEmail|parseAddress|onlyEmail|stripDisplayName|addressOnly|emailOnly|parseMailbox" -C3 || true
echo
echo "== headers.*< pattern (display-name with angle brackets) =="
rg -n --hidden -S "headers\.(to|cc|from|reply-to).*<" -C3 || trueLength of output: 102752
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== apps/web/utils/email/reply-all.ts =="
if [ -f apps/web/utils/email/reply-all.ts ]; then
sed -n '1,240p' apps/web/utils/email/reply-all.ts || true
else
echo "MISSING: apps/web/utils/email/reply-all.ts"
fi
echo
echo "== apps/web/utils/email.ts (extract helpers) =="
if [ -f apps/web/utils/email.ts ]; then
sed -n '1,240p' apps/web/utils/email.ts || true
else
echo "MISSING: apps/web/utils/email.ts"
fi
echo
echo "== apps/web/utils/email/reply-all.test.ts =="
if [ -f apps/web/utils/email/reply-all.test.ts ]; then
sed -n '1,240p' apps/web/utils/email/reply-all.test.ts || true
else
echo "MISSING: apps/web/utils/email/reply-all.test.ts"
fiLength of output: 10989
Sanitize reply-all recipients to bare emails before calling Outlook Graph
buildReplyAllRecipients can return display‑name formats (e.g. "Alice Smith alice@example.com"); Outlook Graph requires bare addresses. Import extractEmailAddress from "@/utils/email" and use extractEmailAddress(addr) || addr.trim() when building toRecipients, ccRecipients, bccRecipients and replyTo in apps/web/utils/outlook/mail.ts (reply flow ~lines 76–96 and draft flow ~lines 186–205).
🤖 Prompt for AI Agents
In apps/web/utils/outlook/mail.ts around lines 76–83 (reply flow, extend to
~76–96) and around lines 186–205 (draft flow), buildReplyAllRecipients can
return display-name formats; Outlook Graph requires bare email addresses. Import
extractEmailAddress from "@/utils/email" and when mapping recipients
(toRecipients, ccRecipients, bccRecipients, replyTo) call
extractEmailAddress(addr) and fall back to addr.trim() if it returns falsy,
using that value for the emailAddress.address fields (or replyTo entries) so all
addresses passed to Graph are sanitized bare emails.
| toRecipients: [ | ||
| { | ||
| emailAddress: { | ||
| address: message.headers["reply-to"] || message.headers.from, | ||
| address: recipients.to, | ||
| }, | ||
| }, | ||
| ], | ||
| ...(ccRecipients.length > 0 ? { ccRecipients } : {}), | ||
| conversationId: message.threadId, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Subject/threading OK; normalize recipients.to for Outlook
Same normalization concern for toRecipients. If upstream may include display names, sanitize before assigning to .address.
- address: recipients.to,
+ address: recipients.to.trim(),🌐 Web query:
Does Microsoft Graph's Message.toRecipients.emailAddress.address accept "Name <email@domain>" or only the raw email address?
💡 Result:
Short answer: Only the raw email address. Use the separate name property for the display name.
Evidence:
- The emailAddress resource documents "address" as "The email address of the person or entity" (string) and has a separate "name" property for the display name. [1]
- Microsoft sendMail/message examples set "emailAddress.address" to an address like "frannis@contoso.com" and put a display name in "emailAddress.name" when needed. [2]
References:
[1] Microsoft Graph — emailAddress resource (address/name).
[2] Microsoft Graph — sendMail / message JSON examples.
Subject/threading OK; split recipients.to into address + name for Outlook
Microsoft Graph requires emailAddress.address to be a raw email; put any display name in emailAddress.name. Parse inputs like "Name email@domain" (or use an RFC‑5322/address parser), set emailAddress.address = parsedEmail and emailAddress.name = parsedName; otherwise fallback to recipients.to.trim().
File: apps/web/utils/outlook/mail.ts — lines 90–98
🤖 Prompt for AI Agents
In apps/web/utils/outlook/mail.ts around lines 90 to 98, the code currently sets
emailAddress.address to recipients.to which may include a display name; instead
parse recipients.to into a raw email and optional display name (e.g., support
formats like "Name <email@domain>" or use an RFC‑5322/address parser library),
then set emailAddress.address = parsedEmail and emailAddress.name = parsedName
(if present); if parsing fails fallback to recipients.to.trim() as the address
and leave name undefined. Ensure ccRecipients and conversationId handling remain
unchanged.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 4 files
Prompt for AI agents (all 4 issues)
Understand the root cause of the following 4 issues and fix them.
<file name="apps/web/utils/email/reply-all.ts">
<violation number="1" location="apps/web/utils/email/reply-all.ts:29">
Splitting address list on a plain comma can break for quoted display names containing commas, producing incorrect CC recipients.</violation>
<violation number="2" location="apps/web/utils/email/reply-all.ts:31">
Excluding the primary recipient via raw string comparison is unreliable. Parse and normalize addresses before comparison so display‑name and case variants are correctly excluded and deduplicated in the CC list.</violation>
</file>
<file name="apps/web/utils/outlook/mail.ts">
<violation number="1" location="apps/web/utils/outlook/mail.ts:81">
Outlook Graph requires emailAddress.address to be a bare email address; passing display‑name formatted values (e.g., "Name <a@b.com>") can cause the request to fail. Sanitize CC entries by extracting the raw email before constructing ccRecipients.</violation>
<violation number="2" location="apps/web/utils/outlook/mail.ts:93">
Normalize the To recipient for Outlook by parsing into raw email and optional display name. Set emailAddress.address to the parsed email and, if present, emailAddress.name to the display name to avoid Graph validation errors.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| // Add original CC recipients if they exist | ||
| if (headers.cc) { | ||
| const originalCcAddresses = headers.cc | ||
| .split(",") |
There was a problem hiding this comment.
Splitting address list on a plain comma can break for quoted display names containing commas, producing incorrect CC recipients.
Prompt for AI agents
Address the following comment on apps/web/utils/email/reply-all.ts at line 29:
<comment>Splitting address list on a plain comma can break for quoted display names containing commas, producing incorrect CC recipients.</comment>
<file context>
@@ -0,0 +1,58 @@
+ // Add original CC recipients if they exist
+ if (headers.cc) {
+ const originalCcAddresses = headers.cc
+ .split(",")
+ .map((addr) => addr.trim())
+ .filter((addr) => addr && addr !== replyTo);
</file context>
apps/web/utils/outlook/mail.ts
Outdated
| { | ||
| emailAddress: { | ||
| address: message.headers["reply-to"] || message.headers.from, | ||
| address: recipients.to, |
There was a problem hiding this comment.
Normalize the To recipient for Outlook by parsing into raw email and optional display name. Set emailAddress.address to the parsed email and, if present, emailAddress.name to the display name to avoid Graph validation errors.
Prompt for AI agents
Address the following comment on apps/web/utils/outlook/mail.ts at line 93:
<comment>Normalize the To recipient for Outlook by parsing into raw email and optional display name. Set emailAddress.address to the parsed email and, if present, emailAddress.name to the display name to avoid Graph validation errors.</comment>
<file context>
@@ -81,10 +90,11 @@ export async function replyToEmail(
{
emailAddress: {
- address: message.headers["reply-to"] || message.headers.from,
+ address: recipients.to,
},
},
</file context>
apps/web/utils/outlook/mail.ts
Outdated
|
|
||
| // Convert CC addresses to Outlook format | ||
| const ccRecipients = recipients.cc.map((addr) => ({ | ||
| emailAddress: { address: addr }, |
There was a problem hiding this comment.
Outlook Graph requires emailAddress.address to be a bare email address; passing display‑name formatted values (e.g., "Name a@b.com") can cause the request to fail. Sanitize CC entries by extracting the raw email before constructing ccRecipients.
Prompt for AI agents
Address the following comment on apps/web/utils/outlook/mail.ts at line 81:
<comment>Outlook Graph requires emailAddress.address to be a bare email address; passing display‑name formatted values (e.g., "Name <a@b.com>") can cause the request to fail. Sanitize CC entries by extracting the raw email before constructing ccRecipients.</comment>
<file context>
@@ -72,6 +73,14 @@ export async function replyToEmail(
+
+ // Convert CC addresses to Outlook format
+ const ccRecipients = recipients.cc.map((addr) => ({
+ emailAddress: { address: addr },
+ }));
+
</file context>
✅ Addressed in fa64e6a
apps/web/utils/email/reply-all.ts
Outdated
| const originalCcAddresses = headers.cc | ||
| .split(",") | ||
| .map((addr) => addr.trim()) | ||
| .filter((addr) => addr && addr !== replyTo); |
There was a problem hiding this comment.
Excluding the primary recipient via raw string comparison is unreliable. Parse and normalize addresses before comparison so display‑name and case variants are correctly excluded and deduplicated in the CC list.
Prompt for AI agents
Address the following comment on apps/web/utils/email/reply-all.ts at line 31:
<comment>Excluding the primary recipient via raw string comparison is unreliable. Parse and normalize addresses before comparison so display‑name and case variants are correctly excluded and deduplicated in the CC list.</comment>
<file context>
@@ -0,0 +1,58 @@
+ const originalCcAddresses = headers.cc
+ .split(",")
+ .map((addr) => addr.trim())
+ .filter((addr) => addr && addr !== replyTo);
+
+ originalCcAddresses.forEach((addr) => ccSet.add(addr));
</file context>
✅ Addressed in a01b857
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/web/utils/gmail/mail.ts (1)
255-258: Reply‑all default for drafts LGTM; no BCC carry‑overUsing buildReplyAllRecipients + formatCcList is correct, and dropping BCC by default matches prior feedback to avoid surprising users.
🧹 Nitpick comments (2)
apps/web/utils/gmail/mail.ts (2)
149-149: Clarify intent: single‑recipient reply hereAvoid confusion now that drafts use reply‑all by default; make the comment explicit.
- // Only replying to the original sender + // Intentionally reply to sender only; draftEmail() applies reply‑all behavior
253-258: Optional: exclude “me” from CC for Gmail‑parityMost clients omit the signed‑in account from CC on reply‑all. Consider filtering it out; keep current behavior if the profile call fails.
const recipients = buildReplyAllRecipients(originalEmail.headers, args.to); + // Exclude the signed‑in account from CC (Gmail UI behavior) + let ccFiltered = recipients.cc; + try { + const { data: profile } = await gmail.users.getProfile({ userId: "me" }); + const myEmail = profile.emailAddress?.toLowerCase(); + if (myEmail) { + ccFiltered = recipients.cc.filter( + (addr) => addr.toLowerCase() !== myEmail, + ); + } + } catch { + // best-effort; proceed without filtering if profile lookup fails + } const raw = await createRawMailMessage({ to: recipients.to, - cc: formatCcList(recipients.cc), + cc: formatCcList(ccFiltered), subject: args.subject || originalEmail.headers.subject,
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/utils/gmail/mail.ts(3 hunks)apps/web/utils/outlook/mail.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/utils/outlook/mail.ts
🧰 Additional context used
📓 Path-based instructions (9)
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/web/CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TypeScript with strict null checks
Path aliases: Use@/for imports from project root
Use proper error handling with try/catch blocks
Format code with Prettier
Leverage TypeScript inference for better DX
Files:
apps/web/utils/gmail/mail.ts
!{.cursor/rules/*.mdc}
📄 CodeRabbit inference engine (.cursor/rules/cursor-rules.mdc)
Never place rule files in the project root, in subdirectories outside .cursor/rules, or in any other location
Files:
apps/web/utils/gmail/mail.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/form-handling.mdc)
**/*.ts: The same validation should be done in the server action too
Define validation schemas using Zod
Files:
apps/web/utils/gmail/mail.ts
apps/web/utils/gmail/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/gmail-api.mdc)
Keep provider-specific implementation details isolated in the appropriate utils subfolder (e.g., 'apps/web/utils/gmail/')
Files:
apps/web/utils/gmail/mail.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/logging.mdc)
**/*.{ts,tsx}: UsecreateScopedLoggerfor logging in backend TypeScript files
Typically add the logger initialization at the top of the file when usingcreateScopedLogger
Only use.with()on a logger instance within a specific function, not for a global loggerImport Prisma in the project using
import prisma from "@/utils/prisma";
**/*.{ts,tsx}: Don't use TypeScript enums.
Don't use TypeScript const enum.
Don't use the TypeScript directive @ts-ignore.
Don't use primitive type aliases or misleading types.
Don't use empty type parameters in type aliases and interfaces.
Don't use any or unknown as type constraints.
Don't use implicit any type on variable declarations.
Don't let variables evolve into any type through reassignments.
Don't use non-null assertions with the ! postfix operator.
Don't misuse the non-null assertion operator (!) in TypeScript files.
Don't use user-defined types.
Use as const instead of literal types and type annotations.
Use export type for types.
Use import type for types.
Don't declare empty interfaces.
Don't merge interfaces and classes unsafely.
Don't use overload signatures that aren't next to each other.
Use the namespace keyword instead of the module keyword to declare TypeScript namespaces.
Don't use TypeScript namespaces.
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 parameter properties in class constructors.
Use either T[] or Array consistently.
Initialize each enum member value explicitly.
Make sure all enum members are literal values.
Files:
apps/web/utils/gmail/mail.ts
apps/web/utils/**
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Create utility functions in
utils/folder for reusable logic
Files:
apps/web/utils/gmail/mail.ts
apps/web/utils/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
apps/web/utils/**/*.ts: Use lodash utilities for common operations (arrays, objects, strings)
Import specific lodash functions to minimize bundle size
Files:
apps/web/utils/gmail/mail.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{js,jsx,ts,tsx}: Don't useelements in Next.js projects.
Don't use elements in Next.js projects.
Don't use namespace imports.
Don't access namespace imports dynamically.
Don't use global eval().
Don't use console.
Don't use debugger.
Don't use var.
Don't use with statements in non-strict contexts.
Don't use the arguments object.
Don't use consecutive spaces in regular expression literals.
Don't use the comma operator.
Don't use unnecessary boolean casts.
Don't use unnecessary callbacks with flatMap.
Use for...of statements instead of Array.forEach.
Don't create classes that only have static members (like a static namespace).
Don't use this and super in static contexts.
Don't use unnecessary catch clauses.
Don't use unnecessary constructors.
Don't use unnecessary continue statements.
Don't export empty modules that don't change anything.
Don't use unnecessary escape sequences in regular expression literals.
Don't use unnecessary labels.
Don't use unnecessary nested block statements.
Don't rename imports, exports, and destructured assignments to the same name.
Don't use unnecessary string or template literal concatenation.
Don't use String.raw in template literals when there are no escape sequences.
Don't use useless case statements in switch statements.
Don't use ternary operators when simpler alternatives exist.
Don't use useless this aliasing.
Don't initialize variables to undefined.
Don't use the void operators (they're not familiar).
Use arrow functions instead of function expressions.
Use Date.now() to get milliseconds since the Unix Epoch.
Use .flatMap() instead of map().flat() when possible.
Use literal property access instead of computed property access.
Don't use parseInt() or Number.parseInt() when binary, octal, or hexadecimal literals work.
Use concise optional chaining instead of chained logical expressions.
Use regular expression literals instead of the RegExp constructor when possible.
Don't use number literal object member names th...
Files:
apps/web/utils/gmail/mail.ts
!pages/_document.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
!pages/_document.{js,jsx,ts,tsx}: Don't import next/document outside of pages/_document.jsx in Next.js projects.
Don't import next/document outside of pages/_document.jsx in Next.js projects.
Files:
apps/web/utils/gmail/mail.ts
🧬 Code graph analysis (1)
apps/web/utils/gmail/mail.ts (1)
apps/web/utils/email/reply-all.ts (2)
buildReplyAllRecipients(16-50)formatCcList(56-58)
🔇 Additional comments (1)
apps/web/utils/gmail/mail.ts (1)
20-20: Good: shared reply‑all helpers via @/ aliasClean import of centralized helpers; aligns with path‑alias and reuse guidelines.
3 participants
Implement default "reply all" behavior for AI draft replies by including original TO and CC recipients in the reply's CC field.
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores