Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions .changeset/improve-playwright-browser-guidance.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

25 changes: 19 additions & 6 deletions .github/aw/playwright.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,25 @@ tools:
playwright:
```

The compiler installs the pinned default `@playwright/cli` package and its agent
skills. It also provisions Chromium, Firefox, and WebKit before the agent starts;
the default `open` session uses Chromium. Do not add installation steps to the
workflow: runtime installation of packages or browsers is prohibited. Pin
`version` only when reproducible browser output is required, such as for visual
baselines:
The compiler installs the pinned default `@playwright/cli` package, its agent
skills, and Chromium before the agent starts. The default `open` session uses
Chromium. To use other browser engines, list them in `browsers`. Playwright's
`chromium` download is the Chrome for Testing distribution; `chrome` and
`chrome-for-testing` are accepted aliases:

```yaml
tools:
playwright:
browsers: [chromium, firefox, webkit]
```

Supported values are `chrome`, `chrome-for-testing`, `chromium`, `firefox`, and
`webkit`. The broader Playwright install-target list also contains system browser
channels and platform-specific tools, but those are not portable browser engines
for this field. Do not add steps such as `npx playwright install` or
`npm exec playwright install`; the compiler provisions the selected engines, and
browser installation during agent execution is prohibited. Pin `version` only
when reproducible browser output is required, such as for visual baselines:

```yaml
tools:
Expand Down
4 changes: 2 additions & 2 deletions docs/src/content/docs/reference/frontmatter-full.md
Original file line number Diff line number Diff line change
Expand Up @@ -4208,8 +4208,8 @@ tools:
# GitHub Actions expression.
mode: "example-value"

# Browsers to provision before the agent starts. Defaults to Chromium. Chrome is
# accepted as an alias for Chromium.
# Browsers to provision before the agent starts. Defaults to Chromium. Chrome and
# Chrome for Testing are accepted as aliases for Chromium.
# (optional)
browsers: []
# Array of strings
Expand Down
8 changes: 5 additions & 3 deletions docs/src/content/docs/reference/playwright.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,18 @@ tools:

The compiler installs `@playwright/cli` as a global npm package, its skills, and
Chromium before the agent runs. The default `open` browser is Chromium. Select
additional browsers with `browsers`; `chrome` is accepted as an alias for
`chromium`:
additional browsers with `browsers`. Playwright's `chromium` download is the
Chrome for Testing distribution; `chrome` and `chrome-for-testing` are accepted
aliases:

```yaml wrap
tools:
playwright:
browsers: [chrome, firefox]
```

