From 44fff1425d8f0ff8328e411c667ec18cc18eaada Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 14:21:58 +0000 Subject: [PATCH 1/9] Initial plan From f89d979ff8a1ffc800a077cb7506388f0f5168e8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 14:38:24 +0000 Subject: [PATCH 2/9] feat: add feature flags for copilot mcpg and firewall version overrides Agent-Logs-Url: https://github.com/github/gh-aw/sessions/9e5aa7a8-e769-4818-81d4-c0db5192182f Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --- pkg/constants/feature_constants.go | 27 +++++++ pkg/workflow/copilot_engine_installation.go | 4 + pkg/workflow/copilot_installer_test.go | 35 +++++++++ pkg/workflow/docker.go | 7 +- pkg/workflow/docker_api_proxy_test.go | 30 ++++++++ pkg/workflow/features.go | 41 ++++++++++ pkg/workflow/features_test.go | 75 +++++++++++++++++++ pkg/workflow/firewall.go | 18 +++++ pkg/workflow/firewall_image_tag_test.go | 31 ++++++++ pkg/workflow/firewall_version_pinning_test.go | 40 ++++++++++ pkg/workflow/mcp_gateway_config.go | 16 +++- pkg/workflow/mcp_gateway_config_test.go | 39 ++++++++++ pkg/workflow/mcp_setup_generator.go | 6 +- pkg/workflow/mcp_setup_generator_test.go | 16 ++++ 14 files changed, 371 insertions(+), 14 deletions(-) diff --git a/pkg/constants/feature_constants.go b/pkg/constants/feature_constants.go index 73fbc93d146..429b7bfed15 100644 --- a/pkg/constants/feature_constants.go +++ b/pkg/constants/feature_constants.go @@ -60,6 +60,33 @@ const ( // features: // byok-copilot: true ByokCopilotFeatureFlag FeatureFlag = "byok-copilot" + // LatestCopilotFeatureFlag forces Copilot CLI installation to use latest. + // When enabled, Copilot CLI installation ignores the pinned default version + // and installs latest for smoke-testing upcoming releases. + // + // Workflow frontmatter usage: + // + // features: + // latest-copilot: true + LatestCopilotFeatureFlag FeatureFlag = "latest-copilot" + // MCPGVersionFeatureFlag overrides the default MCP gateway container version. + // When set to a non-empty string, MCP gateway image references use this + // version instead of DefaultMCPGatewayVersion. + // + // Workflow frontmatter usage: + // + // features: + // mcpg-version: "v0.2.27" + MCPGVersionFeatureFlag FeatureFlag = "mcpg-version" + // FirewallVersionFeatureFlag overrides the default firewall container version. + // When set to a non-empty string, AWF installation and image references use + // this version instead of DefaultFirewallVersion. + // + // Workflow frontmatter usage: + // + // features: + // firewall-version: "v0.25.27" + FirewallVersionFeatureFlag FeatureFlag = "firewall-version" // IntegrityReactionsFeatureFlag enables reaction-based integrity promotion/demotion // in the MCPG allow-only policy. When enabled, the compiler injects // endorsement-reactions and disapproval-reactions fields into the allow-only policy. diff --git a/pkg/workflow/copilot_engine_installation.go b/pkg/workflow/copilot_engine_installation.go index 1b13a8ad4b1..1a21257b923 100644 --- a/pkg/workflow/copilot_engine_installation.go +++ b/pkg/workflow/copilot_engine_installation.go @@ -69,6 +69,10 @@ func (e *CopilotEngine) GetInstallationSteps(workflowData *WorkflowData) []GitHu if workflowData.EngineConfig != nil && workflowData.EngineConfig.Version != "" { copilotVersion = workflowData.EngineConfig.Version } + if isFeatureEnabled(constants.LatestCopilotFeatureFlag, workflowData) { + copilotVersion = "latest" + copilotInstallLog.Print("latest-copilot enabled: forcing Copilot CLI install version to latest") + } if isFeatureEnabled(constants.ByokCopilotFeatureFlag, workflowData) { copilotVersion = "latest" copilotInstallLog.Print("byok-copilot enabled: forcing Copilot CLI install version to latest") diff --git a/pkg/workflow/copilot_installer_test.go b/pkg/workflow/copilot_installer_test.go index 561386d6194..8f55ad5af25 100644 --- a/pkg/workflow/copilot_installer_test.go +++ b/pkg/workflow/copilot_installer_test.go @@ -281,3 +281,38 @@ func TestCopilotInstallerByokFeatureUsesLatestVersion(t *testing.T) { t.Errorf("Expected byok-copilot to ignore pinned version, got:\n%s", installStep) } } + +func TestCopilotInstallerLatestCopilotFeatureUsesLatestVersion(t *testing.T) { + engine := NewCopilotEngine() + workflowData := &WorkflowData{ + Name: "test-workflow", + EngineConfig: &EngineConfig{ + Version: "1.0.0", + }, + Features: map[string]any{ + string(constants.LatestCopilotFeatureFlag): true, + }, + } + + steps := engine.GetInstallationSteps(workflowData) + + var installStep string + for _, step := range steps { + stepContent := strings.Join(step, "\n") + if strings.Contains(stepContent, "install_copilot_cli.sh") { + installStep = stepContent + break + } + } + + if installStep == "" { + t.Fatal("Could not find install step with install_copilot_cli.sh") + } + + if !strings.Contains(installStep, `install_copilot_cli.sh" latest`) { + t.Errorf("Expected latest-copilot to force latest Copilot CLI install, got:\n%s", installStep) + } + if strings.Contains(installStep, `install_copilot_cli.sh" 1.0.0`) { + t.Errorf("Expected latest-copilot to ignore pinned version, got:\n%s", installStep) + } +} diff --git a/pkg/workflow/docker.go b/pkg/workflow/docker.go index 38bef5149de..dea5155ea38 100644 --- a/pkg/workflow/docker.go +++ b/pkg/workflow/docker.go @@ -125,12 +125,7 @@ func collectDockerImages(tools map[string]any, workflowData *WorkflowData, actio mcpGateway := workflowData.SandboxConfig.MCP if mcpGateway.Container != "" { image := mcpGateway.Container - if mcpGateway.Version != "" { - image += ":" + mcpGateway.Version - } else { - // Use default version if not specified (consistent with mcp_servers.go) - image += ":" + string(constants.DefaultMCPGatewayVersion) - } + image += ":" + getMCPGatewayVersion(workflowData, mcpGateway.Version) if !imageSet[image] { images = append(images, image) imageSet[image] = true diff --git a/pkg/workflow/docker_api_proxy_test.go b/pkg/workflow/docker_api_proxy_test.go index 3b3bf625f1c..1e73737cebe 100644 --- a/pkg/workflow/docker_api_proxy_test.go +++ b/pkg/workflow/docker_api_proxy_test.go @@ -79,3 +79,33 @@ func TestCollectDockerImages_APIProxyForEnginesWithLLMGateway(t *testing.T) { }) } } + +func TestCollectDockerImages_FirewallVersionFeatureOverride(t *testing.T) { + featureVersion := "v0.25.27" + featureTag := "0.25.27" + workflowData := &WorkflowData{ + AI: "claude", + Features: map[string]any{ + string(constants.FirewallVersionFeatureFlag): featureVersion, + }, + NetworkPermissions: &NetworkPermissions{ + Firewall: &FirewallConfig{ + Enabled: true, + }, + }, + } + + images := collectDockerImages(nil, workflowData, ActionModeRelease) + + expectedImages := []string{ + constants.DefaultFirewallRegistry + "/squid:" + featureTag, + constants.DefaultFirewallRegistry + "/agent:" + featureTag, + constants.DefaultFirewallRegistry + "/api-proxy:" + featureTag, + } + + for _, expectedImage := range expectedImages { + if !slices.Contains(images, expectedImage) { + t.Errorf("Expected image %q in collected images, got %v", expectedImage, images) + } + } +} diff --git a/pkg/workflow/features.go b/pkg/workflow/features.go index 2c7a0126922..a122d26f13c 100644 --- a/pkg/workflow/features.go +++ b/pkg/workflow/features.go @@ -86,3 +86,44 @@ func isFeatureEnabled(flag constants.FeatureFlag, workflowData *WorkflowData) bo featuresLog.Printf("Feature not found: %s=false", flagLower) return false } + +// getFeatureValue returns the string value for a feature flag from workflow frontmatter. +// Returns an empty string when: +// - workflowData is nil +// - features are not configured +// - the feature is missing +// - the feature value is not a string +// - the feature value is an empty/whitespace string +func getFeatureValue(flag constants.FeatureFlag, workflowData *WorkflowData) string { + if workflowData == nil || workflowData.Features == nil { + return "" + } + + flagLower := strings.ToLower(strings.TrimSpace(string(flag))) + + if value, exists := workflowData.Features[flagLower]; exists { + if strVal, ok := value.(string); ok { + trimmed := strings.TrimSpace(strVal) + if trimmed != "" { + featuresLog.Printf("Feature value found in frontmatter: %s=%s", flagLower, trimmed) + } + return trimmed + } + return "" + } + + for key, value := range workflowData.Features { + if strings.ToLower(key) == flagLower { + if strVal, ok := value.(string); ok { + trimmed := strings.TrimSpace(strVal) + if trimmed != "" { + featuresLog.Printf("Feature value found in frontmatter (case-insensitive): %s=%s", flagLower, trimmed) + } + return trimmed + } + return "" + } + } + + return "" +} diff --git a/pkg/workflow/features_test.go b/pkg/workflow/features_test.go index f4c9fb02ca6..960e58f829e 100644 --- a/pkg/workflow/features_test.go +++ b/pkg/workflow/features_test.go @@ -302,3 +302,78 @@ func TestMergedFeaturesTopLevelPrecedence(t *testing.T) { t.Errorf("isFeatureEnabled(\"import-only\") = %v, want true (from import)", importOnlyResult) } } + +func TestGetFeatureValue(t *testing.T) { + tests := []struct { + name string + workflow *WorkflowData + flag constants.FeatureFlag + expectedVal string + }{ + { + name: "returns exact string value", + workflow: &WorkflowData{ + Features: map[string]any{ + "mcpg-version": "v0.2.27", + }, + }, + flag: constants.MCPGVersionFeatureFlag, + expectedVal: "v0.2.27", + }, + { + name: "returns case-insensitive key match", + workflow: &WorkflowData{ + Features: map[string]any{ + "MCPG-Version": "v0.2.28", + }, + }, + flag: constants.MCPGVersionFeatureFlag, + expectedVal: "v0.2.28", + }, + { + name: "trims whitespace from value", + workflow: &WorkflowData{ + Features: map[string]any{ + "mcpg-version": " v0.2.29 ", + }, + }, + flag: constants.MCPGVersionFeatureFlag, + expectedVal: "v0.2.29", + }, + { + name: "returns empty for non-string value", + workflow: &WorkflowData{ + Features: map[string]any{ + "mcpg-version": true, + }, + }, + flag: constants.MCPGVersionFeatureFlag, + expectedVal: "", + }, + { + name: "returns empty for missing value", + workflow: &WorkflowData{ + Features: map[string]any{ + "other-feature": "v1", + }, + }, + flag: constants.MCPGVersionFeatureFlag, + expectedVal: "", + }, + { + name: "returns empty for nil workflow", + workflow: nil, + flag: constants.MCPGVersionFeatureFlag, + expectedVal: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := getFeatureValue(tt.flag, tt.workflow) + if got != tt.expectedVal { + t.Errorf("getFeatureValue(%q) = %q, want %q", tt.flag, got, tt.expectedVal) + } + }) + } +} diff --git a/pkg/workflow/firewall.go b/pkg/workflow/firewall.go index a04060a36d5..b8cff744b9c 100644 --- a/pkg/workflow/firewall.go +++ b/pkg/workflow/firewall.go @@ -65,9 +65,20 @@ func getFirewallConfig(workflowData *WorkflowData) *FirewallConfig { return nil } + featureVersion := getFeatureValue(constants.FirewallVersionFeatureFlag, workflowData) + // Check network.firewall configuration if workflowData.NetworkPermissions != nil && workflowData.NetworkPermissions.Firewall != nil { config := workflowData.NetworkPermissions.Firewall + if config.Version == "" && featureVersion != "" { + // Use a copy to avoid mutating parsed frontmatter structures. + configCopy := *config + configCopy.Version = featureVersion + if firewallLog.Enabled() { + firewallLog.Printf("Applied firewall-version feature override: version=%s", featureVersion) + } + return &configCopy + } if firewallLog.Enabled() { firewallLog.Printf("Retrieved firewall config: enabled=%v, version=%s, logLevel=%s", config.Enabled, config.Version, config.LogLevel) @@ -75,6 +86,13 @@ func getFirewallConfig(workflowData *WorkflowData) *FirewallConfig { return config } + if featureVersion != "" { + if firewallLog.Enabled() { + firewallLog.Printf("Using firewall-version feature override without explicit firewall config: version=%s", featureVersion) + } + return &FirewallConfig{Version: featureVersion} + } + return nil } diff --git a/pkg/workflow/firewall_image_tag_test.go b/pkg/workflow/firewall_image_tag_test.go index c85bc1189ba..5109ce91047 100644 --- a/pkg/workflow/firewall_image_tag_test.go +++ b/pkg/workflow/firewall_image_tag_test.go @@ -119,6 +119,37 @@ func TestClaudeEngineAWFImageTag(t *testing.T) { t.Errorf("Expected AWF command to contain '%s', got:\n%s", expectedImageTag, stepContent) } }) + + t.Run("AWF command includes image-tag with firewall-version feature override", func(t *testing.T) { + featureVersion := "v0.25.27" + workflowData := &WorkflowData{ + Name: "test-workflow", + EngineConfig: &EngineConfig{ + ID: "claude", + }, + Features: map[string]any{ + string(constants.FirewallVersionFeatureFlag): featureVersion, + }, + NetworkPermissions: &NetworkPermissions{ + Firewall: &FirewallConfig{ + Enabled: true, + }, + }, + } + + engine := NewClaudeEngine() + steps := engine.GetExecutionSteps(workflowData, "test.log") + + if len(steps) == 0 { + t.Fatal("Expected at least one execution step") + } + + stepContent := strings.Join(steps[0], "\n") + expectedImageTag := "--image-tag " + strings.TrimPrefix(featureVersion, "v") + if !strings.Contains(stepContent, expectedImageTag) { + t.Errorf("Expected AWF command to contain '%s', got:\n%s", expectedImageTag, stepContent) + } + }) } // TestCodexEngineAWFImageTag tests that Codex engine includes --image-tag in AWF commands diff --git a/pkg/workflow/firewall_version_pinning_test.go b/pkg/workflow/firewall_version_pinning_test.go index dacb807be51..19c6a6ee329 100644 --- a/pkg/workflow/firewall_version_pinning_test.go +++ b/pkg/workflow/firewall_version_pinning_test.go @@ -158,6 +158,46 @@ func TestCopilotEngineFirewallInstallation(t *testing.T) { } }) + t.Run("uses feature override version when firewall config version is not specified", func(t *testing.T) { + engine := NewCopilotEngine() + featureVersion := "v0.25.27" + workflowData := &WorkflowData{ + Name: "test-workflow", + EngineConfig: &EngineConfig{ + ID: "copilot", + }, + Features: map[string]any{ + string(constants.FirewallVersionFeatureFlag): featureVersion, + }, + NetworkPermissions: &NetworkPermissions{ + Firewall: &FirewallConfig{ + Enabled: true, + }, + }, + } + + steps := engine.GetInstallationSteps(workflowData) + + var foundAWFStep bool + var awfStepStr string + for _, step := range steps { + stepStr := strings.Join(step, "\n") + if strings.Contains(stepStr, "Install AWF binary") { + foundAWFStep = true + awfStepStr = stepStr + break + } + } + + if !foundAWFStep { + t.Fatal("Expected to find AWF installation step when firewall is enabled") + } + + if !strings.Contains(awfStepStr, featureVersion) { + t.Errorf("AWF installation step should pass feature override version %s to script", featureVersion) + } + }) + t.Run("does not include AWF installation when firewall disabled", func(t *testing.T) { engine := NewCopilotEngine() workflowData := &WorkflowData{ diff --git a/pkg/workflow/mcp_gateway_config.go b/pkg/workflow/mcp_gateway_config.go index f191b24a6a1..90a4f6f79da 100644 --- a/pkg/workflow/mcp_gateway_config.go +++ b/pkg/workflow/mcp_gateway_config.go @@ -52,6 +52,16 @@ import ( var mcpGatewayConfigLog = logger.New("workflow:mcp_gateway_config") +func getMCPGatewayVersion(workflowData *WorkflowData, configuredVersion string) string { + if configuredVersion != "" { + return configuredVersion + } + if featureVersion := getFeatureValue(constants.MCPGVersionFeatureFlag, workflowData); featureVersion != "" { + return featureVersion + } + return string(constants.DefaultMCPGatewayVersion) +} + // ensureDefaultMCPGatewayConfig ensures MCP gateway has default configuration if not provided // The MCP gateway is mandatory and defaults to github/gh-aw-mcpg func ensureDefaultMCPGatewayConfig(workflowData *WorkflowData) { @@ -69,7 +79,7 @@ func ensureDefaultMCPGatewayConfig(workflowData *WorkflowData) { mcpGatewayConfigLog.Print("No MCP gateway configuration found, setting default configuration") workflowData.SandboxConfig.MCP = &MCPGatewayRuntimeConfig{ Container: constants.DefaultMCPGatewayContainer, - Version: string(constants.DefaultMCPGatewayVersion), + Version: getMCPGatewayVersion(workflowData, ""), Port: int(DefaultMCPGatewayPort), } } else { @@ -77,9 +87,9 @@ func ensureDefaultMCPGatewayConfig(workflowData *WorkflowData) { if workflowData.SandboxConfig.MCP.Container == "" { workflowData.SandboxConfig.MCP.Container = constants.DefaultMCPGatewayContainer } - // Only replace empty version with default - preserve user-specified versions including "latest" + // Only replace empty version - preserve user-specified versions including "latest" if workflowData.SandboxConfig.MCP.Version == "" { - workflowData.SandboxConfig.MCP.Version = string(constants.DefaultMCPGatewayVersion) + workflowData.SandboxConfig.MCP.Version = getMCPGatewayVersion(workflowData, "") } if workflowData.SandboxConfig.MCP.Port == 0 { workflowData.SandboxConfig.MCP.Port = int(DefaultMCPGatewayPort) diff --git a/pkg/workflow/mcp_gateway_config_test.go b/pkg/workflow/mcp_gateway_config_test.go index 840796557ca..94510f0d3e0 100644 --- a/pkg/workflow/mcp_gateway_config_test.go +++ b/pkg/workflow/mcp_gateway_config_test.go @@ -68,6 +68,45 @@ func TestEnsureDefaultMCPGatewayConfig(t *testing.T) { assert.Equal(t, 8080, wd.SandboxConfig.MCP.Port, "Port should be preserved") }, }, + { + name: "fills in missing version field from mcpg-version feature override", + workflowData: &WorkflowData{ + Features: map[string]any{ + string(constants.MCPGVersionFeatureFlag): "v0.2.27", + }, + SandboxConfig: &SandboxConfig{ + MCP: &MCPGatewayRuntimeConfig{ + Container: "custom-container", + Port: 8080, + }, + }, + }, + validate: func(t *testing.T, wd *WorkflowData) { + assert.Equal(t, "custom-container", wd.SandboxConfig.MCP.Container, "Container should be preserved") + assert.Equal(t, "v0.2.27", wd.SandboxConfig.MCP.Version, "Version should use feature override") + assert.Equal(t, 8080, wd.SandboxConfig.MCP.Port, "Port should be preserved") + }, + }, + { + name: "preserves explicit version over mcpg-version feature override", + workflowData: &WorkflowData{ + Features: map[string]any{ + string(constants.MCPGVersionFeatureFlag): "v0.2.27", + }, + SandboxConfig: &SandboxConfig{ + MCP: &MCPGatewayRuntimeConfig{ + Container: "custom-container", + Version: "v0.3.0", + Port: 8080, + }, + }, + }, + validate: func(t *testing.T, wd *WorkflowData) { + assert.Equal(t, "custom-container", wd.SandboxConfig.MCP.Container, "Container should be preserved") + assert.Equal(t, "v0.3.0", wd.SandboxConfig.MCP.Version, "Explicit version should be preserved") + assert.Equal(t, 8080, wd.SandboxConfig.MCP.Port, "Port should be preserved") + }, + }, { name: "fills in missing port field", workflowData: &WorkflowData{ diff --git a/pkg/workflow/mcp_setup_generator.go b/pkg/workflow/mcp_setup_generator.go index b75eee3b1bd..4f9e25c8204 100644 --- a/pkg/workflow/mcp_setup_generator.go +++ b/pkg/workflow/mcp_setup_generator.go @@ -688,11 +688,7 @@ func (c *Compiler) generateMCPSetup(yaml *strings.Builder, tools map[string]any, // Build container command containerImage := gatewayConfig.Container - if gatewayConfig.Version != "" { - containerImage += ":" + gatewayConfig.Version - } else { - containerImage += ":" + string(constants.DefaultMCPGatewayVersion) - } + containerImage += ":" + getMCPGatewayVersion(workflowData, gatewayConfig.Version) var containerCmd strings.Builder containerCmd.WriteString("docker run -i --rm --network host") diff --git a/pkg/workflow/mcp_setup_generator_test.go b/pkg/workflow/mcp_setup_generator_test.go index bda8e6cc2bd..8910ac1c15b 100644 --- a/pkg/workflow/mcp_setup_generator_test.go +++ b/pkg/workflow/mcp_setup_generator_test.go @@ -118,6 +118,7 @@ func TestMCPGatewayVersionFromFrontmatter(t *testing.T) { tests := []struct { name string sandboxConfig *SandboxConfig + features map[string]any expectedVersion string description string }{ @@ -180,6 +181,20 @@ func TestMCPGatewayVersionFromFrontmatter(t *testing.T) { expectedVersion: "1.2.3", description: "should use custom version 1.2.3", }, + { + name: "mcpg-version feature override applies when sandbox.mcp.version is not set", + sandboxConfig: &SandboxConfig{ + MCP: &MCPGatewayRuntimeConfig{ + Container: constants.DefaultMCPGatewayContainer, + Port: 8080, + }, + }, + features: map[string]any{ + string(constants.MCPGVersionFeatureFlag): "v0.2.27", + }, + expectedVersion: "v0.2.27", + description: "should use mcpg-version feature override when no explicit sandbox.mcp.version is set", + }, } for _, tt := range tests { @@ -187,6 +202,7 @@ func TestMCPGatewayVersionFromFrontmatter(t *testing.T) { workflowData := &WorkflowData{ SandboxConfig: tt.sandboxConfig, Tools: map[string]any{"github": map[string]any{}}, + Features: tt.features, } // Ensure MCP gateway config is applied (includes normalization of "latest") From dcf63b3fe0b9213ac4f86607bad8aa0af375c767 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 14:50:34 +0000 Subject: [PATCH 3/9] refactor: rename version override feature flags to *-latest Agent-Logs-Url: https://github.com/github/gh-aw/sessions/8f382246-ff49-4533-bf8d-dde4344bf6c0 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --- pkg/constants/feature_constants.go | 18 ++++++++--------- pkg/workflow/copilot_engine_installation.go | 4 ++-- pkg/workflow/copilot_installer_test.go | 6 +++--- pkg/workflow/docker_api_proxy_test.go | 2 +- pkg/workflow/features_test.go | 20 +++++++++---------- pkg/workflow/firewall.go | 6 +++--- pkg/workflow/firewall_image_tag_test.go | 4 ++-- pkg/workflow/firewall_version_pinning_test.go | 2 +- pkg/workflow/mcp_gateway_config.go | 2 +- pkg/workflow/mcp_gateway_config_test.go | 8 ++++---- pkg/workflow/mcp_setup_generator_test.go | 6 +++--- 11 files changed, 39 insertions(+), 39 deletions(-) diff --git a/pkg/constants/feature_constants.go b/pkg/constants/feature_constants.go index 429b7bfed15..f9fa34b07de 100644 --- a/pkg/constants/feature_constants.go +++ b/pkg/constants/feature_constants.go @@ -60,33 +60,33 @@ const ( // features: // byok-copilot: true ByokCopilotFeatureFlag FeatureFlag = "byok-copilot" - // LatestCopilotFeatureFlag forces Copilot CLI installation to use latest. + // CopilotLatestFeatureFlag forces Copilot CLI installation to use latest. // When enabled, Copilot CLI installation ignores the pinned default version // and installs latest for smoke-testing upcoming releases. // // Workflow frontmatter usage: // // features: - // latest-copilot: true - LatestCopilotFeatureFlag FeatureFlag = "latest-copilot" - // MCPGVersionFeatureFlag overrides the default MCP gateway container version. + // copilot-latest: true + CopilotLatestFeatureFlag FeatureFlag = "copilot-latest" + // MCPGLatestFeatureFlag overrides the default MCP gateway container version. // When set to a non-empty string, MCP gateway image references use this // version instead of DefaultMCPGatewayVersion. // // Workflow frontmatter usage: // // features: - // mcpg-version: "v0.2.27" - MCPGVersionFeatureFlag FeatureFlag = "mcpg-version" - // FirewallVersionFeatureFlag overrides the default firewall container version. + // mcpg-latest: "v0.2.27" + MCPGLatestFeatureFlag FeatureFlag = "mcpg-latest" + // FirewallLatestFeatureFlag overrides the default firewall container version. // When set to a non-empty string, AWF installation and image references use // this version instead of DefaultFirewallVersion. // // Workflow frontmatter usage: // // features: - // firewall-version: "v0.25.27" - FirewallVersionFeatureFlag FeatureFlag = "firewall-version" + // firewall-latest: "v0.25.27" + FirewallLatestFeatureFlag FeatureFlag = "firewall-latest" // IntegrityReactionsFeatureFlag enables reaction-based integrity promotion/demotion // in the MCPG allow-only policy. When enabled, the compiler injects // endorsement-reactions and disapproval-reactions fields into the allow-only policy. diff --git a/pkg/workflow/copilot_engine_installation.go b/pkg/workflow/copilot_engine_installation.go index 1a21257b923..f0ed130926f 100644 --- a/pkg/workflow/copilot_engine_installation.go +++ b/pkg/workflow/copilot_engine_installation.go @@ -69,9 +69,9 @@ func (e *CopilotEngine) GetInstallationSteps(workflowData *WorkflowData) []GitHu if workflowData.EngineConfig != nil && workflowData.EngineConfig.Version != "" { copilotVersion = workflowData.EngineConfig.Version } - if isFeatureEnabled(constants.LatestCopilotFeatureFlag, workflowData) { + if isFeatureEnabled(constants.CopilotLatestFeatureFlag, workflowData) { copilotVersion = "latest" - copilotInstallLog.Print("latest-copilot enabled: forcing Copilot CLI install version to latest") + copilotInstallLog.Print("copilot-latest enabled: forcing Copilot CLI install version to latest") } if isFeatureEnabled(constants.ByokCopilotFeatureFlag, workflowData) { copilotVersion = "latest" diff --git a/pkg/workflow/copilot_installer_test.go b/pkg/workflow/copilot_installer_test.go index 8f55ad5af25..046f46a2031 100644 --- a/pkg/workflow/copilot_installer_test.go +++ b/pkg/workflow/copilot_installer_test.go @@ -290,7 +290,7 @@ func TestCopilotInstallerLatestCopilotFeatureUsesLatestVersion(t *testing.T) { Version: "1.0.0", }, Features: map[string]any{ - string(constants.LatestCopilotFeatureFlag): true, + string(constants.CopilotLatestFeatureFlag): true, }, } @@ -310,9 +310,9 @@ func TestCopilotInstallerLatestCopilotFeatureUsesLatestVersion(t *testing.T) { } if !strings.Contains(installStep, `install_copilot_cli.sh" latest`) { - t.Errorf("Expected latest-copilot to force latest Copilot CLI install, got:\n%s", installStep) + t.Errorf("Expected copilot-latest to force latest Copilot CLI install, got:\n%s", installStep) } if strings.Contains(installStep, `install_copilot_cli.sh" 1.0.0`) { - t.Errorf("Expected latest-copilot to ignore pinned version, got:\n%s", installStep) + t.Errorf("Expected copilot-latest to ignore pinned version, got:\n%s", installStep) } } diff --git a/pkg/workflow/docker_api_proxy_test.go b/pkg/workflow/docker_api_proxy_test.go index 1e73737cebe..a00c9dd9644 100644 --- a/pkg/workflow/docker_api_proxy_test.go +++ b/pkg/workflow/docker_api_proxy_test.go @@ -86,7 +86,7 @@ func TestCollectDockerImages_FirewallVersionFeatureOverride(t *testing.T) { workflowData := &WorkflowData{ AI: "claude", Features: map[string]any{ - string(constants.FirewallVersionFeatureFlag): featureVersion, + string(constants.FirewallLatestFeatureFlag): featureVersion, }, NetworkPermissions: &NetworkPermissions{ Firewall: &FirewallConfig{ diff --git a/pkg/workflow/features_test.go b/pkg/workflow/features_test.go index 960e58f829e..378de8788ba 100644 --- a/pkg/workflow/features_test.go +++ b/pkg/workflow/features_test.go @@ -314,40 +314,40 @@ func TestGetFeatureValue(t *testing.T) { name: "returns exact string value", workflow: &WorkflowData{ Features: map[string]any{ - "mcpg-version": "v0.2.27", + "mcpg-latest": "v0.2.27", }, }, - flag: constants.MCPGVersionFeatureFlag, + flag: constants.MCPGLatestFeatureFlag, expectedVal: "v0.2.27", }, { name: "returns case-insensitive key match", workflow: &WorkflowData{ Features: map[string]any{ - "MCPG-Version": "v0.2.28", + "MCPG-LATEST": "v0.2.28", }, }, - flag: constants.MCPGVersionFeatureFlag, + flag: constants.MCPGLatestFeatureFlag, expectedVal: "v0.2.28", }, { name: "trims whitespace from value", workflow: &WorkflowData{ Features: map[string]any{ - "mcpg-version": " v0.2.29 ", + "mcpg-latest": " v0.2.29 ", }, }, - flag: constants.MCPGVersionFeatureFlag, + flag: constants.MCPGLatestFeatureFlag, expectedVal: "v0.2.29", }, { name: "returns empty for non-string value", workflow: &WorkflowData{ Features: map[string]any{ - "mcpg-version": true, + "mcpg-latest": true, }, }, - flag: constants.MCPGVersionFeatureFlag, + flag: constants.MCPGLatestFeatureFlag, expectedVal: "", }, { @@ -357,13 +357,13 @@ func TestGetFeatureValue(t *testing.T) { "other-feature": "v1", }, }, - flag: constants.MCPGVersionFeatureFlag, + flag: constants.MCPGLatestFeatureFlag, expectedVal: "", }, { name: "returns empty for nil workflow", workflow: nil, - flag: constants.MCPGVersionFeatureFlag, + flag: constants.MCPGLatestFeatureFlag, expectedVal: "", }, } diff --git a/pkg/workflow/firewall.go b/pkg/workflow/firewall.go index b8cff744b9c..c661441b144 100644 --- a/pkg/workflow/firewall.go +++ b/pkg/workflow/firewall.go @@ -65,7 +65,7 @@ func getFirewallConfig(workflowData *WorkflowData) *FirewallConfig { return nil } - featureVersion := getFeatureValue(constants.FirewallVersionFeatureFlag, workflowData) + featureVersion := getFeatureValue(constants.FirewallLatestFeatureFlag, workflowData) // Check network.firewall configuration if workflowData.NetworkPermissions != nil && workflowData.NetworkPermissions.Firewall != nil { @@ -75,7 +75,7 @@ func getFirewallConfig(workflowData *WorkflowData) *FirewallConfig { configCopy := *config configCopy.Version = featureVersion if firewallLog.Enabled() { - firewallLog.Printf("Applied firewall-version feature override: version=%s", featureVersion) + firewallLog.Printf("Applied firewall-latest feature override: version=%s", featureVersion) } return &configCopy } @@ -88,7 +88,7 @@ func getFirewallConfig(workflowData *WorkflowData) *FirewallConfig { if featureVersion != "" { if firewallLog.Enabled() { - firewallLog.Printf("Using firewall-version feature override without explicit firewall config: version=%s", featureVersion) + firewallLog.Printf("Using firewall-latest feature override without explicit firewall config: version=%s", featureVersion) } return &FirewallConfig{Version: featureVersion} } diff --git a/pkg/workflow/firewall_image_tag_test.go b/pkg/workflow/firewall_image_tag_test.go index 5109ce91047..222ac3e3f6e 100644 --- a/pkg/workflow/firewall_image_tag_test.go +++ b/pkg/workflow/firewall_image_tag_test.go @@ -120,7 +120,7 @@ func TestClaudeEngineAWFImageTag(t *testing.T) { } }) - t.Run("AWF command includes image-tag with firewall-version feature override", func(t *testing.T) { + t.Run("AWF command includes image-tag with firewall-latest feature override", func(t *testing.T) { featureVersion := "v0.25.27" workflowData := &WorkflowData{ Name: "test-workflow", @@ -128,7 +128,7 @@ func TestClaudeEngineAWFImageTag(t *testing.T) { ID: "claude", }, Features: map[string]any{ - string(constants.FirewallVersionFeatureFlag): featureVersion, + string(constants.FirewallLatestFeatureFlag): featureVersion, }, NetworkPermissions: &NetworkPermissions{ Firewall: &FirewallConfig{ diff --git a/pkg/workflow/firewall_version_pinning_test.go b/pkg/workflow/firewall_version_pinning_test.go index 19c6a6ee329..40621656a55 100644 --- a/pkg/workflow/firewall_version_pinning_test.go +++ b/pkg/workflow/firewall_version_pinning_test.go @@ -167,7 +167,7 @@ func TestCopilotEngineFirewallInstallation(t *testing.T) { ID: "copilot", }, Features: map[string]any{ - string(constants.FirewallVersionFeatureFlag): featureVersion, + string(constants.FirewallLatestFeatureFlag): featureVersion, }, NetworkPermissions: &NetworkPermissions{ Firewall: &FirewallConfig{ diff --git a/pkg/workflow/mcp_gateway_config.go b/pkg/workflow/mcp_gateway_config.go index 90a4f6f79da..9250347e525 100644 --- a/pkg/workflow/mcp_gateway_config.go +++ b/pkg/workflow/mcp_gateway_config.go @@ -56,7 +56,7 @@ func getMCPGatewayVersion(workflowData *WorkflowData, configuredVersion string) if configuredVersion != "" { return configuredVersion } - if featureVersion := getFeatureValue(constants.MCPGVersionFeatureFlag, workflowData); featureVersion != "" { + if featureVersion := getFeatureValue(constants.MCPGLatestFeatureFlag, workflowData); featureVersion != "" { return featureVersion } return string(constants.DefaultMCPGatewayVersion) diff --git a/pkg/workflow/mcp_gateway_config_test.go b/pkg/workflow/mcp_gateway_config_test.go index 94510f0d3e0..24df74f0cfc 100644 --- a/pkg/workflow/mcp_gateway_config_test.go +++ b/pkg/workflow/mcp_gateway_config_test.go @@ -69,10 +69,10 @@ func TestEnsureDefaultMCPGatewayConfig(t *testing.T) { }, }, { - name: "fills in missing version field from mcpg-version feature override", + name: "fills in missing version field from mcpg-latest feature override", workflowData: &WorkflowData{ Features: map[string]any{ - string(constants.MCPGVersionFeatureFlag): "v0.2.27", + string(constants.MCPGLatestFeatureFlag): "v0.2.27", }, SandboxConfig: &SandboxConfig{ MCP: &MCPGatewayRuntimeConfig{ @@ -88,10 +88,10 @@ func TestEnsureDefaultMCPGatewayConfig(t *testing.T) { }, }, { - name: "preserves explicit version over mcpg-version feature override", + name: "preserves explicit version over mcpg-latest feature override", workflowData: &WorkflowData{ Features: map[string]any{ - string(constants.MCPGVersionFeatureFlag): "v0.2.27", + string(constants.MCPGLatestFeatureFlag): "v0.2.27", }, SandboxConfig: &SandboxConfig{ MCP: &MCPGatewayRuntimeConfig{ diff --git a/pkg/workflow/mcp_setup_generator_test.go b/pkg/workflow/mcp_setup_generator_test.go index 8910ac1c15b..2591579f7a9 100644 --- a/pkg/workflow/mcp_setup_generator_test.go +++ b/pkg/workflow/mcp_setup_generator_test.go @@ -182,7 +182,7 @@ func TestMCPGatewayVersionFromFrontmatter(t *testing.T) { description: "should use custom version 1.2.3", }, { - name: "mcpg-version feature override applies when sandbox.mcp.version is not set", + name: "mcpg-latest feature override applies when sandbox.mcp.version is not set", sandboxConfig: &SandboxConfig{ MCP: &MCPGatewayRuntimeConfig{ Container: constants.DefaultMCPGatewayContainer, @@ -190,10 +190,10 @@ func TestMCPGatewayVersionFromFrontmatter(t *testing.T) { }, }, features: map[string]any{ - string(constants.MCPGVersionFeatureFlag): "v0.2.27", + string(constants.MCPGLatestFeatureFlag): "v0.2.27", }, expectedVersion: "v0.2.27", - description: "should use mcpg-version feature override when no explicit sandbox.mcp.version is set", + description: "should use mcpg-latest feature override when no explicit sandbox.mcp.version is set", }, } From a576634e0a4c7e561ee4646f83fb7721df81a32c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 15:33:18 +0000 Subject: [PATCH 4/9] feat: add version feature flags across engines and runtime components Agent-Logs-Url: https://github.com/github/gh-aw/sessions/a77776fd-69ce-451f-a672-9e755656680e Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --- actions/setup/sh/install_awf_binary.sh | 8 ++- pkg/constants/feature_constants.go | 55 ++++++++++++--- pkg/workflow/claude_engine.go | 1 + pkg/workflow/claude_engine_test.go | 10 +++ pkg/workflow/codex_engine.go | 9 ++- pkg/workflow/codex_engine_test.go | 19 ++++++ pkg/workflow/copilot_engine_installation.go | 8 ++- pkg/workflow/copilot_installer_test.go | 67 +++++++++++++++++-- pkg/workflow/crush_engine.go | 1 + pkg/workflow/docker_api_proxy_test.go | 2 +- pkg/workflow/engine_helpers.go | 11 ++- pkg/workflow/engine_helpers_test.go | 17 +++++ pkg/workflow/features_test.go | 20 +++--- pkg/workflow/firewall.go | 6 +- pkg/workflow/firewall_image_tag_test.go | 4 +- pkg/workflow/firewall_version_pinning_test.go | 39 ++++++++++- pkg/workflow/gemini_engine.go | 1 + pkg/workflow/gemini_engine_test.go | 15 +++++ pkg/workflow/mcp_gateway_config.go | 2 +- pkg/workflow/mcp_gateway_config_test.go | 8 +-- pkg/workflow/mcp_setup_generator_test.go | 6 +- pkg/workflow/opencode_engine.go | 1 + pkg/workflow/opencode_engine_test.go | 12 ++++ 23 files changed, 273 insertions(+), 49 deletions(-) diff --git a/actions/setup/sh/install_awf_binary.sh b/actions/setup/sh/install_awf_binary.sh index e95bff73f8b..4be495420c1 100755 --- a/actions/setup/sh/install_awf_binary.sh +++ b/actions/setup/sh/install_awf_binary.sh @@ -6,7 +6,7 @@ # its SHA256 checksum before installation to protect against supply chain attacks. # # Arguments: -# VERSION - AWF version to install (e.g., v0.25.10) +# VERSION - AWF version to install (e.g., v0.25.10) or "latest" # # Install strategy: # 1. If Node.js >= 20 is available, download the lightweight awf-bundle.js (~357KB) @@ -44,7 +44,11 @@ ARCH="$(uname -m)" echo "Installing awf with checksum verification (version: ${AWF_VERSION}, os: ${OS}, arch: ${ARCH})" # Download URLs -BASE_URL="https://github.com/${AWF_REPO}/releases/download/${AWF_VERSION}" +if [ "$AWF_VERSION" = "latest" ]; then + BASE_URL="https://github.com/${AWF_REPO}/releases/latest/download" +else + BASE_URL="https://github.com/${AWF_REPO}/releases/download/${AWF_VERSION}" +fi CHECKSUMS_URL="${BASE_URL}/checksums.txt" # Platform-portable SHA256 function diff --git a/pkg/constants/feature_constants.go b/pkg/constants/feature_constants.go index f9fa34b07de..f5dc2540921 100644 --- a/pkg/constants/feature_constants.go +++ b/pkg/constants/feature_constants.go @@ -60,33 +60,66 @@ const ( // features: // byok-copilot: true ByokCopilotFeatureFlag FeatureFlag = "byok-copilot" - // CopilotLatestFeatureFlag forces Copilot CLI installation to use latest. - // When enabled, Copilot CLI installation ignores the pinned default version - // and installs latest for smoke-testing upcoming releases. + // CopilotVersionFeatureFlag overrides the default Copilot CLI version. + // When set to a non-empty string, Copilot CLI installation uses this + // version instead of DefaultCopilotVersion. Supports explicit versions + // (e.g. "1.0.21") and the "latest" tag. // // Workflow frontmatter usage: // // features: - // copilot-latest: true - CopilotLatestFeatureFlag FeatureFlag = "copilot-latest" - // MCPGLatestFeatureFlag overrides the default MCP gateway container version. + // copilot-version: "latest" + CopilotVersionFeatureFlag FeatureFlag = "copilot-version" + // MCPGVersionFeatureFlag overrides the default MCP gateway container version. // When set to a non-empty string, MCP gateway image references use this // version instead of DefaultMCPGatewayVersion. // // Workflow frontmatter usage: // // features: - // mcpg-latest: "v0.2.27" - MCPGLatestFeatureFlag FeatureFlag = "mcpg-latest" - // FirewallLatestFeatureFlag overrides the default firewall container version. + // mcpg-version: "v0.2.27" + MCPGVersionFeatureFlag FeatureFlag = "mcpg-version" + // FirewallVersionFeatureFlag overrides the default firewall container version. // When set to a non-empty string, AWF installation and image references use // this version instead of DefaultFirewallVersion. // // Workflow frontmatter usage: // // features: - // firewall-latest: "v0.25.27" - FirewallLatestFeatureFlag FeatureFlag = "firewall-latest" + // firewall-version: "v0.25.27" + FirewallVersionFeatureFlag FeatureFlag = "firewall-version" + // CodexVersionFeatureFlag overrides the default Codex CLI version. + // Supports explicit versions (e.g. "0.121.0") and the "latest" tag. + // + // Workflow frontmatter usage: + // + // features: + // codex-version: "latest" + CodexVersionFeatureFlag FeatureFlag = "codex-version" + // ClaudeVersionFeatureFlag overrides the default Claude CLI version. + // Supports explicit versions (e.g. "2.1.47") and the "latest" tag. + // + // Workflow frontmatter usage: + // + // features: + // claude-version: "latest" + ClaudeVersionFeatureFlag FeatureFlag = "claude-version" + // OpenCodeVersionFeatureFlag overrides the default OpenCode CLI version. + // Supports explicit versions and the "latest" tag. + // + // Workflow frontmatter usage: + // + // features: + // opencode-version: "latest" + OpenCodeVersionFeatureFlag FeatureFlag = "opencode-version" + // GeminiVersionFeatureFlag overrides the default Gemini CLI version. + // Supports explicit versions and the "latest" tag. + // + // Workflow frontmatter usage: + // + // features: + // gemini-version: "latest" + GeminiVersionFeatureFlag FeatureFlag = "gemini-version" // IntegrityReactionsFeatureFlag enables reaction-based integrity promotion/demotion // in the MCPG allow-only policy. When enabled, the compiler injects // endorsement-reactions and disapproval-reactions fields into the allow-only policy. diff --git a/pkg/workflow/claude_engine.go b/pkg/workflow/claude_engine.go index 02ad9051270..5ce6d4f1432 100644 --- a/pkg/workflow/claude_engine.go +++ b/pkg/workflow/claude_engine.go @@ -78,6 +78,7 @@ func (e *ClaudeEngine) GetInstallationSteps(workflowData *WorkflowData) []GitHub string(constants.DefaultClaudeCodeVersion), "Install Claude Code CLI", "claude", + constants.ClaudeVersionFeatureFlag, workflowData, ) return BuildNpmEngineInstallStepsWithAWF(npmSteps, workflowData) diff --git a/pkg/workflow/claude_engine_test.go b/pkg/workflow/claude_engine_test.go index 14d57db5d92..a224d7539ab 100644 --- a/pkg/workflow/claude_engine_test.go +++ b/pkg/workflow/claude_engine_test.go @@ -62,6 +62,16 @@ func TestClaudeEngine(t *testing.T) { t.Errorf("Expected '%s' in install step, got: %s", expectedInstallCommand, installStep) } + featureInstallSteps := engine.GetInstallationSteps(&WorkflowData{ + Features: map[string]any{ + string(constants.ClaudeVersionFeatureFlag): "latest", + }, + }) + featureInstallStep := strings.Join([]string(featureInstallSteps[1]), "\n") + if !strings.Contains(featureInstallStep, "npm install --ignore-scripts -g @anthropic-ai/claude-code@latest") { + t.Errorf("Expected claude-version feature override to use latest, got: %s", featureInstallStep) + } + // Test execution steps workflowData := &WorkflowData{ Name: "test-workflow", diff --git a/pkg/workflow/codex_engine.go b/pkg/workflow/codex_engine.go index 049de526804..0f2c6c34d54 100644 --- a/pkg/workflow/codex_engine.go +++ b/pkg/workflow/codex_engine.go @@ -80,11 +80,18 @@ func (e *CodexEngine) GetInstallationSteps(workflowData *WorkflowData) []GitHubA } // Use base installation steps (npm install only; secret validation is in the activation job) + codexVersion := string(constants.DefaultCodexVersion) + if workflowData.EngineConfig != nil && workflowData.EngineConfig.Version != "" { + codexVersion = workflowData.EngineConfig.Version + } else if featureVersion := getFeatureValue(constants.CodexVersionFeatureFlag, workflowData); featureVersion != "" { + codexVersion = featureVersion + } + steps := GetBaseInstallationSteps(EngineInstallConfig{ Secrets: []string{"CODEX_API_KEY", "OPENAI_API_KEY"}, DocsURL: "https://github.github.com/gh-aw/reference/engines/#openai-codex", NpmPackage: "@openai/codex", - Version: string(constants.DefaultCodexVersion), + Version: codexVersion, Name: "Codex CLI", InstallStepName: "Install Codex CLI", CliName: "codex", diff --git a/pkg/workflow/codex_engine_test.go b/pkg/workflow/codex_engine_test.go index d4680f3388c..09e1b52a202 100644 --- a/pkg/workflow/codex_engine_test.go +++ b/pkg/workflow/codex_engine_test.go @@ -135,6 +135,25 @@ func TestCodexEngineWithVersion(t *testing.T) { if !foundVersionInstall { t.Error("Expected versioned npm install command with @openai/codex@3.0.1") } + + // Test installation steps with feature-based version override + stepsWithFeatureVersion := engine.GetInstallationSteps(&WorkflowData{ + Features: map[string]any{ + string(constants.CodexVersionFeatureFlag): "latest", + }, + }) + foundFeatureVersionInstall := false + for _, step := range stepsWithFeatureVersion { + for _, line := range step { + if strings.Contains(line, "npm install") && strings.Contains(line, "@openai/codex@latest") { + foundFeatureVersionInstall = true + break + } + } + } + if !foundFeatureVersionInstall { + t.Error("Expected feature version override npm install command with @openai/codex@latest") + } } func TestCodexEngineExecutionIncludesGitHubAWPrompt(t *testing.T) { diff --git a/pkg/workflow/copilot_engine_installation.go b/pkg/workflow/copilot_engine_installation.go index f0ed130926f..061564b1b42 100644 --- a/pkg/workflow/copilot_engine_installation.go +++ b/pkg/workflow/copilot_engine_installation.go @@ -69,9 +69,11 @@ func (e *CopilotEngine) GetInstallationSteps(workflowData *WorkflowData) []GitHu if workflowData.EngineConfig != nil && workflowData.EngineConfig.Version != "" { copilotVersion = workflowData.EngineConfig.Version } - if isFeatureEnabled(constants.CopilotLatestFeatureFlag, workflowData) { - copilotVersion = "latest" - copilotInstallLog.Print("copilot-latest enabled: forcing Copilot CLI install version to latest") + if workflowData.EngineConfig == nil || workflowData.EngineConfig.Version == "" { + if featureVersion := getFeatureValue(constants.CopilotVersionFeatureFlag, workflowData); featureVersion != "" { + copilotVersion = featureVersion + copilotInstallLog.Printf("copilot-version feature override applied: version=%s", featureVersion) + } } if isFeatureEnabled(constants.ByokCopilotFeatureFlag, workflowData) { copilotVersion = "latest" diff --git a/pkg/workflow/copilot_installer_test.go b/pkg/workflow/copilot_installer_test.go index 046f46a2031..75f1b4bebdd 100644 --- a/pkg/workflow/copilot_installer_test.go +++ b/pkg/workflow/copilot_installer_test.go @@ -282,7 +282,36 @@ func TestCopilotInstallerByokFeatureUsesLatestVersion(t *testing.T) { } } -func TestCopilotInstallerLatestCopilotFeatureUsesLatestVersion(t *testing.T) { +func TestCopilotInstallerCopilotVersionFeatureUsesLatestVersion(t *testing.T) { + engine := NewCopilotEngine() + workflowData := &WorkflowData{ + Name: "test-workflow", + Features: map[string]any{ + string(constants.CopilotVersionFeatureFlag): "latest", + }, + } + + steps := engine.GetInstallationSteps(workflowData) + + var installStep string + for _, step := range steps { + stepContent := strings.Join(step, "\n") + if strings.Contains(stepContent, "install_copilot_cli.sh") { + installStep = stepContent + break + } + } + + if installStep == "" { + t.Fatal("Could not find install step with install_copilot_cli.sh") + } + + if !strings.Contains(installStep, `install_copilot_cli.sh" latest`) { + t.Errorf("Expected copilot-version=latest to force latest Copilot CLI install, got:\n%s", installStep) + } +} + +func TestCopilotInstallerEngineConfigVersionTakesPrecedenceOverFeatureVersion(t *testing.T) { engine := NewCopilotEngine() workflowData := &WorkflowData{ Name: "test-workflow", @@ -290,7 +319,7 @@ func TestCopilotInstallerLatestCopilotFeatureUsesLatestVersion(t *testing.T) { Version: "1.0.0", }, Features: map[string]any{ - string(constants.CopilotLatestFeatureFlag): true, + string(constants.CopilotVersionFeatureFlag): "latest", }, } @@ -309,10 +338,36 @@ func TestCopilotInstallerLatestCopilotFeatureUsesLatestVersion(t *testing.T) { t.Fatal("Could not find install step with install_copilot_cli.sh") } - if !strings.Contains(installStep, `install_copilot_cli.sh" latest`) { - t.Errorf("Expected copilot-latest to force latest Copilot CLI install, got:\n%s", installStep) + if !strings.Contains(installStep, `install_copilot_cli.sh" 1.0.0`) { + t.Errorf("Expected engine.version to take precedence over copilot-version feature flag, got:\n%s", installStep) } - if strings.Contains(installStep, `install_copilot_cli.sh" 1.0.0`) { - t.Errorf("Expected copilot-latest to ignore pinned version, got:\n%s", installStep) +} + +func TestCopilotInstallerCopilotVersionFeatureSupportsSpecificVersion(t *testing.T) { + engine := NewCopilotEngine() + workflowData := &WorkflowData{ + Name: "test-workflow", + Features: map[string]any{ + string(constants.CopilotVersionFeatureFlag): "1.2.3", + }, + } + + steps := engine.GetInstallationSteps(workflowData) + + var installStep string + for _, step := range steps { + stepContent := strings.Join(step, "\n") + if strings.Contains(stepContent, "install_copilot_cli.sh") { + installStep = stepContent + break + } + } + + if installStep == "" { + t.Fatal("Could not find install step with install_copilot_cli.sh") + } + + if !strings.Contains(installStep, `install_copilot_cli.sh" 1.2.3`) { + t.Errorf("Expected copilot-version to support specific version values, got:\n%s", installStep) } } diff --git a/pkg/workflow/crush_engine.go b/pkg/workflow/crush_engine.go index fdd630ca1c6..540a66cf81e 100644 --- a/pkg/workflow/crush_engine.go +++ b/pkg/workflow/crush_engine.go @@ -103,6 +103,7 @@ func (e *CrushEngine) GetInstallationSteps(workflowData *WorkflowData) []GitHubA string(constants.DefaultCrushVersion), "Install Crush CLI", "crush", + "", workflowData, ) return BuildNpmEngineInstallStepsWithAWF(npmSteps, workflowData) diff --git a/pkg/workflow/docker_api_proxy_test.go b/pkg/workflow/docker_api_proxy_test.go index a00c9dd9644..1e73737cebe 100644 --- a/pkg/workflow/docker_api_proxy_test.go +++ b/pkg/workflow/docker_api_proxy_test.go @@ -86,7 +86,7 @@ func TestCollectDockerImages_FirewallVersionFeatureOverride(t *testing.T) { workflowData := &WorkflowData{ AI: "claude", Features: map[string]any{ - string(constants.FirewallLatestFeatureFlag): featureVersion, + string(constants.FirewallVersionFeatureFlag): featureVersion, }, NetworkPermissions: &NetworkPermissions{ Firewall: &FirewallConfig{ diff --git a/pkg/workflow/engine_helpers.go b/pkg/workflow/engine_helpers.go index b8d08739885..e29c30da77f 100644 --- a/pkg/workflow/engine_helpers.go +++ b/pkg/workflow/engine_helpers.go @@ -37,6 +37,7 @@ import ( "sort" "strings" + "github.com/github/gh-aw/pkg/constants" "github.com/github/gh-aw/pkg/logger" ) @@ -107,6 +108,7 @@ func GetBaseInstallationSteps(config EngineInstallConfig, workflowData *Workflow config.Version, stepName, config.CliName, + "", workflowData, ) steps = append(steps, npmSteps...) @@ -131,15 +133,22 @@ func BuildStandardNpmEngineInstallSteps( defaultVersion string, stepName string, cacheKeyPrefix string, + featureVersionFlag constants.FeatureFlag, workflowData *WorkflowData, ) []GitHubActionStep { engineHelpersLog.Printf("Building npm engine install steps: package=%s, version=%s", packageName, defaultVersion) // Use version from engine config if provided, otherwise default to pinned version version := defaultVersion - if workflowData.EngineConfig != nil && workflowData.EngineConfig.Version != "" { + engineVersionSet := workflowData.EngineConfig != nil && workflowData.EngineConfig.Version != "" + if engineVersionSet { version = workflowData.EngineConfig.Version engineHelpersLog.Printf("Using engine config version: %s", version) + } else if featureVersionFlag != "" { + if featureVersion := getFeatureValue(featureVersionFlag, workflowData); featureVersion != "" { + version = featureVersion + engineHelpersLog.Printf("Using feature override version (%s): %s", featureVersionFlag, version) + } } // Add npm package installation steps (includes Node.js setup) diff --git a/pkg/workflow/engine_helpers_test.go b/pkg/workflow/engine_helpers_test.go index e72f73ebae7..6de68f4ae8a 100644 --- a/pkg/workflow/engine_helpers_test.go +++ b/pkg/workflow/engine_helpers_test.go @@ -18,12 +18,14 @@ func TestBuildStandardNpmEngineInstallSteps(t *testing.T) { tests := []struct { name string workflowData *WorkflowData + featureFlag constants.FeatureFlag expectedSteps int // Number of steps expected (Node.js setup + npm install) expectedInStep string }{ { name: "with default version", workflowData: &WorkflowData{}, + featureFlag: "", expectedSteps: 2, // Node.js setup + npm install expectedInStep: string(constants.DefaultCopilotVersion), }, @@ -34,6 +36,7 @@ func TestBuildStandardNpmEngineInstallSteps(t *testing.T) { Version: "1.2.3", }, }, + featureFlag: constants.CopilotVersionFeatureFlag, expectedSteps: 2, expectedInStep: "1.2.3", }, @@ -44,9 +47,21 @@ func TestBuildStandardNpmEngineInstallSteps(t *testing.T) { Version: "", }, }, + featureFlag: "", expectedSteps: 2, expectedInStep: string(constants.DefaultCopilotVersion), }, + { + name: "with version override from feature flag", + workflowData: &WorkflowData{ + Features: map[string]any{ + string(constants.CopilotVersionFeatureFlag): "latest", + }, + }, + featureFlag: constants.CopilotVersionFeatureFlag, + expectedSteps: 2, + expectedInStep: "latest", + }, } for _, tt := range tests { @@ -56,6 +71,7 @@ func TestBuildStandardNpmEngineInstallSteps(t *testing.T) { string(constants.DefaultCopilotVersion), "Install GitHub Copilot CLI", "copilot", + tt.featureFlag, tt.workflowData, ) @@ -121,6 +137,7 @@ func TestBuildStandardNpmEngineInstallSteps_AllEngines(t *testing.T) { tt.defaultVersion, tt.stepName, tt.cacheKeyPrefix, + "", workflowData, ) diff --git a/pkg/workflow/features_test.go b/pkg/workflow/features_test.go index 378de8788ba..fadc694b18d 100644 --- a/pkg/workflow/features_test.go +++ b/pkg/workflow/features_test.go @@ -314,40 +314,40 @@ func TestGetFeatureValue(t *testing.T) { name: "returns exact string value", workflow: &WorkflowData{ Features: map[string]any{ - "mcpg-latest": "v0.2.27", + "mcpg-version": "v0.2.27", }, }, - flag: constants.MCPGLatestFeatureFlag, + flag: constants.MCPGVersionFeatureFlag, expectedVal: "v0.2.27", }, { name: "returns case-insensitive key match", workflow: &WorkflowData{ Features: map[string]any{ - "MCPG-LATEST": "v0.2.28", + "MCPG-VERSION": "v0.2.28", }, }, - flag: constants.MCPGLatestFeatureFlag, + flag: constants.MCPGVersionFeatureFlag, expectedVal: "v0.2.28", }, { name: "trims whitespace from value", workflow: &WorkflowData{ Features: map[string]any{ - "mcpg-latest": " v0.2.29 ", + "mcpg-version": " v0.2.29 ", }, }, - flag: constants.MCPGLatestFeatureFlag, + flag: constants.MCPGVersionFeatureFlag, expectedVal: "v0.2.29", }, { name: "returns empty for non-string value", workflow: &WorkflowData{ Features: map[string]any{ - "mcpg-latest": true, + "mcpg-version": true, }, }, - flag: constants.MCPGLatestFeatureFlag, + flag: constants.MCPGVersionFeatureFlag, expectedVal: "", }, { @@ -357,13 +357,13 @@ func TestGetFeatureValue(t *testing.T) { "other-feature": "v1", }, }, - flag: constants.MCPGLatestFeatureFlag, + flag: constants.MCPGVersionFeatureFlag, expectedVal: "", }, { name: "returns empty for nil workflow", workflow: nil, - flag: constants.MCPGLatestFeatureFlag, + flag: constants.MCPGVersionFeatureFlag, expectedVal: "", }, } diff --git a/pkg/workflow/firewall.go b/pkg/workflow/firewall.go index c661441b144..b8cff744b9c 100644 --- a/pkg/workflow/firewall.go +++ b/pkg/workflow/firewall.go @@ -65,7 +65,7 @@ func getFirewallConfig(workflowData *WorkflowData) *FirewallConfig { return nil } - featureVersion := getFeatureValue(constants.FirewallLatestFeatureFlag, workflowData) + featureVersion := getFeatureValue(constants.FirewallVersionFeatureFlag, workflowData) // Check network.firewall configuration if workflowData.NetworkPermissions != nil && workflowData.NetworkPermissions.Firewall != nil { @@ -75,7 +75,7 @@ func getFirewallConfig(workflowData *WorkflowData) *FirewallConfig { configCopy := *config configCopy.Version = featureVersion if firewallLog.Enabled() { - firewallLog.Printf("Applied firewall-latest feature override: version=%s", featureVersion) + firewallLog.Printf("Applied firewall-version feature override: version=%s", featureVersion) } return &configCopy } @@ -88,7 +88,7 @@ func getFirewallConfig(workflowData *WorkflowData) *FirewallConfig { if featureVersion != "" { if firewallLog.Enabled() { - firewallLog.Printf("Using firewall-latest feature override without explicit firewall config: version=%s", featureVersion) + firewallLog.Printf("Using firewall-version feature override without explicit firewall config: version=%s", featureVersion) } return &FirewallConfig{Version: featureVersion} } diff --git a/pkg/workflow/firewall_image_tag_test.go b/pkg/workflow/firewall_image_tag_test.go index 222ac3e3f6e..5109ce91047 100644 --- a/pkg/workflow/firewall_image_tag_test.go +++ b/pkg/workflow/firewall_image_tag_test.go @@ -120,7 +120,7 @@ func TestClaudeEngineAWFImageTag(t *testing.T) { } }) - t.Run("AWF command includes image-tag with firewall-latest feature override", func(t *testing.T) { + t.Run("AWF command includes image-tag with firewall-version feature override", func(t *testing.T) { featureVersion := "v0.25.27" workflowData := &WorkflowData{ Name: "test-workflow", @@ -128,7 +128,7 @@ func TestClaudeEngineAWFImageTag(t *testing.T) { ID: "claude", }, Features: map[string]any{ - string(constants.FirewallLatestFeatureFlag): featureVersion, + string(constants.FirewallVersionFeatureFlag): featureVersion, }, NetworkPermissions: &NetworkPermissions{ Firewall: &FirewallConfig{ diff --git a/pkg/workflow/firewall_version_pinning_test.go b/pkg/workflow/firewall_version_pinning_test.go index 40621656a55..90893f5a778 100644 --- a/pkg/workflow/firewall_version_pinning_test.go +++ b/pkg/workflow/firewall_version_pinning_test.go @@ -167,7 +167,7 @@ func TestCopilotEngineFirewallInstallation(t *testing.T) { ID: "copilot", }, Features: map[string]any{ - string(constants.FirewallLatestFeatureFlag): featureVersion, + string(constants.FirewallVersionFeatureFlag): featureVersion, }, NetworkPermissions: &NetworkPermissions{ Firewall: &FirewallConfig{ @@ -198,6 +198,43 @@ func TestCopilotEngineFirewallInstallation(t *testing.T) { } }) + t.Run("supports latest feature override version", func(t *testing.T) { + engine := NewCopilotEngine() + workflowData := &WorkflowData{ + Name: "test-workflow", + EngineConfig: &EngineConfig{ + ID: "copilot", + }, + Features: map[string]any{ + string(constants.FirewallVersionFeatureFlag): "latest", + }, + NetworkPermissions: &NetworkPermissions{ + Firewall: &FirewallConfig{ + Enabled: true, + }, + }, + } + + steps := engine.GetInstallationSteps(workflowData) + + var awfStepStr string + for _, step := range steps { + stepStr := strings.Join(step, "\n") + if strings.Contains(stepStr, "Install AWF binary") { + awfStepStr = stepStr + break + } + } + + if awfStepStr == "" { + t.Fatal("Expected to find AWF installation step when firewall is enabled") + } + + if !strings.Contains(awfStepStr, `install_awf_binary.sh" latest`) { + t.Errorf("AWF installation step should pass latest to script when firewall-version=latest, got:\n%s", awfStepStr) + } + }) + t.Run("does not include AWF installation when firewall disabled", func(t *testing.T) { engine := NewCopilotEngine() workflowData := &WorkflowData{ diff --git a/pkg/workflow/gemini_engine.go b/pkg/workflow/gemini_engine.go index 05d9d5b676b..2324c77edf2 100644 --- a/pkg/workflow/gemini_engine.go +++ b/pkg/workflow/gemini_engine.go @@ -91,6 +91,7 @@ func (e *GeminiEngine) GetInstallationSteps(workflowData *WorkflowData) []GitHub string(constants.DefaultGeminiVersion), "Install Gemini CLI", "gemini", + constants.GeminiVersionFeatureFlag, workflowData, ) return BuildNpmEngineInstallStepsWithAWF(npmSteps, workflowData) diff --git a/pkg/workflow/gemini_engine_test.go b/pkg/workflow/gemini_engine_test.go index 454722b2223..3653a1e891c 100644 --- a/pkg/workflow/gemini_engine_test.go +++ b/pkg/workflow/gemini_engine_test.go @@ -6,6 +6,7 @@ import ( "strings" "testing" + "github.com/github/gh-aw/pkg/constants" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -111,6 +112,20 @@ func TestGeminiEngineInstallation(t *testing.T) { assert.Empty(t, steps, "Should skip installation when custom command is specified") }) + t.Run("feature version override supports latest", func(t *testing.T) { + workflowData := &WorkflowData{ + Name: "test-workflow", + Features: map[string]any{ + string(constants.GeminiVersionFeatureFlag): "latest", + }, + } + + steps := engine.GetInstallationSteps(workflowData) + require.GreaterOrEqual(t, len(steps), 2, "Should have install step") + stepContent := strings.Join(steps[1], "\n") + assert.Contains(t, stepContent, "@google/gemini-cli@latest", "Should install latest Gemini CLI when gemini-version is latest") + }) + t.Run("with firewall", func(t *testing.T) { workflowData := &WorkflowData{ Name: "test-workflow", diff --git a/pkg/workflow/mcp_gateway_config.go b/pkg/workflow/mcp_gateway_config.go index 9250347e525..90a4f6f79da 100644 --- a/pkg/workflow/mcp_gateway_config.go +++ b/pkg/workflow/mcp_gateway_config.go @@ -56,7 +56,7 @@ func getMCPGatewayVersion(workflowData *WorkflowData, configuredVersion string) if configuredVersion != "" { return configuredVersion } - if featureVersion := getFeatureValue(constants.MCPGLatestFeatureFlag, workflowData); featureVersion != "" { + if featureVersion := getFeatureValue(constants.MCPGVersionFeatureFlag, workflowData); featureVersion != "" { return featureVersion } return string(constants.DefaultMCPGatewayVersion) diff --git a/pkg/workflow/mcp_gateway_config_test.go b/pkg/workflow/mcp_gateway_config_test.go index 24df74f0cfc..94510f0d3e0 100644 --- a/pkg/workflow/mcp_gateway_config_test.go +++ b/pkg/workflow/mcp_gateway_config_test.go @@ -69,10 +69,10 @@ func TestEnsureDefaultMCPGatewayConfig(t *testing.T) { }, }, { - name: "fills in missing version field from mcpg-latest feature override", + name: "fills in missing version field from mcpg-version feature override", workflowData: &WorkflowData{ Features: map[string]any{ - string(constants.MCPGLatestFeatureFlag): "v0.2.27", + string(constants.MCPGVersionFeatureFlag): "v0.2.27", }, SandboxConfig: &SandboxConfig{ MCP: &MCPGatewayRuntimeConfig{ @@ -88,10 +88,10 @@ func TestEnsureDefaultMCPGatewayConfig(t *testing.T) { }, }, { - name: "preserves explicit version over mcpg-latest feature override", + name: "preserves explicit version over mcpg-version feature override", workflowData: &WorkflowData{ Features: map[string]any{ - string(constants.MCPGLatestFeatureFlag): "v0.2.27", + string(constants.MCPGVersionFeatureFlag): "v0.2.27", }, SandboxConfig: &SandboxConfig{ MCP: &MCPGatewayRuntimeConfig{ diff --git a/pkg/workflow/mcp_setup_generator_test.go b/pkg/workflow/mcp_setup_generator_test.go index 2591579f7a9..8910ac1c15b 100644 --- a/pkg/workflow/mcp_setup_generator_test.go +++ b/pkg/workflow/mcp_setup_generator_test.go @@ -182,7 +182,7 @@ func TestMCPGatewayVersionFromFrontmatter(t *testing.T) { description: "should use custom version 1.2.3", }, { - name: "mcpg-latest feature override applies when sandbox.mcp.version is not set", + name: "mcpg-version feature override applies when sandbox.mcp.version is not set", sandboxConfig: &SandboxConfig{ MCP: &MCPGatewayRuntimeConfig{ Container: constants.DefaultMCPGatewayContainer, @@ -190,10 +190,10 @@ func TestMCPGatewayVersionFromFrontmatter(t *testing.T) { }, }, features: map[string]any{ - string(constants.MCPGLatestFeatureFlag): "v0.2.27", + string(constants.MCPGVersionFeatureFlag): "v0.2.27", }, expectedVersion: "v0.2.27", - description: "should use mcpg-latest feature override when no explicit sandbox.mcp.version is set", + description: "should use mcpg-version feature override when no explicit sandbox.mcp.version is set", }, } diff --git a/pkg/workflow/opencode_engine.go b/pkg/workflow/opencode_engine.go index 879b59cd892..ee3beda00e0 100644 --- a/pkg/workflow/opencode_engine.go +++ b/pkg/workflow/opencode_engine.go @@ -96,6 +96,7 @@ func (e *OpenCodeEngine) GetInstallationSteps(workflowData *WorkflowData) []GitH string(constants.DefaultOpenCodeVersion), "Install OpenCode CLI", "opencode", + constants.OpenCodeVersionFeatureFlag, workflowData, ) return BuildNpmEngineInstallStepsWithAWF(npmSteps, workflowData) diff --git a/pkg/workflow/opencode_engine_test.go b/pkg/workflow/opencode_engine_test.go index 4938b21d074..1004b881012 100644 --- a/pkg/workflow/opencode_engine_test.go +++ b/pkg/workflow/opencode_engine_test.go @@ -39,6 +39,18 @@ func TestOpenCodeEngineInstallationAndExecution(t *testing.T) { assert.Contains(t, stepContent, "Setup Node.js", "Should include Node setup") }) + t.Run("feature version override supports latest", func(t *testing.T) { + steps := engine.GetInstallationSteps(&WorkflowData{ + Name: "test-workflow", + Features: map[string]any{ + string(constants.OpenCodeVersionFeatureFlag): "latest", + }, + }) + require.GreaterOrEqual(t, len(steps), 2, "Should generate install step") + stepContent := strings.Join(steps[1], "\n") + assert.Contains(t, stepContent, "opencode-ai@latest", "Should install latest OpenCode CLI when opencode-version is latest") + }) + t.Run("execution uses opencode command and config", func(t *testing.T) { steps := engine.GetExecutionSteps(&WorkflowData{Name: "test-workflow"}, "/tmp/test.log") require.Len(t, steps, 2, "Should generate config step and execution step") From d4ec25d3e5ee2cc7c8545ee21d1ec033f09cce60 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 15:35:27 +0000 Subject: [PATCH 5/9] refactor: use explicit NoFeatureFlag sentinel Agent-Logs-Url: https://github.com/github/gh-aw/sessions/a77776fd-69ce-451f-a672-9e755656680e Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --- pkg/constants/feature_constants.go | 2 ++ pkg/workflow/crush_engine.go | 2 +- pkg/workflow/engine_helpers.go | 2 +- pkg/workflow/engine_helpers_test.go | 6 +++--- 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/pkg/constants/feature_constants.go b/pkg/constants/feature_constants.go index f5dc2540921..4e303a4416c 100644 --- a/pkg/constants/feature_constants.go +++ b/pkg/constants/feature_constants.go @@ -12,6 +12,8 @@ type FeatureFlag string // Feature flag identifiers const ( + // NoFeatureFlag indicates that no feature-flag-based override should be applied. + NoFeatureFlag FeatureFlag = "" // MCPScriptsFeatureFlag is the name of the feature flag for mcp-scripts MCPScriptsFeatureFlag FeatureFlag = "mcp-scripts" // MCPGatewayFeatureFlag is the feature flag name for enabling MCP gateway diff --git a/pkg/workflow/crush_engine.go b/pkg/workflow/crush_engine.go index 540a66cf81e..4e524c18c7f 100644 --- a/pkg/workflow/crush_engine.go +++ b/pkg/workflow/crush_engine.go @@ -103,7 +103,7 @@ func (e *CrushEngine) GetInstallationSteps(workflowData *WorkflowData) []GitHubA string(constants.DefaultCrushVersion), "Install Crush CLI", "crush", - "", + constants.NoFeatureFlag, workflowData, ) return BuildNpmEngineInstallStepsWithAWF(npmSteps, workflowData) diff --git a/pkg/workflow/engine_helpers.go b/pkg/workflow/engine_helpers.go index e29c30da77f..4158f7f8d06 100644 --- a/pkg/workflow/engine_helpers.go +++ b/pkg/workflow/engine_helpers.go @@ -108,7 +108,7 @@ func GetBaseInstallationSteps(config EngineInstallConfig, workflowData *Workflow config.Version, stepName, config.CliName, - "", + constants.NoFeatureFlag, workflowData, ) steps = append(steps, npmSteps...) diff --git a/pkg/workflow/engine_helpers_test.go b/pkg/workflow/engine_helpers_test.go index 6de68f4ae8a..c3ea0880965 100644 --- a/pkg/workflow/engine_helpers_test.go +++ b/pkg/workflow/engine_helpers_test.go @@ -25,7 +25,7 @@ func TestBuildStandardNpmEngineInstallSteps(t *testing.T) { { name: "with default version", workflowData: &WorkflowData{}, - featureFlag: "", + featureFlag: constants.NoFeatureFlag, expectedSteps: 2, // Node.js setup + npm install expectedInStep: string(constants.DefaultCopilotVersion), }, @@ -47,7 +47,7 @@ func TestBuildStandardNpmEngineInstallSteps(t *testing.T) { Version: "", }, }, - featureFlag: "", + featureFlag: constants.NoFeatureFlag, expectedSteps: 2, expectedInStep: string(constants.DefaultCopilotVersion), }, @@ -137,7 +137,7 @@ func TestBuildStandardNpmEngineInstallSteps_AllEngines(t *testing.T) { tt.defaultVersion, tt.stepName, tt.cacheKeyPrefix, - "", + constants.NoFeatureFlag, workflowData, ) From c3c759d574f83404fd82bccfbe0ab7e9c403388d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 15:37:19 +0000 Subject: [PATCH 6/9] refactor: centralize engine version feature resolution Agent-Logs-Url: https://github.com/github/gh-aw/sessions/a77776fd-69ce-451f-a672-9e755656680e Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --- pkg/workflow/copilot_engine_installation.go | 15 +++----- pkg/workflow/engine_helpers.go | 40 ++++++++++++++------- 2 files changed, 33 insertions(+), 22 deletions(-) diff --git a/pkg/workflow/copilot_engine_installation.go b/pkg/workflow/copilot_engine_installation.go index 061564b1b42..cc01cd73dc4 100644 --- a/pkg/workflow/copilot_engine_installation.go +++ b/pkg/workflow/copilot_engine_installation.go @@ -65,16 +65,11 @@ func (e *CopilotEngine) GetInstallationSteps(workflowData *WorkflowData) []GitHu } // Determine Copilot version - copilotVersion := string(constants.DefaultCopilotVersion) - if workflowData.EngineConfig != nil && workflowData.EngineConfig.Version != "" { - copilotVersion = workflowData.EngineConfig.Version - } - if workflowData.EngineConfig == nil || workflowData.EngineConfig.Version == "" { - if featureVersion := getFeatureValue(constants.CopilotVersionFeatureFlag, workflowData); featureVersion != "" { - copilotVersion = featureVersion - copilotInstallLog.Printf("copilot-version feature override applied: version=%s", featureVersion) - } - } + copilotVersion := resolveEngineInstallVersion( + string(constants.DefaultCopilotVersion), + constants.CopilotVersionFeatureFlag, + workflowData, + ) if isFeatureEnabled(constants.ByokCopilotFeatureFlag, workflowData) { copilotVersion = "latest" copilotInstallLog.Print("byok-copilot enabled: forcing Copilot CLI install version to latest") diff --git a/pkg/workflow/engine_helpers.go b/pkg/workflow/engine_helpers.go index 4158f7f8d06..9e620c314ad 100644 --- a/pkg/workflow/engine_helpers.go +++ b/pkg/workflow/engine_helpers.go @@ -138,18 +138,7 @@ func BuildStandardNpmEngineInstallSteps( ) []GitHubActionStep { engineHelpersLog.Printf("Building npm engine install steps: package=%s, version=%s", packageName, defaultVersion) - // Use version from engine config if provided, otherwise default to pinned version - version := defaultVersion - engineVersionSet := workflowData.EngineConfig != nil && workflowData.EngineConfig.Version != "" - if engineVersionSet { - version = workflowData.EngineConfig.Version - engineHelpersLog.Printf("Using engine config version: %s", version) - } else if featureVersionFlag != "" { - if featureVersion := getFeatureValue(featureVersionFlag, workflowData); featureVersion != "" { - version = featureVersion - engineHelpersLog.Printf("Using feature override version (%s): %s", featureVersionFlag, version) - } - } + version := resolveEngineInstallVersion(defaultVersion, featureVersionFlag, workflowData) // Add npm package installation steps (includes Node.js setup) // Always pass false for runInstallScripts: engine CLI installs must never run @@ -165,6 +154,33 @@ func BuildStandardNpmEngineInstallSteps( ) } +// resolveEngineInstallVersion resolves engine CLI install version with this precedence: +// 1. explicit engine.version in frontmatter +// 2. feature flag version override (if provided) +// 3. default pinned version +func resolveEngineInstallVersion( + defaultVersion string, + featureVersionFlag constants.FeatureFlag, + workflowData *WorkflowData, +) string { + version := defaultVersion + hasExplicitEngineVersion := workflowData.EngineConfig != nil && workflowData.EngineConfig.Version != "" + if hasExplicitEngineVersion { + version = workflowData.EngineConfig.Version + engineHelpersLog.Printf("Using engine config version: %s", version) + return version + } + + if featureVersionFlag != constants.NoFeatureFlag { + if featureVersion := getFeatureValue(featureVersionFlag, workflowData); featureVersion != "" { + version = featureVersion + engineHelpersLog.Printf("Using feature override version (%s): %s", featureVersionFlag, version) + } + } + + return version +} + // BuildNpmEngineInstallStepsWithAWF injects an AWF installation step between the Node.js // setup step and the CLI install steps when the firewall is enabled. This eliminates the // duplicated AWF-injection pattern shared by Claude, Gemini, and Copilot engines. From 55be821a27d39d2d2cc71e32bf38f41cca1da0f2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 15:39:02 +0000 Subject: [PATCH 7/9] refactor: reuse shared engine version resolver for codex Agent-Logs-Url: https://github.com/github/gh-aw/sessions/a77776fd-69ce-451f-a672-9e755656680e Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --- pkg/workflow/codex_engine.go | 11 +++++------ pkg/workflow/engine_helpers.go | 12 +++++------- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/pkg/workflow/codex_engine.go b/pkg/workflow/codex_engine.go index 0f2c6c34d54..dd3ff85acb8 100644 --- a/pkg/workflow/codex_engine.go +++ b/pkg/workflow/codex_engine.go @@ -80,12 +80,11 @@ func (e *CodexEngine) GetInstallationSteps(workflowData *WorkflowData) []GitHubA } // Use base installation steps (npm install only; secret validation is in the activation job) - codexVersion := string(constants.DefaultCodexVersion) - if workflowData.EngineConfig != nil && workflowData.EngineConfig.Version != "" { - codexVersion = workflowData.EngineConfig.Version - } else if featureVersion := getFeatureValue(constants.CodexVersionFeatureFlag, workflowData); featureVersion != "" { - codexVersion = featureVersion - } + codexVersion := resolveEngineInstallVersion( + string(constants.DefaultCodexVersion), + constants.CodexVersionFeatureFlag, + workflowData, + ) steps := GetBaseInstallationSteps(EngineInstallConfig{ Secrets: []string{"CODEX_API_KEY", "OPENAI_API_KEY"}, diff --git a/pkg/workflow/engine_helpers.go b/pkg/workflow/engine_helpers.go index 9e620c314ad..49b91d0a062 100644 --- a/pkg/workflow/engine_helpers.go +++ b/pkg/workflow/engine_helpers.go @@ -163,22 +163,20 @@ func resolveEngineInstallVersion( featureVersionFlag constants.FeatureFlag, workflowData *WorkflowData, ) string { - version := defaultVersion hasExplicitEngineVersion := workflowData.EngineConfig != nil && workflowData.EngineConfig.Version != "" if hasExplicitEngineVersion { - version = workflowData.EngineConfig.Version - engineHelpersLog.Printf("Using engine config version: %s", version) - return version + engineHelpersLog.Printf("Using engine config version: %s", workflowData.EngineConfig.Version) + return workflowData.EngineConfig.Version } if featureVersionFlag != constants.NoFeatureFlag { if featureVersion := getFeatureValue(featureVersionFlag, workflowData); featureVersion != "" { - version = featureVersion - engineHelpersLog.Printf("Using feature override version (%s): %s", featureVersionFlag, version) + engineHelpersLog.Printf("Using feature override version (%s): %s", featureVersionFlag, featureVersion) + return featureVersion } } - return version + return defaultVersion } // BuildNpmEngineInstallStepsWithAWF injects an AWF installation step between the Node.js From 15da9eb4fe1eca0013d94c6ac74965878bf93320 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 15:40:28 +0000 Subject: [PATCH 8/9] test: cover engine version precedence in helper Agent-Logs-Url: https://github.com/github/gh-aw/sessions/a77776fd-69ce-451f-a672-9e755656680e Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --- pkg/workflow/engine_helpers.go | 4 ++++ pkg/workflow/engine_helpers_test.go | 14 ++++++++++++++ 2 files changed, 18 insertions(+) diff --git a/pkg/workflow/engine_helpers.go b/pkg/workflow/engine_helpers.go index 49b91d0a062..83b95cfcf89 100644 --- a/pkg/workflow/engine_helpers.go +++ b/pkg/workflow/engine_helpers.go @@ -158,6 +158,10 @@ func BuildStandardNpmEngineInstallSteps( // 1. explicit engine.version in frontmatter // 2. feature flag version override (if provided) // 3. default pinned version +// +// Note: some engines may apply additional post-resolution overrides. +// For example, Copilot's byok-copilot feature forces "latest" after this +// resolver returns. func resolveEngineInstallVersion( defaultVersion string, featureVersionFlag constants.FeatureFlag, diff --git a/pkg/workflow/engine_helpers_test.go b/pkg/workflow/engine_helpers_test.go index c3ea0880965..fee7078ef31 100644 --- a/pkg/workflow/engine_helpers_test.go +++ b/pkg/workflow/engine_helpers_test.go @@ -62,6 +62,20 @@ func TestBuildStandardNpmEngineInstallSteps(t *testing.T) { expectedSteps: 2, expectedInStep: "latest", }, + { + name: "engine config version takes precedence over feature override", + workflowData: &WorkflowData{ + EngineConfig: &EngineConfig{ + Version: "1.2.3", + }, + Features: map[string]any{ + string(constants.CopilotVersionFeatureFlag): "latest", + }, + }, + featureFlag: constants.CopilotVersionFeatureFlag, + expectedSteps: 2, + expectedInStep: "1.2.3", + }, } for _, tt := range tests { From bfb80dc91b9961e84523f942532881e4f5da5484 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 15:42:51 +0000 Subject: [PATCH 9/9] docs: clarify copilot-only post-version override semantics Agent-Logs-Url: https://github.com/github/gh-aw/sessions/a77776fd-69ce-451f-a672-9e755656680e Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --- pkg/workflow/engine_helpers.go | 6 +++--- pkg/workflow/engine_helpers_test.go | 12 ++++++++++++ 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/pkg/workflow/engine_helpers.go b/pkg/workflow/engine_helpers.go index 83b95cfcf89..2610fa11d63 100644 --- a/pkg/workflow/engine_helpers.go +++ b/pkg/workflow/engine_helpers.go @@ -159,9 +159,9 @@ func BuildStandardNpmEngineInstallSteps( // 2. feature flag version override (if provided) // 3. default pinned version // -// Note: some engines may apply additional post-resolution overrides. -// For example, Copilot's byok-copilot feature forces "latest" after this -// resolver returns. +// Note: Copilot currently applies an additional post-resolution override: +// byok-copilot forces "latest" after this resolver returns. Other engines +// should not add post-resolution overrides unless explicitly designed to. func resolveEngineInstallVersion( defaultVersion string, featureVersionFlag constants.FeatureFlag, diff --git a/pkg/workflow/engine_helpers_test.go b/pkg/workflow/engine_helpers_test.go index fee7078ef31..8bb5fec79e1 100644 --- a/pkg/workflow/engine_helpers_test.go +++ b/pkg/workflow/engine_helpers_test.go @@ -21,6 +21,7 @@ func TestBuildStandardNpmEngineInstallSteps(t *testing.T) { featureFlag constants.FeatureFlag expectedSteps int // Number of steps expected (Node.js setup + npm install) expectedInStep string + unexpectedInStep string }{ { name: "with default version", @@ -75,6 +76,7 @@ func TestBuildStandardNpmEngineInstallSteps(t *testing.T) { featureFlag: constants.CopilotVersionFeatureFlag, expectedSteps: 2, expectedInStep: "1.2.3", + unexpectedInStep: "latest", }, } @@ -107,6 +109,16 @@ func TestBuildStandardNpmEngineInstallSteps(t *testing.T) { if !found { t.Errorf("Expected version %s not found in steps", tt.expectedInStep) } + + if tt.unexpectedInStep != "" { + for _, step := range steps { + for _, line := range step { + if strings.Contains(line, tt.unexpectedInStep) { + t.Errorf("Did not expect to find %q in steps when explicit version takes precedence", tt.unexpectedInStep) + } + } + } + } }) } }