Repository navigation
feat(ai): set_home_page tool — agents can set/clear a drive's landing page - #1644
Conversation
… page Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 23 minutes and 20 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ 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 (5)
📝 WalkthroughWalkthroughAdds a new AI tool ChangesDrive Home Page Tool Implementation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b70f397fe2
ℹ️ 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".
| } | ||
|
|
||
| try { | ||
| const result = await getDriveAccessWithDrive(driveId, userId); |
There was a problem hiding this comment.
Gate page agents by their own drive membership
When this tool is invoked by a page agent, userId identifies the triggering user, so this check authorizes using that user's owner/admin status and ignores the agent's drive membership. An enabled agent with no membership can therefore change the home page of any drive administered by its caller if given the drive ID, while a valid member agent invoked by a non-admin is incorrectly denied. Use the actor-aware canActorManageDrive gate in actor-permissions.ts, which explicitly applies agent membership and MCP ceilings.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in cb1dde0: replaced the manual driveDeniedByAppToken + getDriveAccessWithDrive + owner/admin check with canActorManageDrive, which properly gates page-agent callers by their drive membership (via hasAgentDriveMembership) rather than the triggering user's role.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/web/src/lib/ai/core/tool-filtering.ts (1)
21-21: ⚡ Quick winAdd a direct read-only filter assertion for
set_home_page.This classification is correct; add an explicit unit assertion in
apps/web/src/lib/ai/core/__tests__/tool-filtering.test.tsto prevent regressions for this tool specifically.🤖 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 `@apps/web/src/lib/ai/core/tool-filtering.ts` at line 21, Add an explicit unit assertion in apps/web/src/lib/ai/core/__tests__/tool-filtering.test.ts that verifies the tool 'set_home_page' is classified as read-only by the filtering logic: locate the test for tool filtering (e.g., the test that calls the filterTools/filterToolList or similar helper) and add an assertion like expect(filteredTools).toContainEqual(expect.objectContaining({name: 'set_home_page', access: 'read'})) or the project's equivalent matcher to assert read-only classification for 'set_home_page' so this specific tool cannot regress.
🤖 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.
Nitpick comments:
In `@apps/web/src/lib/ai/core/tool-filtering.ts`:
- Line 21: Add an explicit unit assertion in
apps/web/src/lib/ai/core/__tests__/tool-filtering.test.ts that verifies the tool
'set_home_page' is classified as read-only by the filtering logic: locate the
test for tool filtering (e.g., the test that calls the
filterTools/filterToolList or similar helper) and add an assertion like
expect(filteredTools).toContainEqual(expect.objectContaining({name:
'set_home_page', access: 'read'})) or the project's equivalent matcher to assert
read-only classification for 'set_home_page' so this specific tool cannot
regress.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 72931d44-64f2-4742-9959-e1f4de25bb4a
📒 Files selected for processing (2)
apps/web/src/lib/ai/core/tool-filtering.tsapps/web/src/lib/ai/tools/drive-tools.ts
Pins the read-only filtering regression guard requested in code review. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2witstudios
left a comment
There was a problem hiding this comment.
Added in a471308: new test in tool-filtering.test.ts asserts isWriteTool('set_home_page') is true and that set_home_page is excluded from read-only mode via filterToolsForReadOnly.
Required by the registry-coverage test which asserts every AI tool has a rich renderer. Adds ActionResultRenderer entry + display label. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Covers tool definition shape and auth guard, matching the pattern for list_drives / create_drive / rename_drive. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…test
The describe was inserted after the outer describe's closing }), creating
a syntax error. Move it inside the outer describe('drive-tools') block.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…home_page - Replace manual driveDeniedByAppToken + getDriveAccessWithDrive + owner/admin check with canActorManageDrive, which correctly gates page-agent callers by their drive membership rather than the triggering user's role. - Wrap broadcast and activity logging in try/catch so a side-effect failure does not return a false-negative on a successful state change. - Guard updateDrive null return with an explicit error. Fixes: Codex P1 (agent membership), CodeRabbit major (side-effect isolation). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
set_home_pagetodriveToolsinapps/web/src/lib/ai/tools/drive-tools.tsWRITE_TOOLSinapps/web/src/lib/ai/core/tool-filtering.tsWhat the tool does
Agents can now call
set_home_pageto designate any non-trashed page within a drive as its landing page. PassingnullforpageIdclears the home page (restoring the default page tree view).Authorization: owner or admin only (via
getDriveAccessWithDrive); scoped MCP tokens gated viadriveDeniedByAppToken.Validation:
isValidDriveHomePageconfirms the target page exists, belongs to the drive, and is not trashed — same check used byPATCH /api/drives/[driveId].Side effects: broadcasts
drive:updatedto all drive members + logs activity with before/afterhomePageIdvalues.Test plan
pageId: null) — drive reverts to default treeset_home_page(it's inWRITE_TOOLS)🤖 Generated with Claude Code
Summary by CodeRabbit