From 9a578217e681d95e3326e8b9fc20045e7bcba41e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 7 May 2026 05:20:27 +0000 Subject: [PATCH 1/4] Initial plan From 1e508d3d42cd5d774dea91072e6da5194b27ed98 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 7 May 2026 05:26:31 +0000 Subject: [PATCH 2/4] chore: initial plan for semantic function clustering refactor Agent-Logs-Url: https://github.com/github/gh-aw/sessions/f4a60ad6-6141-4cf9-a3d8-663f664bfe03 Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- .github/workflows/smoke-project.lock.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/smoke-project.lock.yml b/.github/workflows/smoke-project.lock.yml index 44ca7d8ef8..907933c4ee 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: From 7546e5f63e510ca4f8f77311289dccab0402e088 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 7 May 2026 05:30:35 +0000 Subject: [PATCH 3/4] refactor: semantic function clustering - rename and reorganize misplaced functions - Rename pkg/workflow/env.go to yaml_env_helpers.go (file contains YAML utilities, not env config) - Move ValidateEffortParam, ValidateTemperatureParam, ValidateKnownParams from model_identifier.go to model_alias_validation.go (business-rule validators belong with alias validation) - Move ValidateActionSHAsInLockFile from action_sha_checker.go to lock_validation.go (user-facing validation orchestrator belongs with lock validation) - Move EngineHasValidateSecretStep from engine_helpers.go to engine_validation.go (validation predicate belongs with engine validators) - Update imports accordingly in affected files Agent-Logs-Url: https://github.com/github/gh-aw/sessions/f4a60ad6-6141-4cf9-a3d8-663f664bfe03 Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- pkg/workflow/action_sha_checker.go | 66 ------------------- pkg/workflow/engine_helpers.go | 21 ------ pkg/workflow/engine_validation.go | 21 ++++++ pkg/workflow/lock_validation.go | 68 ++++++++++++++++++++ pkg/workflow/model_alias_validation.go | 48 ++++++++++++++ pkg/workflow/model_identifier.go | 48 -------------- pkg/workflow/{env.go => yaml_env_helpers.go} | 0 7 files changed, 137 insertions(+), 135 deletions(-) rename pkg/workflow/{env.go => yaml_env_helpers.go} (100%) diff --git a/pkg/workflow/action_sha_checker.go b/pkg/workflow/action_sha_checker.go index 2c7dacde83..57a3ea3c3d 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 7788088045..3fc7c9fc19 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 f327519ce7..1527d0229f 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 c518fe3383..aca0139ec2 100644 --- a/pkg/workflow/lock_validation.go +++ b/pkg/workflow/lock_validation.go @@ -20,7 +20,10 @@ 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 +60,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 4b16e71f5c..7e6c3ae763 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 7ecda81210..f185aa3b4c 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 From cf356bbc4fa21622c04b9357269c9c4b4ca41fe8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 7 May 2026 14:40:36 +0000 Subject: [PATCH 4/4] docs: update lock_validation.go header to document ValidateActionSHAsInLockFile Agent-Logs-Url: https://github.com/github/gh-aw/sessions/a32cb6f4-f5ef-4aa9-a776-19120b7b8b20 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/lock_validation.go | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/pkg/workflow/lock_validation.go b/pkg/workflow/lock_validation.go index aca0139ec2..cd4fea4370 100644 --- a/pkg/workflow/lock_validation.go +++ b/pkg/workflow/lock_validation.go @@ -1,20 +1,25 @@ -// 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