-
Notifications
You must be signed in to change notification settings - Fork 885
feat: Delayed actions #519
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
@edulelis is attempting to deploy a commit to the Inbox Zero Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThis update introduces a delayed actions feature for email rule automation. It adds a Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WebApp
participant DB
participant QStash
participant Executor
User->>WebApp: Configure rule with delayed action
WebApp->>DB: Save rule and action (with delayInMinutes)
WebApp->>QStash: Schedule delayed action (with deduplication ID)
QStash->>WebApp: At scheduled time, POST to /api/scheduled-actions/execute
WebApp->>DB: Retrieve ScheduledAction, mark as EXECUTING
WebApp->>Executor: Execute scheduled action
Executor->>DB: Update ScheduledAction (COMPLETED/FAILED)
Executor->>WebApp: Return execution result
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
apps/web/app/(app)/[emailAccountId]/assistant/RuleForm.tsxOops! Something went wrong! :( ESLint: 9.28.0 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by apps/web/utils/delayed-actions.tsOops! Something went wrong! :( ESLint: 9.28.0 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🔇 Additional comments (3)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (5)
apps/web/app/api/cron/scheduled-actions/route.ts (1)
6-6
: Use import type for NextRequest.The static analysis tool correctly identifies that
NextRequest
is only used as a type. Consider usingimport type
for better tree-shaking.-import { NextRequest } from "next/server"; +import type { NextRequest } from "next/server";apps/web/app/(app)/[emailAccountId]/assistant/RuleForm.tsx (1)
1450-1451
: Consider more robust number parsing.The current implementation might not handle all edge cases properly.
Consider using a more defensive approach:
- const numValue = Number.parseInt(value, 10); - if (Number.isNaN(numValue) || numValue <= 0) return null; + const trimmedValue = value.trim(); + if (!trimmedValue || !/^\d+$/.test(trimmedValue)) return null; + const numValue = Number.parseInt(trimmedValue, 10); + if (Number.isNaN(numValue) || numValue <= 0) return null;This ensures only valid numeric strings are parsed.
apps/web/utils/scheduled-actions/executor.ts (1)
155-157
: Consider using static import instead of dynamic import.Dynamic imports add overhead and complexity when not needed.
Move to top-level import:
+import { parseMessage } from "@/utils/mail"; // In the function: - const parsedMessage = await import("@/utils/mail").then((m) => - m.parseMessage(message), - ); + const parsedMessage = await parseMessage(message);apps/web/utils/scheduled-actions/scheduler.ts (1)
83-136
: Consider using database transactions for consistency.When scheduling multiple delayed actions, consider wrapping the operations in a transaction to ensure all-or-nothing behavior.
Example approach:
const scheduledActions = await prisma.$transaction(async (tx) => { const actions = []; for (const actionItem of delayedActions) { const scheduledFor = new Date( emailInternalDate.getTime() + actionItem.delayInMinutes! * 60 * 1000, ); const action = await tx.scheduledAction.create({ data: { /* ... */ } }); actions.push(action); } return actions; });This ensures either all actions are scheduled or none are, preventing partial scheduling on errors.
apps/web/prisma/schema.prisma (1)
348-348
: Consider adding a comment about delay constraintsThe documentation mentions delays should be between 1 minute and 30 days (43200 minutes). Consider adding a comment here to document these constraints for future developers.
- delayInMinutes Int? // Optional delay in minutes (1 min to 30 days) + delayInMinutes Int? // Optional delay in minutes (min: 1, max: 43200 = 30 days)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
DELAYED_ACTIONS.mdc
(1 hunks)apps/web/app/(app)/[emailAccountId]/assistant/RuleForm.tsx
(3 hunks)apps/web/app/api/cron/scheduled-actions/route.ts
(1 hunks)apps/web/prisma/schema.prisma
(7 hunks)apps/web/utils/actions/rule.validation.ts
(1 hunks)apps/web/utils/ai/choose-rule/run-rules.ts
(5 hunks)apps/web/utils/ai/types.ts
(1 hunks)apps/web/utils/delayed-actions.ts
(1 hunks)apps/web/utils/scheduled-actions/executor.test.ts
(1 hunks)apps/web/utils/scheduled-actions/executor.ts
(1 hunks)apps/web/utils/scheduled-actions/scheduler.test.ts
(1 hunks)apps/web/utils/scheduled-actions/scheduler.ts
(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
apps/web/app/api/cron/scheduled-actions/route.ts
[error] 6-6: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
🔇 Additional comments (21)
apps/web/utils/delayed-actions.ts (1)
1-17
: LGTM! Clean and well-structured utility module.The implementation correctly defines the supported delayed action types and provides a type-safe validation function. The documentation is clear and the code follows good practices.
apps/web/utils/ai/types.ts (1)
25-25
: LGTM! Appropriate type addition for delayed actions.The optional and nullable
delayInMinutes
field is correctly typed and well-documented, maintaining backward compatibility while supporting the new delayed actions feature.apps/web/utils/actions/rule.validation.ts (1)
76-76
: LGTM! Appropriate validation constraints for delay field.The validation correctly enforces a 1-minute minimum and 30-day maximum (43,200 minutes) for delayed actions, with proper optional/nullable typing that matches the type definitions.
apps/web/utils/scheduled-actions/scheduler.test.ts (1)
1-94
: Excellent test coverage for scheduler functionality.The test suite is comprehensive and well-structured, covering both positive and negative scenarios for action type validation and scheduled action cancellation. The proper mocking and clear assertions provide good confidence in the scheduler implementation.
apps/web/app/api/cron/scheduled-actions/route.ts (1)
20-102
: Robust cron job implementation with excellent error handling.The handler is well-implemented with:
- Proper authorization and logging
- Sequential processing to avoid race conditions
- Comprehensive error handling at multiple levels
- Good result tracking and monitoring
- Appropriate timeout configuration (5 minutes)
The duplicate processing prevention using
markActionAsExecuting
is a nice touch for reliability.apps/web/utils/scheduled-actions/executor.test.ts (2)
1-52
: Well-structured test setup with comprehensive mocking.The test setup properly mocks all dependencies and provides a complete mock scheduled action object with all required fields.
53-213
: Comprehensive test coverage for executor functionality.The test cases effectively cover:
- Successful execution with proper status updates
- Error handling with retry logic
- Permanent failure scenarios
Good job on testing both the happy path and error conditions.
apps/web/app/(app)/[emailAccountId]/assistant/RuleForm.tsx (2)
94-94
: Clean integration of delay input for supported actions.The conditional rendering using
isSupportedDelayedAction
ensures the delay input only appears for action types that support delays.Also applies to: 1227-1237
1409-1531
: Well-implemented delay input component with proper unit conversion.The component correctly handles:
- Conversion between minutes, hours, and days
- Validation constraints (1 minute to 30 days)
- Bidirectional sync between display value and stored minutes
- Hidden input for form integration
Good UX with the unit selector and clear max limit display.
apps/web/utils/scheduled-actions/executor.ts (1)
20-362
: Solid implementation of scheduled action execution with robust error handling.The executor properly handles:
- Email validation before execution
- Error classification (permanent vs transient)
- Retry logic with appropriate delays
- Status updates and completion tracking
- Comprehensive logging for monitoring
Good architectural decisions on retry attempts and error categorization.
apps/web/utils/ai/choose-rule/run-rules.ts (2)
102-110
: Good practice: Cancel superseded scheduled actions.Canceling existing scheduled actions before applying new rules prevents duplicate or conflicting actions. The test mode check appropriately skips this in test scenarios.
112-149
: Well-structured separation of immediate and delayed action handling.The implementation correctly:
- Filters actions based on delay criteria
- Saves only immediate actions to ExecutedActions
- Schedules delayed actions separately with proper metadata
- Sets appropriate status (SCHEDULED vs PENDING) based on action types
This maintains a clean separation between immediate execution and scheduled execution paths.
Also applies to: 224-228
apps/web/utils/scheduled-actions/scheduler.ts (3)
32-36
: Good validation of supported action types.Early validation with a clear error message prevents invalid scheduled actions from being created.
113-113
: Safe use of non-null assertion.The non-null assertion on
delayInMinutes!
is safe here because the filter on lines 99-103 ensures it's defined and greater than 0.
1-246
: Well-implemented scheduler service with comprehensive functionality.The scheduler properly handles:
- Action validation and type checking
- Scheduling with proper time calculations
- Cancellation of superseded actions
- Retrieval of due actions for execution
- Concurrency control with status updates
Good architectural decisions and error handling throughout.
apps/web/prisma/schema.prisma (3)
122-122
: IsdigestSchedule
related to the delayed actions feature?This field appears to be for digest email scheduling rather than the delayed actions feature that this PR implements. Consider moving this to a separate PR to keep changes focused.
468-469
: Well-designed indexes for performanceThe indexes are properly optimized for the cron job queries and cancellation operations. Good performance considerations.
846-846
: Appropriate status addition for delayed actionsThe
SCHEDULED
status properly represents rules with pending delayed actions, aligning with the documented status flow.DELAYED_ACTIONS.mdc (3)
120-120
: ****
298-315
: Excellent production readiness documentationThe performance metrics and deployment checklist provide valuable operational guidance. The future enhancement ideas are well thought out.
251-252
: Clarify WIP status vs. completion claimsThe document claims the implementation is 100% complete, but the PR title indicates "WIP: Delayed actions". Please clarify whether this is still a work in progress or ready for review.
if (!isAdmin({ email: session.user.email })) { | ||
return NextResponse.json({ error: "Forbidden" }, { status: 403 }); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra endpoints to perform actions in the admin.
Explicitly checking for admin permissions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
♻️ Duplicate comments (3)
apps/web/app/api/admin/scheduled-actions/[id]/retry/route.ts (1)
1-1
: Fix import type usage for NextRequest.Same issue as the cancel route - NextRequest should be imported as a type since it's only used for type annotation.
-import { NextRequest, NextResponse } from "next/server"; +import { NextResponse, type NextRequest } from "next/server";apps/web/app/api/admin/scheduled-actions/route.ts (1)
1-1
: Fix import type usage for NextRequest.NextRequest should be imported as a type since it's only used for type annotation.
-import { type NextRequest, NextResponse } from "next/server"; +import { NextResponse, type NextRequest } from "next/server";apps/web/utils/scheduled-actions/executor.ts (1)
24-24
: Great improvement on type safety!The previous type safety concerns have been properly addressed:
- Function parameter uses proper
ScheduledAction
type- No more
as any
type casting in the codeAlso applies to: 216-223
🧹 Nitpick comments (3)
apps/web/utils/delayed-actions.ts (1)
1-16
: LGTM! Well-structured utility for action delay validation.The utility provides a clean, client-safe way to check action delay support. The choice of unsupported actions (MARK_SPAM, TRACK_THREAD, DIGEST) makes sense as these are typically immediate-effect actions.
Consider documenting why these specific actions don't support delays to help future maintainers understand the rationale.
apps/web/app/api/admin/scheduled-actions/[id]/cancel/route.ts (1)
1-1
: Fix import type usage for NextRequest.NextRequest is only used as a type parameter, so it should be imported with
import type
to ensure it's removed during compilation.-import { type NextRequest, NextResponse } from "next/server"; +import { NextResponse, type NextRequest } from "next/server";apps/web/utils/scheduled-actions/scheduler.ts (1)
138-186
: Consider persisting the cancellation reason in the database.The
reason
parameter is only used for logging but not stored in the database. For better audit trails and debugging, consider adding acancellationReason
field to theScheduledAction
model.Add the cancellation reason to the update data:
data: { status: ScheduledActionStatus.CANCELLED, + cancellationReason: reason, },
This would require adding a
cancellationReason
field to the Prisma schema.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
apps/web/__tests__/ai-choose-rule.test.ts
(1 hunks)apps/web/app/(app)/[emailAccountId]/assistant/RuleForm.tsx
(10 hunks)apps/web/app/(app)/[emailAccountId]/assistant/Rules.tsx
(2 hunks)apps/web/app/(app)/[emailAccountId]/assistant/onboarding/CategoriesSetup.tsx
(1 hunks)apps/web/app/(app)/admin/page.tsx
(2 hunks)apps/web/app/(app)/admin/scheduled-actions/ScheduledActionsTable.tsx
(1 hunks)apps/web/app/(app)/admin/scheduled-actions/page.tsx
(1 hunks)apps/web/app/(app)/admin/scheduled-actions/types.ts
(1 hunks)apps/web/app/api/admin/scheduled-actions/[id]/cancel/route.ts
(1 hunks)apps/web/app/api/admin/scheduled-actions/[id]/retry/route.ts
(1 hunks)apps/web/app/api/admin/scheduled-actions/route.ts
(1 hunks)apps/web/app/api/cron/scheduled-actions/route.ts
(1 hunks)apps/web/components/TooltipExplanation.tsx
(2 hunks)apps/web/prisma/migrations/20250625053006_add_delayed_actions/migration.sql
(1 hunks)apps/web/prisma/schema.prisma
(6 hunks)apps/web/utils/action-item.ts
(2 hunks)apps/web/utils/actions/rule.ts
(10 hunks)apps/web/utils/actions/rule.validation.ts
(3 hunks)apps/web/utils/ai/assistant/chat.ts
(4 hunks)apps/web/utils/ai/choose-rule/run-rules.ts
(3 hunks)apps/web/utils/ai/rule/create-rule-schema.ts
(2 hunks)apps/web/utils/date.ts
(1 hunks)apps/web/utils/delayed-actions.ts
(1 hunks)apps/web/utils/reply-tracker/enable.ts
(1 hunks)apps/web/utils/rule/rule.ts
(1 hunks)apps/web/utils/schedule.test.ts
(1 hunks)apps/web/utils/scheduled-actions/executor.test.ts
(1 hunks)apps/web/utils/scheduled-actions/executor.ts
(1 hunks)apps/web/utils/scheduled-actions/scheduler.test.ts
(1 hunks)apps/web/utils/scheduled-actions/scheduler.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (8)
- apps/web/app/(app)/admin/page.tsx
- apps/web/utils/reply-tracker/enable.ts
- apps/web/tests/ai-choose-rule.test.ts
- apps/web/app/(app)/admin/scheduled-actions/page.tsx
- apps/web/app/(app)/[emailAccountId]/assistant/Rules.tsx
- apps/web/app/(app)/admin/scheduled-actions/types.ts
- apps/web/utils/schedule.test.ts
- apps/web/utils/date.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/web/utils/ai/choose-rule/run-rules.ts
- apps/web/utils/scheduled-actions/scheduler.test.ts
- apps/web/utils/actions/rule.validation.ts
- apps/web/prisma/schema.prisma
🧰 Additional context used
📓 Path-based instructions (17)
`apps/web/**/app/**`: Follow NextJS app router structure by organizing code within the app directory.
apps/web/**/app/**
: Follow NextJS app router structure by organizing code within the app directory.
apps/web/app/(app)/[emailAccountId]/assistant/onboarding/CategoriesSetup.tsx
apps/web/app/api/admin/scheduled-actions/[id]/cancel/route.ts
apps/web/app/api/admin/scheduled-actions/route.ts
apps/web/app/api/cron/scheduled-actions/route.ts
apps/web/app/(app)/admin/scheduled-actions/ScheduledActionsTable.tsx
apps/web/app/api/admin/scheduled-actions/[id]/retry/route.ts
apps/web/app/(app)/[emailAccountId]/assistant/RuleForm.tsx
`apps/web/**/*.{ts,tsx}`: Use TypeScript with strict null checks enabled. Use path aliases with @/ for imports from the project root. Use proper error handling with try/catch block...
apps/web/**/*.{ts,tsx}
: Use TypeScript with strict null checks enabled.
Use path aliases with @/ for imports from the project root.
Use proper error handling with try/catch blocks.
Use the LoadingContent component for async data loading states.
Prefix client-side environment variables with NEXT_PUBLIC_.
apps/web/app/(app)/[emailAccountId]/assistant/onboarding/CategoriesSetup.tsx
apps/web/utils/rule/rule.ts
apps/web/utils/delayed-actions.ts
apps/web/utils/action-item.ts
apps/web/utils/ai/rule/create-rule-schema.ts
apps/web/components/TooltipExplanation.tsx
apps/web/app/api/admin/scheduled-actions/[id]/cancel/route.ts
apps/web/app/api/admin/scheduled-actions/route.ts
apps/web/app/api/cron/scheduled-actions/route.ts
apps/web/utils/scheduled-actions/executor.ts
apps/web/utils/scheduled-actions/executor.test.ts
apps/web/utils/ai/assistant/chat.ts
apps/web/app/(app)/admin/scheduled-actions/ScheduledActionsTable.tsx
apps/web/utils/actions/rule.ts
apps/web/utils/scheduled-actions/scheduler.ts
apps/web/app/api/admin/scheduled-actions/[id]/retry/route.ts
apps/web/app/(app)/[emailAccountId]/assistant/RuleForm.tsx
`apps/web/**/*.{ts,tsx,js,jsx}`: Format code with Prettier and follow tailwindcss patterns using prettier-plugin-tailwindcss.
apps/web/**/*.{ts,tsx,js,jsx}
: Format code with Prettier and follow tailwindcss patterns using prettier-plugin-tailwindcss.
apps/web/app/(app)/[emailAccountId]/assistant/onboarding/CategoriesSetup.tsx
apps/web/utils/rule/rule.ts
apps/web/utils/delayed-actions.ts
apps/web/utils/action-item.ts
apps/web/utils/ai/rule/create-rule-schema.ts
apps/web/components/TooltipExplanation.tsx
apps/web/app/api/admin/scheduled-actions/[id]/cancel/route.ts
apps/web/app/api/admin/scheduled-actions/route.ts
apps/web/app/api/cron/scheduled-actions/route.ts
apps/web/utils/scheduled-actions/executor.ts
apps/web/utils/scheduled-actions/executor.test.ts
apps/web/utils/ai/assistant/chat.ts
apps/web/app/(app)/admin/scheduled-actions/ScheduledActionsTable.tsx
apps/web/utils/actions/rule.ts
apps/web/utils/scheduled-actions/scheduler.ts
apps/web/app/api/admin/scheduled-actions/[id]/retry/route.ts
apps/web/app/(app)/[emailAccountId]/assistant/RuleForm.tsx
`apps/web/**`: Install packages only within the 'apps/web' directory, not at the repository root.
apps/web/**
: Install packages only within the 'apps/web' directory, not at the repository root.
apps/web/app/(app)/[emailAccountId]/assistant/onboarding/CategoriesSetup.tsx
apps/web/utils/rule/rule.ts
apps/web/utils/delayed-actions.ts
apps/web/utils/action-item.ts
apps/web/utils/ai/rule/create-rule-schema.ts
apps/web/components/TooltipExplanation.tsx
apps/web/app/api/admin/scheduled-actions/[id]/cancel/route.ts
apps/web/app/api/admin/scheduled-actions/route.ts
apps/web/app/api/cron/scheduled-actions/route.ts
apps/web/utils/scheduled-actions/executor.ts
apps/web/utils/scheduled-actions/executor.test.ts
apps/web/utils/ai/assistant/chat.ts
apps/web/app/(app)/admin/scheduled-actions/ScheduledActionsTable.tsx
apps/web/utils/actions/rule.ts
apps/web/utils/scheduled-actions/scheduler.ts
apps/web/app/api/admin/scheduled-actions/[id]/retry/route.ts
apps/web/prisma/migrations/20250625053006_add_delayed_actions/migration.sql
apps/web/app/(app)/[emailAccountId]/assistant/RuleForm.tsx
`apps/web/app/(app)/**/*.{js,jsx,ts,tsx}`: If you need to use onClick in a component, that component is a client component and file must start with 'use client'.
apps/web/app/(app)/**/*.{js,jsx,ts,tsx}
: If you need to use onClick in a component, that component is a client component and file must start with 'use client'.
apps/web/app/(app)/[emailAccountId]/assistant/onboarding/CategoriesSetup.tsx
apps/web/app/(app)/admin/scheduled-actions/ScheduledActionsTable.tsx
apps/web/app/(app)/[emailAccountId]/assistant/RuleForm.tsx
`**/*.{js,jsx,ts,tsx}`: Use Shadcn UI and Tailwind for components and styling. Implement responsive design with Tailwind CSS using a mobile-first approach. Use the `next/image` pac...
**/*.{js,jsx,ts,tsx}
: Use Shadcn UI and Tailwind for components and styling.
Implement responsive design with Tailwind CSS using a mobile-first approach.
Use thenext/image
package for images.
apps/web/app/(app)/[emailAccountId]/assistant/onboarding/CategoriesSetup.tsx
apps/web/utils/rule/rule.ts
apps/web/utils/delayed-actions.ts
apps/web/utils/action-item.ts
apps/web/utils/ai/rule/create-rule-schema.ts
apps/web/components/TooltipExplanation.tsx
apps/web/app/api/admin/scheduled-actions/[id]/cancel/route.ts
apps/web/app/api/admin/scheduled-actions/route.ts
apps/web/app/api/cron/scheduled-actions/route.ts
apps/web/utils/scheduled-actions/executor.ts
apps/web/utils/scheduled-actions/executor.test.ts
apps/web/utils/ai/assistant/chat.ts
apps/web/app/(app)/admin/scheduled-actions/ScheduledActionsTable.tsx
apps/web/utils/actions/rule.ts
apps/web/utils/scheduled-actions/scheduler.ts
apps/web/app/api/admin/scheduled-actions/[id]/retry/route.ts
apps/web/app/(app)/[emailAccountId]/assistant/RuleForm.tsx
`apps/web/utils/**/*`: Create utility functions in utils/ folder for reusable logic.
apps/web/utils/**/*
: Create utility functions in utils/ folder for reusable logic.
apps/web/utils/rule/rule.ts
apps/web/utils/delayed-actions.ts
apps/web/utils/action-item.ts
apps/web/utils/ai/rule/create-rule-schema.ts
apps/web/utils/scheduled-actions/executor.ts
apps/web/utils/scheduled-actions/executor.test.ts
apps/web/utils/ai/assistant/chat.ts
apps/web/utils/actions/rule.ts
apps/web/utils/scheduled-actions/scheduler.ts
`apps/web/utils/ai/**`: Main LLM implementations should be placed in this directory.
apps/web/utils/ai/**
: Main LLM implementations should be placed in this directory.
apps/web/utils/ai/rule/create-rule-schema.ts
apps/web/utils/ai/assistant/chat.ts
`apps/web/**/components/**/*.{ts,tsx}`: Use PascalCase naming convention for component files. Prefer functional components with hooks. Use shadcn/ui components when available. Ensu...
apps/web/**/components/**/*.{ts,tsx}
: Use PascalCase naming convention for component files.
Prefer functional components with hooks.
Use shadcn/ui components when available.
Ensure responsive design with a mobile-first approach.
apps/web/components/TooltipExplanation.tsx
`apps/web/components/**/*`: All other components are in components/.
apps/web/components/**/*
: All other components are in components/.
apps/web/components/TooltipExplanation.tsx
`apps/web/app/api/**/*`: All API route handlers must use authentication middleware such as withAuth, withEmailAccount, or withError with custom authentication logic. All database q...
apps/web/app/api/**/*
: All API route handlers must use authentication middleware such as withAuth, withEmailAccount, or withError with custom authentication logic.
All database queries must include user/account filtering, using emailAccountId or userId in WHERE clauses.
Parameters must be validated before use; do not use direct parameter values in queries without validation.
Request bodies should use Zod schemas for validation.
Only necessary fields should be returned in API responses; use Prisma's select to limit fields.
Do not include sensitive data in error messages; use generic errors and SafeError for user-facing errors.
Cron endpoints must use hasCronSecret or hasPostCronSecret for authentication.
Do not hardcode weak secrets in cron endpoints; secrets should not be plain strings in code except for environment variables like CRON_SECRET.
apps/web/app/api/admin/scheduled-actions/[id]/cancel/route.ts
apps/web/app/api/admin/scheduled-actions/route.ts
apps/web/app/api/cron/scheduled-actions/route.ts
apps/web/app/api/admin/scheduled-actions/[id]/retry/route.ts
`**/api/**/*.ts`: ALL API routes that handle user data MUST use appropriate authentication and authorization middleware such as withAuth or withEmailAccount. ALL database queries i...
**/api/**/*.ts
: ALL API routes that handle user data MUST use appropriate authentication and authorization middleware such as withAuth or withEmailAccount.
ALL database queries in API routes MUST be scoped to the authenticated user/account (e.g., include userId or emailAccountId in query filters).
Always validate that resources being accessed or modified belong to the authenticated user before performing operations.
All parameters (route, query, body) in API routes MUST be validated for type, format, and length before use.
Request bodies in API routes MUST be validated using Zod schemas or equivalent.
Error responses in API routes MUST NOT leak sensitive information; use generic error messages and consistent error formats.
All findUnique/findFirst database calls in API routes MUST include ownership filters (e.g., userId or emailAccountId).
All findMany database calls in API routes MUST be scoped to the authenticated user's data.
API routes MUST NOT return sensitive fields or data from other users.
API routes MUST NOT use direct object references (IDs) without ownership checks to prevent IDOR vulnerabilities.
API routes MUST use explicit whitelisting of allowed fields for updates to prevent mass assignment and privilege escalation.
API routes MUST NOT use user input directly in queries; always validate and sanitize inputs.
API routes MUST use SafeError or equivalent for error handling to prevent information disclosure.
API routes MUST use withError middleware (not withAuth or withEmailAccount) for public endpoints, webhooks, or cron endpoints, and MUST implement custom authentication/validation as appropriate.
apps/web/app/api/admin/scheduled-actions/[id]/cancel/route.ts
apps/web/app/api/admin/scheduled-actions/route.ts
apps/web/app/api/cron/scheduled-actions/route.ts
apps/web/app/api/admin/scheduled-actions/[id]/retry/route.ts
`**/api/**/cron/**/*.ts`: Cron endpoints MUST use withError middleware and validate cron secret using hasCronSecret(request) or hasPostCronSecret(request). Cron endpoints MUST capt...
**/api/**/cron/**/*.ts
: Cron endpoints MUST use withError middleware and validate cron secret using hasCronSecret(request) or hasPostCronSecret(request).
Cron endpoints MUST capture unauthorized attempts with captureException and return 401 status for unauthorized requests.
Cron endpoints MUST NOT use predictable or weak secrets for authentication; use strong, random secrets stored in environment variables.
apps/web/app/api/cron/scheduled-actions/route.ts
`**/*.test.ts`: Tests should be colocated next to the tested file, following the pattern: 'dir/format.ts' and 'dir/format.test.ts'.
**/*.test.ts
: Tests should be colocated next to the tested file, following the pattern: 'dir/format.ts' and 'dir/format.test.ts'.
apps/web/utils/scheduled-actions/executor.test.ts
`apps/web/utils/actions/**/*`: Server actions are in apps/web/utils/actions folder.
apps/web/utils/actions/**/*
: Server actions are in apps/web/utils/actions folder.
apps/web/utils/actions/rule.ts
`apps/web/utils/actions/*`: Server action files must start with 'use server'.
apps/web/utils/actions/*
: Server action files must start with 'use server'.
apps/web/utils/actions/rule.ts
`apps/web/utils/actions/*.ts`: Implement all server actions using the next-safe-action library for type safety, input validation, context management, and error handling. Use the ap...
apps/web/utils/actions/*.ts
: Implement all server actions using the next-safe-action library for type safety, input validation, context management, and error handling. Use the appropriate client (actionClientUser, actionClient, or adminActionClient) as needed.
Server Actions are strictly for mutations (operations that change data, e.g., creating, updating, deleting). Do NOT use Server Actions for data fetching (GET operations).
Use the .metadata({ name: "actionName" }) method to provide a meaningful name for monitoring and instrumentation.
If an action modifies data displayed elsewhere, use revalidatePath or revalidateTag from next/cache within the action handler as needed for cache invalidation.
apps/web/utils/actions/rule.ts
🪛 Biome (1.9.4)
apps/web/app/api/admin/scheduled-actions/[id]/retry/route.ts
[error] 1-1: Some named imports are only used as types.
This import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Add inline type keywords.
(lint/style/useImportType)
🔇 Additional comments (27)
apps/web/app/(app)/[emailAccountId]/assistant/onboarding/CategoriesSetup.tsx (1)
210-212
: LGTM! Clean addition of delayed archive option.The new "Label + Archive after a week" option follows the existing SelectItem pattern and provides a clear, user-friendly description of the delayed action functionality.
apps/web/utils/rule/rule.ts (1)
350-350
: LGTM! Consistent field mapping for delay support.The addition of
delayInMinutes
to the action field mapping follows the existing pattern and properly handles the optional nature of the field.apps/web/utils/ai/rule/create-rule-schema.ts (2)
7-7
: LGTM! Proper import for validation constant.Good practice importing the NINETY_DAYS_MINUTES constant for consistent validation limits across the application.
80-87
: LGTM! Well-structured delay field validation.The delayInMinutes field is properly implemented with:
- Appropriate type and constraints (1 minute to 90 days)
- Nullish for optional nature
- Clear, descriptive validation messages
- Helpful field description
The validation range strikes a good balance between flexibility and preventing abuse.
apps/web/utils/action-item.ts (2)
165-174
: LGTM! Consistent type extension for delay support.The ActionFieldsSelection type properly includes delayInMinutes alongside existing action fields, maintaining consistency with the existing pattern.
179-189
: LGTM! Proper initialization of delay field.The delayInMinutes field is correctly initialized in the base object using the same null-coalescing pattern as other optional fields, ensuring consistent handling throughout the sanitization process.
apps/web/app/api/admin/scheduled-actions/[id]/cancel/route.ts (1)
7-58
: LGTM! Solid admin API implementation with proper security checks.The route correctly implements:
- Session authentication and admin authorization
- Resource existence validation
- Business logic validation (only PENDING actions can be cancelled)
- Proper status transition and error handling
apps/web/app/api/admin/scheduled-actions/[id]/retry/route.ts (1)
7-58
: LGTM! Consistent implementation with proper retry logic.The route correctly:
- Validates that only FAILED actions can be retried
- Transitions status from FAILED back to PENDING for reprocessing
- Follows the same secure patterns as the cancel route
apps/web/app/api/cron/scheduled-actions/route.ts (1)
15-74
: LGTM! Excellent cron job implementation with proper safeguards.The implementation correctly follows all coding guidelines:
- Uses
withError
middleware andhasPostCronSecret
for authentication- Implements proper concurrency control by marking actions as executing
- Isolates errors so individual failures don't stop batch processing
- Includes comprehensive logging for monitoring and debugging
- Has appropriate timeout (5 minutes) for long-running operations
apps/web/components/TooltipExplanation.tsx (1)
4-52
: LGTM! Excellent refactor to controlled tooltip system.The refactor effectively addresses modal tooltip positioning issues by:
- Adding controlled state for manual tooltip visibility management
- Introducing configurable
side
positioning prop- Implementing onClick toggle for better user control in complex UI contexts
The implementation follows React best practices and maintains type safety.
apps/web/app/api/admin/scheduled-actions/route.ts (1)
6-148
: LGTM! Comprehensive admin API with excellent filtering and performance optimization.The implementation excels in:
- Flexible filtering system (email, status, rule name search)
- Performance optimization with concurrent Promise.all queries
- Proper data aggregation for status counts and totals
- Reasonable result limiting (100 items) to prevent performance issues
- Comprehensive error handling and proper admin authorization
apps/web/utils/ai/assistant/chat.ts (1)
18-18
: Well-implemented delay feature integration!The addition of
delayInMinutes
field is properly implemented with:
- Appropriate validation constraints (1 minute to 90 days)
- Clear description for API consumers
- Consistent type definitions
- Correct handling in the execution flow
Also applies to: 95-103, 126-126, 766-766
apps/web/utils/scheduled-actions/executor.test.ts (1)
1-274
: Excellent test coverage for the executor module!The test suite comprehensively covers:
- Successful execution with proper state transitions
- Error handling with failed status updates
- Edge case of missing email account
All dependencies are properly mocked and assertions validate the expected behavior.
apps/web/utils/actions/rule.ts (1)
48-48
: Solid implementation of delayed actions in rule management!The changes properly integrate the delay feature:
- Correctly handles
delayInMinutes
in both create and update operations- Implements the new "label_archive_1_week" onboarding option consistently
- Updates user-facing descriptions appropriately
Also applies to: 80-101, 232-232, 251-251, 483-484, 559-566, 598-605, 622-627
apps/web/utils/scheduled-actions/executor.ts (1)
255-267
: I didn’t find theScheduledAction
model via ast-grep. Let’s locate the Prisma schema file and print theScheduledAction
block to check for anerror
field:#!/bin/bash # Locate a Prisma schema file schema_file=$(fd --extension prisma | head -n1) echo "Using schema file: $schema_file" # Print the ScheduledAction model definition sed -n '/model ScheduledAction {/,/}/p' "$schema_file"apps/web/utils/scheduled-actions/scheduler.ts (4)
1-78
: LGTM! Well-structured scheduler service with comprehensive error handling.The
createScheduledAction
function properly validates action types, stores all necessary ActionItem data, and includes appropriate error handling with logging.
80-136
: Function correctly handles scheduling of delayed actions.The non-null assertion on line 113 is safe due to the filtering logic that ensures
delayInMinutes
is not null. The time calculation properly adds the delay to the email's internal date.
188-219
: Efficient query for retrieving due scheduled actions.The function properly filters for PENDING actions that are due, includes necessary relations, and implements sensible ordering and limiting.
221-245
: Good implementation of optimistic locking for concurrent execution prevention.The function correctly uses the status field in the WHERE clause to prevent race conditions when multiple workers might try to execute the same action.
apps/web/prisma/migrations/20250625053006_add_delayed_actions/migration.sql (2)
1-21
: Verify the intentional removal ofdigestScheduleId
column.The migration will drop the
digestScheduleId
column from theEmailAccount
table, resulting in data loss. Please confirm this is intentional and that any existing digest schedule data has been migrated or is no longer needed.
22-63
: Well-designed ScheduledAction table with appropriate indexes.The table structure includes all necessary fields for delayed action execution, and the indexes are optimized for the expected query patterns (finding due actions and querying by email).
apps/web/app/(app)/[emailAccountId]/assistant/RuleForm.tsx (6)
94-94
: LGTM! Appropriate import for delayed action support.
129-142
: Correctly preserves delayInMinutes when loading existing rules.
860-864
: Appropriate default value for new actions.Setting
delayInMinutes: null
ensures new actions don't have delays by default, which is the expected behavior.
1082-1139
: Well-structured UI for delay configuration.The toggle controls are properly organized and only shown for relevant action types. The tooltip provides clear guidance about the delay feature.
1481-1582
: Excellent implementation of delay input with unit conversion.The
DelayInputControls
component properly handles conversion between minutes, hours, and days, with correct calculations and good UX.
1286-1297
: ```shell
#!/bin/bash
echo "=== create-rule-schema.ts around delayInMinutes ==="
rg -n 'delayInMinutes' -A 5 apps/web/utils/ai/rule/create-rule-schema.tsecho "=== rule.validation.ts around delayInMinutes ==="
rg -n 'delayInMinutes' -A 5 apps/web/utils/actions/rule.validation.ts</details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
apps/web/app/(app)/admin/scheduled-actions/ScheduledActionsTable.tsx
Outdated
Show resolved
Hide resolved
apps/web/app/(app)/admin/scheduled-actions/ScheduledActionsTable.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/web/components/ui/tooltip.tsx (1)
16-27
: Excellent portal implementation with one consideration.The portal wrapping is a best practice that prevents z-index and overflow issues. However, the z-index value of
9999
is extremely high and might cause issues if other elements need to appear above tooltips.Consider using a more reasonable z-index value like
z-[100]
orz-[200]
unless there's a specific requirement for such a high value.- "z-[9999] overflow-hidden rounded-md border border-slate-700 bg-gray-900 px-3 py-1.5 text-sm text-slate-50 shadow-md animate-in fade-in-0 zoom-in-95 data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=closed]:zoom-out-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2 dark:border-slate-100 dark:bg-slate-50 dark:text-slate-950", + "z-[100] overflow-hidden rounded-md border border-slate-700 bg-gray-900 px-3 py-1.5 text-sm text-slate-50 shadow-md animate-in fade-in-0 zoom-in-95 data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=closed]:zoom-out-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2 dark:border-slate-100 dark:bg-slate-50 dark:text-slate-950",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
.cursor/rules/features/delayed-actions.mdc
(1 hunks)apps/web/app/(app)/[emailAccountId]/assistant/RuleForm.tsx
(10 hunks)apps/web/app/api/admin/scheduled-actions/[id]/cancel/route.ts
(1 hunks)apps/web/app/api/admin/scheduled-actions/[id]/retry/route.ts
(1 hunks)apps/web/app/api/scheduled-actions/execute/route.ts
(1 hunks)apps/web/components/ui/tooltip.tsx
(1 hunks)apps/web/prisma/migrations/20250626043046_add_scheduled_actions/migration.sql
(1 hunks)apps/web/prisma/schema.prisma
(6 hunks)apps/web/utils/ai/choose-rule/run-rules.ts
(4 hunks)apps/web/utils/scheduled-actions/executor.ts
(1 hunks)apps/web/utils/scheduled-actions/scheduler.test.ts
(1 hunks)apps/web/utils/scheduled-actions/scheduler.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- apps/web/utils/ai/choose-rule/run-rules.ts
- apps/web/utils/scheduled-actions/scheduler.test.ts
- apps/web/app/api/admin/scheduled-actions/[id]/cancel/route.ts
- apps/web/utils/scheduled-actions/executor.ts
- apps/web/app/(app)/[emailAccountId]/assistant/RuleForm.tsx
- apps/web/prisma/schema.prisma
- apps/web/app/api/admin/scheduled-actions/[id]/retry/route.ts
🧰 Additional context used
📓 Path-based instructions (11)
`apps/web/**/*.{ts,tsx}`: Use TypeScript with strict null checks enabled. Use pa...
apps/web/**/*.{ts,tsx}
: Use TypeScript with strict null checks enabled.
Use path aliases with @/ for imports from the project root.
Use proper error handling with try/catch blocks.
Use the LoadingContent component for async data loading states.
Prefix client-side environment variables with NEXT_PUBLIC_.
📄 Source: CodeRabbit Inference Engine (apps/web/CLAUDE.md)
List of files the instruction was applied to:
apps/web/components/ui/tooltip.tsx
apps/web/app/api/scheduled-actions/execute/route.ts
apps/web/utils/scheduled-actions/scheduler.ts
`apps/web/**/*.{ts,tsx,js,jsx}`: Format code with Prettier and follow tailwindcss patterns using prettier-plugin-tailwindcss.
apps/web/**/*.{ts,tsx,js,jsx}
: Format code with Prettier and follow tailwindcss patterns using prettier-plugin-tailwindcss.
📄 Source: CodeRabbit Inference Engine (apps/web/CLAUDE.md)
List of files the instruction was applied to:
apps/web/components/ui/tooltip.tsx
apps/web/app/api/scheduled-actions/execute/route.ts
apps/web/utils/scheduled-actions/scheduler.ts
`apps/web/**/components/**/*.{ts,tsx}`: Use PascalCase naming convention for com...
apps/web/**/components/**/*.{ts,tsx}
: Use PascalCase naming convention for component files.
Prefer functional components with hooks.
Use shadcn/ui components when available.
Ensure responsive design with a mobile-first approach.
📄 Source: CodeRabbit Inference Engine (apps/web/CLAUDE.md)
List of files the instruction was applied to:
apps/web/components/ui/tooltip.tsx
`apps/web/**`: Install packages only within the 'apps/web' directory, not at the repository root.
apps/web/**
: Install packages only within the 'apps/web' directory, not at the repository root.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/installing-packages.mdc)
List of files the instruction was applied to:
apps/web/components/ui/tooltip.tsx
apps/web/app/api/scheduled-actions/execute/route.ts
apps/web/utils/scheduled-actions/scheduler.ts
apps/web/prisma/migrations/20250626043046_add_scheduled_actions/migration.sql
`apps/web/components/ui/**/*`: Shadcn components are in components/ui.
apps/web/components/ui/**/*
: Shadcn components are in components/ui.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/project-structure.mdc)
List of files the instruction was applied to:
apps/web/components/ui/tooltip.tsx
`apps/web/components/**/*`: All other components are in components/.
apps/web/components/**/*
: All other components are in components/.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/project-structure.mdc)
List of files the instruction was applied to:
apps/web/components/ui/tooltip.tsx
`**/*.{js,jsx,ts,tsx}`: Use Shadcn UI and Tailwind for components and styling. I...
**/*.{js,jsx,ts,tsx}
: Use Shadcn UI and Tailwind for components and styling.
Implement responsive design with Tailwind CSS using a mobile-first approach.
Use thenext/image
package for images.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/ui-components.mdc)
List of files the instruction was applied to:
apps/web/components/ui/tooltip.tsx
apps/web/app/api/scheduled-actions/execute/route.ts
apps/web/utils/scheduled-actions/scheduler.ts
`apps/web/**/app/**`: Follow NextJS app router structure by organizing code within the app directory.
apps/web/**/app/**
: Follow NextJS app router structure by organizing code within the app directory.
📄 Source: CodeRabbit Inference Engine (apps/web/CLAUDE.md)
List of files the instruction was applied to:
apps/web/app/api/scheduled-actions/execute/route.ts
`apps/web/app/api/**/*`: All API route handlers must use authentication middlewa...
apps/web/app/api/**/*
: All API route handlers must use authentication middleware such as withAuth, withEmailAccount, or withError with custom authentication logic.
All database queries must include user/account filtering, using emailAccountId or userId in WHERE clauses.
Parameters must be validated before use; do not use direct parameter values in queries without validation.
Request bodies should use Zod schemas for validation.
Only necessary fields should be returned in API responses; use Prisma's select to limit fields.
Do not include sensitive data in error messages; use generic errors and SafeError for user-facing errors.
Cron endpoints must use hasCronSecret or hasPostCronSecret for authentication.
Do not hardcode weak secrets in cron endpoints; secrets should not be plain strings in code except for environment variables like CRON_SECRET.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/security-audit.mdc)
List of files the instruction was applied to:
apps/web/app/api/scheduled-actions/execute/route.ts
`**/api/**/*.ts`: ALL API routes that handle user data MUST use appropriate auth...
**/api/**/*.ts
: ALL API routes that handle user data MUST use appropriate authentication and authorization middleware such as withAuth or withEmailAccount.
ALL database queries in API routes MUST be scoped to the authenticated user/account (e.g., include userId or emailAccountId in query filters).
Always validate that resources being accessed or modified belong to the authenticated user before performing operations.
All parameters (route, query, body) in API routes MUST be validated for type, format, and length before use.
Request bodies in API routes MUST be validated using Zod schemas or equivalent.
Error responses in API routes MUST NOT leak sensitive information; use generic error messages and consistent error formats.
All findUnique/findFirst database calls in API routes MUST include ownership filters (e.g., userId or emailAccountId).
All findMany database calls in API routes MUST be scoped to the authenticated user's data.
API routes MUST NOT return sensitive fields or data from other users.
API routes MUST NOT use direct object references (IDs) without ownership checks to prevent IDOR vulnerabilities.
API routes MUST use explicit whitelisting of allowed fields for updates to prevent mass assignment and privilege escalation.
API routes MUST NOT use user input directly in queries; always validate and sanitize inputs.
API routes MUST use SafeError or equivalent for error handling to prevent information disclosure.
API routes MUST use withError middleware (not withAuth or withEmailAccount) for public endpoints, webhooks, or cron endpoints, and MUST implement custom authentication/validation as appropriate.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/security.mdc)
List of files the instruction was applied to:
apps/web/app/api/scheduled-actions/execute/route.ts
`apps/web/utils/**/*`: Create utility functions in utils/ folder for reusable logic.
apps/web/utils/**/*
: Create utility functions in utils/ folder for reusable logic.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/project-structure.mdc)
List of files the instruction was applied to:
apps/web/utils/scheduled-actions/scheduler.ts
🧬 Code Graph Analysis (2)
apps/web/components/ui/tooltip.tsx (1)
apps/web/utils/index.ts (1)
cn
(4-6)
apps/web/utils/scheduled-actions/scheduler.ts (4)
apps/web/utils/logger.ts (1)
createScopedLogger
(17-65)apps/web/utils/ai/types.ts (1)
ActionItem
(15-26)apps/web/utils/delayed-actions.ts (1)
canActionBeDelayed
(14-16)apps/web/utils/cron.ts (1)
getCronSecretHeader
(26-28)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Static Code Analysis Js
- GitHub Check: Secret Detection
- GitHub Check: Jit Security
🔇 Additional comments (19)
apps/web/components/ui/tooltip.tsx (4)
11-11
: LGTM! Good addition for flexibility.Adding
TooltipPortal
as a separate export provides flexibility for cases where direct portal control is needed.
13-13
: Good TypeScript improvement.The change from
React.ElementRef
toReact.ComponentRef
is the correct modern TypeScript pattern for component refs.
22-22
: Verify the dark theme styling is intentional.The tooltip styling has changed from a light default theme to a dark theme (gray-900 background). This is a significant visual change that will affect the appearance across the application.
Please confirm this dark theme styling is intentional and aligns with the overall design system.
31-37
: LGTM! Clean export structure.The export structure is well-organized and includes the new
TooltipPortal
component appropriately.apps/web/app/api/scheduled-actions/execute/route.ts (3)
42-48
: Database query appears secure.The query properly filters by the scheduled action ID and includes necessary relations. No user input is directly used in the query, which prevents injection attacks.
100-103
: Comprehensive error handling implemented.The try-catch block properly handles unexpected errors and provides appropriate logging and HTTP responses.
74-76
: Verify atomic operation behavior.The
markQStashActionAsExecuting
function should use an atomic update with a WHERE condition to prevent race conditions.#!/bin/bash # Check the implementation of markQStashActionAsExecuting for atomic updates ast-grep --pattern 'export async function markQStashActionAsExecuting($$$) { $$$ }'.cursor/rules/features/delayed-actions.mdc (1)
1-151
: Comprehensive and well-structured feature documentation.The documentation effectively covers:
- Architecture overview and use cases
- Database schema design
- QStash integration details
- API endpoints and error handling
- Benefits over alternative approaches
The code examples are accurate and the explanations are clear for developers implementing or maintaining this feature.
apps/web/prisma/migrations/20250626043046_add_scheduled_actions/migration.sql (4)
8-8
: Well-designed status enum for action lifecycle.The
ScheduledActionStatus
enum covers all necessary states: PENDING, EXECUTING, COMPLETED, FAILED, CANCELLED. This provides clear status tracking throughout the action lifecycle.
23-46
: Comprehensive table design with proper field coverage.The
ScheduledAction
table includes:
- All necessary action metadata (type, timing, status)
- Email context (messageId, threadId, emailAccountId)
- Action-specific fields (label, subject, content, etc.)
- QStash integration (qstashMessageId)
- Execution tracking (executedAt, executedActionId)
The design supports all documented action types and use cases.
48-58
: Efficient indexing strategy implemented.The indexes are well-chosen for the expected query patterns:
status_scheduledFor_idx
: For finding pending actions to executeemailAccountId_messageId_idx
: For canceling actions by emailqstashMessageId_idx
: For QStash message managementThis should provide good performance for the scheduler operations.
60-67
: Proper foreign key relationships with appropriate cascade behavior.The foreign key constraints ensure data integrity:
executedRuleId
→ExecutedRule
with CASCADE DELETE (appropriate for audit trail)executedActionId
→ExecutedAction
with SET NULL (allows independent lifecycle)emailAccountId
→EmailAccount
with CASCADE DELETE (proper cleanup)apps/web/utils/scheduled-actions/scheduler.ts (7)
27-30
: Proper environment-based QStash client initialization.The client factory function correctly handles the case where QStash token is not available, returning null for graceful degradation.
50-54
: Essential validation for action type support.The function properly validates that the action type supports delayed execution before proceeding, preventing invalid scheduled actions.
58-76
: Comprehensive database record creation.The scheduled action creation includes all necessary fields from the ActionItem and properly maps the data structure for storage.
266-286
: Robust error handling for QStash cancellation.The code properly handles QStash cancellation failures without failing the entire operation, which is appropriate since database state is the source of truth.
334-334
: Environment variable fallback strategy.The URL construction properly falls back from
WEBHOOK_URL
toNEXT_PUBLIC_BASE_URL
, providing flexibility for different deployment environments.
340-348
: Secure QStash message publishing configuration.The QStash publishing configuration includes proper security headers and deduplication settings:
- Uses cron secret headers for authentication
- Disables content-based deduplication in favor of explicit IDs
- Uses absolute timestamps for precise scheduling
407-415
: Atomic database operation for status transitions.The
markQStashActionAsExecuting
function uses a proper WHERE condition with the PENDING status, ensuring atomic updates and preventing race conditions.
@@ -867,9 +869,10 @@ export function RuleForm({ | |||
</div> | |||
)} | |||
|
|||
<div className="mt-8 flex items-center justify-end space-x-2"> | |||
<div className="mt-[67px] flex items-center justify-end space-x-2"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reason for 67px hard coded?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary by CodeRabbit
New Features
Bug Fixes
Chores