fix(cli): do not pass PLAYWRIGHT_CLI_SESSION as daemon --endpoint#41019
Open
Skn0tt wants to merge 1 commit into
Open
fix(cli): do not pass PLAYWRIGHT_CLI_SESSION as daemon --endpoint#41019Skn0tt wants to merge 1 commit into
Skn0tt wants to merge 1 commit into
Conversation
PLAYWRIGHT_CLI_SESSION is the env-var equivalent of -s/--session (the session name). It is already consumed as such in startDaemon via resolveSessionName. The leftover else-if branch additionally re-passed it as the daemon's --endpoint= argument when mode is 'attach', which made the daemon try to connect to a socket at the literal session-name path. Repro: PLAYWRIGHT_CLI_SESSION=myname playwright-cli attach --cdp=http://127.0.0.1:99999 -> Error: Daemon pid=...: connect ENOENT myname The fallback dates back to microsoft#39650, when PLAYWRIGHT_CLI_SESSION only meant "endpoint to attach to". After microsoft#39707 it gained its current session-name meaning, but the obsolete fallback survived through the --attach -> --endpoint rename (microsoft#39972) and the mode-guard refactor (microsoft#40176). Fixes microsoft/playwright-cli#414
Contributor
Test results for "MCP"7188 passed, 1119 skipped Merge workflow run. |
| args.push(`--cdp=${cliArgs.cdp}`); | ||
| if (cliArgs.endpoint) | ||
| args.push(`--endpoint=${cliArgs.endpoint}`); | ||
| else if (mode === 'attach' && process.env.PLAYWRIGHT_CLI_SESSION) |
Member
There was a problem hiding this comment.
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.
Member
Author
There was a problem hiding this comment.
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?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PLAYWRIGHT_CLI_SESSIONwas being passed to the daemon as--endpoint=<value>inattachmode, in addition to its correct use as the session name. The daemon then tried to connect to a socket at the literal session-name path.startDaemon.resolveSessionNamealready consumes the env var with its documented meaning.Fixes microsoft/playwright-cli#414