-
Notifications
You must be signed in to change notification settings - Fork 17
feat: added bookmarks tool and instructions in prompt #276
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
Greptile OverviewGreptile SummaryThis PR adds Key changes:
Bookmarks section improvements:
Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Client as AI Agent/MCP Client
participant Server as HTTP Server (Hono)
participant Tool as browser_update_bookmark Tool
participant Bridge as ControllerBridge (WebSocket)
participant Controller as BrowserOSController
participant Action as UpdateBookmarkAction
participant Adapter as BookmarkAdapter
participant Chrome as Chrome Bookmarks API
Client->>Server: Call browser_update_bookmark(bookmarkId, title?, url?)
Server->>Tool: Handler invoked with params
Tool->>Bridge: executeAction('updateBookmark', {id, title, url})
Bridge->>Controller: Send action request via WebSocket
Controller->>Action: Execute UpdateBookmarkAction
Action->>Action: Validate at least one field provided
Action->>Adapter: updateBookmark(id, changes)
Adapter->>Chrome: chrome.bookmarks.update(id, changes)
Chrome-->>Adapter: Updated bookmark node
Adapter-->>Action: Return updated bookmark
Action-->>Controller: Return {id, title, url}
Controller-->>Bridge: Action result
Bridge-->>Tool: Execution result
Tool->>Tool: Format response text
Tool-->>Server: Response with updated bookmark details
Server-->>Client: MCP response
|
Code ReviewFound 1 issue that needs attention: Schema Validation Mismatch in
|
| title: z.string().optional().describe('New title for the bookmark'), | |
| url: z.string().optional().describe('New URL for the bookmark'), | |
| windowId: z.number().optional().describe('Window ID for routing'), |
However, the extension-side schema requires a valid URL format using Zod's .url() validator:
BrowserOS-agent/apps/controller-ext/src/actions/bookmark/UpdateBookmarkAction.ts
Lines 13 to 15 in a674439
| title: z.string().optional().describe('New bookmark title'), | |
| url: z.string().url().optional().describe('New bookmark URL'), | |
| }) |
Impact: A user could call browser_update_bookmark with an invalid URL string (e.g., "not-a-valid-url"), which would:
- Pass server-side validation
- Be sent to the extension via WebSocket
- Fail at the extension level with a Zod validation error
This creates a poor user experience because validation errors occur in the wrong layer and with less helpful error messages.
Suggested fix: Update line 135 in apps/server/src/tools/controller-based/tools/bookmarks.ts to:
url: z.string().url().optional().describe('New URL for the bookmark'),This ensures validation happens as early as possible in the request pipeline and provides clearer error messages to users.
No description provided.