The supported values are `chrome`, `chromium`, `firefox`, and `webkit`.
The supported values are `chrome`, `chrome-for-testing`, `chromium`, `firefox`,
and `webkit`.
Requested browsers are downloaded with retries before the agent starts; package
and browser installation during agent execution is prohibited. The agent
invokes `playwright-cli <command>` from bash:
Expand Down
4 changes: 2 additions & 2 deletions pkg/parser/schemas/main_workflow_schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -4966,10 +4966,10 @@
},
"browsers": {
"type": "array",
"description": "Browsers to provision before the agent starts. Defaults to Chromium. Chrome is accepted as an alias for Chromium.",
"description": "Browsers to provision before the agent starts. Defaults to Chromium. Chrome and Chrome for Testing are accepted as aliases for Chromium.",
"items": {
"type": "string",
"enum": ["chrome", "chromium", "firefox", "webkit"]
"enum": ["chrome", "chrome-for-testing", "chromium", "firefox", "webkit"]
},
"minItems": 1,
"uniqueItems": true
Expand Down
1 change: 1 addition & 0 deletions pkg/workflow/compiler_validators.go
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,7 @@ func (c *Compiler) emitGeneralToolWarnings(workflowData *WorkflowData, markdownP
}
c.emitSandboxRuntimeWarnings(workflowData, markdownPath)
c.emitPiThreatDetectionAuthWarning(workflowData, markdownPath)
c.emitPlaywrightBrowserInstallWarning(workflowData, markdownPath)
if workflowData.SafeOutputs != nil && workflowData.SafeOutputs.AssignToAgent != nil &&
workflowData.SafeOutputs.GitHubApp != nil && workflowData.SafeOutputs.AssignToAgent.GitHubToken == "" {
fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr(
Expand Down
2 changes: 1 addition & 1 deletion pkg/workflow/playwright_cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func TestGeneratePlaywrightCLIInstallSteps_ModeOmitted(t *testing.T) {
func TestGeneratePlaywrightCLIInstallSteps_SelectedBrowsers(t *testing.T) {
steps := generatePlaywrightCLIInstallSteps(&WorkflowData{
Tools: map[string]any{"playwright": map[string]any{
"browsers": []any{"chrome", "Firefox", "webkit", "chrome"},
"browsers": []any{"chrome", "Firefox", "webkit", "chrome-for-testing", "chrome"},
}},
})

Expand Down
47 changes: 45 additions & 2 deletions pkg/workflow/playwright_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,18 @@ package workflow

import (
"fmt"
"os"
"regexp"
"strings"

"github.com/goccy/go-yaml"
)

var playwrightBrowserInstallPattern = regexp.MustCompile(`(?im)(?:^|&&|\|\||;)[ \t]*(?:(?:npx|npm[ \t]+(?:exec|x)|pnpm[ \t]+(?:exec|dlx)|yarn(?:[ \t]+(?:exec|dlx))?|bunx)[ \t]+(?:(?:--yes|--no-install|--)[ \t]+)*)?playwright(?:@[^\s;&|]+)?[ \t]+install(?:[ \t]|$)`)

func normalizePlaywrightBrowser(browser string) string {
switch strings.ToLower(strings.TrimSpace(browser)) {
case "chrome", "chromium":
case "chrome", "chrome-for-testing", "chromium":
return "chromium"
case "firefox":
return "firefox"
Expand Down Expand Up @@ -85,7 +91,7 @@ func (c *Compiler) validatePlaywrightMode(workflowData *WorkflowData) error {
return NewValidationError(
"tools.playwright.browsers",
fmt.Sprint(browser),
"unsupported browser; choose chrome, chromium, firefox, or webkit",
"unsupported browser; choose chrome, chrome-for-testing, chromium, firefox, or webkit",
"Set browsers to a list containing supported Playwright browser names",
)
}
Expand All @@ -94,3 +100,40 @@ func (c *Compiler) validatePlaywrightMode(workflowData *WorkflowData) error {
}
return nil
}

func (c *Compiler) emitPlaywrightBrowserInstallWarning(workflowData *WorkflowData, markdownPath string) {
if workflowData == nil || !isPlaywrightCLIMode(workflowData.Tools) || !hasPlaywrightBrowserInstallStep(workflowData) {
return
}

fmt.Fprintln(os.Stderr, formatCompilerMessage(markdownPath, "warning",
"custom steps install Playwright browser engines. Remove those installation commands and use `tools.playwright.browsers` instead; the compiler provisions the selected browsers before the agent starts."))
c.IncrementWarningCount()
}

func hasPlaywrightBrowserInstallStep(workflowData *WorkflowData) bool {
sections := []string{
workflowData.PreSteps,
workflowData.CustomSteps,
workflowData.PreAgentSteps,
workflowData.PostSteps,
}

for _, section := range sections {
if section == "" {
continue
}
var wrapper map[string][]WorkflowStep
if err := yaml.Unmarshal([]byte(section), &wrapper); err != nil {
continue
}
for _, steps := range wrapper {
for _, step := range steps {
if playwrightBrowserInstallPattern.MatchString(step.Run) {
return true
}
}
}
}
return false
}
141 changes: 140 additions & 1 deletion pkg/workflow/playwright_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"path/filepath"
"testing"

"github.com/github/gh-aw/pkg/testutil"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -212,7 +213,7 @@ func TestValidatePlaywrightModeNilWorkflow(t *testing.T) {
func TestValidatePlaywrightBrowsers(t *testing.T) {
compiler := NewCompiler()
err := compiler.validatePlaywrightMode(&WorkflowData{Tools: map[string]any{
"playwright": map[string]any{"browsers": []any{"chrome", "firefox"}},
"playwright": map[string]any{"browsers": []any{"chrome", "chrome-for-testing", "firefox"}},
}})
require.NoError(t, err)

Expand All @@ -221,3 +222,141 @@ func TestValidatePlaywrightBrowsers(t *testing.T) {
}})
require.Error(t, err)
}

func TestEmitPlaywrightBrowserInstallWarning(t *testing.T) {
tests := []struct {
name string
tools map[string]any
preSteps string
customSteps string
preAgentSteps string
postSteps string
wantWarning bool
}{
{
name: "npm exec browser install",
tools: map[string]any{"playwright": nil},
customSteps: `steps:
- name: Install Playwright Chromium
run: npm exec playwright install --with-deps chromium
`,
wantWarning: true,
},
{
name: "npx browser install",
tools: map[string]any{"playwright": nil},
customSteps: `steps:
- run: npx --yes playwright@latest install firefox
`,
wantWarning: true,
},
{
name: "browser install after another command",
tools: map[string]any{"playwright": nil},
customSteps: `steps:
- run: npm ci && pnpm exec playwright install webkit
`,
wantWarning: true,
},
{
name: "bare browser install in pre-steps",
tools: map[string]any{"playwright": nil},
preSteps: `pre-steps:
- run: playwright install chromium
`,
wantWarning: true,
},
{
name: "browser install in pre-agent steps",
tools: map[string]any{"playwright": nil},
preAgentSteps: `pre-agent-steps:
- run: bunx playwright install chromium
`,
wantWarning: true,
},
{
name: "browser install in post-steps",
tools: map[string]any{"playwright": nil},
postSteps: `post-steps:
- run: yarn exec playwright install webkit
`,
wantWarning: true,
},
{
name: "skills install is not a browser install",
tools: map[string]any{"playwright": nil},
customSteps: `steps:
- run: playwright-cli install --skills
`,
},
{
name: "package install is not a browser install",
tools: map[string]any{"playwright": nil},
customSteps: `steps:
- run: npm install playwright
`,
},
{
name: "disabled Playwright tool",
tools: map[string]any{"playwright": false},
customSteps: `steps:
- run: npx playwright install chromium
`,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
compiler := NewCompiler()
output := testutil.CaptureStderr(t, func() {
compiler.emitPlaywrightBrowserInstallWarning(&WorkflowData{
Tools: tt.tools,
PreSteps: tt.preSteps,
CustomSteps: tt.customSteps,
PreAgentSteps: tt.preAgentSteps,
PostSteps: tt.postSteps,
}, "test.md")
})

if tt.wantWarning {
assert.Contains(t, output, "use `tools.playwright.browsers` instead")
assert.Equal(t, 1, compiler.GetWarningCount())
} else {
assert.Empty(t, output)
assert.Zero(t, compiler.GetWarningCount())
}
})
}
}

func TestCompileWorkflowWarnsAboutPlaywrightBrowserInstallStep(t *testing.T) {
tmpDir := t.TempDir()
mdPath := filepath.Join(tmpDir, "test-workflow.md")
content := `---
on: push
permissions:
contents: read
engine: copilot
tools:
playwright:
browsers: [chrome-for-testing]
steps:
- name: Install Playwright Chromium
run: npm exec playwright install --with-deps chromium
---

# Test Workflow
`
require.NoError(t, os.WriteFile(mdPath, []byte(content), 0o644))

compiler := NewCompiler()
output := testutil.CaptureStderr(t, func() {
require.NoError(t, compiler.CompileWorkflow(mdPath))
})

assert.Contains(t, output, "use `tools.playwright.browsers` instead")
assert.Equal(t, 1, compiler.GetWarningCount())
lockContent, err := os.ReadFile(filepath.Join(tmpDir, "test-workflow.lock.yml"))
require.NoError(t, err)
assert.Contains(t, string(lockContent), `install_playwright_browsers.sh" chromium`)
}
Loading