Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded@elie222 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 7 minutes and 11 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 (2)
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. WalkthroughThis PR introduces a new NOTIFY_SENDER action type for cold email rules, enabling automated notifications to senders of detected cold emails. Changes span the full stack: schema definitions, database migration, UI components for action configuration and display, action execution handlers, email notification templates, and premium feature gating for bulk operations. Changes
Sequence DiagramsequenceDiagram
participant Rule as Rule Engine
participant Action as Action Handler
participant Notif as Notification Service
participant Resend as Resend API
participant Logger as Logger
Rule->>Action: Execute NOTIFY_SENDER action
Action->>Action: Extract sender email & subject
Action->>Notif: sendColdEmailNotification()
alt Configuration check
Notif->>Logger: Log warning if RESEND_API_KEY missing
Notif-->>Action: Return error
else Key present
Notif->>Notif: Format reply subject & build payload
Notif->>Resend: Send email via Resend API
alt Success
Resend-->>Notif: Return messageId
Notif->>Logger: Log acceptance with messageId
Notif-->>Action: Return { success: true }
else Failure
Resend-->>Notif: Return error
Notif->>Logger: Log error message
Notif-->>Action: Return { success: false, error }
end
end
Action->>Logger: Log result (best-effort, continue on failure)
Action-->>Rule: Complete (non-blocking)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 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 |
Add a premium-gated toggle in BulkRunRules to include read emails in bulk processing and pass the flag to
|
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 23 files
Prompt for AI agents (all 2 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/resend/emails/cold-email-notification.tsx">
<violation number="1" location="packages/resend/emails/cold-email-notification.tsx:34">
P2: Image source is hardcoded while `baseUrl` prop is available for configurable URLs. This inconsistency means the image will always point to production even when `baseUrl` is overridden (e.g., in development/staging environments). Consider using the `baseUrl` prop for consistency.</violation>
</file>
<file name="apps/web/app/(app)/[emailAccountId]/assistant/constants.ts">
<violation number="1" location="apps/web/app/(app)/[emailAccountId]/assistant/constants.ts:66">
P2: The `|| lowerExample.includes("sender")` condition is too broad and will cause false matches. Strings like "Forward to sender" or "Reply to sender" will incorrectly return NOTIFY_SENDER color because "sender" matches first. Consider using only `lowerExample.includes("notify")` or a more specific combined check like `lowerExample.includes("notify sender")`.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
| <Container className="mx-auto w-full max-w-[600px] p-0"> | ||
| <Section className="p-8 text-center"> | ||
| <Link href={baseUrl} className="text-[15px]"> | ||
| <Img |
There was a problem hiding this comment.
P2: Image source is hardcoded while baseUrl prop is available for configurable URLs. This inconsistency means the image will always point to production even when baseUrl is overridden (e.g., in development/staging environments). Consider using the baseUrl prop for consistency.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/resend/emails/cold-email-notification.tsx, line 34:
<comment>Image source is hardcoded while `baseUrl` prop is available for configurable URLs. This inconsistency means the image will always point to production even when `baseUrl` is overridden (e.g., in development/staging environments). Consider using the `baseUrl` prop for consistency.</comment>
<file context>
@@ -0,0 +1,89 @@
+ <Container className="mx-auto w-full max-w-[600px] p-0">
+ <Section className="p-8 text-center">
+ <Link href={baseUrl} className="text-[15px]">
+ <Img
+ src={"https://www.getinboxzero.com/icon.png"}
+ width="40"
</file context>
There was a problem hiding this comment.
P2: The || lowerExample.includes("sender") condition is too broad and will cause false matches. Strings like "Forward to sender" or "Reply to sender" will incorrectly return NOTIFY_SENDER color because "sender" matches first. Consider using only lowerExample.includes("notify") or a more specific combined check like lowerExample.includes("notify sender").
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/app/(app)/[emailAccountId]/assistant/constants.ts, line 66:
<comment>The `|| lowerExample.includes("sender")` condition is too broad and will cause false matches. Strings like "Forward to sender" or "Reply to sender" will incorrectly return NOTIFY_SENDER color because "sender" matches first. Consider using only `lowerExample.includes("notify")` or a more specific combined check like `lowerExample.includes("notify sender")`.</comment>
<file context>
@@ -53,12 +56,16 @@ export const ACTION_TYPE_ICONS = {
export function getActionTypeColor(example: string): string {
const lowerExample = example.toLowerCase();
+ if (lowerExample.includes("notify") || lowerExample.includes("sender")) {
+ return ACTION_TYPE_COLORS[ActionType.NOTIFY_SENDER];
+ }
</file context>
| if (lowerExample.includes("notify") || lowerExample.includes("sender")) { | |
| if (lowerExample.includes("notify")) { |
✅ Addressed in 4e976a9
![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
🧹 Nitpick comments (1)
apps/web/app/(app)/[emailAccountId]/assistant/AvailableActionsPanel.tsx (1)
24-24: Consider adding a tooltip for NOTIFY_SENDER.While ActionSummaryCard includes a tooltip for this action, adding one here would provide consistent user guidance across the UI. Other actions like CALL_WEBHOOK and DIGEST have tooltips in this component.
🔎 Proposed addition
const actionTooltips: Partial<Record<ActionType, string>> = { [ActionType.CALL_WEBHOOK]: "For developers: trigger external integrations by sending email data to a custom URL", [ActionType.DIGEST]: "Group emails together and receive them as a daily summary", + [ActionType.NOTIFY_SENDER]: + "Sends an automated notification from Inbox Zero (not from your email) informing the sender their email was filtered as cold outreach", };
There was a problem hiding this comment.
Keyword "sender" may be too broad for action type detection.
The condition checks for "sender" OR "notify" in the example string, but "sender" is a generic word that could appear in other action contexts (e.g., "reply to sender", "forward to sender"). This could cause false positives where non-NOTIFY_SENDER actions incorrectly match this condition.
Consider making the check more specific (e.g., "notify sender" as a phrase) or placing this condition lower in the priority chain to allow more specific action types to match first.
🔎 Suggested fix
- if (lowerExample.includes("notify") || lowerExample.includes("sender")) {
+ if (lowerExample.includes("notify") && lowerExample.includes("sender")) {
return ACTION_TYPE_COLORS[ActionType.NOTIFY_SENDER];
}Or alternatively, check for the phrase "notify sender":
- if (lowerExample.includes("notify") || lowerExample.includes("sender")) {
+ if (lowerExample.includes("notify sender")) {
return ACTION_TYPE_COLORS[ActionType.NOTIFY_SENDER];
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (lowerExample.includes("notify") || lowerExample.includes("sender")) { | |
| return ACTION_TYPE_COLORS[ActionType.NOTIFY_SENDER]; | |
| } | |
| if (lowerExample.includes("notify") && lowerExample.includes("sender")) { | |
| return ACTION_TYPE_COLORS[ActionType.NOTIFY_SENDER]; | |
| } |
🤖 Prompt for AI Agents
In apps/web/app/(app)/[emailAccountId]/assistant/constants.ts around lines 66 to
68, the condition treating any example containing "sender" as NOTIFY_SENDER is
too broad and can produce false positives; change the detection to match a more
specific phrase such as "notify sender" (or use word-boundary/regex checks for
the phrase) and/or move this branch lower in the priority order so more specific
actions (e.g., "reply to sender", "forward to sender") are matched first; update
the conditional accordingly to check for the full phrase or use a regex like
/\bnotify\s+sender\b/i and ensure ordering of checks prevents premature matches.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.