Skip to content

fix(cli): do not pass PLAYWRIGHT_CLI_SESSION as daemon --endpoint #41019

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

Open
Skn0tt wants to merge 2 commits into
base: main
Choose a base branch
from
+20 −9
Open

fix(cli): do not pass PLAYWRIGHT_CLI_SESSION as daemon --endpoint #41019

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bb46da6
fix(cli): do not pass PLAYWRIGHT_CLI_SESSION as daemon --endpoint
Skn0tt May 27, 2026
25fa0e6
fix(cli): enforce mutual exclusivity of attach target flags
Skn0tt Jun 1, 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
4 changes: 2 additions & 2 deletions packages/playwright-core/src/tools/cli-client/output.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ export class TextOutput implements Output {
}

errorAttachConflict(): never {
console.error(`Error: cannot use target name with --cdp, --endpoint, or --extension`);
console.error(`Error: only one of [name], --cdp, --endpoint, or --extension can be specified`);
return process.exit(1);
}

Expand Down Expand Up @@ -288,7 +288,7 @@ export class JsonOutput implements Output {
}

errorAttachConflict(): never {
this._emit({ isError: true, error: `cannot use target name with --cdp, --endpoint, or --extension` });
this._emit({ isError: true, error: `only one of [name], --cdp, --endpoint, or --extension can be specified` });
return process.exit(1);
}

Expand Down
3 changes: 2 additions & 1 deletion packages/playwright-core/src/tools/cli-client/program.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,8 @@ export async function program(options?: { embedderVersion?: string}) {
}
case 'attach': {
const attachTarget = args._[1] as string | undefined;
if (attachTarget && (args.cdp || args.endpoint || args.extension))
const targetCount = (attachTarget ? 1 : 0) + (args.cdp ? 1 : 0) + (args.endpoint ? 1 : 0) + (args.extension ? 1 : 0);
if (targetCount > 1)
output.errorAttachConflict();
if (attachTarget)
args.endpoint = attachTarget;
Expand Down
10 changes: 4 additions & 6 deletions packages/playwright-core/src/tools/cli-client/session.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,6 @@ export class Session {
];
if (cliArgs.headed)
args.push('--headed');
if (cliArgs.extension)
args.push('--extension');
if (cliArgs.browser)
args.push(`--browser=${cliArgs.browser}`);
if (cliArgs.persistent)
Expand All @@ -135,12 +133,12 @@ export class Session {
args.push(`--profile=${cliArgs.profile}`);
if (cliArgs.config)
args.push(`--config=${cliArgs.config}`);
if (cliArgs.cdp)
if (cliArgs.extension)
args.push('--extension');
else if (cliArgs.cdp)
args.push(`--cdp=${cliArgs.cdp}`);
if (cliArgs.endpoint)
else if (cliArgs.endpoint)
args.push(`--endpoint=${cliArgs.endpoint}`);
else if (mode === 'attach' && process.env.PLAYWRIGHT_CLI_SESSION)
Copy link
Copy Markdown
Member

@pavelfeldman pavelfeldman May 27, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

wouldn't it break PLAYWRIGHT_CLI_SESSION=myname npx @playwright/cli attach? I think we are missing else in line 140 which would make --cdp, --cdp and --endpoint mutually exclusive.

Copy link
Copy Markdown
Member Author

@Skn0tt Skn0tt May 28, 2026

Choose a reason for hiding this comment

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

That command doesn't work for me, here's the output on ToT:

 $ PLAYWRIGHT_CLI_SESSION=myname playwright-cli attach
 Error: no target specified for attach command; use one of [name], --cdp, --endpoint, or --extension to specify the target to attach to.

Maybe what you have in mind regressed in #39707?

Copy link
Copy Markdown
Member

@pavelfeldman pavelfeldman May 29, 2026

Choose a reason for hiding this comment

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

In either case, I think what I'm suggesting is a meaningful improvement.

Copy link
Copy Markdown
Member Author

@Skn0tt Skn0tt Jun 1, 2026

Choose a reason for hiding this comment

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

Done.

args.push(`--endpoint=${process.env.PLAYWRIGHT_CLI_SESSION}`);

const child = spawn(process.execPath, args, {
detached: true,
Expand Down
12 changes: 12 additions & 0 deletions tests/mcp/cli-cdp.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,18 @@ test('attach via cdp URL keeps the default session', async ({ cdpServer, cli, se
expect(listOutput).toContain('(attached)');
});

test('attach via cdp URL honors PLAYWRIGHT_CLI_SESSION as session name', { annotation: { type: 'issue', description: 'https://github.com/microsoft/playwright-cli/issues/414' } }, async ({ cdpServer, cli }) => {
await cdpServer.start();
const { exitCode } = await cli('attach', `--cdp=${cdpServer.endpoint}`, { env: { PLAYWRIGHT_CLI_SESSION: 'myname' } });
expect(exitCode).toBe(0);
});

test('attach rejects combining --cdp, --endpoint, or --extension', async ({ cli }) => {
const { error, exitCode } = await cli('attach', '--cdp=chrome-dev', '--endpoint=/tmp/foo');
expect(exitCode).toBe(1);
expect(error).toContain('only one of [name], --cdp, --endpoint, or --extension can be specified');
});

test('detach tears down an attached session', async ({ cdpServer, cli }) => {
await cdpServer.start();

Expand Down
Loading