Skip to content

Commit f7555ef

Browse files
committed
chore(cli): require explicit target for attach
- attach now errors if no positional target / --cdp / --extension is supplied; the implicit fallback through PLAYWRIGHT_CLI_SESSION / --config is gone. - list output drops the per-server status line; serverRegistry.list() already filters to connectable descriptors, so dashboard and sessionSidebar drop their redundant canConnect filters too. - Attach helper text suggests the short --s= form.
1 parent 815d051 commit f7555ef

9 files changed

Lines changed: 46 additions & 75 deletions

File tree

packages/dashboard/src/dashboardChannel.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,9 @@
1515
*/
1616

1717
import type { ClientInfo } from '../../playwright-core/src/tools/cli-client/registry';
18-
import type { BrowserStatus } from '../../playwright-core/src/serverRegistry';
18+
import type { BrowserDescriptor } from '../../playwright-core/src/serverRegistry';
1919

20-
export type SessionStatus = BrowserStatus;
20+
export type SessionStatus = BrowserDescriptor;
2121

2222
export type Tab = {
2323
browser: string;

packages/dashboard/src/sessionSidebar.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ const TabRow: React.FC<{ tab: Tab; model: DashboardModel }> = ({ tab, model }) =
7878

7979
export const SessionSidebar: React.FC<SessionSidebarProps> = ({ model }) => {
8080
const { sessions, clientInfo, loadingSessions, tabs: allTabs } = model.state;
81-
const openSessions = React.useMemo(() => sessions.filter(session => session.canConnect), [sessions]);
81+
const openSessions = sessions;
8282

8383
const tabsByBrowserAndContext = React.useMemo(() => {
8484
const map = new Map<string, Map<string, Tab[]>>();

packages/playwright-core/src/serverRegistry.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ class ServerRegistry extends EventEmitter {
8989
return this._ready ?? Promise.resolve();
9090
}
9191

92-
async list(): Promise<Map<string, BrowserStatus[]>> {
92+
async list(): Promise<Map<string, BrowserDescriptor[]>> {
9393
const ownWatcher = !this._watcher;
9494
let dispose: (() => void) | undefined;
9595
if (ownWatcher)
@@ -102,7 +102,7 @@ class ServerRegistry extends EventEmitter {
102102
return { descriptor, canConnect };
103103
}),
104104
);
105-
const result = new Map<string, BrowserStatus[]>();
105+
const result = new Map<string, BrowserDescriptor[]>();
106106
for (const { descriptor, canConnect } of statuses) {
107107
if (!canConnect) {
108108
await fs.promises.unlink(path.join(this._browsersDir(), descriptor.browser.guid)).catch(() => {});
@@ -114,7 +114,7 @@ class ServerRegistry extends EventEmitter {
114114
list = [];
115115
result.set(key, list);
116116
}
117-
list.push({ ...descriptor, canConnect });
117+
list.push(descriptor);
118118
}
119119
return result;
120120
} finally {
@@ -133,7 +133,7 @@ class ServerRegistry extends EventEmitter {
133133
endpoint: endpoint.endpoint,
134134
workspaceDir: endpoint.workspaceDir,
135135
};
136-
await fs.promises.writeFile(file, JSON.stringify(descriptor), 'utf-8');
136+
await fs.promises.writeFile(file, JSON.stringify(descriptor, null, 2), 'utf-8');
137137
}
138138

139139
async delete(guid: string): Promise<void> {

packages/playwright-core/src/tools/cli-client/output.ts

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import path from 'path';
2222
import { playwrightExtensionInstallUrl } from '../utils/extension';
2323

2424
import type { ChannelSession } from './channelSessions';
25-
import type { BrowserStatus } from '../../serverRegistry';
25+
import type { BrowserDescriptor } from '../../serverRegistry';
2626

2727
export type ListedBrowser = {
2828
name: string;
@@ -40,7 +40,7 @@ export type ListedBrowser = {
4040
export type ListData = {
4141
all: boolean;
4242
browsers: ListedBrowser[];
43-
servers?: BrowserStatus[];
43+
servers?: BrowserDescriptor[];
4444
channelSessions?: ChannelSession[];
4545
};
4646

@@ -55,6 +55,7 @@ export interface Output {
5555
errorAttachConflict(): never;
5656
errorDetachNotAttached(session: string): never;
5757
errorBrowserNotOpenForTool(session: string): never;
58+
errorAttachNoTarget(): never;
5859

5960
list(data: ListData): void;
6061
closeAll(sessions: string[]): void;
@@ -112,6 +113,11 @@ export class TextOutput implements Output {
112113
return process.exit(1);
113114
}
114115

116+
errorAttachNoTarget(): never {
117+
console.error(`Error: no target specified for attach command; use one of [name], --cdp, --endpoint, or --extension to specify the target to attach to.`);
118+
return process.exit(1);
119+
}
120+
115121
list({ all, browsers, servers, channelSessions }: ListData): void {
116122
const byWorkspace = new Map<string, ListedBrowser[]>();
117123
for (const browser of browsers) {
@@ -144,7 +150,7 @@ export class TextOutput implements Output {
144150
if (count)
145151
console.log('');
146152
console.log('### Browser servers available for attach');
147-
const serversByWorkspace = new Map<string, BrowserStatus[]>();
153+
const serversByWorkspace = new Map<string, BrowserDescriptor[]>();
148154
for (const server of servers) {
149155
let list = serversByWorkspace.get(server.workspaceDir ?? '');
150156
if (!list) {
@@ -204,7 +210,8 @@ export class TextOutput implements Output {
204210
attach(session: string, pid: number | undefined, endpoint: string | undefined, toolResult: string): void {
205211
if (endpoint) {
206212
console.log(`### Session \`${session}\` created, attached to \`${endpoint}\`.`);
207-
console.log(`Run commands with: playwright-cli --session=${session} <command>`);
213+
console.log(`Run commands with: playwright-cli --s=${session} <command>`);
214+
console.log('');
208215
} else {
209216
console.log(`### Browser \`${session}\` opened with pid ${pid}.`);
210217
}
@@ -282,6 +289,11 @@ export class JsonOutput implements Output {
282289
return process.exit(1);
283290
}
284291

292+
errorAttachNoTarget(): never {
293+
this._emit({ isError: true, error: `no target specified for attach command; use one of [name], --cdp, --endpoint, or --extension to specify the target to attach to.` });
294+
return process.exit(1);
295+
}
296+
285297
list({ all, browsers, servers, channelSessions }: ListData): void {
286298
const payload: Record<string, unknown> = { browsers };
287299
if (all) {
@@ -372,11 +384,10 @@ function renderBrowser(browser: ListedBrowser): string {
372384
return lines.join('\n');
373385
}
374386

375-
function renderServer(server: BrowserStatus): string {
387+
function renderServer(server: BrowserDescriptor): string {
376388
const lines = [`- browser "${server.title}":`];
377389
lines.push(` - browser: ${server.browser.browserName}`);
378390
lines.push(` - version: v${server.playwrightVersion}`);
379-
lines.push(` - status: ${server.canConnect ? 'open' : 'closed'}`);
380391
if (server.browser.userDataDir)
381392
lines.push(` - data-dir: ${server.browser.userDataDir}`);
382393
else

packages/playwright-core/src/tools/cli-client/program.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -165,13 +165,17 @@ export async function program(options?: { embedderVersion?: string}) {
165165
args.browser = args.extension;
166166
args.extension = true;
167167
}
168+
168169
const cdpChannel = typeof args.cdp === 'string' && isKnownChannel(args.cdp) ? args.cdp : undefined;
170+
const targetName = attachTarget ?? cdpChannel ?? extensionChannel ?? args.cdp as string;
171+
if (!targetName)
172+
output.errorAttachNoTarget();
169173
const attachSessionName = explicitSessionName(args.session as string) ?? attachTarget ?? cdpChannel ?? extensionChannel ?? sessionName;
170174
args.session = attachSessionName;
171-
const { pid, endpoint } = await startSession(attachSessionName, registry, clientInfo, args, 'attach');
175+
const { pid } = await startSession(attachSessionName, registry, clientInfo, args, 'attach');
172176
const newEntry = await registry.loadEntry(clientInfo, attachSessionName);
173177
const toolText = await runInSession(newEntry, clientInfo, { _: ['snapshot'], filename: '<auto>' }, output);
174-
output.attach(attachSessionName, pid, endpoint, toolText);
178+
output.attach(attachSessionName, pid, targetName, toolText);
175179
return;
176180
}
177181
case 'close': {
@@ -341,14 +345,17 @@ async function killAllDaemons(): Promise<number[]> {
341345
async function collectList(registry: Registry, clientInfo: ClientInfo, all: boolean): Promise<ListData> {
342346
const browsers: ListedBrowser[] = [];
343347
const entries = registry.entryMap();
348+
349+
// List early to GC.
350+
const serverEntries = await serverRegistry.list();
344351
const key = clientKey(clientInfo);
345352
for (const [workspaceKey, list] of entries) {
346353
if (!all && workspaceKey !== key)
347354
continue;
348355
for (const entry of list) {
349356
const session = new Session(entry);
350357
const canConnect = await session.canConnect();
351-
if (!canConnect && !session.config.cli.persistent) {
358+
if (!canConnect) {
352359
await session.deleteSessionConfig();
353360
continue;
354361
}
@@ -372,7 +379,6 @@ async function collectList(registry: Registry, clientInfo: ClientInfo, all: bool
372379
if (!all)
373380
return { all, browsers };
374381

375-
const serverEntries = await serverRegistry.list();
376382
const servers = [...serverEntries.values()].flat();
377383
return { all, browsers, servers, channelSessions: await listChannelSessions() };
378384
}

packages/playwright-core/src/tools/cli-client/skill/SKILL.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -209,8 +209,8 @@ playwright-cli open --persistent
209209
# Use persistent profile with custom directory
210210
playwright-cli open --profile=/path/to/profile
211211
212-
# Connect to browser via extension
213-
playwright-cli attach --extension
212+
# Connect to browser via Playwright Extension
213+
playwright-cli attach --extension=chrome
214214
215215
# Connect to a running Chrome or Edge by channel name
216216
playwright-cli attach --cdp=chrome

packages/playwright-core/src/tools/dashboard/dashboardController.ts

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ import { createClientInfo } from '../cli-client/registry';
2828
import type * as api from '../../..';
2929
import type { Transport } from '@utils/httpServer';
3030
import type { AnnotationData, Tab } from '@dashboard/dashboardChannel';
31-
import type { BrowserDescriptor, BrowserStatus } from '../../serverRegistry';
31+
import type { BrowserDescriptor } from '../../serverRegistry';
3232

3333
type BrowserTrackerCallbacks = {
3434
onTabsChanged: () => void;
@@ -274,7 +274,7 @@ export class DashboardConnection implements Transport {
274274
return this._visible;
275275
}
276276

277-
emitSessions(sessions: BrowserStatus[]) {
277+
emitSessions(sessions: BrowserDescriptor[]) {
278278
this.sendEvent?.('sessions', { sessions, clientInfo: createClientInfo() });
279279
}
280280

@@ -379,7 +379,7 @@ export class DashboardConnection implements Transport {
379379
this._pushSessionsScheduled = false;
380380
try {
381381
const byWs = await serverRegistry.list();
382-
const sessions: BrowserStatus[] = [];
382+
const sessions: BrowserDescriptor[] = [];
383383
for (const list of byWs.values()) {
384384
for (const status of list) {
385385
if (status.title.startsWith('--playwright-internal'))
@@ -397,12 +397,10 @@ export class DashboardConnection implements Transport {
397397
});
398398
};
399399

400-
private async _reconcile(sessions: BrowserStatus[]) {
401-
const connectable = new Map<string, BrowserStatus>();
402-
for (const status of sessions) {
403-
if (status.canConnect)
404-
connectable.set(status.browser.guid, status);
405-
}
400+
private async _reconcile(sessions: BrowserDescriptor[]) {
401+
const connectable = new Map<string, BrowserDescriptor>();
402+
for (const status of sessions)
403+
connectable.set(status.browser.guid, status);
406404

407405
for (const [guid, slot] of this._browsers) {
408406
if (connectable.has(guid))

tests/mcp/cli-cdp.spec.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,10 +93,9 @@ test('cdp server', async ({ cdpServer, cli, server }) => {
9393

9494
const configPath = test.info().outputPath('config.ini');
9595
await fs.promises.writeFile(configPath, `
96-
browser.cdpEndpoint=${cdpServer.endpoint}
9796
browser.isolated=false
9897
`);
99-
await cli('attach', `--config=${configPath}`);
98+
await cli('attach', `--cdp=${cdpServer.endpoint}`, `--config=${configPath}`);
10099
const { inlineSnapshot } = await cli('snapshot');
101100
expect(inlineSnapshot).toContain(`- generic [active] [ref=e1]: Hello, world!`);
102101
});

tests/mcp/cli-session.spec.ts

Lines changed: 2 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -69,19 +69,6 @@ test('close non-running session', async ({ cli }) => {
6969
expect(output).toContain(`Browser 'nonexistent' is not open.`);
7070
});
7171

72-
test('persistent session shows in list after close', async ({ cli, server }) => {
73-
await cli('open', server.HELLO_WORLD, '--persistent');
74-
75-
const { output: listBefore } = await cli('list');
76-
expect(listBefore).toContain('- default:');
77-
expect(listBefore).not.toContain('<in-memory>');
78-
79-
await cli('close');
80-
81-
const { output: listAfter } = await cli('list');
82-
expect(listAfter).toContain('- default:');
83-
});
84-
8572
test('close-all', async ({ cli, server }) => {
8673
await cli('-s', 'session1', 'open', server.HELLO_WORLD);
8774
await cli('-s', 'session2', 'open', server.HELLO_WORLD);
@@ -310,7 +297,6 @@ workspace1:
310297
- browser "foobar":
311298
- browser: ${/* FIX browser._options */ mcpBrowser.replace('chrome', 'chromium')}
312299
- version: ${version}
313-
- status: open
314300
- data-dir: <in-memory>
315301
- run \`playwright-cli attach "foobar"\` to attach`);
316302
});
@@ -323,7 +309,7 @@ workspace1:
323309
await page.setContent('<title>My Page</title>');
324310
const { output: openOutput } = await cli('attach', 'foobar');
325311
expect(openOutput).toContain('### Session `foobar` created, attached to `foobar`.');
326-
expect(openOutput).toContain('Run commands with: playwright-cli --session=foobar <command>');
312+
expect(openOutput).toContain('Run commands with: playwright-cli --s=foobar <command>');
327313
const { output: listOutput } = await cli('list', '--all');
328314
expect(listOutput).toBe(`### Browsers
329315
/:
@@ -336,7 +322,6 @@ workspace1:
336322
- browser "foobar":
337323
- browser: ${mcpBrowser.replace('chrome', 'chromium')}
338324
- version: ${version}
339-
- status: open
340325
- data-dir: <in-memory>
341326
- run \`playwright-cli attach "foobar"\` to attach`);
342327
});
@@ -349,33 +334,6 @@ workspace1:
349334
expect(error).toContain('Error: unable to connect to a browser that does not have any contexts');
350335
});
351336

352-
test('attach via PLAYWRIGHT_CLI_SESSION env', async ({ cli, mcpBrowser }) => {
353-
const browserName = mcpBrowser.replace('chrome', 'chromium');
354-
await using browser = await playwright[browserName].launch({ headless: true });
355-
const page = await browser.newPage();
356-
await page.setContent('<title>Env Page</title><h1>Hello from env</h1>');
357-
await browser.bind('foobar', { workspaceDir: 'workspace1' });
358-
const { output: openOutput, snapshot } = await cli('attach', { env: { PLAYWRIGHT_CLI_SESSION: 'foobar' } });
359-
expect(openOutput).toContain('### Browser `foobar` opened with pid');
360-
expect(openOutput).toContain('Env Page');
361-
expect(snapshot).toContain('Hello from env');
362-
const { output: listOutput } = await cli('list', '--all');
363-
expect(listOutput).toBe(`### Browsers
364-
/:
365-
- foobar:
366-
- status: open
367-
- browser-type: ${mcpBrowser.replace('chrome', 'chromium')} (attached)
368-
369-
### Browser servers available for attach
370-
workspace1:
371-
- browser "foobar":
372-
- browser: ${mcpBrowser.replace('chrome', 'chromium')}
373-
- version: ${version}
374-
- status: open
375-
- data-dir: <in-memory>
376-
- run \`playwright-cli attach "foobar"\` to attach`);
377-
});
378-
379337
test('attach with session alias', async ({ cli, mcpBrowser }) => {
380338
const browserName = mcpBrowser.replace('chrome', 'chromium');
381339
await using browser = await playwright[browserName].launch({ headless: true });
@@ -384,7 +342,7 @@ workspace1:
384342
await page.setContent('<title>Alias Page</title>');
385343
const { output: openOutput } = await cli('attach', 'foobar', '--session=mybrowser');
386344
expect(openOutput).toContain('### Session `mybrowser` created, attached to `foobar`.');
387-
expect(openOutput).toContain('Run commands with: playwright-cli --session=mybrowser <command>');
345+
expect(openOutput).toContain('Run commands with: playwright-cli --s=mybrowser <command>');
388346
await cli('-s', 'mybrowser', 'close');
389347
});
390348

@@ -402,7 +360,6 @@ workspace1:
402360
- browser \"foobar\":
403361
- browser: ${/* FIX browser._options */ mcpBrowser.replace('chrome', 'chromium')}
404362
- version: ${version}
405-
- status: open
406363
- data-dir: <in-memory>
407364
- run \`playwright-cli attach \"foobar\"\` to attach`);
408365
});

0 commit comments

Comments
 (0)