feat(mcp): add browser_set_geolocation tool #38645
Conversation
| import { defineTool } from './tool'; | ||
|
|
||
| const setGeolocation = defineTool({ | ||
| capability: 'core', |
There was a problem hiding this comment.
These don't sound core to me, please kick them out into a separate geolocation capability.
pavelfeldman
left a comment
pavelfeldman
left a comment
There was a problem hiding this comment.
Can't have a patch w/o tests either.
|
Thanks for the review! I've addressed both feedback items: 1. Separate capability: Changed from 2. Added tests: Created tests/mcp/geolocation.spec.ts with:
All 3 tests pass locally. Let me know your feedback :) |
| accuracy: params.accuracy ?? 0, | ||
| }); | ||
|
|
||
| response.addCode(`await context.grantPermissions(['geolocation']);`); |
There was a problem hiding this comment.
Let's generate page.context().grantPermission to give LLM a better clue if it needs to generate the source code.
| const browserContext = await context.ensureBrowserContext(); | ||
|
|
||
| // Clear permissions revokes geolocation access | ||
| await browserContext.clearPermissions(); |
There was a problem hiding this comment.
You would want to keep the permission and run browserContext.setGeolocation(null) instead.
| import { defineTool } from './tool'; | ||
|
|
||
| const setGeolocation = defineTool({ | ||
| capability: 'geolocation', |
There was a problem hiding this comment.
This property is typed, make sure you update the types and the docs.
Adds geolocation mocking support to Playwright MCP via a new \�rowser_set_geolocation\ tool that uses the browser context's setGeolocation() method. Also adds \�rowser_clear_geolocation\ to reset the mocked state. The tool: - Accepts latitude (-90 to 90), longitude (-180 to 180), and optional accuracy - Automatically grants geolocation permissions - Works at the browser context level, affecting all pages Fixes microsoft/playwright-mcp#1114
Addresses PR review feedback: - Changed capability from 'core' to 'geolocation' - Tools now require --caps=geolocation flag to enable - Added geolocation.spec.ts with tests for: - Capability availability with --caps=geolocation - browser_set_geolocation functional test - browser_clear_geolocation test
- Use page.context().grantPermissions/setGeolocation for better LLM code hints - Use setGeolocation(null) instead of clearPermissions() to preserve permissions - Add 'geolocation' to ToolCapability type in config.d.ts - Update capabilities documentation in config.d.ts - Update --caps CLI help text to include geolocation - Update test assertions to match new generated code format
AdamParker19
force-pushed
the
feat/mcp-geolocation
branch
from
236f9d4 to
80f5a61
Compare
|
Thanks for the detailed review @pavelfeldman! I've addressed all feedback and rebased on latest main: Changes made:1. Generated code uses - await context.grantPermissions(['geolocation']);
- await context.setGeolocation({ latitude: 37.77, longitude: -122.42, accuracy: 100 });
+ await page.context().grantPermissions(['geolocation']);
+ await page.context().setGeolocation({ latitude: 37.77, longitude: -122.42, accuracy: 100 });2. - await browserContext.clearPermissions();
+ await browserContext.setGeolocation(null);3. Updated types and documentation
4. Rebased on latest main - resolved conflict with new All 3 tests pass locally: |
Test results for "MCP"14 failed 2847 passed, 116 skipped Merge workflow run. |
Fixed 41 ESLint indentation errors - project style requires 2-space indentation, not 4-space.
|
Good morning and Happy new year to you @pavelfeldman I have fixed the indentation issue and the docs & lint test should pass now what do you want me to do about the other tests please let me know |
2 participants
Adds geolocation mocking support to Playwright MCP via a new \�rowser_set_geolocation\ tool that uses the browser context's setGeolocation() method. Also adds \�rowser_clear_geolocation\ to reset the mocked state.
The tool:
Fixes microsoft/playwright-mcp#1114