diff --git a/packages/playwright-core/src/tools/backend/navigate.ts b/packages/playwright-core/src/tools/backend/navigate.ts index fcaa3b0d4693e..460642cd92068 100644 --- a/packages/playwright-core/src/tools/backend/navigate.ts +++ b/packages/playwright-core/src/tools/backend/navigate.ts @@ -50,7 +50,7 @@ const goBack = defineTabTool({ }, handle: async (tab, params, response) => { - await tab.page.goBack(tab.navigationTimeoutOptions); + await tab.page.goBack({ waitUntil: 'commit', ...tab.navigationTimeoutOptions }); response.setIncludeSnapshot(); response.addCode(`await page.goBack();`); }, @@ -68,7 +68,7 @@ const goForward = defineTabTool({ }, handle: async (tab, params, response) => { - await tab.page.goForward(tab.navigationTimeoutOptions); + await tab.page.goForward({ waitUntil: 'commit', ...tab.navigationTimeoutOptions }); response.setIncludeSnapshot(); response.addCode(`await page.goForward();`); }, diff --git a/packages/playwright-core/src/tools/cli-client/output.ts b/packages/playwright-core/src/tools/cli-client/output.ts index 601384469c0c3..1972f1862adf6 100644 --- a/packages/playwright-core/src/tools/cli-client/output.ts +++ b/packages/playwright-core/src/tools/cli-client/output.ts @@ -105,7 +105,7 @@ export class TextOutput implements Output { } errorAttachConflict(): never { - console.error(`Error: cannot use target name with --cdp, --endpoint, or --extension`); + console.error(`Error: only one of [name], --cdp, --endpoint, or --extension can be specified`); return process.exit(1); } @@ -288,7 +288,7 @@ export class JsonOutput implements Output { } errorAttachConflict(): never { - this._emit({ isError: true, error: `cannot use target name with --cdp, --endpoint, or --extension` }); + this._emit({ isError: true, error: `only one of [name], --cdp, --endpoint, or --extension can be specified` }); return process.exit(1); } diff --git a/packages/playwright-core/src/tools/cli-client/program.ts b/packages/playwright-core/src/tools/cli-client/program.ts index 86a3db49e68e4..1d9dc5d657c2d 100644 --- a/packages/playwright-core/src/tools/cli-client/program.ts +++ b/packages/playwright-core/src/tools/cli-client/program.ts @@ -156,7 +156,8 @@ export async function program(options?: { embedderVersion?: string}) { } case 'attach': { const attachTarget = args._[1] as string | undefined; - if (attachTarget && (args.cdp || args.endpoint || args.extension)) + const targetCount = (attachTarget ? 1 : 0) + (args.cdp ? 1 : 0) + (args.endpoint ? 1 : 0) + (args.extension ? 1 : 0); + if (targetCount > 1) output.errorAttachConflict(); if (attachTarget) args.endpoint = attachTarget; diff --git a/packages/playwright-core/src/tools/cli-client/session.ts b/packages/playwright-core/src/tools/cli-client/session.ts index 3547230b56b35..e9d79783b0885 100644 --- a/packages/playwright-core/src/tools/cli-client/session.ts +++ b/packages/playwright-core/src/tools/cli-client/session.ts @@ -125,8 +125,6 @@ export class Session { ]; if (cliArgs.headed) args.push('--headed'); - if (cliArgs.extension) - args.push('--extension'); if (cliArgs.browser) args.push(`--browser=${cliArgs.browser}`); if (cliArgs.persistent) @@ -135,12 +133,12 @@ export class Session { args.push(`--profile=${cliArgs.profile}`); if (cliArgs.config) args.push(`--config=${cliArgs.config}`); - if (cliArgs.cdp) + if (cliArgs.extension) + args.push('--extension'); + else if (cliArgs.cdp) args.push(`--cdp=${cliArgs.cdp}`); - if (cliArgs.endpoint) + else if (cliArgs.endpoint) args.push(`--endpoint=${cliArgs.endpoint}`); - else if (mode === 'attach' && process.env.PLAYWRIGHT_CLI_SESSION) - args.push(`--endpoint=${process.env.PLAYWRIGHT_CLI_SESSION}`); const child = spawn(process.execPath, args, { detached: true, diff --git a/tests/mcp/cli-cdp.spec.ts b/tests/mcp/cli-cdp.spec.ts index 880e358c6e05e..48c3a237a7683 100644 --- a/tests/mcp/cli-cdp.spec.ts +++ b/tests/mcp/cli-cdp.spec.ts @@ -57,6 +57,18 @@ test('attach via cdp URL keeps the default session', async ({ cdpServer, cli, se expect(listOutput).toContain('(attached)'); }); +test('attach via cdp URL honors PLAYWRIGHT_CLI_SESSION as session name', { annotation: { type: 'issue', description: 'https://github.com/microsoft/playwright-cli/issues/414' } }, async ({ cdpServer, cli }) => { + await cdpServer.start(); + const { exitCode } = await cli('attach', `--cdp=${cdpServer.endpoint}`, { env: { PLAYWRIGHT_CLI_SESSION: 'myname' } }); + expect(exitCode).toBe(0); +}); + +test('attach rejects combining --cdp, --endpoint, or --extension', async ({ cli }) => { + const { error, exitCode } = await cli('attach', '--cdp=chrome-dev', '--endpoint=/tmp/foo'); + expect(exitCode).toBe(1); + expect(error).toContain('only one of [name], --cdp, --endpoint, or --extension can be specified'); +}); + test('detach tears down an attached session', async ({ cdpServer, cli }) => { await cdpServer.start(); diff --git a/tests/mcp/core.spec.ts b/tests/mcp/core.spec.ts index 06423357b550d..8e6040a3f5781 100644 --- a/tests/mcp/core.spec.ts +++ b/tests/mcp/core.spec.ts @@ -86,6 +86,32 @@ test('browser_navigate can navigate to file:// URLs allowUnrestrictedFileAccess }); }); +test('browser_navigate_back does not time out when load never fires', async ({ client, server }) => { + // https://github.com/microsoft/playwright-mcp/issues/1635 + // Page A never fires the `load` event because the image request hangs forever. + // Going back to it should still succeed because we wait for `commit`, not `load`. + server.setRoute('/hang', () => {}); + server.setContent('/page-a', `Page APage A`, 'text/html'); + server.setContent('/page-b', `Page BPage B`, 'text/html'); + + await client.callTool({ + name: 'browser_navigate', + arguments: { url: `${server.PREFIX}/page-a` }, + }); + await client.callTool({ + name: 'browser_navigate', + arguments: { url: `${server.PREFIX}/page-b` }, + }); + + expect(await client.callTool({ + name: 'browser_navigate_back', + arguments: {}, + })).toHaveResponse({ + code: `await page.goBack();`, + page: expect.stringContaining(`- Page URL: ${server.PREFIX}/page-a`), + }); +}); + test('browser_select_option', async ({ client, server }) => { server.setContent('/', ` Title diff --git a/utils/build/build.js b/utils/build/build.js index 814da980b2b29..e748eca2529c2 100644 --- a/utils/build/build.js +++ b/utils/build/build.js @@ -80,12 +80,21 @@ function filePath(relative) { } /** - * @param {string} path + * Resolve a CLI shipped by a node_modules package to an absolute path, so we + * can spawn it via `node` directly instead of going through `npx`/`npm exec` + * (which adds a shell + npm wrapper process per concurrent build). + * @param {string} pkg + * @param {string} binName * @returns {string} */ -function quotePath(path) { - return "\"" + path + "\""; +function resolveNodeBin(pkg, binName) { + // Resolve via package.json (always allowed) rather than a subpath that may + // be excluded by the package's `exports` field. + const pkgJson = require.resolve(`${pkg}/package.json`, { paths: [ROOT] }); + return path.join(path.dirname(pkgJson), require(pkgJson).bin[binName]); } +const VITE_BIN = resolveNodeBin('vite', 'vite'); +const TSC_BIN = resolveNodeBin('typescript', 'tsc'); class Step { /** @@ -866,15 +875,15 @@ const pkgSizePlugin = { const webPackages = ['html-reporter', 'recorder', 'trace-viewer', 'dashboard']; for (const webPackage of webPackages) { steps.push(new ProgramStep({ - command: 'npx', + command: process.execPath, args: [ - 'vite', + VITE_BIN, 'build', ...(watchMode ? ['--watch', '--minify=false'] : []), ...(withSourceMaps ? ['--sourcemap=inline'] : []), '--clearScreen=false', ], - shell: true, + shell: false, cwd: path.join(__dirname, '..', '..', 'packages', webPackage), concurrent: true, })); @@ -882,15 +891,15 @@ for (const webPackage of webPackages) { // Build/watch extension steps.push(new ProgramStep({ - command: 'npx', + command: process.execPath, args: [ - 'vite', + VITE_BIN, 'build', ...(watchMode ? ['--watch', '--minify=false'] : []), ...(withSourceMaps ? ['--sourcemap=inline'] : []), '--clearScreen=false', ], - shell: true, + shell: false, cwd: path.join(__dirname, '..', '..', 'packages', 'extension'), concurrent: true, })); @@ -1014,9 +1023,9 @@ copyFiles.push({ if (watchMode) { // Run TypeScript for type checking. steps.push(new ProgramStep({ - command: 'npx', - args: ['tsc', '-w', '--preserveWatchOutput', '-p', quotePath(filePath('.'))], - shell: true, + command: process.execPath, + args: [TSC_BIN, '-w', '--preserveWatchOutput', '-p', filePath('.')], + shell: false, concurrent: true, })); }