feat(tasks): timezone-aware scheduling — pass timezone to the Trigger.dev schedule (chat#1881 3c) - #780
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Warning Review limit reached
Next review available in: 23 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughTask creation and updates now accept validated IANA timezones, avoid persisting them in scheduled actions, and propagate them to Trigger.dev schedule creation or synchronization. Existing schedule timezones are retrieved when updates omit a replacement. ChangesTimezone-aware task scheduling
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant updateTask
participant syncTriggerSchedule
participant TriggerSchedules as Trigger.dev schedules API
updateTask->>syncTriggerSchedule: pass schedule and optional timezone
syncTriggerSchedule->>TriggerSchedules: retrieve existing timezone when omitted
TriggerSchedules-->>syncTriggerSchedule: return timezone
syncTriggerSchedule->>TriggerSchedules: update schedule with resolved timezone
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 4 files
Confidence score: 5/5
- Safe to merge after the addressed issues were fixed.
Architecture diagram
sequenceDiagram
participant Frontend as Frontend UI
participant API as API Route
participant Validator as isValidTimeZone
participant CronCore as getNextUtcRunForLocalCron
participant Intl as Intl.DateTimeFormat
participant DB as Database
participant TriggerDev as Trigger.dev
Note over Frontend,TriggerDev: NEW: Timezone-Aware Weekly Scheduling Flow
Frontend->>API: POST /scheduled_actions (timezone: "America/New_York")
API->>Validator: Validate IANA timezone
Validator-->>API: true/false
alt Invalid Timezone
API-->>Frontend: 400 Bad Request
else Valid Timezone
API->>CronCore: getNextUtcRunForLocalCron("0 9 * * 1", "America/New_York", from)
Note over CronCore: Resolve UTC offset for local cron time
CronCore->>Intl: formatToParts(instant) for DST offset
Intl-->>CronCore: UTC offset in ms
loop For next 8 days
CronCore->>CronCore: Check if weekday matches cron DOW
alt Weekday matches
CronCore->>Intl: zonedWallTimeToUtc(year, month, day, hour, minute)
Intl-->>CronCore: UTC candidate instant
alt candidate > from
CronCore-->>API: Next UTC run time
end
end
end
API->>DB: INSERT scheduled_actions (..., timezone, next_run)
DB-->>API: Created
API->>TriggerDev: createSchedule(id, cron, timezone)
TriggerDev-->>API: Schedule created
API-->>Frontend: 201 Created + task details
end
Note over Frontend,TriggerDev: Follow-Up: Update Schedule Flow
Frontend->>API: PUT /scheduled_actions/:id (new timezone)
API->>Validator: Validate new timezone
API->>CronCore: Recalculate next_run with new timezone
API->>DB: UPDATE scheduled_actions (timezone, next_run)
API->>TriggerDev: updateSchedule(id, cron, timezone)
TriggerDev-->>API: Updated
API-->>Frontend: 200 OK
Note over Frontend,TriggerDev: DST Transition Handling (automatic)
CronCore->>Intl: getTimeZoneOffsetMs(instant)
alt Offset differs from previous calculation
Note over CronCore: DST boundary crossed
CronCore->>Intl: Refine with corrected instant
Intl-->>CronCore: Final DST-aware UTC offset
end
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…sks (#1881 item 3c) Weekly-report emails currently fire on a UTC cron (`0 9 * * 1` = 9am UTC), which is the middle of the US night. Item 3c makes the send land at 9am in the customer's *local* timezone. This commit adds the pure, DST-aware core the feature is built on: - getNextUtcRunForLocalCron(cron, ianaTz, from): computes the next UTC instant for an `M H * * D` cron interpreted in an IANA timezone. Handles EST/EDT (and any zone) correctly across DST boundaries. - isValidTimeZone(tz): IANA validation via Intl. TDD red→green: 12 tests including winter (UTC-5) vs summer (UTC-4) New York Mondays, UTC/Tokyo, week rollover, and daily-wildcard cases. BLOCKER: persisting the timezone is blocked on a `database` migration adding `scheduled_actions.timezone` (no such column exists today). Contract acceptance, persistence, and schedule wiring follow once that lands. See PR body for details. Chat-side timezone picker is a sibling PR. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…p local UTC conversion (chat#1881)
Trigger.dev's schedule already owns the timezone: schedules.create/update({ cron,
timezone }) interprets the cron in that IANA zone (DST-aware) and IS the source
of truth for when a task fires. So instead of computing UTC ourselves and storing
a copy of the timezone in scheduled_actions (both of which can diverge from the
schedule), we just thread the timezone through:
- create/update task contracts accept an optional IANA `timezone` (validated via
isValidTimeZone).
- createTask → createSchedule({ timezone }); the timezone is stripped before the
Supabase insert (not a scheduled_actions column).
- updateTask → syncTriggerSchedule({ timezone }); a timezone-only change re-syncs
the schedule, and a cron-only edit preserves the current timezone by reading it
back from Trigger.dev (retrieveScheduleTimezone) rather than resetting to UTC.
- Removed getNextUtcRunForLocalCron (redundant with Trigger.dev's native tz).
- database#48 (scheduled_actions.timezone) closed — Trigger.dev is the single
source of truth; no denormalized copy to maintain.
Tests: syncTriggerSchedule 4/4 (create/update/preserve/delete), createTask 3/3
(passes tz to schedule, strips from insert, UTC default). Full lib/tasks +
lib/trigger green; tsc + eslint clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
e1dbe17 to
5d311e3
Compare
sweetmantech
changed the title
Reworked — Trigger.dev owns the timezone (no local conversion, no Supabase copy)Per the chat#1881 3c design discussion: Trigger.dev's schedule already interprets the cron in an IANA Changes (force-pushed):
Tests: Follow-up: the docs task contract (OpenAPI |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
lib/tasks/createTaskSchemas.ts (1)
36-42: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCentralize the duplicated timezone contract.
Both schemas independently define the same refinement, error message, and documentation. Extract one shared
timezoneSchemaand use it in both contracts.
lib/tasks/createTaskSchemas.ts#L36-L42: replace the inline schema with the shared timezone schema.lib/tasks/updateTaskSchemas.ts#L25-L31: import and reuse the same shared schema.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/tasks/createTaskSchemas.ts` around lines 36 - 42, Extract the duplicated timezone refinement, error message, and documentation into a shared timezoneSchema, then replace the inline schema in lib/tasks/createTaskSchemas.ts lines 36-42 and import and reuse it in lib/tasks/updateTaskSchemas.ts lines 25-31.Sources: Coding guidelines, Path instructions
lib/tasks/createTask.ts (1)
14-49: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftReduce the size of the orchestration functions.
These functions combine several responsibilities and exceed the repository’s small-function guidance:
lib/tasks/createTask.ts#L14-L49: separate persistence from schedule registration and compensation.lib/tasks/updateTask.ts#L19-L70: extract authorization/patch construction and schedule synchronization.lib/trigger/syncTriggerSchedule.ts#L27-L62: extract the delete, update, and create operations.As per coding guidelines, flag functions longer than 20 lines and keep functions small and focused; as per path instructions, domain functions must stay under 50 lines.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/tasks/createTask.ts` around lines 14 - 49, Reduce orchestration size by extracting focused helpers: in lib/tasks/createTask.ts lines 14-49, separate scheduled-action persistence, Trigger.dev schedule registration, and compensation; in lib/tasks/updateTask.ts lines 19-70, extract authorization/patch construction and schedule synchronization; in lib/trigger/syncTriggerSchedule.ts lines 27-62, extract the delete, update, and create operations. Keep each domain function under 50 lines and the extracted functions focused and small.Sources: Coding guidelines, Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/tasks/createTask.ts`:
- Around line 22-32: Update the task-creation flow surrounding
insertScheduledAction and createSchedule so a Trigger.dev failure cannot leave
an unscheduled persisted task: add an appropriate idempotency, compensating
deletion, or outbox/reconciliation mechanism. Ensure retries do not create
duplicate tasks or schedules, and preserve the existing validation of the
created task ID.
In `@lib/tasks/timezone/isValidTimeZone.ts`:
- Around line 13-15: Update isValidTimeZone to reject offset-shaped identifiers
such as +01:00, -02:30, +0100, and -23 before invoking Intl.DateTimeFormat;
retain the existing Intl-based validation for named IANA timezones and return
false for rejected offsets.
In `@lib/tasks/updateTask.ts`:
- Around line 39-49: Preserve timezone updates across disabled schedule deletion
and recreation: in lib/tasks/updateTask.ts lines 39-49, retain the supplied
timezone as pending state or reject timezone-only updates when no schedule
exists; in lib/trigger/syncTriggerSchedule.ts lines 31-38, do not discard that
timezone on the disabled/delete path; and in lines 51-57, use the retained
timezone when recreating the schedule.
---
Nitpick comments:
In `@lib/tasks/createTask.ts`:
- Around line 14-49: Reduce orchestration size by extracting focused helpers: in
lib/tasks/createTask.ts lines 14-49, separate scheduled-action persistence,
Trigger.dev schedule registration, and compensation; in lib/tasks/updateTask.ts
lines 19-70, extract authorization/patch construction and schedule
synchronization; in lib/trigger/syncTriggerSchedule.ts lines 27-62, extract the
delete, update, and create operations. Keep each domain function under 50 lines
and the extracted functions focused and small.
In `@lib/tasks/createTaskSchemas.ts`:
- Around line 36-42: Extract the duplicated timezone refinement, error message,
and documentation into a shared timezoneSchema, then replace the inline schema
in lib/tasks/createTaskSchemas.ts lines 36-42 and import and reuse it in
lib/tasks/updateTaskSchemas.ts lines 25-31.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3396c91d-aff9-484a-b78b-88612913013a
⛔ Files ignored due to path filters (3)
lib/tasks/__tests__/createTask.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/tasks/timezone/__tests__/isValidTimeZone.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/trigger/__tests__/syncTriggerSchedule.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**
📒 Files selected for processing (8)
lib/tasks/createTask.tslib/tasks/createTaskSchemas.tslib/tasks/timezone/isValidTimeZone.tslib/tasks/updateTask.tslib/tasks/updateTaskSchemas.tslib/trigger/retrieveScheduleTimezone.tslib/trigger/syncTriggerSchedule.tslib/trigger/updateSchedule.ts
| // Insert the task into the database | ||
| const tasks = await insertScheduledAction(validatedInput); | ||
| const tasks = await insertScheduledAction(scheduledActionInput); | ||
| const created = tasks[0]; | ||
|
|
||
| if (!created || !created.id) { | ||
| throw new Error("Failed to create task: missing Supabase id for scheduling"); | ||
| } | ||
|
|
||
| // Create the Trigger.dev schedule | ||
| // Create the Trigger.dev schedule (it interprets the cron in `timezone`, | ||
| // DST-aware; defaults to UTC when omitted). | ||
| const triggerSchedule = await createSchedule({ |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Make task creation recoverable across the DB/Trigger.dev boundary.
The database insert completes before createSchedule. If Trigger.dev fails, this method throws after leaving a persisted task without a schedule. Add idempotency, compensating cleanup, or an outbox/reconciliation flow to prevent partial task creation.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/tasks/createTask.ts` around lines 22 - 32, Update the task-creation flow
surrounding insertScheduledAction and createSchedule so a Trigger.dev failure
cannot leave an unscheduled persisted task: add an appropriate idempotency,
compensating deletion, or outbox/reconciliation mechanism. Ensure retries do not
create duplicate tasks or schedules, and preserve the existing validation of the
created task ID.
| try { | ||
| new Intl.DateTimeFormat("en-US", { timeZone }); | ||
| return true; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the target file without executing repository code.
if [ -f lib/tasks/timezone/isValidTimeZone.ts ]; then
echo "== file exists =="
wc -l lib/tasks/timezone/isValidTimeZone.ts
cat -n lib/tasks/timezone/isValidTimeZone.ts
else
echo "target file not found"
fd -a 'isValidTimeZone' . || true
fi
echo "== usages of isValidTimeZone =="
rg -n "isValidTimeZone|timeZone|UTC|timezone" lib/tasks/timezone lib 2>/dev/null | head -200 || true
echo "== package/runtime Intl behavior probe =="
node - <<'JS'
const candidates = [
"+01:00",
"+0100",
"+01",
"-02:30",
"UTC",
"Etc/UTC",
"America/New_York",
"0",
"+12:15:45",
"-23",
"",
];
for (const tz of candidates) {
let ok, msg;
try {
new Intl.DateTimeFormat("en-US", { timeZone: tz });
ok = true;
} catch (e) {
ok = false;
msg = e && e.message ? e.message : String(e);
}
console.log(JSON.stringify({value: tz, accepted: ok, message: msg}));
}
JSRepository: recoupable/api
Length of output: 13897
Reject offset identifiers before relying on Intl validation.
isValidTimeZone is only used as an IANA timezone validator, but Intl.DateTimeFormat also accepts offset identifiers like +01:00, -02:30, +0100, and -23; those values would pass this check. Add a precheck for offset-shaped input or validate against a named timezone set so the schema reflects the contract.
Suggested narrow fix
if (typeof timeZone !== "string" || timeZone.length === 0) {
return false;
}
+ if (/^[+-]\d{2}(?::?\d{2})?$/.test(timeZone)) {
+ return false;
+ }📝 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.
| try { | |
| new Intl.DateTimeFormat("en-US", { timeZone }); | |
| return true; | |
| if (/^[+-]\d{2}(?::?\d{2})?$/.test(timeZone)) { | |
| return false; | |
| } | |
| try { | |
| new Intl.DateTimeFormat("en-US", { timeZone }); | |
| return true; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/tasks/timezone/isValidTimeZone.ts` around lines 13 - 15, Update
isValidTimeZone to reject offset-shaped identifiers such as +01:00, -02:30,
+0100, and -23 before invoking Intl.DateTimeFormat; retain the existing
Intl-based validation for named IANA timezones and return false for rejected
offsets.
| // `timezone` lives on the Trigger.dev schedule, not scheduled_actions. | ||
| if (key === "id" || key === "resolvedAccountId" || key === "timezone") return false; | ||
| return true; | ||
| }), | ||
| ) as Partial<TablesUpdate<"scheduled_actions">>; | ||
|
|
||
| const finalEnabled = enabled !== undefined ? enabled : (existingTask.enabled ?? true); | ||
| const cronExpression = schedule ?? existingTask.schedule; | ||
| const scheduleChanged = schedule !== undefined; | ||
| // A timezone-only change still needs the schedule re-synced (the cron falls | ||
| // back to the existing expression), so treat it as a schedule change. | ||
| const scheduleChanged = schedule !== undefined || timezone !== undefined; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Do not lose timezone changes when no active schedule exists.
The task layer strips timezone from persistence, while the synchronization layer deletes or skips schedules when disabled. Re-enabling without a new timezone therefore creates a UTC schedule instead of preserving the accepted value. (trigger.dev)
lib/tasks/updateTask.ts#L39-L49: retain pending timezone state or reject timezone-only updates when no schedule exists.lib/trigger/syncTriggerSchedule.ts#L31-L38: do not discard the supplied timezone on the disabled/delete path.lib/trigger/syncTriggerSchedule.ts#L51-L57: use the retained timezone when recreating the schedule.
📍 Affects 2 files
lib/tasks/updateTask.ts#L39-L49(this comment)lib/trigger/syncTriggerSchedule.ts#L31-L38lib/trigger/syncTriggerSchedule.ts#L51-L57
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/tasks/updateTask.ts` around lines 39 - 49, Preserve timezone updates
across disabled schedule deletion and recreation: in lib/tasks/updateTask.ts
lines 39-49, retain the supplied timezone as pending state or reject
timezone-only updates when no schedule exists; in
lib/trigger/syncTriggerSchedule.ts lines 31-38, do not discard that timezone on
the disabled/delete path; and in lines 51-57, use the retained timezone when
recreating the schedule.
…chat#1881)
Preview verification caught a 500 on a timezone-only PATCH: the timezone lives on
the Trigger.dev schedule, not scheduled_actions, so `updateData` was empty and
updateScheduledAction({ id }) failed ("multiple (or no) rows returned") on the
empty update. The Trigger.dev schedule DID update correctly — only the trailing
Supabase write errored. Skip the DB write when there are no column changes and
return the unchanged row. Adds updateTask tests (tz-only passes tz to sync +
skips the DB write; real column changes still persist).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Auto-approved: Adds optional IANA timezone support to task scheduling, passing it to Trigger.dev for DST-aware local-time firing. No database changes, no schema migration, no breaking contract changes – the feature is opt-in and defaults to UTC. Well-tested and bounded.
Re-trigger cubic
✅ Preview verification — timezone flows to the Trigger.dev schedule (+ found & fixed a tz-only 500)Tested against the preview with a Privy Bearer, creating/patching a real task and reading the schedule back from Trigger.dev directly (
Bug found + fixed (
|
1 participant
Item 3c (api) — Timezone-aware weekly scheduling
Part of recoupable/chat#1881.
Problem
Weekly-report emails fire on a UTC cron (
0 9 * * 1= 9am UTC), which is the middle of the US night. The goal is to make the send land at 9am local in the customer's IANA timezone (e.g.America/New_York), DST-aware.What this PR ships (un-blocked, TDD'd core)
lib/tasks/timezone/getNextUtcRunForLocalCron.ts— pure function: given anM H * * Dcron, an IANA timezone, and afrominstant, returns the next UTC instant the schedule fires at, interpreting the cron in local wall-clock time. DST-aware (usesIntloffsets, refines across boundaries).lib/tasks/timezone/isValidTimeZone.ts— IANA timezone validation.UTCandAsia/Tokyopnpm test lib/tasks/timezone→ 12 passed. eslint clean.🚧 BLOCKER — needs a
databasemigration firstAccepting and persisting the IANA
timezoneon a task requires a new column that does not exist today:scheduled_actionshas notimezonecolumn (confirmed intypes/database.types.tsand indatabase/supabase/migrations/— nothing adds one).Per the issue's guardrail I did not invent a migration. A prerequisite
databasePR must add:Follow-up wiring (lands after the column exists)
Once
scheduled_actions.timezoneis available:timezonein the create/update contracts (createTaskSchemas.ts/updateTaskSchemas.ts), validated withisValidTimeZone.insertScheduledAction/updateScheduledAction.timezoneto Trigger.dev —createSchedulealready accepts it; thread it throughupdateSchedule+syncTriggerSchedule.getNextUtcRunForLocalCronto populate the existingnext_runcolumn so the API reflects the correct local send time.Related
🤖 Generated with Claude Code
Summary by cubic
Adds timezone-aware scheduling so weekly/daily tasks run at the user’s local time (IANA, DST-aware), e.g. 9am local instead of 9am UTC. Part of #1881 item 3c.
New Features
timezonein task create/update; validated withisValidTimeZone.createTask/syncTriggerSchedulepass it through; cron-only edits preserve the existing timezone viaretrieveScheduleTimezonefrom@trigger.dev/sdk.Bug Fixes
scheduled_actionswrite on a timezone-only update to prevent a 500 from an empty DB update; return the unchanged row instead.Written for commit f5a7d83. Summary will update on new commits.
Summary by CodeRabbit