Allow explicit "latest" version for Playwright while defaulting to pinned version

pkg/workflow/custom_engine_test.go

@@ -300,9 +300,9 @@ func TestCustomEngineRenderPlaywrightMCPConfigWithDomainConfiguration(t *testing
 		t.Errorf("Expected Playwright configuration in output")
 	}
 
-	// Check that it contains Playwright MCP npx configuration
-	if !strings.Contains(output, "@playwright/mcp@latest") {
-		t.Errorf("Expected Playwright MCP npx package in output")
+	// Check that it contains Playwright MCP npx configuration with the specified version
+	if !strings.Contains(output, "@playwright/mcp@v1.40.0") {
+		t.Errorf("Expected Playwright MCP npx package with v1.40.0 in output")
 	}
 
 	// Check that it contains --allowed-origins flag when domains are configured
@@ -351,9 +351,9 @@ func TestCustomEngineRenderPlaywrightMCPConfigDefaultDomains(t *testing.T) {
 		t.Errorf("Expected Playwright configuration in output")
 	}
 
-	// Check that it contains Playwright MCP npx configuration
-	if !strings.Contains(output, "@playwright/mcp@latest") {
-		t.Errorf("Expected Playwright MCP npx package in output")
+	// Check that it contains Playwright MCP npx configuration with the specified version
+	if !strings.Contains(output, "@playwright/mcp@v1.40.0") {
+		t.Errorf("Expected Playwright MCP npx package with v1.40.0 in output")
 	}
 
 	// Check that it contains --allowed-origins flag for default domains

pkg/workflow/mcp-config.go

@@ -36,9 +36,9 @@ func renderPlaywrightMCPConfigWithOptions(yaml *strings.Builder, playwrightTool
 	}
 
 	// Determine version to use - respect version configuration if provided
-	// Default to the pinned version from constants
+	// Default to the pinned version from constants when no version is specified
 	playwrightPackage := "@playwright/mcp@" + constants.DefaultPlaywrightVersion
-	if includeCopilotFields && args.ImageVersion != "" && args.ImageVersion != "latest" {
+	if args.ImageVersion != "" {
 		playwrightPackage = "@playwright/mcp@" + args.ImageVersion
 	}
 
@@ -200,11 +200,18 @@ func renderPlaywrightMCPConfigTOML(yaml *strings.Builder, playwrightTool any) {
 	args := generatePlaywrightDockerArgs(playwrightTool)
 	customArgs := getPlaywrightCustomArgs(playwrightTool)
 
+	// Determine version to use - respect version configuration if provided
+	// Default to the pinned version from constants when no version is specified
+	version := constants.DefaultPlaywrightVersion
+	if args.ImageVersion != "" {
+		version = args.ImageVersion
+	}
+
 	yaml.WriteString("          \n")
 	yaml.WriteString("          [mcp_servers.playwright]\n")
 	yaml.WriteString("          command = \"npx\"\n")
 	yaml.WriteString("          args = [\n")
-	yaml.WriteString(fmt.Sprintf("            \"@playwright/mcp@%s\",\n", constants.DefaultPlaywrightVersion))
+	yaml.WriteString(fmt.Sprintf("            \"@playwright/mcp@%s\",\n", version))
 	yaml.WriteString("            \"--output-dir\",\n")
 	yaml.WriteString("            \"/tmp/gh-aw/mcp-logs/playwright\"")
 	if len(args.AllowedDomains) > 0 {

pkg/workflow/mcp_config_refactor_test.go

@@ -3,6 +3,8 @@ package workflow
 import (
 	"strings"
 	"testing"
+
+	"github.com/githubnext/gh-aw/pkg/constants"
 )
 
 // TestRenderPlaywrightMCPConfigWithOptions verifies the shared Playwright config helper
@@ -48,7 +50,7 @@ func TestRenderPlaywrightMCPConfigWithOptions(t *testing.T) {
 				`"playwright": {`,
 				`"command": "npx"`,
 				`"args": [`,
-				`"@playwright/mcp@latest"`,
+				`"@playwright/mcp@` + constants.DefaultPlaywrightVersion + `"`,
 				`"--output-dir"`,
 				`"/tmp/gh-aw/mcp-logs/playwright"`,
 				`              },`,
@@ -269,7 +271,7 @@ func TestRenderPlaywrightMCPConfigTOML(t *testing.T) {
 				`[mcp_servers.playwright]`,
 				`command = "npx"`,
 				`args = [`,
-				`"@playwright/mcp@latest"`,
+				`"@playwright/mcp@` + constants.DefaultPlaywrightVersion + `"`,
 				`"--output-dir"`,
 				`"/tmp/gh-aw/mcp-logs/playwright"`,
 			},

pkg/workflow/mcp_config_shared_test.go

@@ -3,6 +3,8 @@ package workflow
 import (
 	"strings"
 	"testing"
+
+	"github.com/githubnext/gh-aw/pkg/constants"
 )
 
 // TestRenderPlaywrightMCPConfigShared tests the shared renderPlaywrightMCPConfig function
@@ -23,7 +25,7 @@ func TestRenderPlaywrightMCPConfigShared(t *testing.T) {
 			wantContains: []string{
 				`"playwright": {`,
 				`"command": "npx"`,
-				`"@playwright/mcp@latest"`,
+				`"@playwright/mcp@` + constants.DefaultPlaywrightVersion + `"`,
 				`"--output-dir"`,
 				`"/tmp/gh-aw/mcp-logs/playwright"`,
 				`"--allowed-origins"`,
@@ -50,7 +52,7 @@ func TestRenderPlaywrightMCPConfigShared(t *testing.T) {
 			wantContains: []string{
 				`"playwright": {`,
 				`"command": "npx"`,
-				`"@playwright/mcp@latest"`,
+				`"@playwright/mcp@` + constants.DefaultPlaywrightVersion + `"`,
 			},
 			wantEnding: "},\n",
 		},

pkg/workflow/mcp_servers.go

@@ -320,7 +320,8 @@ func getGitHubAllowedTools(githubTool any) []string {
 }
 
 func getPlaywrightDockerImageVersion(playwrightTool any) string {
-	playwrightDockerImageVersion := "latest" // Default Playwright Docker image version
+	// Default to empty string - caller will use pinned version when empty
+	playwrightDockerImageVersion := ""
 	// Extract version setting from tool properties
 	if toolConfig, ok := playwrightTool.(map[string]any); ok {
 		if versionSetting, exists := toolConfig["version"]; exists {

pkg/workflow/playwright_version_latest_test.go

@@ -0,0 +1,55 @@
+package workflow
+
+import (
+"strings"
+"testing"
+)
+
+// TestPlaywrightExplicitLatestVersion verifies that when user explicitly sets version: "latest"
+// it is respected and used in the rendered output
+func TestPlaywrightExplicitLatestVersion(t *testing.T) {
+t.Run("Explicit latest version for Copilot engine", func(t *testing.T) {
+var yaml strings.Builder
+playwrightTool := map[string]any{
+"version":         "latest",
+"allowed_domains": []string{"example.com"},
+}
+
+renderPlaywrightMCPConfigWithOptions(&yaml, playwrightTool, false, true, true)
+output := yaml.String()
+
+if !strings.Contains(output, "@playwright/mcp@latest") {
+t.Errorf("Expected @playwright/mcp@latest when user explicitly sets version: latest, got: %s", output)
+}
+})
+
+t.Run("Explicit latest version for Claude engine", func(t *testing.T) {
+var yaml strings.Builder
+playwrightTool := map[string]any{
+"version":         "latest",
+"allowed_domains": []string{"example.com"},
+}
+
+renderPlaywrightMCPConfigWithOptions(&yaml, playwrightTool, false, false, false)
+output := yaml.String()
+
+if !strings.Contains(output, "@playwright/mcp@latest") {
+t.Errorf("Expected @playwright/mcp@latest when user explicitly sets version: latest, got: %s", output)
+}
+})
+
+t.Run("Explicit latest version for Codex engine (TOML)", func(t *testing.T) {
+var yaml strings.Builder
+playwrightTool := map[string]any{
+"version":         "latest",
+"allowed_domains": []string{"example.com"},
+}
+
+renderPlaywrightMCPConfigTOML(&yaml, playwrightTool)
+output := yaml.String()
+
+if !strings.Contains(output, "@playwright/mcp@latest") {
+t.Errorf("Expected @playwright/mcp@latest when user explicitly sets version: latest, got: %s", output)
+}
+})
+}

pkg/workflow/version_field_test.go

@@ -33,7 +33,7 @@ func TestVersionField(t *testing.T) {
 
 	// Test Playwright tool version extraction
 	t.Run("Playwright version field extraction", func(t *testing.T) {
-		// Test "version" field
+		// Test "version" field with specific version
 		playwrightTool := map[string]any{
 			"allowed_domains": []any{"example.com"},
 			"version":         "v1.41.0",
@@ -43,13 +43,23 @@ func TestVersionField(t *testing.T) {
 			t.Errorf("Expected v1.41.0, got %s", result)
 		}
 
-		// Test default value when version field is not present
+		// Test explicit "latest" version is respected
+		playwrightToolLatest := map[string]any{
+			"allowed_domains": []any{"example.com"},
+			"version":         "latest",
+		}
+		result = getPlaywrightDockerImageVersion(playwrightToolLatest)
+		if result != "latest" {
+			t.Errorf("Expected latest (user explicitly set it), got %s", result)
+		}
+
+		// Test default value when version field is not present (empty string, caller uses pinned version)
 		playwrightToolDefault := map[string]any{
 			"allowed_domains": []any{"example.com"},
 		}
 		result = getPlaywrightDockerImageVersion(playwrightToolDefault)
-		if result != "latest" {
-			t.Errorf("Expected default latest, got %s", result)
+		if result != "" {
+			t.Errorf("Expected default empty string, got %s", result)
 		}
 	})