diff --git a/.github/workflows/smoke-project.lock.yml b/.github/workflows/smoke-project.lock.yml index 44ca7d8ef83..907933c4ee5 100644 --- a/.github/workflows/smoke-project.lock.yml +++ b/.github/workflows/smoke-project.lock.yml @@ -1193,7 +1193,7 @@ jobs: permissions: contents: write discussions: write - issues: read + issues: write pull-requests: write concurrency: group: "gh-aw-conclusion-smoke-project" @@ -1604,7 +1604,7 @@ jobs: permissions: contents: write discussions: write - issues: read + issues: write pull-requests: write timeout-minutes: 15 env: diff --git a/pkg/workflow/action_sha_checker.go b/pkg/workflow/action_sha_checker.go index 2c7dacde838..57a3ea3c3dc 100644 --- a/pkg/workflow/action_sha_checker.go +++ b/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 -} diff --git a/pkg/workflow/engine_helpers.go b/pkg/workflow/engine_helpers.go index 77880880456..3fc7c9fc19e 100644 --- a/pkg/workflow/engine_helpers.go +++ b/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 -} diff --git a/pkg/workflow/engine_validation.go b/pkg/workflow/engine_validation.go index f327519ce72..1527d0229fa 100644 --- a/pkg/workflow/engine_validation.go +++ b/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 +} diff --git a/pkg/workflow/lock_validation.go b/pkg/workflow/lock_validation.go index c518fe33835..cd4fea43704 100644 --- a/pkg/workflow/lock_validation.go +++ b/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 +} diff --git a/pkg/workflow/model_alias_validation.go b/pkg/workflow/model_alias_validation.go index 4b16e71f5c6..7e6c3ae7638 100644 --- a/pkg/workflow/model_alias_validation.go +++ b/pkg/workflow/model_alias_validation.go @@ -22,8 +22,10 @@ package workflow import ( "fmt" + "math" "os" "sort" + "strconv" "strings" "github.com/github/gh-aw/pkg/console" @@ -31,6 +33,52 @@ import ( 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). diff --git a/pkg/workflow/model_identifier.go b/pkg/workflow/model_identifier.go index 7ecda81210c..f185aa3b4c0 100644 --- a/pkg/workflow/model_identifier.go +++ b/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 diff --git a/pkg/workflow/env.go b/pkg/workflow/yaml_env_helpers.go similarity index 100% rename from pkg/workflow/env.go rename to pkg/workflow/yaml_env_helpers.go