fix(mcp): keep upload modal, skip short secrets and bad headers

packages/playwright-core/src/tools/backend/context.ts

@@ -408,7 +408,7 @@ export class Context {
 
   redactSecrets(text: string): string {
     for (const [secretName, secretValue] of Object.entries(this.config.secrets ?? {})) {
-      if (!secretValue)
+      if (!secretValue || secretValue.length < 4)
         continue;
       text = text.replaceAll(secretValue, `<secret>${secretName}</secret>`);
     }

packages/playwright-core/src/tools/backend/files.ts

@@ -43,11 +43,16 @@ export const uploadFile = defineTabTool({
 
     response.addCode(`await fileChooser.setFiles(${JSON.stringify(paths)})`);
 
-    tab.clearModalState(modalState);
-    await tab.waitForCompletion(async () => {
-      if (paths)
+    if (paths) {
+      try {
         await modalState.fileChooser.setFiles(paths);
-    });
+      } catch (e) {
+        response.addError(e instanceof Error ? e.message : String(e));
+        return;
+      }
+    }
+    tab.clearModalState(modalState);
+    await tab.waitForCompletion(async () => {});
   },
 
   clearsModalState: 'fileChooser',

packages/playwright-core/src/tools/backend/route.ts

@@ -40,10 +40,18 @@ const route = defineTool({
   },
 
   handle: async (context, params, response) => {
-    const addHeaders = params.headers ? Object.fromEntries(params.headers.map(h => {
-      const colonIndex = h.indexOf(':');
-      return [h.substring(0, colonIndex).trim(), h.substring(colonIndex + 1).trim()];
-    })) : undefined;
+    let addHeaders: Record<string, string> | undefined;
+    if (params.headers) {
+      addHeaders = {};
+      for (const h of params.headers) {
+        const colonIndex = h.indexOf(':');
+        if (colonIndex <= 0) {
+          response.addError(`Invalid header "${h}": expected "Name: Value"`);
+          return;
+        }
+        addHeaders[h.substring(0, colonIndex).trim()] = h.substring(colonIndex + 1).trim();
+      }
+    }
     const removeHeaders = params.removeHeaders ? params.removeHeaders.split(',').map(h => h.trim()) : undefined;
 
     const handler = async (route: playwright.Route) => {

tests/mcp/files.spec.ts

@@ -103,6 +103,43 @@ test('browser_file_upload', async ({ client, server }, testInfo) => {
   }
 });
 
+test('browser_file_upload keeps chooser when setFiles fails', async ({ client, server }, testInfo) => {
+  server.setContent('/', `<input type="file" />`, 'text/html');
+
+  await client.callTool({
+    name: 'browser_navigate',
+    arguments: { url: server.PREFIX },
+  });
+
+  await client.callTool({
+    name: 'browser_click',
+    arguments: {
+      element: 'Textbox',
+      target: 'e2',
+    },
+  });
+
+  const missing = testInfo.outputPath('missing.txt');
+  const failed = await client.callTool({
+    name: 'browser_file_upload',
+    arguments: { paths: [missing] },
+  });
+  expect(failed).toHaveResponse({
+    isError: true,
+    modalState: expect.stringContaining(`[File chooser]`),
+  });
+
+  const filePath = testInfo.outputPath('retry.txt');
+  await fs.writeFile(filePath, 'retry');
+  const retried = await client.callTool({
+    name: 'browser_file_upload',
+    arguments: { paths: [filePath] },
+  });
+  expect(retried).toHaveResponse({
+    modalState: undefined,
+  });
+});
+
 test('clicking on download link emits download', async ({ startClient, server }, testInfo) => {
   const { client } = await startClient({
     config: { outputDir: testInfo.outputPath('output') },

tests/mcp/route.spec.ts

@@ -136,6 +136,24 @@ test('browser_route modifies request headers', async ({ client, server }) => {
   expect(receivedHeaders['x-custom-header']).toBe('test-value');
 });
 
+test('browser_route errors on header lines without a colon', async ({ client, server }) => {
+  await client.callTool({
+    name: 'browser_navigate',
+    arguments: { url: server.EMPTY_PAGE },
+  });
+
+  expect(await client.callTool({
+    name: 'browser_route',
+    arguments: {
+      pattern: '**/api/check',
+      headers: ['NotAHeader', 'X-Custom-Header: test-value'],
+    },
+  })).toHaveResponse({
+    isError: true,
+    error: expect.stringContaining('Invalid header "NotAHeader"'),
+  });
+});
+
 test('browser_route_list shows active routes', async ({ client, server }) => {
   await client.callTool({
     name: 'browser_navigate',

tests/mcp/secrets.spec.ts

@@ -164,6 +164,32 @@ await page.getByRole('textbox', { name: 'Password' }).fill(process.env['X-PASSWO
   });
 });
 
+test('short secret values are not redacted', async ({ startClient, server }) => {
+  const secretsFile = test.info().outputPath('secrets.env');
+  await fs.promises.writeFile(secretsFile, 'X-TINY=e');
+
+  const { client } = await startClient({
+    args: ['--secrets', secretsFile],
+  });
+
+  server.setContent('/', `
+    <!DOCTYPE html>
+    <html>
+      <body><p>hello</p></body>
+    </html>
+  `, 'text/html');
+
+  const response = await client.callTool({
+    name: 'browser_navigate',
+    arguments: { url: server.PREFIX },
+  });
+
+  expect(response).toHaveResponse({
+    snapshot: expect.stringContaining('hello'),
+  });
+  expect(JSON.stringify(response)).not.toContain('<secret>X-TINY</secret>');
+});
+
 test('empty secret value is ignored', async ({ startClient, server }) => {
   const secretsFile = test.info().outputPath('secrets.env');
   await fs.promises.writeFile(secretsFile, 'EMPTY_SECRET=\nX-PASSWORD=password123');