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
4 changes: 2 additions & 2 deletions .github/workflows/smoke-codex.lock.yml

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

6 changes: 3 additions & 3 deletions pkg/workflow/args.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,9 @@ func getGitHubCustomArgs(githubTool any) []string {
}

// getPlaywrightCustomArgs extracts custom args from Playwright tool configuration
func getPlaywrightCustomArgs(playwrightTool any) []string {
if toolConfig, ok := playwrightTool.(map[string]any); ok {
return extractCustomArgs(toolConfig)
func getPlaywrightCustomArgs(playwrightConfig *PlaywrightToolConfig) []string {
if playwrightConfig != nil && len(playwrightConfig.Args) > 0 {
return playwrightConfig.Args
}
return nil
}
Expand Down
9 changes: 6 additions & 3 deletions pkg/workflow/args_field_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ func TestArgsField(t *testing.T) {
"allowed_domains": []any{"example.com"},
"args": []any{"--browser", "firefox"},
}
result := getPlaywrightCustomArgs(playwrightTool)
playwrightConfig := parsePlaywrightTool(playwrightTool)
result := getPlaywrightCustomArgs(playwrightConfig)
if len(result) != 2 {
t.Errorf("Expected 2 args, got %d", len(result))
}
Expand All @@ -66,7 +67,8 @@ func TestArgsField(t *testing.T) {
"allowed_domains": []any{"example.com"},
"args": []string{"--headless"},
}
result = getPlaywrightCustomArgs(playwrightToolString)
playwrightConfigString := parsePlaywrightTool(playwrightToolString)
result = getPlaywrightCustomArgs(playwrightConfigString)
if len(result) != 1 {
t.Errorf("Expected 1 arg, got %d", len(result))
}
Expand All @@ -78,7 +80,8 @@ func TestArgsField(t *testing.T) {
playwrightToolDefault := map[string]any{
"allowed_domains": []any{"example.com"},
}
result = getPlaywrightCustomArgs(playwrightToolDefault)
playwrightConfigDefault := parsePlaywrightTool(playwrightToolDefault)
result = getPlaywrightCustomArgs(playwrightConfigDefault)
if result != nil {
t.Errorf("Expected nil args, got %v", result)
}
Expand Down
16 changes: 8 additions & 8 deletions pkg/workflow/mcp-config-playwright.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,17 @@ var mcpPlaywrightLog = logger.New("workflow:mcp-config-playwright")
// renderPlaywrightMCPConfig generates the Playwright MCP server configuration
// Uses Docker container to launch Playwright MCP for consistent browser environment
// This is a shared function used by both Claude and Custom engines
func renderPlaywrightMCPConfig(yaml *strings.Builder, playwrightTool any, isLast bool) {
func renderPlaywrightMCPConfig(yaml *strings.Builder, playwrightConfig *PlaywrightToolConfig, isLast bool) {
mcpPlaywrightLog.Print("Rendering Playwright MCP configuration")
renderPlaywrightMCPConfigWithOptions(yaml, playwrightTool, isLast, false, false)
renderPlaywrightMCPConfigWithOptions(yaml, playwrightConfig, isLast, false, false)
}

// renderPlaywrightMCPConfigWithOptions generates the Playwright MCP server configuration with engine-specific options
// Per MCP Gateway Specification v1.0.0 section 3.2.1, stdio-based MCP servers MUST be containerized.
// Uses MCP Gateway spec format: container, entrypointArgs, mounts, and args fields.
func renderPlaywrightMCPConfigWithOptions(yaml *strings.Builder, playwrightTool any, isLast bool, includeCopilotFields bool, inlineArgs bool) {
args := generatePlaywrightDockerArgs(playwrightTool)
customArgs := getPlaywrightCustomArgs(playwrightTool)
func renderPlaywrightMCPConfigWithOptions(yaml *strings.Builder, playwrightConfig *PlaywrightToolConfig, isLast bool, includeCopilotFields bool, inlineArgs bool) {
args := generatePlaywrightDockerArgs(playwrightConfig)
customArgs := getPlaywrightCustomArgs(playwrightConfig)

// Extract all expressions from playwright arguments and replace them with env var references
expressions := extractExpressionsFromPlaywrightArgs(args.AllowedDomains, customArgs)
Expand Down Expand Up @@ -119,9 +119,9 @@ func renderPlaywrightMCPConfigWithOptions(yaml *strings.Builder, playwrightTool
// renderPlaywrightMCPConfigTOML generates the Playwright MCP server configuration in TOML format for Codex
// Per MCP Gateway Specification v1.0.0 section 3.2.1, stdio-based MCP servers MUST be containerized.
// Uses MCP Gateway spec format: container, entrypointArgs, mounts, and args fields.
func renderPlaywrightMCPConfigTOML(yaml *strings.Builder, playwrightTool any) {
args := generatePlaywrightDockerArgs(playwrightTool)
customArgs := getPlaywrightCustomArgs(playwrightTool)
func renderPlaywrightMCPConfigTOML(yaml *strings.Builder, playwrightConfig *PlaywrightToolConfig) {
args := generatePlaywrightDockerArgs(playwrightConfig)
customArgs := getPlaywrightCustomArgs(playwrightConfig)

// Use official Playwright MCP Docker image (no version tag - only one image)
playwrightImage := "mcr.microsoft.com/playwright/mcp"
Expand Down
9 changes: 6 additions & 3 deletions pkg/workflow/mcp_benchmark_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,12 @@ func BenchmarkRenderPlaywrightMCPConfig(b *testing.B) {
"container": "mcr.microsoft.com/playwright:v1.41.0",
"allowed-domains": []any{"github.com", "*.github.io"},
}
playwrightConfig := parsePlaywrightTool(playwrightTool)

b.ResetTimer()
for i := 0; i < b.N; i++ {
var yaml strings.Builder
renderPlaywrightMCPConfig(&yaml, playwrightTool, true)
renderPlaywrightMCPConfig(&yaml, playwrightConfig, true)
}
}

Expand All @@ -30,10 +31,11 @@ func BenchmarkGeneratePlaywrightDockerArgs(b *testing.B) {
"*.googleapis.com",
},
}
playwrightConfig := parsePlaywrightTool(playwrightTool)

b.ResetTimer()
for i := 0; i < b.N; i++ {
_ = generatePlaywrightDockerArgs(playwrightTool)
_ = generatePlaywrightDockerArgs(playwrightConfig)
}
}

Expand All @@ -49,11 +51,12 @@ func BenchmarkRenderPlaywrightMCPConfig_Complex(b *testing.B) {
},
"args": []any{"--debug", "--timeout", "30000"},
}
playwrightConfig := parsePlaywrightTool(playwrightTool)

b.ResetTimer()
for i := 0; i < b.N; i++ {
var yaml strings.Builder
renderPlaywrightMCPConfig(&yaml, playwrightTool, true)
renderPlaywrightMCPConfig(&yaml, playwrightConfig, true)
}
}

Expand Down
6 changes: 4 additions & 2 deletions pkg/workflow/mcp_config_refactor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,8 @@ func TestRenderPlaywrightMCPConfigWithOptions(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
var output strings.Builder

renderPlaywrightMCPConfigWithOptions(&output, tt.playwrightTool, tt.isLast, tt.includeCopilotFields, tt.inlineArgs)
playwrightConfig := parsePlaywrightTool(tt.playwrightTool)
renderPlaywrightMCPConfigWithOptions(&output, playwrightConfig, tt.isLast, tt.includeCopilotFields, tt.inlineArgs)

result := output.String()

Expand Down Expand Up @@ -321,7 +322,8 @@ func TestRenderPlaywrightMCPConfigTOML(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
var output strings.Builder

renderPlaywrightMCPConfigTOML(&output, tt.playwrightTool)
playwrightConfig := parsePlaywrightTool(tt.playwrightTool)
renderPlaywrightMCPConfigTOML(&output, playwrightConfig)

result := output.String()

Expand Down
3 changes: 2 additions & 1 deletion pkg/workflow/mcp_config_shared_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ func TestRenderPlaywrightMCPConfigShared(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
var yaml strings.Builder
renderPlaywrightMCPConfig(&yaml, tt.playwrightTool, tt.isLast)
playwrightConfig := parsePlaywrightTool(tt.playwrightTool)
renderPlaywrightMCPConfig(&yaml, playwrightConfig, tt.isLast)

result := yaml.String()

Expand Down
13 changes: 8 additions & 5 deletions pkg/workflow/mcp_renderer.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,21 +122,24 @@ func (r *MCPConfigRendererUnified) RenderGitHubMCP(yaml *strings.Builder, github
func (r *MCPConfigRendererUnified) RenderPlaywrightMCP(yaml *strings.Builder, playwrightTool any) {
mcpRendererLog.Printf("Rendering Playwright MCP: format=%s, inline_args=%t", r.options.Format, r.options.InlineArgs)

// Parse playwright tool configuration to strongly-typed struct
playwrightConfig := parsePlaywrightTool(playwrightTool)

if r.options.Format == "toml" {
r.renderPlaywrightTOML(yaml, playwrightTool)
r.renderPlaywrightTOML(yaml, playwrightConfig)
return
}

// JSON format
renderPlaywrightMCPConfigWithOptions(yaml, playwrightTool, r.options.IsLast, r.options.IncludeCopilotFields, r.options.InlineArgs)
renderPlaywrightMCPConfigWithOptions(yaml, playwrightConfig, r.options.IsLast, r.options.IncludeCopilotFields, r.options.InlineArgs)
}

// renderPlaywrightTOML generates Playwright MCP configuration in TOML format
// Per MCP Gateway Specification v1.0.0 section 3.2.1, stdio-based MCP servers MUST be containerized.
// Uses MCP Gateway spec format: container, entrypointArgs, mounts, and args fields.
func (r *MCPConfigRendererUnified) renderPlaywrightTOML(yaml *strings.Builder, playwrightTool any) {
args := generatePlaywrightDockerArgs(playwrightTool)
customArgs := getPlaywrightCustomArgs(playwrightTool)
func (r *MCPConfigRendererUnified) renderPlaywrightTOML(yaml *strings.Builder, playwrightConfig *PlaywrightToolConfig) {
args := generatePlaywrightDockerArgs(playwrightConfig)
customArgs := getPlaywrightCustomArgs(playwrightConfig)

// Use official Playwright MCP Docker image (no version tag - only one image)
playwrightImage := "mcr.microsoft.com/playwright/mcp"
Expand Down
71 changes: 21 additions & 50 deletions pkg/workflow/mcp_servers.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,9 @@ func collectMCPEnvironmentVariables(tools map[string]any, mcpTools []string, wor
if hasPlaywright {
// Extract all expressions from playwright arguments using ExpressionExtractor
if playwrightTool, ok := tools["playwright"]; ok {
allowedDomains := generatePlaywrightAllowedDomains(playwrightTool)
customArgs := getPlaywrightCustomArgs(playwrightTool)
playwrightConfig := parsePlaywrightTool(playwrightTool)
allowedDomains := generatePlaywrightAllowedDomains(playwrightConfig)
customArgs := getPlaywrightCustomArgs(playwrightConfig)
playwrightAllowedDomainsSecrets := extractExpressionsFromPlaywrightArgs(allowedDomains, customArgs)
for envVarName, originalExpr := range playwrightAllowedDomainsSecrets {
envVars[envVarName] = originalExpr
Expand Down Expand Up @@ -967,70 +968,40 @@ func getGitHubAllowedTools(githubTool any) []string {
return nil
}

func getPlaywrightDockerImageVersion(playwrightTool any) string {
func getPlaywrightDockerImageVersion(playwrightConfig *PlaywrightToolConfig) string {
playwrightDockerImageVersion := string(constants.DefaultPlaywrightBrowserVersion) // Default Playwright browser Docker image version
// Extract version setting from tool properties
if toolConfig, ok := playwrightTool.(map[string]any); ok {
if versionSetting, exists := toolConfig["version"]; exists {
switch v := versionSetting.(type) {
case string:
playwrightDockerImageVersion = v
case int:
playwrightDockerImageVersion = fmt.Sprintf("%d", v)
case int64:
playwrightDockerImageVersion = fmt.Sprintf("%d", v)
case uint64:
playwrightDockerImageVersion = fmt.Sprintf("%d", v)
case float64:
playwrightDockerImageVersion = fmt.Sprintf("%g", v)
}
}
if playwrightConfig != nil && playwrightConfig.Version != "" {
playwrightDockerImageVersion = playwrightConfig.Version
}
return playwrightDockerImageVersion
}

// getPlaywrightMCPPackageVersion extracts version setting for the @playwright/mcp NPM package
// This is separate from the Docker image version because they follow different versioning schemes
func getPlaywrightMCPPackageVersion(playwrightTool any) string {
func getPlaywrightMCPPackageVersion(playwrightConfig *PlaywrightToolConfig) string {
// Always use the default @playwright/mcp package version.
return string(constants.DefaultPlaywrightMCPVersion)
}

// generatePlaywrightAllowedDomains extracts domain list from Playwright tool configuration with bundle resolution
// Uses the same domain bundle resolution as top-level network configuration, defaulting to localhost only
func generatePlaywrightAllowedDomains(playwrightTool any) []string {
func generatePlaywrightAllowedDomains(playwrightConfig *PlaywrightToolConfig) []string {
// Default to localhost with all port variations (same as Copilot agent default)
allowedDomains := constants.DefaultAllowedDomains

// Extract allowed_domains from Playwright tool configuration
if toolConfig, ok := playwrightTool.(map[string]any); ok {
if domainsConfig, exists := toolConfig["allowed_domains"]; exists {
// Create a mock NetworkPermissions structure to use the same domain resolution logic
playwrightNetwork := &NetworkPermissions{}

switch domains := domainsConfig.(type) {
case []string:
playwrightNetwork.Allowed = domains
case []any:
// Convert []any to []string
allowedDomainsSlice := make([]string, len(domains))
for i, domain := range domains {
if domainStr, ok := domain.(string); ok {
allowedDomainsSlice[i] = domainStr
}
}
playwrightNetwork.Allowed = allowedDomainsSlice
case string:
// Single domain as string
playwrightNetwork.Allowed = []string{domains}
}
if playwrightConfig != nil && len(playwrightConfig.AllowedDomains) > 0 {
// Create a mock NetworkPermissions structure to use the same domain resolution logic
playwrightNetwork := &NetworkPermissions{
Allowed: playwrightConfig.AllowedDomains.ToStringSlice(),
}

// Use the same domain bundle resolution as the top-level network configuration
resolvedDomains := GetAllowedDomains(playwrightNetwork)
// Use the same domain bundle resolution as the top-level network configuration
resolvedDomains := GetAllowedDomains(playwrightNetwork)

// Ensure localhost domains are always included
allowedDomains = parser.EnsureLocalhostDomains(resolvedDomains)
}
// Ensure localhost domains are always included
allowedDomains = parser.EnsureLocalhostDomains(resolvedDomains)
}

return allowedDomains
Expand All @@ -1044,11 +1015,11 @@ type PlaywrightDockerArgs struct {
}

// generatePlaywrightDockerArgs creates the common Docker arguments for Playwright MCP server
func generatePlaywrightDockerArgs(playwrightTool any) PlaywrightDockerArgs {
func generatePlaywrightDockerArgs(playwrightConfig *PlaywrightToolConfig) PlaywrightDockerArgs {
return PlaywrightDockerArgs{
ImageVersion: getPlaywrightDockerImageVersion(playwrightTool),
MCPPackageVersion: getPlaywrightMCPPackageVersion(playwrightTool),
AllowedDomains: generatePlaywrightAllowedDomains(playwrightTool),
ImageVersion: getPlaywrightDockerImageVersion(playwrightConfig),
MCPPackageVersion: getPlaywrightMCPPackageVersion(playwrightConfig),
AllowedDomains: generatePlaywrightAllowedDomains(playwrightConfig),
}
}

Expand Down
28 changes: 20 additions & 8 deletions pkg/workflow/tools_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,30 +264,42 @@ func parsePlaywrightTool(val any) *PlaywrightToolConfig {
if configMap, ok := val.(map[string]any); ok {
config := &PlaywrightToolConfig{}

// Handle version field - can be string or number
if version, ok := configMap["version"].(string); ok {
config.Version = version
} else if versionNum, ok := configMap["version"].(int); ok {
config.Version = fmt.Sprintf("%d", versionNum)
} else if versionNum, ok := configMap["version"].(int64); ok {
config.Version = fmt.Sprintf("%d", versionNum)
} else if versionNum, ok := configMap["version"].(float64); ok {
config.Version = fmt.Sprintf("%g", versionNum)
}

// Handle allowed_domains - can be string or array
if allowedDomains, ok := configMap["allowed_domains"]; ok {
if str, ok := allowedDomains.(string); ok {
config.AllowedDomains = []string{str}
config.AllowedDomains = PlaywrightAllowedDomains{PlaywrightDomain(str)}
} else if arr, ok := allowedDomains.([]any); ok {
config.AllowedDomains = make([]string, 0, len(arr))
config.AllowedDomains = make(PlaywrightAllowedDomains, 0, len(arr))
for _, item := range arr {
if str, ok := item.(string); ok {
config.AllowedDomains = append(config.AllowedDomains, str)
config.AllowedDomains = append(config.AllowedDomains, PlaywrightDomain(str))
}
}
}
}

if args, ok := configMap["args"].([]any); ok {
config.Args = make([]string, 0, len(args))
for _, item := range args {
if str, ok := item.(string); ok {
config.Args = append(config.Args, str)
// Handle args field - can be []any or []string
if argsValue, ok := configMap["args"]; ok {
if arr, ok := argsValue.([]any); ok {
config.Args = make([]string, 0, len(arr))
for _, item := range arr {
if str, ok := item.(string); ok {
config.Args = append(config.Args, str)
}
}
} else if arr, ok := argsValue.([]string); ok {
config.Args = arr
}
}

Expand Down
21 changes: 18 additions & 3 deletions pkg/workflow/tools_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -260,11 +260,26 @@ type GitHubToolConfig struct {
Lockdown bool `yaml:"lockdown,omitempty"`
}

// PlaywrightDomain represents a domain name allowed for Playwright browser automation
type PlaywrightDomain string

// PlaywrightAllowedDomains is a slice of allowed domain names for Playwright
type PlaywrightAllowedDomains []PlaywrightDomain

// ToStringSlice converts PlaywrightAllowedDomains to []string
func (p PlaywrightAllowedDomains) ToStringSlice() []string {
result := make([]string, len(p))
for i, domain := range p {
result[i] = string(domain)
}
return result
}

// PlaywrightToolConfig represents the configuration for the Playwright tool
type PlaywrightToolConfig struct {
Version string `yaml:"version,omitempty"`
AllowedDomains []string `yaml:"allowed_domains,omitempty"`
Args []string `yaml:"args,omitempty"`
Version string `yaml:"version,omitempty"`
AllowedDomains PlaywrightAllowedDomains `yaml:"allowed_domains,omitempty"`
Args []string `yaml:"args,omitempty"`
}

// SerenaToolConfig represents the configuration for the Serena MCP tool
Expand Down
Loading
Loading