From d4667f865d7102ef8c62aca82acbad043c3ad4e1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 1 Aug 2025 23:59:38 +0000 Subject: [PATCH] Address review feedback: move permissions to contextOptions, remove redundant tests and files Co-authored-by: pavelfeldman <883973+pavelfeldman@users.noreply.github.com> --- config.d.ts | 5 -- examples/clipboard-permissions.md | 95 ------------------------------- src/browserContextFactory.ts | 17 +----- src/config.ts | 4 +- tests/permissions.spec.ts | 65 ++------------------- 5 files changed, 10 insertions(+), 176 deletions(-) delete mode 100644 examples/clipboard-permissions.md diff --git a/config.d.ts b/config.d.ts index 14f98dc..d63b061 100644 --- a/config.d.ts +++ b/config.d.ts @@ -54,11 +54,6 @@ export type Config = { */ contextOptions?: playwright.BrowserContextOptions; - /** - * Permissions to grant to the browser context, for example ["clipboard-read", "clipboard-write"]. - */ - permissions?: string[]; - /** * Chrome DevTools Protocol endpoint to connect to an existing browser instance in case of Chromium family browsers. */ diff --git a/examples/clipboard-permissions.md b/examples/clipboard-permissions.md deleted file mode 100644 index 42e2d8d..0000000 --- a/examples/clipboard-permissions.md +++ /dev/null @@ -1,95 +0,0 @@ -# Clipboard Permissions Example - -This example demonstrates how to use the new `--permissions` feature to enable clipboard operations without user permission prompts. - -## Usage - -### Via Command Line Arguments - -```json -{ - "mcpServers": { - "playwright": { - "command": "npx", - "args": [ - "@playwright/mcp@latest", - "--permissions", "clipboard-read,clipboard-write" - ] - } - } -} -``` - -### Via Configuration File - -Create a config file `playwright-mcp-config.json`: - -```json -{ - "browser": { - "permissions": ["clipboard-read", "clipboard-write"] - } -} -``` - -Then use it: - -```json -{ - "mcpServers": { - "playwright": { - "command": "npx", - "args": [ - "@playwright/mcp@latest", - "--config", "playwright-mcp-config.json" - ] - } - } -} -``` - -### Via Environment Variable - -```bash -export PLAYWRIGHT_MCP_PERMISSIONS="clipboard-read,clipboard-write" -``` - -## Clipboard Operations - -Once permissions are granted, you can use clipboard APIs via `browser_evaluate`: - -```javascript -// Write to clipboard (no permission prompt) -await browser_evaluate({ - function: "() => navigator.clipboard.writeText('Copy this!')" -}) - -// Read from clipboard (no permission prompt) -await browser_evaluate({ - function: "() => navigator.clipboard.readText()" -}) -``` - -## Supported Permissions - -You can grant multiple permissions as a comma-separated list: - -- `clipboard-read` -- `clipboard-write` -- `geolocation` -- `camera` -- `microphone` -- `notifications` -- And any other [Web API permissions](https://developer.mozilla.org/en-US/docs/Web/API/Permissions_API) - -Example with multiple permissions: - -```bash ---permissions "clipboard-read,clipboard-write,geolocation,notifications" -``` - -## Notes - -- Permissions are applied when the browser context is created -- The clipboard API requires secure contexts (HTTPS) in production environments -- Some permissions may not be supported in all browsers or may require additional user activation \ No newline at end of file diff --git a/src/browserContextFactory.ts b/src/browserContextFactory.ts index 2ff811b..eef59d2 100644 --- a/src/browserContextFactory.ts +++ b/src/browserContextFactory.ts @@ -113,10 +113,7 @@ class IsolatedContextFactory extends BaseContextFactory { } protected override async _doCreateContext(browser: playwright.Browser): Promise { - const contextOptions = { ...this.browserConfig.contextOptions }; - if (this.browserConfig.permissions) - contextOptions.permissions = this.browserConfig.permissions; - return browser.newContext(contextOptions); + return browser.newContext(this.browserConfig.contextOptions); } } @@ -131,10 +128,7 @@ class CdpContextFactory extends BaseContextFactory { protected override async _doCreateContext(browser: playwright.Browser): Promise { if (this.browserConfig.isolated) { - const contextOptions: playwright.BrowserContextOptions = {}; - if (this.browserConfig.permissions) - contextOptions.permissions = this.browserConfig.permissions; - return browser.newContext(contextOptions); + return browser.newContext(this.browserConfig.contextOptions); } return browser.contexts()[0]; } @@ -154,10 +148,7 @@ class RemoteContextFactory extends BaseContextFactory { } protected override async _doCreateContext(browser: playwright.Browser): Promise { - const contextOptions: playwright.BrowserContextOptions = {}; - if (this.browserConfig.permissions) - contextOptions.permissions = this.browserConfig.permissions; - return browser.newContext(contextOptions); + return browser.newContext(this.browserConfig.contextOptions); } } @@ -186,8 +177,6 @@ class PersistentContextFactory implements BrowserContextFactory { handleSIGINT: false, handleSIGTERM: false, }; - if (this.browserConfig.permissions) - contextOptions.permissions = this.browserConfig.permissions; const browserContext = await browserType.launchPersistentContext(userDataDir, contextOptions); const close = () => this._closeBrowserContext(browserContext, userDataDir); return { browserContext, close }; diff --git a/src/config.ts b/src/config.ts index 9388c31..745041b 100644 --- a/src/config.ts +++ b/src/config.ts @@ -173,12 +173,14 @@ export function configFromCLIOptions(cliOptions: CLIOptions): Config { if (cliOptions.blockServiceWorkers) contextOptions.serviceWorkers = 'block'; + if (cliOptions.permissions) + contextOptions.permissions = cliOptions.permissions; + const result: Config = { browser: { browserName, isolated: cliOptions.isolated, userDataDir: cliOptions.userDataDir, - permissions: cliOptions.permissions, launchOptions, contextOptions, cdpEndpoint: cliOptions.cdpEndpoint, diff --git a/tests/permissions.spec.ts b/tests/permissions.spec.ts index 0f09803..a8b79c5 100644 --- a/tests/permissions.spec.ts +++ b/tests/permissions.spec.ts @@ -68,7 +68,7 @@ test('clipboard permissions support via CLI argument', async ({ startClient, ser }); }); -test('clipboard permissions support via config file', async ({ startClient, server }) => { +test('clipboard permissions support via config file contextOptions', async ({ startClient, server }) => { server.setContent('/', ` @@ -79,7 +79,9 @@ test('clipboard permissions support via config file', async ({ startClient, serv const config = { browser: { - permissions: ['clipboard-read', 'clipboard-write'] + contextOptions: { + permissions: ['clipboard-read', 'clipboard-write'] + } } }; @@ -166,62 +168,3 @@ test('multiple permissions can be granted', async ({ startClient, server }) => { result: '"granted"' }); }); - -test('clipboard permissions via environment variable', async ({ startClient, server }) => { - server.setContent('/', ` - - -

Environment Variable Test

- - - `, 'text/html'); - - // Set environment variable - process.env.PLAYWRIGHT_MCP_PERMISSIONS = 'clipboard-read,clipboard-write'; - - try { - const { client } = await startClient({ args: [] }); - - // Navigate to server page - const navigateResponse = await client.callTool({ - name: 'browser_navigate', - arguments: { url: server.PREFIX }, - }); - expect(navigateResponse.isError).toBeFalsy(); - - // Verify permissions are granted via environment variable - const permissionsResponse = await client.callTool({ - name: 'browser_evaluate', - arguments: { - function: '() => navigator.permissions.query({ name: "clipboard-write" }).then(result => result.state)' - } - }); - expect(permissionsResponse.isError).toBeFalsy(); - expect(permissionsResponse).toHaveResponse({ - result: '"granted"' - }); - - // Test clipboard operations work - const writeResponse = await client.callTool({ - name: 'browser_evaluate', - arguments: { - function: '() => navigator.clipboard.writeText("env test content")' - } - }); - expect(writeResponse.isError).toBeFalsy(); - - const readResponse = await client.callTool({ - name: 'browser_evaluate', - arguments: { - function: '() => navigator.clipboard.readText()' - } - }); - expect(readResponse.isError).toBeFalsy(); - expect(readResponse).toHaveResponse({ - result: '"env test content"' - }); - } finally { - // Clean up environment variable - delete process.env.PLAYWRIGHT_MCP_PERMISSIONS; - } -});