fix: address review comments in PR #25948 integrity-reactions feature

- Move validateIntegrityReactions after MergeFeatures in both
  compiler_string_api.go and compiler_orchestrator_workflow.go so
  imported feature flags are visible during validation
- Add semverutil.IsValid() guard in mcpgSupportsIntegrityReactions
  to safely handle non-semver strings (branch names, etc.)
- Extend min-integrity requirement to all 4 integrity-reactions fields
- Fix error message to show correct YAML multi-line format
- Update tests to reflect new behavior

Agent-Logs-Url: https://github.com/github/gh-aw/sessions/c9ae237b-f229-4918-98e7-7d057c6618ca

Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>

Author: Copilot

pkg/workflow/compiler_orchestrator_workflow.go

@@ -112,15 +112,6 @@ func (c *Compiler) ParseWorkflowFile(markdownPath string) (*WorkflowData, error)
 		return nil, fmt.Errorf("%s: %w", cleanPath, err)
 	}
 
-	// Validate integrity-reactions feature configuration
-	var gatewayConfig *MCPGatewayRuntimeConfig
-	if workflowData.SandboxConfig != nil {
-		gatewayConfig = workflowData.SandboxConfig.MCP
-	}
-	if err := validateIntegrityReactions(workflowData.ParsedTools, workflowData.Name, workflowData, gatewayConfig); err != nil {
-		return nil, fmt.Errorf("%s: %w", cleanPath, err)
-	}
-
 	// Use shared action cache and resolver from the compiler
 	actionCache, actionResolver := c.getSharedActionResolver()
 	workflowData.ActionCache = actionCache
@@ -155,6 +146,15 @@ func (c *Compiler) ParseWorkflowFile(markdownPath string) (*WorkflowData, error)
 		workflowData.Features = mergedFeatures
 	}
 
+	// Validate integrity-reactions feature configuration (after features are merged so imports are visible)
+	var gatewayConfig *MCPGatewayRuntimeConfig
+	if workflowData.SandboxConfig != nil {
+		gatewayConfig = workflowData.SandboxConfig.MCP
+	}
+	if err := validateIntegrityReactions(workflowData.ParsedTools, workflowData.Name, workflowData, gatewayConfig); err != nil {
+		return nil, fmt.Errorf("%s: %w", cleanPath, err)
+	}
+
 	// Process and merge custom steps with imported steps
 	c.processAndMergeSteps(result.Frontmatter, workflowData, engineSetup.importsResult)
 

pkg/workflow/compiler_string_api.go

@@ -150,15 +150,6 @@ func (c *Compiler) ParseWorkflowString(content string, virtualPath string) (*Wor
 		return nil, fmt.Errorf("%s: %w", cleanPath, err)
 	}
 
-	// Validate integrity-reactions feature configuration
-	var gatewayConfig *MCPGatewayRuntimeConfig
-	if workflowData.SandboxConfig != nil {
-		gatewayConfig = workflowData.SandboxConfig.MCP
-	}
-	if err := validateIntegrityReactions(workflowData.ParsedTools, workflowData.Name, workflowData, gatewayConfig); err != nil {
-		return nil, fmt.Errorf("%s: %w", cleanPath, err)
-	}
-
 	// Setup action cache and resolver
 	actionCache, actionResolver := c.getSharedActionResolver()
 	workflowData.ActionCache = actionCache
@@ -178,6 +169,15 @@ func (c *Compiler) ParseWorkflowString(content string, virtualPath string) (*Wor
 		workflowData.Features = mergedFeatures
 	}
 
+	// Validate integrity-reactions feature configuration (after features are merged so imports are visible)
+	var gatewayConfig *MCPGatewayRuntimeConfig
+	if workflowData.SandboxConfig != nil {
+		gatewayConfig = workflowData.SandboxConfig.MCP
+	}
+	if err := validateIntegrityReactions(workflowData.ParsedTools, workflowData.Name, workflowData, gatewayConfig); err != nil {
+		return nil, fmt.Errorf("%s: %w", cleanPath, err)
+	}
+
 	// Process and merge custom steps
 	c.processAndMergeSteps(parseResult.frontmatterResult.Frontmatter, workflowData, engineSetup.importsResult)
 

pkg/workflow/mcp_github_config.go

