refactor: reorganize misplaced functions per semantic clustering analysis (#30770)

Author: Copilot

pkg/workflow/action_sha_checker.go

@@ -6,7 +6,6 @@ import (
 	"os"
 	"regexp"
 
-	"github.com/github/gh-aw/pkg/console"
 	"github.com/github/gh-aw/pkg/logger"
 	"github.com/goccy/go-yaml"
 )
@@ -144,68 +143,3 @@ func CheckActionSHAUpdates(actions []ActionUsage, resolver *ActionResolver) []Ac
 
 	return results
 }
-
-// ValidateActionSHAsInLockFile validates action SHAs in a lock file and emits warnings
-func ValidateActionSHAsInLockFile(lockFilePath string, cache *ActionCache, verbose bool) error {
-	actionSHACheckerLog.Printf("Validating action SHAs in: %s", lockFilePath)
-
-	// Extract actions from lock file
-	actions, err := ExtractActionsFromLockFile(lockFilePath)
-	if err != nil {
-		return fmt.Errorf("failed to extract actions: %w", err)
-	}
-
-	if len(actions) == 0 {
-		actionSHACheckerLog.Print("No pinned actions found in lock file")
-		if verbose {
-			fmt.Fprintln(os.Stderr, console.FormatInfoMessage("No pinned actions to validate"))
-		}
-		return nil
-	}
-
-	// Create resolver for checking latest SHAs
-	resolver := NewActionResolver(cache)
-
-	// Check for updates
-	checks := CheckActionSHAUpdates(actions, resolver)
-
-	// Count and report updates
-	updateCount := 0
-	for _, check := range checks {
-		if check.NeedsUpdate {
-			updateCount++
-			// Emit warning (FormatWarningMessage adds the warning emoji)
-			warningMsg := fmt.Sprintf("%s@%s has a newer SHA available: %s → %s",
-				check.Action.Repo,
-				check.Action.Version,
-				check.Action.SHA[:7],
-				check.LatestSHA[:7])
-			fmt.Fprintln(os.Stderr, console.FormatWarningMessage(warningMsg))
-
-			// Show full SHA in verbose mode
-			if verbose {
-				fmt.Fprintf(os.Stderr, "    Current: %s\n", check.Action.SHA)
-				fmt.Fprintf(os.Stderr, "    Latest:  %s\n", check.LatestSHA)
-			}
-		}
-	}
-
-	if updateCount > 0 {
-		actionSHACheckerLog.Printf("Found %d actions that need updating", updateCount)
-		// Save the cache with updated SHAs so the next compilation will use them
-		if err := cache.Save(); err != nil {
-			actionSHACheckerLog.Printf("Warning: failed to save action cache: %v", err)
-		} else {
-			actionSHACheckerLog.Print("Saved updated action cache")
-		}
-		// Provide suggestion to fix the issue
-		fmt.Fprintln(os.Stderr, console.FormatInfoMessage("To apply updated action SHAs, recompile with: gh aw compile"))
-		if verbose {
-			fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Found %d action(s) with available updates", updateCount)))
-		}
-	} else {
-		actionSHACheckerLog.Print("All actions are up to date")
-	}
-
-	return nil
-}

pkg/workflow/engine_helpers.go

@@ -360,24 +360,3 @@ func FilterEnvForSecrets(env map[string]string, allowedNamesAndKeys []string) ma
 	engineHelpersLog.Printf("Filtered environment variables: kept=%d, removed=%d", len(filtered), secretsRemoved)
 	return filtered
 }
-
-// EngineHasValidateSecretStep checks if the engine provides a validate-secret step.
-// This is used to determine whether the secret_verification_result job output should be added.
-//
-// The validate-secret step is provided by engines that override GetSecretValidationStep():
-//   - Copilot engine: Adds step unless copilot-requests feature is enabled or custom command is set
-//   - Claude engine: Adds step unless custom command is set
-//   - Codex engine: Adds step unless custom command is set
-//   - Gemini engine: Adds step unless custom command is set
-//   - Custom engine: Never adds this step (uses BaseEngine default which returns empty)
-//
-// Parameters:
-//   - engine: The agentic engine to check
-//   - data: The workflow data (needed for GetSecretValidationStep)
-//
-// Returns:
-//   - bool: true if the engine provides a validate-secret step, false otherwise
-func EngineHasValidateSecretStep(engine CodingAgentEngine, data *WorkflowData) bool {
-	step := engine.GetSecretValidationStep(data)
-	return len(step) > 0
-}

pkg/workflow/engine_validation.go

@@ -399,3 +399,24 @@ func (c *Compiler) validatePiEngineRequirements(workflowData *WorkflowData) erro
 	engineValidationLog.Print("Pi engine requirements satisfied: gh-proxy and cli-proxy are enabled")
 	return nil
 }
