Conversation
Add LLM-based logic to determine if Recoup should reply when only CC'd (not in TO). When Recoup is only in the CC array, uses an LLM call to analyze the email context and decide if a reply is expected or if Recoup is just being kept in the loop. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded@sweetmantech has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 1 minutes and 14 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR introduces CC email handling to the inbound email processing system. It adds logic to determine whether the Recoup assistant should reply when CC'd (but not TO'd), updates the sender lookup to consider CC emails as a fallback, and establishes testing infrastructure with Vitest, GitHub Actions workflows, and unit tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Incoming Email
participant Handler as respondToInboundEmail
participant Validator as validateCcReplyExpected
participant LLM as LLM (generateText)
participant Response as Email Response
Client->>Handler: Email received (to, cc, from, subject, body)
Handler->>Validator: validateCcReplyExpected(original, emailText)
rect rgb(220, 240, 250)
Note over Validator: Check Recoup position
alt Recoup in TO
Validator->>Handler: null (continue normal flow)
else Recoup in CC only
Validator->>LLM: shouldReplyToCcEmail(context)
alt LLM returns true
LLM->>Validator: true
Validator->>Handler: null (continue)
else LLM returns false
LLM->>Validator: false
Validator->>Handler: {response: 200 JSON}
end
else Recoup not in TO/CC
Validator->>Handler: {response: 200 JSON}
end
end
alt Early exit (CC-only, no reply needed)
Handler->>Response: 200 JSON response
else Continue normal flow
Handler->>Response: Generate and send reply
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Fix all issues with AI Agents 🤖
In @lib/emails/inbound/isRecoupOnlyInCC.ts:
- Around line 8-16: The function isRecoupOnlyInCC calls .some() on to and cc
without guarding for null/undefined; update isRecoupOnlyInCC to defensively
handle missing inputs by treating to and cc as empty arrays when undefined/null
(e.g., coerce inputs to [] or add null checks) before calling .some(), keep the
existing recoupDomain logic and return semantics (hasRecoupInCC &&
!hasRecoupInTo) unchanged.
In @lib/emails/inbound/shouldReplyToCcEmail.ts:
- Around line 21-56: The shouldReplyToCcEmail function currently interpolates
the raw email body into the LLM prompt and has no error handling; wrap the
generateText call in a try/catch that logs or captures the error and returns a
safe default (false) on failure, and sanitize/truncate the body before
interpolation (e.g., strip or neutralize lines that look like
instructions/prompts, limit to a safe max length) so prompt-injection is
mitigated; keep references to CcEmailContext, LIGHTWEIGHT_MODEL and generateText
when implementing the changes.
🧹 Nitpick comments (3)
lib/emails/inbound/isRecoupOnlyInCC.ts (1)
9-9: Extract the Recoup domain to a shared constant.The domain
"@mail.recoupable.com"appears in multiple files (isRecoupOnlyInCC.ts,getFromWithName.ts). Consider extracting it to a shared constant inlib/const.tsto follow DRY principles and ensure consistency.🔎 Proposed refactor
In
lib/const.ts, add:export const RECOUP_EMAIL_DOMAIN = "@mail.recoupable.com";Then update this file:
+import { RECOUP_EMAIL_DOMAIN } from "@/lib/const"; + export function isRecoupOnlyInCC(to: string[], cc: string[]): boolean { - const recoupDomain = "@mail.recoupable.com"; - - const hasRecoupInTo = to.some(email => email.toLowerCase().endsWith(recoupDomain)); + const hasRecoupInTo = to.some(email => email.toLowerCase().endsWith(RECOUP_EMAIL_DOMAIN)); - const hasRecoupInCC = cc.some(email => email.toLowerCase().endsWith(recoupDomain)); + const hasRecoupInCC = cc.some(email => email.toLowerCase().endsWith(RECOUP_EMAIL_DOMAIN)); return hasRecoupInCC && !hasRecoupInTo; }lib/emails/inbound/shouldReplyToCcEmail.ts (2)
4-10: Consider moving CcEmailContext interface to a shared types file.The
CcEmailContextinterface is defined locally but represents a reusable email context structure. If this interface might be used in other parts of the codebase (e.g., tests, other email utilities), consider moving it to a shared types file.
54-55: Consider more robust LLM response parsing.The current implementation only checks if the response exactly equals
"true"(after trim and lowercase). While this works for compliant responses, consider handling variations or unexpected formats more gracefully.🔎 Alternative: More flexible parsing
const result = response.text.trim().toLowerCase(); - return result === "true"; + // Handle variations like "true", "yes", or sentences containing affirmative indicators + if (result === "true" || result === "yes" || result.includes("reply is expected")) { + return true; + } + if (result === "false" || result === "no" || result.includes("no reply")) { + return false; + } + // Log unexpected responses for monitoring + console.warn("[shouldReplyToCcEmail] Unexpected LLM response:", result); + return false; // Safe default
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.claude/settings.jsonlib/emails/inbound/getFromWithName.tslib/emails/inbound/isRecoupOnlyInCC.tslib/emails/inbound/respondToInboundEmail.tslib/emails/inbound/shouldReplyToCcEmail.ts
🧰 Additional context used
🧬 Code graph analysis (2)
lib/emails/inbound/shouldReplyToCcEmail.ts (1)
lib/const.ts (1)
LIGHTWEIGHT_MODEL(13-13)
lib/emails/inbound/respondToInboundEmail.ts (3)
lib/emails/inbound/getFromWithName.ts (1)
getFromWithName(9-28)lib/emails/inbound/isRecoupOnlyInCC.ts (1)
isRecoupOnlyInCC(8-16)lib/emails/inbound/shouldReplyToCcEmail.ts (1)
shouldReplyToCcEmail(21-56)
🔇 Additional comments (4)
lib/emails/inbound/getFromWithName.ts (1)
9-22: LGTM! Clean fallback implementation.The addition of the
ccEmailsparameter with fallback logic is well-implemented. The function correctly:
- Maintains the original
toEmailspriority- Falls back to
ccEmailswhen needed- Provides a clear error message covering both arrays
- Uses a default parameter to handle undefined
ccEmailsNote: As mentioned in the review of
isRecoupOnlyInCC.ts, consider extracting"@mail.recoupable.com"to a shared constant inlib/const.tsto ensure consistency across files.lib/emails/inbound/respondToInboundEmail.ts (2)
37-60: Well-structured CC handling logic with good observability.The CC-aware flow is properly integrated:
- Correctly checks if Recoup is only CC'd before deciding on reply
- Uses LLM-based decision making for nuanced scenarios
- Provides clear logging for both paths (skip vs. continue)
- Returns appropriate 200 response when skipping (avoids errors)
- Maintains existing flow when reply is expected
The extraction of
emailTextfromvalidationResultaligns with the enriched summary context.One suggestion: Consider tracking these CC-related decisions in metrics/monitoring to understand how often the "CC'd for visibility only" case occurs, which could inform future optimizations.
29-29: The concern aboutoriginal.ccbeing undefined is unfounded. The Zod schema invalidateInboundEmailEvent.tsdefinesccas a required array field (z.array(z.string())), not optional. TheResendEmailReceivedEventtype guarantees thatccis always present as an array. BothgetFromWithName(which has a default parameterccEmails: string[] = []) andisRecoupOnlyInCCare designed to safely handle arrays, including empty ones. There is no undefined-handling issue here.Likely an incorrect or invalid review comment.
.claude/settings.json (1)
1-5: Consider whether this IDE configuration should be committed to the repository.The
.claude/settings.jsonfile is currently tracked in git (not in.gitignore). Decide whether:
- The
ralph-wiggumplugin is required for all developers, or if it's a personal preference- The file should be added to
.gitignorefor local development only- There's documentation explaining the plugin's purpose and why it's needed
- Add vitest for unit testing - Add test script to package.json - Create GitHub Actions workflow to run tests on PRs - Add unit tests for isRecoupOnlyInCC function (8 tests) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/test.yml (1)
22-28: Consider adding pnpm caching to speed up CI runs.Adding a cache step after pnpm setup would significantly reduce installation time on subsequent runs.
🔎 Suggested enhancement with caching
- name: Setup pnpm uses: pnpm/action-setup@v4 with: version: 9 + - name: Get pnpm store directory + shell: bash + run: | + echo "STORE_PATH=$(pnpm store path --silent)" >> $GITHUB_ENV + + - name: Setup pnpm cache + uses: actions/cache@v4 + with: + path: ${{ env.STORE_PATH }} + key: ${{ runner.os }}-pnpm-store-${{ hashFiles('**/pnpm-lock.yaml') }} + restore-keys: | + ${{ runner.os }}-pnpm-store- + - name: Install dependencies run: pnpm install
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
.github/workflows/test.ymllib/emails/inbound/__tests__/isRecoupOnlyInCC.test.tspackage.json
🧰 Additional context used
🧬 Code graph analysis (1)
lib/emails/inbound/__tests__/isRecoupOnlyInCC.test.ts (1)
lib/emails/inbound/isRecoupOnlyInCC.ts (1)
isRecoupOnlyInCC(8-16)
⏰ 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: Vercel Agent Review
🔇 Additional comments (5)
package.json (2)
14-15: LGTM! Standard vitest test scripts.The test scripts follow vitest conventions correctly:
vitest runfor CI andvitestfor watch mode.
61-62: vitest 3.2.4 is stable and compatible with Node.js 20.No vitest configuration file is required—vitest works with default settings and the test scripts are correctly configured.
lib/emails/inbound/__tests__/isRecoupOnlyInCC.test.ts (2)
1-7: LGTM! Clean test setup.Imports and test constants are well-organized and follow vitest conventions.
9-53: Excellent test coverage!The test suite comprehensively covers all logical branches and edge cases:
- ✅ True positive: Recoup only in CC
- ✅ True negative: Recoup in TO (various combinations)
- ✅ Edge cases: empty arrays, multiple recipients, case-insensitivity
- ✅ Domain variants: different email addresses with same Recoup domain
The tests are well-structured with clear naming and focused assertions.
.github/workflows/test.yml (1)
30-31: No action needed. Tests do not require environment variables or external API configuration—no env var usage was found in the test code.Likely an incorrect or invalid review comment.
Move CC reply validation logic from respondToInboundEmail into its own validateCcReplyExpected module, following the same pattern as validateNewEmailMemory. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Pass original email data object directly instead of destructuring individual properties at the call site. Export ResendEmailData type for reuse. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @lib/emails/inbound/validateCcReplyExpected.ts:
- Around line 32-39: The call to shouldReplyToCcEmail can throw (LLM/network/API
errors); wrap the await shouldReplyToCcEmail(...) call in a try/catch, log the
caught error (using the existing logger like processLogger or console if none
available) and choose a safe default (e.g., set shouldReply = false) in the
catch block so the inbound handler does not propagate an unhandled rejection or
crash.
🧹 Nitpick comments (1)
lib/emails/inbound/validateCcReplyExpected.ts (1)
41-52: Consider structured logging for better observability.The current
console.logstatements work but lack structure for production monitoring. Consider using a structured logger that includes metadata like email identifiers, timestamps, and log levels.Example with a hypothetical logger:
if (!shouldReply) { logger.info("CC validation: no reply expected", { from, isRecoupOnlyCC: true, action: "skip_reply" }); return { response: NextResponse.json( { message: "CC'd for visibility only, no reply sent" }, { status: 200 }, ), }; } logger.info("CC validation: reply expected", { from, isRecoupOnlyCC: true, action: "continue_reply" }); return null;
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/emails/inbound/respondToInboundEmail.tslib/emails/inbound/validateCcReplyExpected.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/emails/inbound/respondToInboundEmail.ts
🧰 Additional context used
🧬 Code graph analysis (1)
lib/emails/inbound/validateCcReplyExpected.ts (2)
lib/emails/inbound/isRecoupOnlyInCC.ts (1)
isRecoupOnlyInCC(8-16)lib/emails/inbound/shouldReplyToCcEmail.ts (1)
shouldReplyToCcEmail(21-56)
⏰ 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: Vercel Agent Review
🔇 Additional comments (2)
lib/emails/inbound/validateCcReplyExpected.ts (2)
1-11: LGTM! Clean interface and imports.The interface clearly defines the required email context parameters, and the imports are appropriate for the CC validation logic.
22-30: The return type pattern is correct and intentional. The caller inrespondToInboundEmail.tsproperly handles it by checking the truthiness ofccValidationand then accessingccValidation.responseto retrieve theNextResponseobject. The wrapping structure allows the function to signal both whether an early return is needed and provide the response in a single return value.
Use Pick<ResendEmailData, ...> instead of duplicating field definitions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Remove isRecoupOnlyInCC helper - inline logic into validateCcReplyExpected - Add comprehensive unit tests for validateCcReplyExpected - Add vitest.config.ts with path aliases and env setup - Simplified logic: - Recoup in TO → always reply (no LLM) - Recoup only in CC → use LLM to determine - Recoup not in TO/CC → no reply 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Simplify validateCcReplyExpected to always call shouldReplyToCcEmail. The LLM prompt now handles all cases: - Recoup in TO → always reply - Recoup only in CC → LLM decides based on context 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
lib/emails/inbound/shouldReplyToCcEmail.ts (1)
16-51: Previous review concerns remain unaddressed.The concerns raised in the earlier review about error handling and prompt injection risks are still applicable:
No error handling: If
generateTextthrows an error, the function will propagate the exception. Consider wrapping in try-catch with a safe default (returnfalseto avoid unsolicited replies).Prompt injection risk: The
bodycontent is directly interpolated into the LLM prompt (line 28). Consider truncating extremely long bodies and sanitizing content that might manipulate the LLM.
🧹 Nitpick comments (1)
lib/emails/inbound/__tests__/validateCcReplyExpected.test.ts (1)
1-12: Consider mock hoisting for clarity.While Vitest automatically hoists
vi.mock()calls, placing them at the very top of the file (before regular imports) makes the hoisting behavior explicit and follows Vitest best practices.🔎 Suggested ordering
+// Mock the generateText function +vi.mock("@/lib/ai/generateText", () => ({ + default: vi.fn(), +})); + import { describe, it, expect, vi, beforeEach } from "vitest"; import { validateCcReplyExpected } from "../validateCcReplyExpected"; import type { ResendEmailData } from "@/lib/emails/validateInboundEmailEvent"; - -// Mock the generateText function -vi.mock("@/lib/ai/generateText", () => ({ - default: vi.fn(), -})); - import generateText from "@/lib/ai/generateText";
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
lib/emails/inbound/__tests__/validateCcReplyExpected.test.tslib/emails/inbound/respondToInboundEmail.tslib/emails/inbound/shouldReplyToCcEmail.tslib/emails/inbound/validateCcReplyExpected.tslib/emails/validateInboundEmailEvent.tsvitest.config.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/emails/inbound/validateCcReplyExpected.ts
🧰 Additional context used
🧬 Code graph analysis (2)
lib/emails/inbound/shouldReplyToCcEmail.ts (2)
lib/emails/validateInboundEmailEvent.ts (1)
ResendEmailData(32-32)lib/const.ts (1)
LIGHTWEIGHT_MODEL(13-13)
lib/emails/inbound/__tests__/validateCcReplyExpected.test.ts (2)
lib/emails/validateInboundEmailEvent.ts (1)
ResendEmailData(32-32)lib/emails/inbound/validateCcReplyExpected.ts (1)
validateCcReplyExpected(18-62)
⏰ 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: Vercel Agent Review
🔇 Additional comments (6)
vitest.config.ts (1)
1-16: Verify that__dirnameworks in this ESM context.Next.js 15 uses ESM by default, and
__dirnameis not available in ES modules without special handling. While Vitest may handle this configuration file as CommonJS, consider using the ESM-compatible approach if issues arise:🔎 ESM-compatible alternative if needed
import { defineConfig } from "vitest/config"; -import path from "path"; +import { fileURLToPath } from "url"; +import { dirname, resolve } from "path"; + +const __filename = fileURLToPath(import.meta.url); +const __dirname = dirname(__filename); export default defineConfig({ test: { globals: true, env: { PRIVY_PROJECT_SECRET: "test-secret", }, }, resolve: { alias: { - "@": path.resolve(__dirname, "./"), + "@": resolve(__dirname, "./"), }, }, });lib/emails/validateInboundEmailEvent.ts (1)
32-32: LGTM! Clean type export for improved type safety.Exporting the
ResendEmailDatatype enables type-safe usage across the CC validation logic and tests without duplicating the schema definition.lib/emails/inbound/respondToInboundEmail.ts (3)
10-10: LGTM! Import added for CC validation.The import is necessary for the new CC validation logic.
28-28: LGTM! CC fallback for sender lookup.Passing
original.ccenables the sender lookup to fall back to CC emails when no@mail.recoupable.comaddress is found in the TO field, correctly supporting CC-only scenarios.
36-42: LGTM! CC validation integrated correctly.The CC validation logic is properly positioned:
- After memory validation (duplicate check)
- Before response generation
The early-return pattern correctly handles cases where Recoup is CC'd for visibility only.
lib/emails/inbound/__tests__/validateCcReplyExpected.test.ts (1)
14-105: LGTM! Comprehensive test coverage.The test suite thoroughly covers all scenarios:
- Recoup in TO (with and without CC) → replies without LLM
- Recoup only in CC → uses LLM to decide
- Recoup not addressed → no reply, no LLM call
Assertions correctly verify both return values and LLM invocation patterns.
Update shouldReplyToCcEmail to use ToolLoopAgent with Output.object() for structured output, consistent with other agents in the codebase. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Update LLM prompt to check body/subject for "don't reply" FIRST before considering TO/CC placement. When in doubt, return false. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Move agent definition from shouldReplyToCcEmail to dedicated lib/agents/EmailReplyAgent module, following existing agent patterns. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
1 participant
Summary by CodeRabbit
Release Notes
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.