Improve Playwright CLI guidance and browser provisioning - #57731
Conversation
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
|
Great work! 🎉 This PR looks ready for review. You've done a thorough job updating the Playwright CLI guidance to match the current
|
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft.
|
|
✅ PR Code Quality Reviewer completed the code quality review.
|
|
🧠 Matt Pocock Skills Reviewer failed during the skills-based review. Warning Threat Detection Engine Failure — The analysis engine could not complete. This is a tooling failure, not a security finding. What happenedThe threat detection engine failed to produce results. Review the workflow run logs for details.
|
|
✅ Test Quality Sentinel completed test quality analysis. Test Quality Sentinel skipped because pre-fetch PR data was unavailable: unable to fetch test file diff
|
|
Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "ab.chatgpt.com"See Network Configuration for more information.
|
Comment MemoryNote This comment is managed by comment memory.It stores persistent context for this thread in the code block at the top of this comment.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict
REQUEST_CHANGES — the documentation update is fine, but the compiler change hard-codes three playwright-cli install-browser steps into every Playwright-enabled job and there is no evidence those commands exist in the pinned CLI/runtime combination.
Blocking themes
- The implementation assumes a browser-install subcommand/API without any compatibility guard or version-gated test against the actual pinned
@playwright/clibehavior. - The new guidance tells users browsers are pre-provisioned, so if those install steps fail at runtime the entire workflow breaks before the agent starts.
🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt54 · 34.3 AIC · ⌖ 7.34 AIC · ⊞ 21.8K
Comment /review to run again
ADR required before mergeI enforced the design-decision gate for this PR because it adds 243 new lines in business-logic directories ( Evidence reviewed
Action takenI added a draft ADR at Next action for the authorReview and refine that ADR with any missing rationale, then keep it with the PR as the decision record for this implementation.
|
There was a problem hiding this comment.
pkg/workflow/playwright_cli.go:114: shrink: helper plus browser-name list for three fixed browser installs. Inline the three step literals and drop the abstraction.
net: -10 lines possible.
Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
ab.chatgpt.com
To allow these domains, add them to the network.allowed list in your workflow frontmatter:
network:
allowed:
- defaults
- "ab.chatgpt.com"See Network Configuration for more information.
Generated by ✂️ Ponytail Reviewer for #57731 · codex · mai10 · 4.53 AIC · ⌖ 0.322 AIC · ⊞ 13.5K
Comment /ponytail to run again
| return steps | ||
| } | ||
|
|
||
| func generatePlaywrightBrowserInstallSteps() []GitHubActionStep { |
There was a problem hiding this comment.
pkg/workflow/playwright_cli.go:114: shrink: helper plus browser-name list for three fixed browser installs. Inline the three step literals and drop the abstraction.
There was a problem hiding this comment.
The browser install steps now use explicit literals, while each step sets the pinned CLI's supported PLAYWRIGHT_BROWSERS_PATH; regenerated lock files include the same commands and environment.
There was a problem hiding this comment.
🟡 Changes recommended
Host-side browser downloads are not made available inside the AWF sandbox, and current tests do not detect that failure.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Updates Playwright CLI provisioning and guidance for browser support, sandbox networking, artifacts, accessibility, and diagnostics.
Changes:
- Provisions Chromium, Firefox, and WebKit before agent execution.
- Updates Playwright documentation and CLI examples.
- Extends compiler fixtures for loopback access and restricted Bash permissions.
File summaries
| File | Description |
|---|---|
pkg/workflow/playwright_cli.go |
Adds browser provisioning steps. |
pkg/workflow/playwright_cli_test.go |
Tests generated provisioning commands. |
pkg/workflow/testdata/wasm_golden/fixtures/playwright-cli-mode.md |
Exercises loopback and restricted Bash. |
pkg/workflow/testdata/TestWasmGolden_CompileFixtures/playwright-cli-mode.golden |
Updates canonical compiled output. |
pkg/workflow/testdata/wasm_golden/WasmBinary/playwright-cli-mode.golden |
Updates WASM output snapshot. |
docs/src/content/docs/reference/playwright.md |
Revises public Playwright reference. |
.github/aw/playwright.md |
Expands internal workflow guidance. |
.github/aw/network.md |
Clarifies loopback behavior. |
.github/aw/actions-lock.json |
Adds a checkout action pin. |
Review details
- Files reviewed: 10/10 changed files
- Comments generated: 3
- Review effort level: Balanced
| for _, browser := range playwrightBrowserNames { | ||
| steps = append(steps, GitHubActionStep{ | ||
| " - name: Install Playwright " + browser + " browser", | ||
| " run: playwright-cli install-browser " + browser, |
There was a problem hiding this comment.
Fixed in pkg/workflow/playwright_cli.go: browsers install into ${RUNNER_TEMP}/gh-aw/playwright-browsers, a runner-temp path mounted into AWF, and the same path is exported to the agent execution environment.
| assert.Contains(t, browserStep, "playwright-cli install-browser "+browser) | ||
| assert.Contains(t, browserStep, "timeout-minutes: 10") |
There was a problem hiding this comment.
Added compiler coverage that verifies all three browser provisioning steps use the shared mounted cache, and synchronized every generated lock file. This catches host-cache versus sandbox-cache mismatches before runtime.
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
|
@copilot address review feedback |
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
|
@copilot playwright cli is the only mode so isPlaywrightCLIMode should always be true. Apply the env var to all agentic engines. |
Addressed in commits |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot the user should specify the list of browser engines it wants as a child field of "playwright": otherwise, by default Chrome is downloaded. Create a bash script actions/setup/sh that takes a list of browser engines and downloads them. Use retries to make it more resilient. |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Implemented in |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
... Implemented in commit |
Playwright guidance now matches the current
@playwright/cliinterface and explicitly defines browser, loopback, artifact, accessibility, and diagnostic behavior in AWF.Provisioning
Workflow guidance
playwright-cli:*access for restricted Bash allowlists.network.allowed: local./tmpfiles, accessibility boundaries, and diagnostics.Public reference
open,snapshot,resize, andscreenshot.local.Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
github.comTo allow these domains, add them to the
network.allowedlist in your workflow frontmatter:See Network Configuration for more information.