+
+// EngineHasValidateSecretStep checks if the engine provides a validate-secret step.
+// This is used to determine whether the secret_verification_result job output should be added.
+//
+// The validate-secret step is provided by engines that override GetSecretValidationStep():
+//   - Copilot engine: Adds step unless copilot-requests feature is enabled or custom command is set
+//   - Claude engine: Adds step unless custom command is set
+//   - Codex engine: Adds step unless custom command is set
+//   - Gemini engine: Adds step unless custom command is set
+//   - Custom engine: Never adds this step (uses BaseEngine default which returns empty)
+//
+// Parameters:
+//   - engine: The agentic engine to check
+//   - data: The workflow data (needed for GetSecretValidationStep)
+//
+// Returns:
+//   - bool: true if the engine provides a validate-secret step, false otherwise
+func EngineHasValidateSecretStep(engine CodingAgentEngine, data *WorkflowData) bool {
+	step := engine.GetSecretValidationStep(data)
+	return len(step) > 0
+}

pkg/workflow/lock_validation.go

@@ -1,26 +1,34 @@
-// This file provides validation for workflow lock file schema compatibility.
+// This file provides validation for workflow lock files.
 //
-// # Lock File Schema Validation
+// # Lock File Validation
 //
-// This file validates that lock files use a schema version that the current
-// build of gh-aw supports. It provides actionable error messages when a
-// lock file was generated by an incompatible version.
+// This file validates lock files at two levels:
+//   - Schema compatibility: ensures the lock file's schema version is supported by the
+//     current build of gh-aw, providing actionable error messages when a lock file
+//     was generated by an incompatible version.
+//   - Action SHA freshness: checks whether pinned GitHub Actions SHAs are up to date
+//     and emits warnings when newer SHAs are available.
 //
 // # Validation Functions
 //
 //   - ValidateLockSchemaCompatibility() - Validates lock file schema version
+//   - ValidateActionSHAsInLockFile()    - Validates action SHAs and emits update warnings
 //
 // # When to Add Validation Here
 //
 // Add validation to this file when:
 //   - Adding new lock file format constraints
 //   - Adding migration validation for schema upgrades
+//   - Adding user-facing orchestration that validates lock file content
 
 package workflow
 
 import (
 	"fmt"
+	"os"
 	"strings"
+
+	"github.com/github/gh-aw/pkg/console"
 )
 
 // ValidateLockSchemaCompatibility validates that a lock file's schema is compatible.
@@ -57,3 +65,68 @@ func ValidateLockSchemaCompatibility(content string, lockFilePath string) error
 	lockSchemaLog.Printf("Lock file schema validated: %s (version=%s)", lockFilePath, metadata.SchemaVersion)
 	return nil
 }
+
+// ValidateActionSHAsInLockFile validates action SHAs in a lock file and emits warnings
+func ValidateActionSHAsInLockFile(lockFilePath string, cache *ActionCache, verbose bool) error {
+	actionSHACheckerLog.Printf("Validating action SHAs in: %s", lockFilePath)
+
+	// Extract actions from lock file
+	actions, err := ExtractActionsFromLockFile(lockFilePath)
+	if err != nil {
+		return fmt.Errorf("failed to extract actions: %w", err)
+	}
+
+	if len(actions) == 0 {
+		actionSHACheckerLog.Print("No pinned actions found in lock file")
+		if verbose {
+			fmt.Fprintln(os.Stderr, console.FormatInfoMessage("No pinned actions to validate"))
+		}
+		return nil
+	}
+
+	// Create resolver for checking latest SHAs
+	resolver := NewActionResolver(cache)
+
+	// Check for updates
+	checks := CheckActionSHAUpdates(actions, resolver)
+
+	// Count and report updates
+	updateCount := 0
+	for _, check := range checks {
+		if check.NeedsUpdate {
+			updateCount++
+			// Emit warning (FormatWarningMessage adds the warning emoji)
+			warningMsg := fmt.Sprintf("%s@%s has a newer SHA available: %s → %s",
+				check.Action.Repo,
+				check.Action.Version,
+				check.Action.SHA[:7],
+				check.LatestSHA[:7])
+			fmt.Fprintln(os.Stderr, console.FormatWarningMessage(warningMsg))
+
+			// Show full SHA in verbose mode
+			if verbose {
+				fmt.Fprintf(os.Stderr, "    Current: %s\n", check.Action.SHA)
+				fmt.Fprintf(os.Stderr, "    Latest:  %s\n", check.LatestSHA)
+			}
+		}
+	}
+
+	if updateCount > 0 {
+		actionSHACheckerLog.Printf("Found %d actions that need updating", updateCount)
+		// Save the cache with updated SHAs so the next compilation will use them
+		if err := cache.Save(); err != nil {
+			actionSHACheckerLog.Printf("Warning: failed to save action cache: %v", err)
+		} else {
+			actionSHACheckerLog.Print("Saved updated action cache")
+		}
+		// Provide suggestion to fix the issue
+		fmt.Fprintln(os.Stderr, console.FormatInfoMessage("To apply updated action SHAs, recompile with: gh aw compile"))
+		if verbose {
+			fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Found %d action(s) with available updates", updateCount)))
+		}
+	} else {
+		actionSHACheckerLog.Print("All actions are up to date")
+	}
+
+	return nil
+}

