Repository navigation
feat(calendar): scheduled LLM agent triggers - #862
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds calendar-event-driven scheduled agent work: DB schema for triggers, AI tools to schedule/cancel triggers, parsing utilities, a cron POST endpoint that claims and executes due triggers in batches, an execution engine that builds prompts and runs workflows, and tests/docs for the end-to-end flow. Changes
Sequence Diagram(s)sequenceDiagram
participant Cron as Cron Job
participant API as /api/cron/calendar-triggers
participant DB as Database
participant Exec as Executor
participant LLM as Workflow/LLM
Cron->>API: POST /api/cron/calendar-triggers
API->>DB: UPDATE running triggers older than 10m -> failed
API->>DB: SELECT pending triggers WHERE triggerAt <= now LIMIT 50
alt No due triggers
API-->>Cron: { executed: 0 }
else Triggers due
loop For each batch (<=5)
API->>DB: UPDATE pending→running (atomic RETURNING)
DB-->>API: claimed triggers
API->>DB: SELECT calendarEvents for claimed triggers
par For each claimed trigger
API->>Exec: executeCalendarTrigger(trigger, event)
Exec->>DB: incrementUsage(scheduledById)
alt Usage denied
Exec->>DB: UPDATE trigger -> failed
Exec-->>API: result failure
else Usage allowed
Exec->>DB: SELECT attendees, instruction page
Exec->>LLM: executeWorkflow(synthetic workflow)
LLM-->>Exec: { success, conversationId?, error? }
Exec->>DB: UPDATE trigger (completed/failed, conversationId, duration)
Exec-->>API: result
end
end
API->>DB: aggregate results / persist failures for exceptions
end
API-->>Cron: { executed: N, total: M, errors?: [...] }
end
sequenceDiagram
participant User as Caller
participant Tool as schedule_agent_work
participant Validate as Validators
participant DB as Database
participant Broadcast as Broadcaster
User->>Tool: schedule_agent_work(input)
Tool->>Tool: require authenticated user
Tool->>Tool: normalize timezone
Tool->>Validate: require prompt OR instructionPageId
Tool->>Validate: verify drive membership for driveId
Tool->>DB: load agent page, verify type/drive/access
alt instructionPageId
Tool->>DB: load instruction page, verify access/not trashed
end
Tool->>Tool: parseDateTime(triggerAt) → must be future
Tool->>DB: BEGIN transaction
Tool->>DB: INSERT calendar_event
Tool->>DB: INSERT calendar_trigger
Tool->>DB: UPDATE calendar_event.metadata with triggerId
Tool->>DB: COMMIT
Tool->>Broadcast: broadcastCalendarEvent(created event)
Tool-->>User: { triggerId, eventId, scheduledFor, agentName }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/lib/ai/tools/calendar-read-tools.ts`:
- Around line 128-134: The response currently includes triggerInfo.agentPageId
inside the scheduledWork object (see scheduledWork, triggerInfo,
fetchTriggerInfoForEvents, formatEventForResponse), which can leak cross-drive
page IDs; remove agentPageId from the returned scheduledWork or conditionally
include it only after calling the centralized permission check (use the
permission helpers from packages/lib/src/permissions/) to verify the caller can
view that page before adding agentPageId; update both occurrences (lines around
the scheduledWork block and the other instance noted) to either omit agentPageId
entirely or wrap its inclusion in the centralized permission function, ensuring
no custom permission logic is introduced.
In `@apps/web/src/lib/ai/tools/calendar-trigger-tools.ts`:
- Around line 237-273: The cancel flow has a race: the select-then-update can be
lost to the cron flipping status to "running"; change cancel routine to perform
an atomic transactional update guarded by status='pending' (and ensure
scheduledById===userId) against calendarTriggers and, in the same transaction,
update calendarEvents (isTrashed/trashedAt/updatedAt); check the affected-rows
count from the guarded update—if zero, do a fresh SELECT of calendarTriggers by
triggerId to read and return the current status/error (e.g., already
running/completed/failed/cancelled or not found); use the existing db,
calendarTriggers, calendarEvents, triggerId and userId symbols and ensure
completedAt is set when marking cancelled.
In `@apps/web/src/lib/workflows/calendar-trigger-executor.ts`:
- Around line 121-131: The current code reads instructionPage by
trigger.instructionPageId without verifying that trigger.scheduledById still has
access; update the logic in calendar-trigger-executor.ts to either (A) snapshot
the instruction page content at schedule time and store it on the trigger so
execution uses the snapshot, or (B) before pushing instructionPage into parts at
runtime, call the centralized permission check from packages/lib/src/permissions
(do not implement custom checks) to confirm trigger.scheduledById can read the
page (use pages and instructionPage identifiers) and only include
instructionPage.content if that permission check passes; make sure to reference
trigger.scheduledById, instructionPage, pages and avoid adding ad-hoc access
checks.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: b7d835d4-413f-4503-b968-02bf9172c0c7
📒 Files selected for processing (17)
apps/web/src/app/api/cron/calendar-triggers/route.tsapps/web/src/lib/ai/core/__tests__/timestamp-utils.test.tsapps/web/src/lib/ai/core/ai-tools.tsapps/web/src/lib/ai/core/timestamp-utils.tsapps/web/src/lib/ai/core/tool-filtering.tsapps/web/src/lib/ai/tools/calendar-read-tools.tsapps/web/src/lib/ai/tools/calendar-trigger-tools.tsapps/web/src/lib/ai/tools/calendar-write-tools.tsapps/web/src/lib/workflows/calendar-trigger-executor.tsapps/web/src/lib/workflows/workflow-executor.tsdocker/cron/crontabdocs/1.0-overview/changelog.mdpackages/db/drizzle/0094_mysterious_vision.sqlpackages/db/drizzle/meta/0094_snapshot.jsonpackages/db/drizzle/meta/_journal.jsonpackages/db/src/schema.tspackages/db/src/schema/calendar-triggers.ts
Agents can now self-schedule future work by creating calendar events that fire LLM executions at the specified time. Any agent can schedule work for itself or another agent (respecting RBAC). Triggers consume from the existing daily AI call rate limits for cost control. - New `calendarTriggers` table + CalendarTriggerStatus enum - `schedule_agent_work` + `cancel_scheduled_work` AI tools - Cron endpoint polling every 2 min with atomic claim pattern - Executor adapter reuses existing workflow-executor - Calendar read tools surface trigger status on events Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…arseDateTime P1 Security: - Enforce drive access on agent and instruction pages in schedule_agent_work - Restrict cancel_scheduled_work to scheduledById (not any drive member) - Sync trigger rows with calendar event mutations (update/delete) - Cron skips triggers whose events are trashed P2 Reliability: - Eliminate stale 'claimed' state — claim directly to 'running' - Add LIMIT 50 to cron discovery query P3 Correctness: - Return conversationId from executeWorkflow so trigger stores actual value - Wrap event+trigger+metadata creation in db.transaction() - Extract parseDateTime to shared timestamp-utils (eliminates duplication) Tests & Docs: - Add parseDateTime unit tests to timestamp-utils.test.ts - Update changelog with feature entry Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ixes 1. Remove agentPageId from scheduledWork response to prevent cross-drive page ID leaks (calendar-read-tools.ts) 2. Reject personal pages (null driveId) for agent, instruction, and context pages. Validate all contextPageIds against accessible drives (calendar-trigger-tools.ts) 3. Make cancel_scheduled_work atomic: single UPDATE with WHERE status='pending' AND scheduledById=userId guard in a transaction, eliminating the read-then-write race with cron (calendar-trigger-tools.ts) 4. Re-check instruction page drive access at execution time in buildTriggerPrompt — if access was revoked after scheduling, the page content is silently excluded (calendar-trigger-executor.ts) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/web/src/lib/workflows/calendar-trigger-executor.ts (1)
109-118: Consider privacy implications of including attendee emails in prompts.The attendee query selects both
nameand♻️ Suggested change to avoid email fallback
if (attendees.length > 0) { - parts.push(`Attendees: ${attendees.map(a => a.name || a.email).join(', ')}`); + parts.push(`Attendees: ${attendees.filter(a => a.name).map(a => a.name).join(', ')}`); }Alternatively, if some attendee identification is necessary when names are absent, consider using a generic placeholder like "unnamed attendee".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/lib/workflows/calendar-trigger-executor.ts` around lines 109 - 118, The attendees query currently selects name and email and the parts.push uses attendees.map(a => a.name || a.email), which can leak emails into prompts; modify the query and formatting to avoid returning or using emails: select only users.name (remove users.email from the .select on eventAttendees/users), filter/skip records with no name or map missing names to a generic placeholder like "unnamed attendee" instead of falling back to a.email, and update the parts.push to join only the safe name/placeholder values (references: attendees variable, eventAttendees, users, and the parts.push mapping expression).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/web/src/lib/workflows/calendar-trigger-executor.ts`:
- Around line 109-118: The attendees query currently selects name and email and
the parts.push uses attendees.map(a => a.name || a.email), which can leak emails
into prompts; modify the query and formatting to avoid returning or using
emails: select only users.name (remove users.email from the .select on
eventAttendees/users), filter/skip records with no name or map missing names to
a generic placeholder like "unnamed attendee" instead of falling back to
a.email, and update the parts.push to join only the safe name/placeholder values
(references: attendees variable, eventAttendees, users, and the parts.push
mapping expression).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f04e540f-00a9-43d7-958a-0ff6af9b32f4
📒 Files selected for processing (3)
apps/web/src/lib/ai/tools/calendar-read-tools.tsapps/web/src/lib/ai/tools/calendar-trigger-tools.tsapps/web/src/lib/workflows/calendar-trigger-executor.ts
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
apps/web/src/lib/ai/tools/calendar-read-tools.ts (1)
162-166: Multiple triggers per recurring event will show only one status.The map overwrites entries by
calendarEventId, so recurring events with multiple trigger rows (differentoccurrenceDatevalues) will only display the last-processed trigger's status. This may be acceptable for the current UI, but consider returning the "most relevant" trigger (e.g., the next pending one) if users need to see upcoming occurrence statuses.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/lib/ai/tools/calendar-read-tools.ts` around lines 162 - 166, The current loop in calendar-read-tools.ts builds a Map keyed by calendarEventId and blindly overwrites entries, losing other triggers for recurring events; update the logic in the loop that populates map (the map variable, the triggers array, and usage of calendarEventId) to select and keep the most relevant trigger per calendarEventId (e.g., prefer the next pending occurrence: compare occurrenceDate and status of the existing entry vs the incoming trigger and only replace when the incoming trigger is a closer upcoming pending occurrence or otherwise more relevant). Ensure you compare the trigger.occurrenceDate and trigger.status when deciding to call map.set so the Map retains the chosen triggerId/status rather than the last-seen row.apps/web/src/lib/ai/tools/calendar-write-tools.ts (1)
509-519: Trigger cancellation on event delete is correct but not transactional with the soft-delete.The cancellation correctly uses atomic
WHERE status='pending'to avoid racing with cron. However, the trigger cancellation (lines 509-519) and event soft-delete (lines 521-529) are separate operations without a transaction wrapper.If the soft-delete fails after trigger cancellation succeeds, the trigger would be
cancelledwhile the event remains active. This is low-risk since:
- The cron already skips trashed events
- The cancelled trigger won't execute regardless
Consider wrapping in a transaction if stricter consistency is needed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/lib/ai/tools/calendar-write-tools.ts` around lines 509 - 519, Wrap the trigger-cancellation and the event soft-delete in a single database transaction so both succeed or both roll back together: when detecting event.metadata as CalendarTriggerMetadata (eventMeta) and cancelling pending triggers via db.update(calendarTriggers).set(...).where(...), perform that update and the subsequent soft-delete update on the calendar event inside a single db transaction (use the existing db.transaction/transactional API) using the same eventId/calendarEventId and status='pending' predicates so either both operations commit or both are rolled back on error.apps/web/src/app/api/cron/calendar-triggers/route.ts (1)
7-8: Stuck trigger timeout could mark legitimately running triggers as failed.The 10-minute timeout resets triggers that are still in
runningstate. Per context snippet 1,executeCalendarTriggerhas no internal timeout and relies on AI provider response times. If an AI call legitimately takes >10 minutes:
- The next cron run marks it
failed- The original execution completes and updates to
completed/failed, overwriting the stuck-timeout statusThis creates a race where the final status depends on timing. Consider either:
- Increasing the timeout for AI workloads
- Adding a check in
executeCalendarTriggerto skip the final update if status is no longerrunningAlso applies to: 19-33
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/api/cron/calendar-triggers/route.ts` around lines 7 - 8, The current STUCK_TRIGGER_TIMEOUT_MS (10 minutes) can mark legitimately long-running executeCalendarTrigger executions as failed; either raise STUCK_TRIGGER_TIMEOUT_MS to a much larger value for AI workloads, or (preferred) make executeCalendarTrigger resilient by reading the trigger's latest status before writing its final state and only perform the completion update if the stored status is still "running" (i.e., add a pre-update check inside executeCalendarTrigger to skip updating when status !== "running"); apply the same guard where the cron resets stuck runs (lines referenced around 19-33) so you don't overwrite a true final result.apps/web/src/lib/ai/tools/calendar-trigger-tools.ts (1)
139-147: Consider clock skew tolerance for "must be in the future" check.The check
parsedTriggerAt <= new Date()may reject valid requests if there's minor clock drift between client and server, or if processing delays pushnew Date()past a very-near-futuretriggerAt. Consider a small tolerance (e.g., 30 seconds) for borderline cases:-if (parsedTriggerAt <= new Date()) { +const MIN_FUTURE_BUFFER_MS = 30_000; // 30 seconds +if (parsedTriggerAt.getTime() <= Date.now() + MIN_FUTURE_BUFFER_MS) {This also prevents scheduling work that would immediately execute before the cron even picks it up.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/lib/ai/tools/calendar-trigger-tools.ts` around lines 139 - 147, The current future-check using parsedTriggerAt <= new Date() is too strict; introduce a small clock-skew/processing tolerance (e.g., const FUTURE_TOLERANCE_MS = 30_000) and change the condition to reject only when parsedTriggerAt.getTime() <= Date.now() + FUTURE_TOLERANCE_MS so near-term times within the tolerance are treated as valid; update the check around parseDateTime/parsedTriggerAt/triggerAt accordingly and keep the same error response when the adjusted check fails.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/lib/ai/core/timestamp-utils.ts`:
- Around line 226-233: The chrono parsing uses a single fixed offset from
ref.instant which breaks across DST; update the logic to follow the two-pass
pattern used in parseNaiveDatetimeInTimezone: (1) build ref with initial
timezone = getTimezoneOffsetMinutes(timezone, ref.instant) and call
chrono.parseDate(input, ref, { forwardDate: true }); (2) if a parsed Date is
returned, recompute newOffset = getTimezoneOffsetMinutes(timezone, parsed) and
if newOffset !== ref.timezone set ref.timezone = newOffset and call
chrono.parseDate(input, ref, { forwardDate: true }) again so the final result
uses the correct offset. Ensure you reference ref, timezone,
getTimezoneOffsetMinutes, chrono.parseDate and parsed in the change.
- Around line 216-222: Handle naive ISO datetimes before falling back to native
Date and only accept strict ISO/date-only/explicit-offset formats for native
parsing: when timezone is provided, check isNaiveISODatetime(input) first and
call parseNaiveDatetimeInTimezone(input, timezone) if true; otherwise only allow
new Date(input) to be returned when input matches a safe ISO date-only or
ISO-with-offset pattern (reject loose formats like "2024-06-15 14:00" so
timezone is not silently ignored). Update the logic around the isoDate/new
Date(input) branch in timestamp-utils.ts to enforce this ordering and pattern
check.
In `@packages/db/drizzle/0095_mysterious_vision.sql`:
- Around line 24-27: The migration's UNIQUE constraint
"calendar_triggers_event_occurrence_key" on columns calendarEventId and
occurrenceDate allows multiple NULL occurrenceDate values and thus duplicate
one-shot triggers; fix this in the calendar-triggers schema source
(calendar-triggers.ts) by changing the constraint to enforce uniqueness only
when occurrenceDate IS NOT NULL (e.g., use a partial/conditional unique index or
Drizzle's conditional constraint on the calendar_triggers table so uniqueness
applies for non-NULL occurrenceDate), or alternatively add application-level
validation to prevent NULL duplicates, then run pnpm db:generate to regenerate
the SQL migration.
In `@packages/db/drizzle/meta/0094_snapshot.json`:
- Around line 888-893: The snapshot incorrectly includes a resurrected nullable
column "password" for table public.users; remove the "password" entry from the
snapshot JSON (the object where "name": "password") so the snapshot matches the
intended schema, then regenerate the snapshot from your current intended DB
schema and commit the regenerated snapshot (ensuring public.users no longer
contains a password key) to avoid a bogus add-column migration in the next
generated migration.
- Around line 11908-11913: The unique constraint on (calendarEventId,
occurrenceDate) fails to dedupe one-shot triggers because occurrenceDate is
nullable and inserts leave it NULL; update the schema to ensure NULLs are
treated consistently by either: 1) make occurrenceDate NOT NULL and require
callers to supply a value (change occurrenceDate definition and callers that
insert into that table), or 2) set a deterministic sentinel timestamp for
one-shot triggers at insert time (modify the insert path to populate
occurrenceDate for one-shot events), or 3) alter the unique constraint to use
NULLS NOT DISTINCT (Postgres 15+) so NULL occurrenceDate values are considered
equal; pick one approach and update the table definition and any insert/update
logic that references occurrenceDate and calendarEventId accordingly.
---
Nitpick comments:
In `@apps/web/src/app/api/cron/calendar-triggers/route.ts`:
- Around line 7-8: The current STUCK_TRIGGER_TIMEOUT_MS (10 minutes) can mark
legitimately long-running executeCalendarTrigger executions as failed; either
raise STUCK_TRIGGER_TIMEOUT_MS to a much larger value for AI workloads, or
(preferred) make executeCalendarTrigger resilient by reading the trigger's
latest status before writing its final state and only perform the completion
update if the stored status is still "running" (i.e., add a pre-update check
inside executeCalendarTrigger to skip updating when status !== "running"); apply
the same guard where the cron resets stuck runs (lines referenced around 19-33)
so you don't overwrite a true final result.
In `@apps/web/src/lib/ai/tools/calendar-read-tools.ts`:
- Around line 162-166: The current loop in calendar-read-tools.ts builds a Map
keyed by calendarEventId and blindly overwrites entries, losing other triggers
for recurring events; update the logic in the loop that populates map (the map
variable, the triggers array, and usage of calendarEventId) to select and keep
the most relevant trigger per calendarEventId (e.g., prefer the next pending
occurrence: compare occurrenceDate and status of the existing entry vs the
incoming trigger and only replace when the incoming trigger is a closer upcoming
pending occurrence or otherwise more relevant). Ensure you compare the
trigger.occurrenceDate and trigger.status when deciding to call map.set so the
Map retains the chosen triggerId/status rather than the last-seen row.
In `@apps/web/src/lib/ai/tools/calendar-trigger-tools.ts`:
- Around line 139-147: The current future-check using parsedTriggerAt <= new
Date() is too strict; introduce a small clock-skew/processing tolerance (e.g.,
const FUTURE_TOLERANCE_MS = 30_000) and change the condition to reject only when
parsedTriggerAt.getTime() <= Date.now() + FUTURE_TOLERANCE_MS so near-term times
within the tolerance are treated as valid; update the check around
parseDateTime/parsedTriggerAt/triggerAt accordingly and keep the same error
response when the adjusted check fails.
In `@apps/web/src/lib/ai/tools/calendar-write-tools.ts`:
- Around line 509-519: Wrap the trigger-cancellation and the event soft-delete
in a single database transaction so both succeed or both roll back together:
when detecting event.metadata as CalendarTriggerMetadata (eventMeta) and
cancelling pending triggers via db.update(calendarTriggers).set(...).where(...),
perform that update and the subsequent soft-delete update on the calendar event
inside a single db transaction (use the existing db.transaction/transactional
API) using the same eventId/calendarEventId and status='pending' predicates so
either both operations commit or both are rolled back on error.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 13f48142-a161-44b7-a9cf-4b5869ec8f1c
📒 Files selected for processing (18)
apps/web/src/app/api/cron/calendar-triggers/route.tsapps/web/src/lib/ai/core/__tests__/timestamp-utils.test.tsapps/web/src/lib/ai/core/ai-tools.tsapps/web/src/lib/ai/core/timestamp-utils.tsapps/web/src/lib/ai/core/tool-filtering.tsapps/web/src/lib/ai/tools/calendar-read-tools.tsapps/web/src/lib/ai/tools/calendar-trigger-tools.tsapps/web/src/lib/ai/tools/calendar-write-tools.tsapps/web/src/lib/workflows/calendar-trigger-executor.tsapps/web/src/lib/workflows/workflow-executor.tsdocker/cron/crontabdocs/1.0-overview/changelog.mdpackages/db/drizzle/0095_mysterious_vision.sqlpackages/db/drizzle/meta/0094_snapshot.jsonpackages/db/drizzle/meta/0095_snapshot.jsonpackages/db/drizzle/meta/_journal.jsonpackages/db/src/schema.tspackages/db/src/schema/calendar-triggers.ts
✅ Files skipped from review due to trivial changes (5)
- docker/cron/crontab
- packages/db/src/schema.ts
- packages/db/drizzle/meta/_journal.json
- apps/web/src/lib/ai/core/tests/timestamp-utils.test.ts
- docs/1.0-overview/changelog.md
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/web/src/lib/ai/core/ai-tools.ts
- apps/web/src/lib/workflows/workflow-executor.ts
- apps/web/src/lib/ai/core/tool-filtering.ts
- apps/web/src/lib/workflows/calendar-trigger-executor.ts
The CI test checks that pageSpaceTools equals the merged object of all tool modules. Add the new calendarTriggerTools mock and import so the assertion matches the updated ai-tools.ts. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1. parseDateTime: check naive ISO before native Date fallback, add two-pass DST resolution for chrono natural language parsing 2. occurrenceDate: make NOT NULL with epoch sentinel for one-shot events so the unique constraint actually deduplicates (NULL != NULL in PG) 3. Regenerate migration as 0095_tiresome_iron_man.sql with correct snapshot (no stale users.password column) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…apshot The previous migration only had ALTER TABLE statements because the snapshot already contained the table from a prior generation cycle. After rebase, the CREATE TABLE was lost. Regenerated from master's 0094 snapshot so Drizzle produces the full CREATE TABLE + indexes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add 18 unit tests for calendar-trigger-tools (schedule validation, cancel race conditions, cross-drive access denial, error leakage) - Add 11 unit tests for calendar-trigger-executor (rate limits, access rechecking, prompt building, workflow delegation) - Add 7 unit tests for cron calendar-triggers route (auth, claiming, trashed events, error handling) - Wrap tool throw statements with generic messages to prevent internal DB error details from leaking to AI/user (details still logged server-side) - Document claimed enum status as reserved for future recurring triggers Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…s test Removes mockSelectFrom and mockSelectWhere that were declared but never referenced, fixing ESLint no-unused-vars errors in CI. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/lib/ai/tools/calendar-trigger-tools.ts`:
- Around line 207-214: The post-commit broadcast via broadcastCalendarEvent is
currently awaited and will throw if it fails, causing callers to potentially
retry non-idempotent scheduling; make this broadcast best-effort by wrapping the
broadcastCalendarEvent call in a try/catch (referencing broadcastCalendarEvent
and the payload using event.id, driveId, userId, operation:'created') and on
error log the failure (with context) but do not rethrow; alternatively you may
dispatch it fire-and-forget (no await) after logging, but ensure errors are not
allowed to bubble up to callers.
- Around line 311-318: The broadcastCalendarEvent call after the cancellation
commit should not turn a successful cancel into a reported failure; change the
code around broadcastCalendarEvent (the call that uses
cancelled.calendarEventId, cancelled.driveId, operation 'deleted') to run
asynchronously without bubbling errors — e.g. await it inside a try/catch and
log any websocket/broadcast errors (do not rethrow), or fire-and-forget the
broadcast with .catch(...) to swallow/log errors; ensure the cancellation flow
(the transaction that produced cancelled) still returns success even if
broadcast fails.
- Around line 117-135: The code currently queries pages with
inArray(contextPageIds) but never rejects IDs that don't exist, allowing
nonexistent contextPageIds to be stored; after fetching ctxPages (the result of
db.select(...).where(inArray(pages.id, contextPageIds))), compare the set of
returned ids (ctxPages.map(p => p.id)) to the original contextPageIds and if any
ids are missing return { success: false, error: `Context page(s) not found:
${missingIds.join(', ')}` }; keep the existing per-page checks (cp.isTrashed,
cp.driveId, isUserDriveMember) but perform this missing-ID validation
immediately after obtaining ctxPages to reject unknown IDs.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 67b7d0a4-08f3-40cc-9bbd-b1bb152bbfec
📒 Files selected for processing (10)
apps/web/src/app/api/cron/calendar-triggers/__tests__/route.test.tsapps/web/src/lib/ai/core/__tests__/ai-tools.test.tsapps/web/src/lib/ai/core/timestamp-utils.tsapps/web/src/lib/ai/tools/__tests__/calendar-trigger-tools.test.tsapps/web/src/lib/ai/tools/calendar-trigger-tools.tsapps/web/src/lib/workflows/__tests__/calendar-trigger-executor.test.tspackages/db/drizzle/0095_blushing_firelord.sqlpackages/db/drizzle/meta/0095_snapshot.jsonpackages/db/drizzle/meta/_journal.jsonpackages/db/src/schema/calendar-triggers.ts
✅ Files skipped from review due to trivial changes (2)
- apps/web/src/lib/ai/tools/tests/calendar-trigger-tools.test.ts
- packages/db/drizzle/meta/_journal.json
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/src/lib/ai/core/timestamp-utils.ts
CodeRabbit round-3 fixes: - Reject unknown contextPageIds (IDs not found in DB) - Make schedule and cancel broadcasts best-effort (try/catch, log errors) Test TypeScript fixes: - Replace banned `as Function`/`as any` casts with @ts-expect-error and try/catch patterns that satisfy strict ESLint rules - Regenerated migration 0095 as full CREATE TABLE from master's snapshot Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
CI's full typecheck generates proper types, making the directive unnecessary and causing TS2578. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Scheduling is a property of a calendar event, not a separate action. Remove standalone schedule_agent_work and cancel_scheduled_work tools (38 tools now, down from 40). Add optional `agentTrigger` param to create_calendar_event instead. Cancel = delete_calendar_event (already wired to cancel triggers). Reschedule = update_calendar_event (already wired to sync triggerAt). All backend infrastructure unchanged (calendarTriggers table, cron poller, executor, migration). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…x changelog Address review findings: add 11 tests for agentTrigger creation validation, trigger sync on update, trigger cancel on delete, and execution-time drive access. Fix changelog to reference actual shipped API (agentTrigger param, not removed schedule_agent_work tool). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ss fixes - Reject recurrence + agentTrigger combination (one trigger row can't serve multiple occurrences; recurring support is a future feature) - Restrict context pages to same drive (cross-drive pages are silently dropped by executeWorkflow, so reject at schedule time) - Cancel running/claimed triggers on event delete (not just pending), closing the race between cron claim and user deletion - Add agent page preflight before consuming usage credit, so deleted agents don't waste the scheduler's daily AI call budget Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Merge origin/master — both the calendar-trigger cron (this branch) and the orphaned-file-cleanup cron (#863) are kept. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
agentTriggerparameter oncreate_calendar_event— any agent can schedule work for itself or another agent (respecting RBAC)delete_calendar_event— trashing a trigger-linked event automatically cancels pending/claimed/running triggerscalendarTriggerstable tracks execution state (pending → running → completed/failed/cancelled)executeWorkflow()— no duplicated infrastructureparseDateTime()extracted totimestamp-utils.ts(was duplicated in write-tools)Security hardening (5 review rounds)
Changed files
packages/db/src/schema/calendar-triggers.tsapps/web/src/lib/workflows/calendar-trigger-executor.tsexecuteWorkflow()with preflight checksapps/web/src/app/api/cron/calendar-triggers/route.tsapps/web/src/lib/ai/tools/calendar-write-tools.tsagentTriggerparam oncreate_calendar_event, trigger sync/cancel on update/deleteapps/web/src/lib/ai/tools/calendar-read-tools.tsapps/web/src/lib/ai/core/timestamp-utils.tsparseDateTime+ timezone utilitiesTest plan
pnpm db:generatesucceeds (migration:0095_blushing_firelord.sql)pnpm --filter web build/ typecheck passes with zero errorspnpm db:migrateapplies cleanly against dev database🤖 Generated with Claude Code