diff --git a/packages/playwright-core/src/tools/cli-client/program.ts b/packages/playwright-core/src/tools/cli-client/program.ts index 8c4de36b1bbd2..86a3db49e68e4 100644 --- a/packages/playwright-core/src/tools/cli-client/program.ts +++ b/packages/playwright-core/src/tools/cli-client/program.ts @@ -204,9 +204,16 @@ export async function program(options?: { embedderVersion?: string}) { const daemonScript = libPath('entry', 'dashboardApp.js'); const daemonArgs = [ daemonScript, - `--sessionName=${sessionName}`, `--workspaceDir=${clientInfo.workspaceDir ?? ''}`, ]; + // Only pass --sessionName when the user explicitly requested a session + // (via -s/--session or PLAYWRIGHT_CLI_SESSION). Bare `playwright cli show` + // opens the dashboard generically, with no specific session to reveal, + // so the daemon should ack as soon as it's ready rather than waiting for + // a reveal that was never asked for. + const explicit = explicitSessionName(args.session as string); + if (explicit) + daemonArgs.push(`--sessionName=${explicit}`); if (args.port !== undefined) daemonArgs.push(`--port=${args.port}`); if (args.host !== undefined) @@ -237,13 +244,17 @@ export async function program(options?: { embedderVersion?: string}) { } const timer = setTimeout(() => child.stdin!.destroy(), 60_000); child.unref(); + let daemonPid: number; try { await new Promise((resolve, reject) => { let outLog = ''; child.stdout!.on('data', data => { outLog += data.toString(); - if (outLog.includes('Dashboard is running')) + const match = outLog.match(/Dashboard is running pid=(\d+)/); + if (match) { + daemonPid = Number(match[1]); resolve(); + } }); child.once('exit', (code, signal) => reject(new Error(`Dashboard daemon exited (code=${code}, signal=${signal}) before signaling READY${outLog ? '\n' + outLog : ''}`))); }); @@ -253,7 +264,7 @@ export async function program(options?: { embedderVersion?: string}) { child.stdin!.destroy(); child.stdout!.destroy(); } - output.show(sessionName, child.pid); + output.show(sessionName, daemonPid!); return; } default: { diff --git a/packages/playwright-core/src/tools/dashboard/dashboardApp.ts b/packages/playwright-core/src/tools/dashboard/dashboardApp.ts index 26afa59aae034..4c969c00da049 100644 --- a/packages/playwright-core/src/tools/dashboard/dashboardApp.ts +++ b/packages/playwright-core/src/tools/dashboard/dashboardApp.ts @@ -22,6 +22,7 @@ import http from 'http'; import { HttpServer } from '@utils/httpServer'; import { makeSocketPath } from '@utils/fileUtils'; import { gracefullyProcessExitDoNotHang } from '@utils/processLauncher'; +import { ManualPromise } from '@isomorphic/manualPromise'; import { libPath } from '../../package'; import { playwright } from '../../inprocess'; import { findChromiumChannelBestEffort, registryDirectory } from '../../server/registry/index'; @@ -41,9 +42,8 @@ declare const __PW_HMR__: boolean; type DashboardServer = { url: string; - reveal: (options: DashboardOptions) => void; - triggerAnnotate: () => void; - registerAnnotateWaiter: (socket: net.Socket) => void; + reveal: (options: DashboardOptions) => Promise; + triggerAnnotate: (socket: net.Socket) => Promise; close: () => Promise; }; @@ -52,8 +52,7 @@ async function startDashboardServer(provider: SessionProvider, options: Dashboar const httpServer = new HttpServer(dashboardDir); const connections = new Set(); - let currentReveal: DashboardOptions = options; - let pendingAnnotate = false; + let connectionLanded = new ManualPromise(); const waitingSockets = new Set(); const submitAnnotation = (frames: SubmittedAnnotationFrame[], feedback: string) => { @@ -70,15 +69,12 @@ async function startDashboardServer(provider: SessionProvider, options: Dashboar httpServer.createWebSocket(() => { let connection: DashboardConnection; // eslint-disable-next-line prefer-const - connection = new DashboardConnection(provider, () => connections.delete(connection), () => { - if (currentReveal.pageId) - connection.revealPage(currentReveal.pageId); - else if (currentReveal.sessionName) - connection.revealSession(currentReveal.sessionName, currentReveal.workspaceDir); - if (pendingAnnotate) { - pendingAnnotate = false; - connection.emitAnnotate(); - } + connection = new DashboardConnection(provider, () => { + connections.delete(connection); + if (connections.size === 0) + connectionLanded = new ManualPromise(); + }, () => { + connectionLanded.resolve(); }, submitAnnotation); connections.add(connection); return connection; @@ -102,48 +98,38 @@ async function startDashboardServer(provider: SessionProvider, options: Dashboar attachDashboardStaticServer(httpServer, dashboardDir); await httpServer.start({ port: options.port, host: options.host }); - const reveal = (next: DashboardOptions) => { - currentReveal = next; - if (next.pageId) { - for (const connection of connections) - connection.revealPage(next.pageId); - return; - } - if (!next.sessionName) - return; - for (const connection of connections) - connection.revealSession(next.sessionName, next.workspaceDir); - }; - - const triggerAnnotate = () => { - if (connections.size === 0) { - pendingAnnotate = true; - return; - } - for (const connection of connections) - connection.emitAnnotate(); - }; - - const notifyAnnotateEnded = () => { - pendingAnnotate = false; - for (const connection of connections) - connection.emitCancelAnnotate(); + const reveal = async (next: DashboardOptions): Promise => { + await connectionLanded; + await Promise.all([...connections].map(async c => { + if (next.pageId) + await c.revealPage(next.pageId); + else if (next.sessionName) + await c.revealSession(next.sessionName, next.workspaceDir); + })); }; - const registerAnnotateWaiter = (socket: net.Socket) => { + const triggerAnnotate = async (socket: net.Socket) => { waitingSockets.add(socket); const cleanup = () => { if (!waitingSockets.delete(socket)) return; - if (waitingSockets.size === 0) - notifyAnnotateEnded(); + if (waitingSockets.size === 0) { + for (const connection of connections) + connection.emitCancelAnnotate(); + } }; socket.on('close', cleanup); socket.on('error', cleanup); + + await connectionLanded; + if (waitingSockets.size === 0) + return; + for (const connection of connections) + connection.emitAnnotate(); }; const close = () => httpServer.stop(); - return { url: httpServer.urlPrefix('human-readable'), reveal, triggerAnnotate, registerAnnotateWaiter, close }; + return { url: httpServer.urlPrefix('human-readable'), reveal, triggerAnnotate, close }; } function attachDashboardStaticServer(httpServer: HttpServer, dashboardDir: string) { @@ -168,6 +154,7 @@ async function attachDashboardDevServer(httpServer: HttpServer) { async function innerOpenDashboardApp(options: DashboardOptions): Promise<{ page: api.Page; server: DashboardServer }> { const server = await startDashboardServer(new RegistrySessionProvider(), options); + void server.reveal(options).catch(() => {}); const { page } = await launchApp('dashboard', { onClose: () => gracefullyProcessExitDoNotHang(0) }); await page.goto(server.url); return { page, server }; @@ -266,25 +253,38 @@ function parseOpenArgs(): DashboardOptions { }; } -async function acquireSingleton(options: DashboardOptions): Promise { +type AcquireResult = + | { role: 'winner', server: net.Server } + | { role: 'loser', daemonPid: number }; + +async function acquireSingleton(options: DashboardOptions): Promise { const socketPath = dashboardSocketPath(); if (process.platform !== 'win32') await fs.promises.mkdir(path.dirname(socketPath), { recursive: true }); return await new Promise((resolve, reject) => { const server = net.createServer(); - server.listen(socketPath, () => resolve(server)); + server.listen(socketPath, () => resolve({ role: 'winner', server })); server.on('error', (err: NodeJS.ErrnoException) => { - if (err.code !== 'EADDRINUSE') + if (err.code !== 'EADDRINUSE' && err.code !== 'EEXIST') return reject(err); + let ackBuffer = ''; const client = net.connect(socketPath, () => { client.write(JSON.stringify(options) + '\n'); - reject(new Error('already running')); + }); + client.on('data', chunk => { ackBuffer += chunk.toString(); }); + client.on('end', () => { + try { + const { pid } = JSON.parse(ackBuffer.trim()); + resolve({ role: 'loser', daemonPid: pid }); + } catch (e) { + reject(e); + } }); client.on('error', () => { if (process.platform !== 'win32') fs.unlinkSync(socketPath); - server.listen(socketPath, () => resolve(server)); + server.listen(socketPath, () => resolve({ role: 'winner', server })); }); }); }); @@ -305,9 +305,10 @@ export async function openDashboardApp() { console.error('Unhandled promise rejection:', error); }); if (options.port !== undefined) { - const { url } = await startDashboardServer(new RegistrySessionProvider(), options); + const server = await startDashboardServer(new RegistrySessionProvider(), options); + void server.reveal(options).catch(() => {}); // eslint-disable-next-line no-console - console.log(`Listening on ${url}`); + console.log(`Listening on ${server.url}`); // eslint-disable-next-line no-restricted-properties await new Promise(f => process.stdout.write('', f)); // Make sure stdout is flushed. selfDestructOnParentGone(); @@ -316,24 +317,23 @@ export async function openDashboardApp() { // Self-destruct if the parent CLI dies before we signal READY. Unregistered // before we signal so the daemon outlives the parent. const stopSelfDestruct = selfDestructOnParentGone(); - let server: net.Server; - try { - server = await acquireSingleton(options); - } catch { + const acquired = await acquireSingleton(options); + if (acquired.role === 'loser') { // Another daemon is already running, signal success. stopSelfDestruct(); // eslint-disable-next-line no-console - console.log('Dashboard is running'); + console.log(`Dashboard is running pid=${acquired.daemonPid}`); // eslint-disable-next-line no-restricted-properties await new Promise(f => process.stdout.write('', f)); // Make sure stdout is flushed. return; } + const { server } = acquired; process.on('exit', () => server.close()); try { await startApp(server, options); stopSelfDestruct(); // eslint-disable-next-line no-console - console.log('Dashboard is running'); + console.log(`Dashboard is running pid=${process.pid}`); } catch (error) { // eslint-disable-next-line no-console console.log(error); @@ -345,7 +345,7 @@ async function startApp(server: net.Server, options: DashboardOptions) { const statePromise = innerOpenDashboardApp(options); server.on('connection', socket => { let buffer = ''; - socket.on('data', data => { + socket.on('data', async data => { buffer += data.toString(); const newlineIndex = buffer.indexOf('\n'); if (newlineIndex === -1) @@ -362,21 +362,27 @@ async function startApp(server: net.Server, options: DashboardOptions) { socket.end(); return; } - void statePromise.then(({ page, server: dashboard }) => { - if (parsed.annotate) { - page?.bringToFront().catch(() => {}); - dashboard.reveal(parsed); - dashboard.triggerAnnotate(); - dashboard.registerAnnotateWaiter(socket); - } else if (parsed.kill) { - socket.end(); - gracefullyProcessExitDoNotHang(0); - } else { - page?.bringToFront().catch(() => {}); - dashboard.reveal(parsed); - socket.end(); + const { page, server: dashboard } = await statePromise; + if (parsed.annotate) { + try { + await page?.bringToFront(); + await dashboard.reveal(parsed); + await dashboard.triggerAnnotate(socket); + } catch (e) { + socket.end(e); } - }); + } else if (parsed.kill) { + socket.end(); + gracefullyProcessExitDoNotHang(0); + } else { + try { + await page?.bringToFront(); + await dashboard.reveal(parsed); + socket.end(JSON.stringify({ pid: process.pid }) + '\n'); + } catch (e) { + socket.end(e); + } + } }); }); await statePromise; diff --git a/packages/playwright-core/src/tools/dashboard/dashboardController.ts b/packages/playwright-core/src/tools/dashboard/dashboardController.ts index b11df9415dea9..dfa9bc2216f81 100644 --- a/packages/playwright-core/src/tools/dashboard/dashboardController.ts +++ b/packages/playwright-core/src/tools/dashboard/dashboardController.ts @@ -20,6 +20,7 @@ import fs from 'fs'; import crypto from 'crypto'; import { execFile } from 'child_process'; import { Disposable } from '@isomorphic/disposable'; +import { ManualPromise } from '@isomorphic/manualPromise'; import { eventsHelper } from '@utils/eventsHelper'; import { createClientInfo } from '../cli-client/registry'; @@ -42,7 +43,7 @@ export class DashboardConnection implements Transport { private _onAnnotationSubmit?: (frames: SubmittedAnnotationFrame[], feedback: string) => void; private _pushTabsScheduled = false; private _visible = true; - private _pendingReveal: { sessionName?: string; workspaceDir?: string; pageId?: string } | undefined; + private _pendingReveal: { sessionName?: string; workspaceDir?: string; pageId?: string; done: ManualPromise } | undefined; private _annotateWaitingForAttach = false; _recordingDir: string; @@ -80,6 +81,9 @@ export class DashboardConnection implements Transport { this._provider.dispose(); this._attachedPage?.dispose(); this._attachedPage = undefined; + // Reject any in-flight reveal so callers awaiting it don't hang. + this._pendingReveal?.done.reject(new Error('Dashboard connection closed')); + this._pendingReveal = undefined; for (const stream of this._streams.values()) { void stream.handle.close() .catch(() => {}) @@ -137,14 +141,29 @@ export class DashboardConnection implements Transport { await this._attachedPage?.setScreencastActive(params.visible); } - revealSession(sessionName: string, workspaceDir?: string) { - this._pendingReveal = { sessionName, workspaceDir }; + revealSession(sessionName: string, workspaceDir?: string): Promise { + const existing = this._pendingReveal; + if (existing + && existing.pageId === undefined + && existing.sessionName === sessionName + && existing.workspaceDir === workspaceDir) + return existing.done; + existing?.done.reject(new Error('Reveal superseded')); + const done = new ManualPromise(); + this._pendingReveal = { sessionName, workspaceDir, done }; void this._tryRevealPending(); + return done; } - revealPage(pageId: string) { - this._pendingReveal = { pageId }; + revealPage(pageId: string): Promise { + const existing = this._pendingReveal; + if (existing && existing.pageId === pageId) + return existing.done; + existing?.done.reject(new Error('Reveal superseded')); + const done = new ManualPromise(); + this._pendingReveal = { pageId, done }; void this._tryRevealPending(); + return done; } private async _tryRevealPending() { @@ -163,8 +182,14 @@ export class DashboardConnection implements Transport { if (!page) return; this._pendingReveal = undefined; - await this._switchAttachedTo(page); - this._pushTabs(); + try { + await this._switchAttachedTo(page); + this._pushTabs(); + pending.done.resolve(); + } catch (e) { + pending.done.reject(e instanceof Error ? e : new Error(String(e))); + throw e; + } } async submitAnnotation(params: { frames: SubmittedAnnotationFrame[]; feedback: string }) { diff --git a/tests/mcp/annotate.spec.ts b/tests/mcp/annotate.spec.ts index eb91e63be3347..0f00ca9d11213 100644 --- a/tests/mcp/annotate.spec.ts +++ b/tests/mcp/annotate.spec.ts @@ -27,8 +27,8 @@ function activeSession(dashboard: import('playwright-core').Page) { } async function drawAndSubmitAnnotation(dashboard: import('playwright-core').Page, text: string) { - await expect(dashboard.getByRole('main', { name: 'Dashboard: annotate' })).toBeVisible(); - await expect(dashboard.locator('.annotate-modal-image')).toBeVisible(); + await expect(dashboard.getByRole('main', { name: 'Dashboard: annotate' })).toBeVisible({ timeout: 15_000 }); + await expect(dashboard.locator('.annotate-modal-image')).toBeVisible({ timeout: 15_000 }); const box = await dashboard.locator('.annotate-modal-image').boundingBox(); const x0 = box!.x + box!.width * 0.3; const y0 = box!.y + box!.height * 0.3; @@ -67,7 +67,7 @@ test('should capture multiple screenshots in one annotation', async ({ connectTo let done = false; void annotatePromise.finally(() => { done = true; }); - await expect(dashboard.locator('.annotate-modal-image')).toBeVisible(); + await expect(dashboard.locator('.annotate-modal-image')).toBeVisible({ timeout: 15_000 }); // First annotation on initial frame. let box = await dashboard.locator('.annotate-modal-image').boundingBox(); await dashboard.mouse.move(box!.x + box!.width * 0.2, box!.y + box!.height * 0.2); @@ -120,7 +120,7 @@ test('should abort annotation when last screenshot is removed', async ({ connect let done = false; void annotatePromise.finally(() => { done = true; }); - await expect(dashboard.locator('.annotate-sidebar-thumb')).toHaveCount(1); + await expect(dashboard.locator('.annotate-sidebar-thumb')).toHaveCount(1, { timeout: 15_000 }); // Close the fullscreen overlay first so the sidebar remove button is accessible. await dashboard.getByRole('button', { name: 'Done annotating' }).click(); @@ -153,7 +153,7 @@ test('should abort MCP annotation when last screenshot is removed', async ({ con const browser = await connectToDashboard(bindTitle); try { const dashboard = browser.contexts()[0].pages()[0]; - await expect(dashboard.locator('.annotate-sidebar-thumb')).toHaveCount(1); + await expect(dashboard.locator('.annotate-sidebar-thumb')).toHaveCount(1, { timeout: 15_000 }); // Close the fullscreen overlay first so the sidebar remove button is accessible. await dashboard.getByRole('button', { name: 'Done annotating' }).click(); @@ -205,7 +205,7 @@ test('user-initiated annotate downloads zip with feedback.md', async ({ connectT await expect(dashboard.locator('.annotate-sidebar-thumb')).toHaveCount(1); // Wait until the in-flight (aborted) submit fully resolves and the button is re-enabled, // otherwise installing the next picker mid-flight would race. - await expect(dashboard.getByRole('button', { name: 'Submit', exact: true })).toBeEnabled(); + await expect(dashboard.getByRole('button', { name: 'Submit', exact: true })).toBeEnabled({ timeout: 15_000 }); // Now install a capturing picker and submit for real. const awaitZipBytes = await installSaveFilePickerMock(dashboard); @@ -271,6 +271,7 @@ test('should start dashboard and annotate when no dashboard is running', async ( }); test('should enter annotate mode on fresh dashboard.tsx mount with -s --annotate', async ({ connectToDashboard, cli, server }) => { + test.slow(); const bindTitle = `--playwright-internal--${crypto.randomUUID()}`; await cli('-s=first', 'open', server.EMPTY_PAGE, { bindTitle }); await cli('-s=second', 'open', server.EMPTY_PAGE, { bindTitle }); @@ -282,7 +283,7 @@ test('should enter annotate mode on fresh dashboard.tsx mount with -s --annotate const browser = await connectToDashboard(bindTitle); try { const dashboard = browser.contexts()[0].pages()[0]; - await expect(dashboard.getByRole('main', { name: 'Dashboard: annotate' })).toBeVisible(); + await expect(dashboard.getByRole('main', { name: 'Dashboard: annotate' })).toBeVisible({ timeout: 15_000 }); await expect(activeSession(dashboard)).toHaveAccessibleName('Session second'); await drawAndSubmitAnnotation(dashboard, 'fresh'); } finally { @@ -314,7 +315,7 @@ test('should annotate via direct browser_annotate MCP call', async ({ connectToD const browser = await connectToDashboard(bindTitle); try { const dashboard = browser.contexts()[0].pages()[0]; - await expect(dashboard.getByRole('main', { name: 'Dashboard: annotate' })).toBeVisible(); + await expect(dashboard.getByRole('main', { name: 'Dashboard: annotate' })).toBeVisible({ timeout: 15_000 }); await drawAndSubmitAnnotation(dashboard, 'direct-mcp'); } finally { await browser.close().catch(() => {}); @@ -351,7 +352,7 @@ test('should annotate when context has no fixed viewport', async ({ connectToDas const browser = await connectToDashboard(bindTitle); try { const dashboard = browser.contexts()[0].pages()[0]; - await expect(dashboard.getByRole('main', { name: 'Dashboard: annotate' })).toBeVisible(); + await expect(dashboard.getByRole('main', { name: 'Dashboard: annotate' })).toBeVisible({ timeout: 15_000 }); await drawAndSubmitAnnotation(dashboard, 'no-viewport'); } finally { await browser.close().catch(() => {}); @@ -383,7 +384,7 @@ test('should cancel browser_annotate when the MCP request is aborted', async ({ const browser = await connectToDashboard(bindTitle); try { const dashboard = browser.contexts()[0].pages()[0]; - await expect(dashboard.getByRole('main', { name: 'Dashboard: annotate' })).toBeVisible(); + await expect(dashboard.getByRole('main', { name: 'Dashboard: annotate' })).toBeVisible({ timeout: 15_000 }); controller.abort(); @@ -413,7 +414,7 @@ test('should cancel browser_annotate when the MCP client disconnects', async ({ const browser = await connectToDashboard(bindTitle); try { const dashboard = browser.contexts()[0].pages()[0]; - await expect(dashboard.getByRole('main', { name: 'Dashboard: annotate' })).toBeVisible(); + await expect(dashboard.getByRole('main', { name: 'Dashboard: annotate' })).toBeVisible({ timeout: 15_000 }); await client.close(); @@ -425,6 +426,7 @@ test('should cancel browser_annotate when the MCP client disconnects', async ({ test('should switch screencast to -s session on show --annotate', async ({ connectToDashboard, cli, server }) => { + test.slow(); server.setContent('/red', '', 'text/html'); server.setContent('/green', '', 'text/html'); @@ -459,7 +461,7 @@ test('should switch screencast to -s session on show --annotate', async ({ conne let done = false; void annotatePromise.finally(() => { done = true; }); - await expect(dashboard.getByRole('main', { name: 'Dashboard: annotate' })).toBeVisible(); + await expect(dashboard.getByRole('main', { name: 'Dashboard: annotate' })).toBeVisible({ timeout: 15_000 }); await expect(activeSession(dashboard)).toHaveAccessibleName('Session second'); await expect.poll(async () => { @@ -474,6 +476,7 @@ test('should switch screencast to -s session on show --annotate', async ({ conne }); test('should disengage annotate mode when --annotate client disconnects', async ({ connectToDashboard, cli, childProcess, cliEnv, mcpBrowser, mcpHeadless, server }) => { + test.slow(); await cli('open', server.EMPTY_PAGE); const bindTitle = `--playwright-internal--${crypto.randomUUID()}`; await cli('show', { bindTitle }); @@ -492,7 +495,7 @@ test('should disengage annotate mode when --annotate client disconnects', async }), }); - await expect(dashboard.getByRole('main', { name: 'Dashboard: annotate' })).toBeVisible(); + await expect(dashboard.getByRole('main', { name: 'Dashboard: annotate' })).toBeVisible({ timeout: 15_000 }); await annotateClient.kill(); diff --git a/tests/mcp/dashboard.spec.ts b/tests/mcp/dashboard.spec.ts index b10a2ec4dff01..eda77af07cce1 100644 --- a/tests/mcp/dashboard.spec.ts +++ b/tests/mcp/dashboard.spec.ts @@ -178,3 +178,10 @@ test('save recording streams WebM bytes to the chosen file', async ({ cli, serve // WebM files start with the EBML magic bytes. expect(bytes.subarray(0, 4)).toEqual(Buffer.from([0x1a, 0x45, 0xdf, 0xa3])); }); + +test('two concurrent cli show invocations both succeed', async ({ cli }) => { + const bindTitle = `--playwright-internal--${crypto.randomUUID()}`; + const [first, second] = await Promise.all([cli('show', { bindTitle }), cli('show', { bindTitle })]); + expect(first.dashboardPid).toBe(second.dashboardPid); + await cli('show', '--kill'); +});