pkg/workflow/model_alias_validation.go

@@ -22,15 +22,63 @@ package workflow
 
 import (
 	"fmt"
+	"math"
 	"os"
 	"sort"
+	"strconv"
 	"strings"
 
 	"github.com/github/gh-aw/pkg/console"
 )
 
 var modelAliasValidationLog = newValidationLogger("model_alias")
 
+// ─── Known-parameter validation ───────────────────────────────────────────────
+
+// ValidateEffortParam validates the "effort" parameter value (V-MAF-002).
+// Allowed values: low, medium, high.
+func ValidateEffortParam(value string) error {
+	switch value {
+	case "low", "medium", "high":
+		return nil
+	default:
+		return fmt.Errorf("model parameter 'effort': value %q is not valid; allowed values are: low, medium, high (V-MAF-002)", value)
+	}
+}
+
+// ValidateTemperatureParam validates the "temperature" parameter value (V-MAF-003).
+// Must be a finite decimal float in [0.0, 2.0].
+func ValidateTemperatureParam(value string) error {
+	f, err := strconv.ParseFloat(value, 64)
+	if err != nil {
+		return fmt.Errorf("model parameter 'temperature': value %q cannot be parsed as a decimal float (V-MAF-003)", value)
+	}
+	if math.IsNaN(f) || math.IsInf(f, 0) {
+		return fmt.Errorf("model parameter 'temperature': value %q is not a finite number (V-MAF-003)", value)
+	}
+	if f < 0.0 || f > 2.0 {
+		return fmt.Errorf("model parameter 'temperature': value %q is out of range; must be in [0.0, 2.0] (V-MAF-003)", value)
+	}
+	return nil
+}
+
+// ValidateKnownParams validates the known parameters in a parsed identifier.
+// Unknown parameters are tolerated (V-MAF-011 emits a warning, not an error).
+// Returns an error if a known parameter has an invalid value.
+func ValidateKnownParams(params map[string]string) error {
+	if v, ok := params[modelParamEffort]; ok {
+		if err := ValidateEffortParam(v); err != nil {
+			return err
+		}
+	}
+	if v, ok := params[modelParamTemperature]; ok {
+		if err := ValidateTemperatureParam(v); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 // validateModelAliasMap is the main entry point for compile-time model-alias validation.
 // It validates:
 //   - The user-supplied alias map entries in frontmatterModels (V-MAF-001..006, V-MAF-011).

pkg/workflow/model_identifier.go

@@ -47,9 +47,7 @@ package workflow
 import (
 	"errors"
 	"fmt"
-	"math"
 	"regexp"
-	"strconv"
 	"strings"
 
 	"github.com/github/gh-aw/pkg/logger"
@@ -354,52 +352,6 @@ func validateParamValue(value string) error {
 	return nil
 }
 
-// ─── Known-parameter validation ───────────────────────────────────────────────
-
-// ValidateEffortParam validates the "effort" parameter value (V-MAF-002).
-// Allowed values: low, medium, high.
-func ValidateEffortParam(value string) error {
-	switch value {
-	case "low", "medium", "high":
-		return nil
-	default:
-		return fmt.Errorf("model parameter 'effort': value %q is not valid; allowed values are: low, medium, high (V-MAF-002)", value)
-	}
-}
-
-// ValidateTemperatureParam validates the "temperature" parameter value (V-MAF-003).
-// Must be a finite decimal float in [0.0, 2.0].
-func ValidateTemperatureParam(value string) error {
-	f, err := strconv.ParseFloat(value, 64)
-	if err != nil {
-		return fmt.Errorf("model parameter 'temperature': value %q cannot be parsed as a decimal float (V-MAF-003)", value)
-	}
-	if math.IsNaN(f) || math.IsInf(f, 0) {
-		return fmt.Errorf("model parameter 'temperature': value %q is not a finite number (V-MAF-003)", value)
-	}
-	if f < 0.0 || f > 2.0 {
-		return fmt.Errorf("model parameter 'temperature': value %q is out of range; must be in [0.0, 2.0] (V-MAF-003)", value)
-	}
-	return nil
-}
-
-// ValidateKnownParams validates the known parameters in a parsed identifier.
-// Unknown parameters are tolerated (V-MAF-011 emits a warning, not an error).
-// Returns an error if a known parameter has an invalid value.
-func ValidateKnownParams(params map[string]string) error {
-	if v, ok := params[modelParamEffort]; ok {
-		if err := ValidateEffortParam(v); err != nil {
-			return err
-		}
-	}
-	if v, ok := params[modelParamTemperature]; ok {
-		if err := ValidateTemperatureParam(v); err != nil {
-			return err
-		}
-	}
-	return nil
-}
-
 // UnrecognizedParams returns the list of parameter keys in params that are not
 // defined in Section 6 (i.e., not effort or temperature).
 // Reserved future parameters (Section 6.3) are returned here as well since they