[pull] main from microsoft:main

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();`);
   },

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);
   }
 

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;

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,

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();
 

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', `<title>Page A</title><body>Page A<img src="/hang"></body>`, 'text/html');
+  server.setContent('/page-b', `<title>Page B</title><body>Page B</body>`, '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>Title</title>

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,31 +875,31 @@ 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,
   }));
 }
 
 // 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,
   }));
 }