Repository navigation
feat(agents): dedicated trigger tools for calendar and task scheduling - #1615
Conversation
… task scheduling Agents were failing at calendar trigger setup because the only path was a nested agentTrigger param buried inside create_calendar_event — nothing in the tool name indicated it was the scheduling primitive. Task triggers had no AI tool at all (REST-only). Add four explicit, intention-first tools: - set_calendar_trigger: attach to existing event OR create new backing event - delete_calendar_trigger: remove trigger without deleting the event - set_task_trigger: upsert due_date/completion trigger on an existing task - delete_task_trigger: disable a task trigger All four reuse the existing service layer (upsertCalendarTriggerWorkflow, createTaskTriggerWorkflow, removeCalendarTrigger, recomputeTaskTriggerMetadata) and the shared agentTriggerBaseSchema validation. Update create_workflow description to point to the new tools instead of the buried agentTrigger params. 20 unit tests, typecheck clean. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
More reviews will be available in 12 minutes and 38 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis PR introduces four new AI tools ( ChangesAgent Trigger Tools Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7b6d1889ee
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!(await canUserEditPage(userId, taskListPageId))) { | ||
| return { success: false, error: 'You do not have edit access to this task list.' }; | ||
| } |
There was a problem hiding this comment.
When this tool runs from an agent chat or a drive-scoped MCP token, canUserEditPage(userId, ...) checks only the underlying user's rights and bypasses the agent ACL, MCP drive scope, and app-role ceiling. Consequently, an actor that cannot edit the task list through update_task can still create a deferred workflow there; delete_task_trigger repeats the same bypass at line 352. Use the established canActorEditPage(ctx, taskListPageId) check used by resolveTaskForUpdate for both operations.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in commit 510f76b. Both set_task_trigger (line 284) and delete_task_trigger (line 351) now use canActorEditPage(ctx, taskListPageId) from actor-permissions, matching the pattern used by all other tools in this codebase. canUserEditPage is no longer imported in trigger-tools.ts.
…ed-vars Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Agents sometimes try to configure triggers via update_agent_config. The updated description now explicitly directs them to set_calendar_trigger, set_task_trigger, or create_workflow instead. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…trigger Updated create_calendar_event and update_calendar_event descriptions to steer agents toward the new dedicated set_calendar_trigger tool. The agentTrigger param remains for UI-driven creation workflows. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…k_trigger Adds a note to the update_task Agent Triggers section steering agents toward the dedicated set_task_trigger tool for attaching triggers to existing tasks, consistent with how calendar-write-tools was updated. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…n test - registry.tsx: add ActionResultRenderer entries for set_calendar_trigger, delete_calendar_trigger, set_task_trigger, delete_task_trigger — required by the registry-coverage guard test - ai-tools.test.ts: mock + import + spread triggerTools so the aggregation test matches the actual pageSpaceTools shape Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replaces canUserEditPage(userId, ...) with canActorEditPage(ctx, ...) in set_task_trigger and delete_task_trigger so that agent ACL, MCP drive scope, and app-role ceiling are enforced — matching the pattern used by all other tool permission checks. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
set_task_trigger and delete_task_trigger now call driveDeniedByAppToken before canActorEditPage, matching the pattern used by calendar trigger tools. Also removes dead agent-trigger-shared mock and unused z import from test file. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The four new trigger tools (set/delete_calendar_trigger, set/delete_task_trigger) were missing from WRITE_TOOLS, so they would have appeared in read-only agent contexts. Adds them to the set and adds corresponding test coverage. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
vi.clearAllMocks() clears call history but not mock implementations. The new token-denial test at the end of set_task_trigger left mockDriveDeniedByAppToken returning true, which bled into all delete_task_trigger tests. Explicitly reset to false in each task trigger describe block's beforeEach. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
task.dueDate!.toISOString() was logically safe (guarded above) but TypeScript can't narrow through a ternary expression. Use optional chaining with fallback to eliminate the assertion. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![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
🤖 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 `@apps/web/src/lib/ai/tools/__tests__/trigger-tools.test.ts`:
- Around line 92-95: The test is mocking the wrong module path so
parseDateTime() and normalizeTimezone() used by trigger-tools.ts are not being
controlled; update the vi.mock call in trigger-tools.test.ts to mock the exact
module path that trigger-tools.ts imports (or add an additional vi.mock for that
import) and return mocked implementations for parseDateTime and
normalizeTimezone (the same vi.fn stubs currently used) so that the functions
used by the TriggerTools logic are intercepted during tests; locate usages of
parseDateTime/normalizeTimezone and the import in trigger-tools.ts to verify the
module specifier you need to mock.
In `@apps/web/src/lib/ai/tools/trigger-tools.ts`:
- Around line 364-371: The update to taskTriggers and the subsequent
recomputeTaskTriggerMetadata call must be made atomic so we don't leave rows
disabled while metadata still reflects the old trigger; modify the code around
db.update(taskTriggers)... to perform both the .update(...) and
recomputeTaskTriggerMetadata(...) inside the same transaction (use
db.transaction(...) or pass the transaction/tx object into
recomputeTaskTriggerMetadata so it runs within the same tx), then only call
broadcastTaskEvent after the transaction commits; reference taskTriggers,
recomputeTaskTriggerMetadata, broadcastTaskEvent and ensure
recomputeTaskTriggerMetadata accepts/uses the same tx or is wrapped in the
db.transaction block.
- Around line 145-147: Wrap normalization/parsing of timezone and triggerAt in a
try/catch so malformed inputs return the file’s tool error shape instead of
throwing: call normalizeTimezone(tzInput ?? ctx.timezone) and
parseDateTime(triggerAt, ...) inside a try block (referencing tzInput,
ctx.timezone, triggerAt, parseDateTime, normalizeTimezone, parsedAt, endAt) and
on any exception return { success: false, error: "Invalid trigger time or
timezone" } (or the module's existing validation error helper) rather than
letting the exception escape; only compute endAt = new Date(parsedAt.getTime() +
60*60*1000) after successful parse.
🪄 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: c9f0b4c2-d5fb-4321-b7d3-684cb660112b
📒 Files selected for processing (11)
apps/web/src/components/ai/shared/chat/tool-calls/registry.tsxapps/web/src/lib/ai/core/__tests__/ai-tools.test.tsapps/web/src/lib/ai/core/__tests__/tool-filtering.test.tsapps/web/src/lib/ai/core/ai-tools.tsapps/web/src/lib/ai/core/tool-filtering.tsapps/web/src/lib/ai/tools/__tests__/trigger-tools.test.tsapps/web/src/lib/ai/tools/agent-tools.tsapps/web/src/lib/ai/tools/calendar-write-tools.tsapps/web/src/lib/ai/tools/task-management-tools.tsapps/web/src/lib/ai/tools/trigger-tools.tsapps/web/src/lib/ai/tools/workflow-tools.ts
…uard - Wrap delete_task_trigger's disable+recompute in db.transaction() so the trigger state and task metadata stay consistent on partial failure - Fix test mock path: '../core/timestamp-utils' → '../../core/timestamp-utils' - Add mockDbTransaction call-through in delete_task_trigger beforeEach Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… tools Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
Agents were consistently failing to schedule triggers because the tool surfaces were named and structured for implementation intent, not agent intent:
Root cause: Three surfaces, three discovery anti-patterns. Only cron (`create_workflow`) was correctly named for its purpose.
Changes
New file: `trigger-tools.ts` — 4 intention-named, flat-schema tools:
All tools reuse existing service layer helpers — no new DB logic.
Permissions — full actor-scoped permission model throughout:
Description redirects — 4 existing tools now steer agents to the dedicated trigger tools:
Registry coverage — 4 new `ActionResultRenderer` entries in `registry.tsx` and corresponding mock/import/spread entries in `ai-tools.test.ts`.
Test plan