Skip to content

fix(mcp): await init page before screenshot #41646

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
yury-s merged 4 commits into from
Jul 8, 2026
+26 −0
Merged

fix(mcp): await init page before screenshot #41646

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9dd8234
fix(mcp): await init page before screenshot
harshmathurx Jul 6, 2026
60bd1c8
fix(mcp): move init-page wait from screenshot to ensureTab
harshmathurx Jul 7, 2026
406ae34
chore: retrigger ci
harshmathurx Jul 8, 2026
f2f3124
chore: retrigger ci
Jul 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/playwright-core/src/tools/backend/screenshot.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ const screenshot = defineTabTool({

const screenshotTargetLabel = params.target ? params.element || 'element' : (params.fullPage ? 'full page' : 'viewport');
const target = params.target ? await tab.targetLocator({ element: params.element, target: params.target }) : null;
if (!target)
await tab.waitForInitialized();

@pavelfeldman pavelfeldman Jul 6, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be earlier in the pipeline, most likely in the context.ensureTab

@harshmathurx harshmathurx Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call — ensureTab is the right place. That way all tools (snapshot, evaluate, screenshot, etc.) get the guarantee automatically instead of each one having to remember to await init. I will move the waitForInitialized() call into context.ensureTab() and keep the method on Tab for direct use when needed.

const data = target ? await target.locator.screenshot(options) : await tab.page.screenshot(options);

const resolvedFile = await response.resolveClientFile({ prefix: target ? 'element' : 'page', ext: fileType, suggestedFilename: params.filename }, `Screenshot of ${screenshotTargetLabel}`);
Expand Down
4 changes: 4 additions & 0 deletions packages/playwright-core/src/tools/backend/tab.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,10 @@ export class Tab extends EventEmitter<TabEventsInterface> {
this._consoleLog.stop();
}

async waitForInitialized() {
await this._initializedPromise;
}

static forPage(page: playwright.Page): Tab | undefined {
// eslint-disable-next-line no-restricted-syntax
return (page as any)[tabSymbol];
Expand Down
21 changes: 21 additions & 0 deletions tests/mcp/init-page.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,27 @@ test('init-page surfaces load errors instead of silently dropping them', async (
expect(JSON.stringify(response.content)).toContain('boom from initPage');
});

test('init-page is applied before the first page screenshot', async ({ startClient }) => {
const initPagePath = test.info().outputPath('screenshotInitPage.ts');
await fs.promises.writeFile(initPagePath, `
export default async ({ page }) => {
page.screenshot = async () => {
throw new Error('initPage screenshot hook applied');
};
};
`);

const { client } = await startClient({
args: [`--init-page=${initPagePath}`],
});
const response: any = await client.callTool({
name: 'browser_take_screenshot',
arguments: {},
});
expect(response.isError).toBe(true);
expect(JSON.stringify(response.content)).toContain('initPage screenshot hook applied');
});

test('--init-page w/ --init-script', async ({ startClient, server }) => {
server.setContent('/', `
<div>Hello world</div>
Expand Down