From c47014a0677ee76934e71618d8588935f13ef3a8 Mon Sep 17 00:00:00 2001 From: Rizwan Saleem Date: Tue, 1 Sep 2026 20:48:29 +0100 Subject: [PATCH] fix(mcp): create the parent directory for an explicit output file name Tools that accept a `filename` resolve it through `Response.resolveClientFile()`, which resolves the name against the client workspace but never creates its parent directory. So `browser_take_screenshot` with `filename: "sub/shot.png"` fails with a raw ENOENT, while the auto-named branch of the same method goes through `outputFile()`, which does create it. Also correct the `filename` description on `browser_take_screenshot` and `browser_pdf_save`. Both said a relative name stays within the output directory; it is resolved against the working directory, which is the behaviour the existing tests pin. Fixes #42487 --- .../playwright-core/src/tools/backend/pdf.ts | 2 +- .../src/tools/backend/response.ts | 8 ++++++-- .../src/tools/backend/screenshot.ts | 2 +- tests/mcp/screenshot.spec.ts | 20 +++++++++++++++++++ 4 files changed, 28 insertions(+), 4 deletions(-) diff --git a/packages/playwright-core/src/tools/backend/pdf.ts b/packages/playwright-core/src/tools/backend/pdf.ts index a31a3569fda1e..95a33aa1daf26 100644 --- a/packages/playwright-core/src/tools/backend/pdf.ts +++ b/packages/playwright-core/src/tools/backend/pdf.ts @@ -20,7 +20,7 @@ import { formatObject } from '@isomorphic/stringUtils'; import { defineTabTool } from './tool'; const pdfSchema = z.object({ - filename: z.string().optional().describe('File name to save the pdf to. Defaults to `page-{timestamp}.pdf` if not specified. Prefer relative file names to stay within the output directory.'), + filename: z.string().optional().describe('File name to save the pdf to. Defaults to `page-{timestamp}.pdf` in the output directory if not specified. An explicit name is resolved against the working directory, not the output directory.'), }); const pdf = defineTabTool({ diff --git a/packages/playwright-core/src/tools/backend/response.ts b/packages/playwright-core/src/tools/backend/response.ts index b5315a40c341e..90e5afb157df2 100644 --- a/packages/playwright-core/src/tools/backend/response.ts +++ b/packages/playwright-core/src/tools/backend/response.ts @@ -87,10 +87,14 @@ export class Response { async resolveClientFile(template: FilenameTemplate, title: string): Promise { let fileName: string; - if (template.suggestedFilename) + if (template.suggestedFilename) { fileName = await this.resolveClientFilename(template.suggestedFilename); - else + // outputFile() creates the parent directory for auto-named files, do the same + // for an explicit one so that "sub/page.png" does not fail with ENOENT. + await fs.promises.mkdir(path.dirname(fileName), { recursive: true }); + } else { fileName = await this._context.outputFile(template, { origin: 'llm' }); + } const relativeName = this._computeRelativeTo(fileName); const printableLink = `- [${title}](${relativeName})`; return { fileName, relativeName, printableLink }; diff --git a/packages/playwright-core/src/tools/backend/screenshot.ts b/packages/playwright-core/src/tools/backend/screenshot.ts index af0cf5e0cf706..ada0363fd889e 100644 --- a/packages/playwright-core/src/tools/backend/screenshot.ts +++ b/packages/playwright-core/src/tools/backend/screenshot.ts @@ -28,7 +28,7 @@ type ImageFormat = 'png' | 'jpeg' | 'webp'; const screenshotSchema = optionalElementSchema.extend({ type: z.enum(['png', 'jpeg', 'webp']).optional().describe('Image format for the screenshot. If unset, inferred from the filename extension, otherwise png.'), - filename: z.string().optional().describe('File name to save the screenshot to. Defaults to `page-{timestamp}.{png|jpeg|webp}` if not specified. Prefer relative file names to stay within the output directory.'), + filename: z.string().optional().describe('File name to save the screenshot to. Defaults to `page-{timestamp}.{png|jpeg|webp}` in the output directory if not specified. An explicit name is resolved against the working directory, not the output directory.'), fullPage: z.boolean().optional().describe('When true, takes a screenshot of the full scrollable page, instead of the currently visible viewport. Cannot be used with element screenshots.'), scale: z.enum(['css', 'device']).default('css').describe('Image resolution scale. "css" produces a screenshot sized in CSS pixels (smaller, consistent across devices). "device" produces a high-resolution screenshot using device pixels (larger, accounts for the device pixel ratio). Default is css.'), }); diff --git a/tests/mcp/screenshot.spec.ts b/tests/mcp/screenshot.spec.ts index 09775eb24cc48..1303d3bc1180d 100644 --- a/tests/mcp/screenshot.spec.ts +++ b/tests/mcp/screenshot.spec.ts @@ -246,6 +246,26 @@ test('browser_take_screenshot (filename: "output.png")', async ({ client, server expect(files[0]).toMatch(/^output\.png$/); }); +test('browser_take_screenshot (filename with a subdirectory)', async ({ client, server }, testInfo) => { + expect(await client.callTool({ + name: 'browser_navigate', + arguments: { url: server.HELLO_WORLD }, + })).toHaveResponse({ + code: expect.stringContaining(`page.goto('http://localhost`), + }); + + expect(await client.callTool({ + name: 'browser_take_screenshot', + arguments: { + filename: 'sub/dir/output.png', + }, + })).toHaveResponse({ + result: expect.stringContaining('output.png'), + }); + + expect(fs.existsSync(testInfo.outputPath('sub', 'dir', 'output.png'))).toBeTruthy(); +}); + test('browser_take_screenshot (imageResponses=omit)', async ({ startClient, server }, testInfo) => { const outputDir = testInfo.outputPath('output'); const { client } = await startClient({