@@ -341,6 +341,11 @@ func mcpgSupportsIntegrityReactions(gatewayConfig *MCPGatewayRuntimeConfig) bool
 		return true
 	}
 
+	if !semverutil.IsValid(version) {
+		// Branch names and non-semver tags are treated conservatively as not supporting the feature.
+		return false
+	}
+
 	minVersion := string(constants.MCPGIntegrityReactionsMinVersion)
 	return semverutil.Compare(version, minVersion) >= 0
 }

pkg/workflow/tools_validation.go

@@ -201,7 +201,7 @@ func validateIntegrityReactions(tools *Tools, workflowName string, data *Workflo
 	// Reaction fields require the integrity-reactions feature flag
 	if !isFeatureEnabled(constants.IntegrityReactionsFeatureFlag, data) {
 		toolsValidationLog.Printf("Reaction fields present but integrity-reactions feature flag not enabled in workflow: %s", workflowName)
-		return errors.New("invalid guard policy: 'endorsement-reactions', 'disapproval-reactions', 'disapproval-integrity', and 'endorser-min-integrity' require the 'integrity-reactions' feature flag to be enabled. Add 'features: integrity-reactions: true' to your workflow")
+		return errors.New("invalid guard policy: 'tools.github.endorsement-reactions', 'tools.github.disapproval-reactions', 'tools.github.disapproval-integrity', and 'tools.github.endorser-min-integrity' require the 'integrity-reactions' feature flag to be enabled. Add the following to your workflow frontmatter:\nfeatures:\n  integrity-reactions: true")
 	}
 
 	// Reaction fields require MCPG >= v0.2.18
@@ -215,10 +215,10 @@ func validateIntegrityReactions(tools *Tools, workflowName string, data *Workflo
 			constants.MCPGIntegrityReactionsMinVersion, version)
 	}
 
-	// Reaction fields require min-integrity to be set
-	if (hasEndorsementReactions || hasDisapprovalReactions) && github.MinIntegrity == "" {
-		toolsValidationLog.Printf("endorsement-reactions/disapproval-reactions without min-integrity in workflow: %s", workflowName)
-		return errors.New("invalid guard policy: 'endorsement-reactions' and 'disapproval-reactions' require 'github.min-integrity' to be set")
+	// Integrity reaction fields require min-integrity to be set so the allow-only guard policy is rendered
+	if (hasEndorsementReactions || hasDisapprovalReactions || hasDisapprovalIntegrity || hasEndorserMinIntegrity) && github.MinIntegrity == "" {
+		toolsValidationLog.Printf("integrity reaction fields without min-integrity in workflow: %s", workflowName)
+		return errors.New("invalid guard policy: 'endorsement-reactions', 'disapproval-reactions', 'disapproval-integrity', and 'endorser-min-integrity' require 'github.min-integrity' to be set")
 	}
 
 	// Validate endorsement-reactions values

pkg/workflow/tools_validation_test.go

@@ -695,6 +695,22 @@ func TestMCPGSupportsIntegrityReactions(t *testing.T) {
 			},
 			want: true,
 		},
+		{
+			name: "non-semver branch name returns false",
+			gatewayConfig: &MCPGatewayRuntimeConfig{
+				Container: "ghcr.io/test/mcpg",
+				Version:   "main",
+			},
+			want: false,
+		},
+		{
+			name: "non-semver tag returns false",
+			gatewayConfig: &MCPGatewayRuntimeConfig{
+				Container: "ghcr.io/test/mcpg",
+				Version:   "feature-branch",
+			},
+			want: false,
+		},
 	}
 
 	for _, tt := range tests {
@@ -878,15 +894,16 @@ func TestValidateIntegrityReactions(t *testing.T) {
 			errorContains: "none",
 		},
 		{
-			name: "only disapproval-integrity (no reaction arrays) without min-integrity is OK",
+			name: "only disapproval-integrity (no reaction arrays) without min-integrity fails",
 			tools: &Tools{
 				GitHub: &GitHubToolConfig{
 					DisapprovalIntegrity: "none",
 				},
 			},
 			data:          makeDataWithFeature(true),
 			gatewayConfig: newGatewayConfig,
-			shouldError:   false, // disapproval-integrity alone doesn't require min-integrity
+			shouldError:   true,
+			errorContains: "min-integrity",
 		},
 	}