diff --git a/pkg/workflow/compiler_yaml_main_job.go b/pkg/workflow/compiler_yaml_main_job.go index 22f322a8a9e..4aaadf633be 100644 --- a/pkg/workflow/compiler_yaml_main_job.go +++ b/pkg/workflow/compiler_yaml_main_job.go @@ -244,7 +244,7 @@ func (c *Compiler) generateRuntimeAndWorkspaceSetupSteps(yaml *strings.Builder, func (c *Compiler) prepareRuntimeSetupAndCheckoutInfo(data *WorkflowData) ([]GitHubActionStep, bool) { // Add automatic runtime setup steps if needed // This detects runtimes from custom steps and MCP configs - runtimeRequirements := DetectRuntimeRequirements(data) + runtimeRequirements := detectRuntimeRequirementsCached(data) // Deduplicate runtime setup steps from custom steps // This removes any runtime setup action steps (like actions/setup-go) from custom steps diff --git a/pkg/workflow/runtime_detection.go b/pkg/workflow/runtime_detection.go index 7ab6e7328f0..e9928ec02c6 100644 --- a/pkg/workflow/runtime_detection.go +++ b/pkg/workflow/runtime_detection.go @@ -1,6 +1,7 @@ package workflow import ( + "maps" "strings" "github.com/github/gh-aw/pkg/constants" @@ -11,6 +12,36 @@ import ( var runtimeSetupLog = logger.New("workflow:runtime_setup") +func cloneRuntimeRequirements(requirements []RuntimeRequirement) []RuntimeRequirement { + if len(requirements) == 0 { + return nil + } + cloned := make([]RuntimeRequirement, len(requirements)) + copy(cloned, requirements) + for i, req := range cloned { + if req.ExtraFields == nil { + continue + } + extraFields := make(map[string]any, len(req.ExtraFields)) + maps.Copy(extraFields, req.ExtraFields) + cloned[i].ExtraFields = extraFields + } + return cloned +} + +func detectRuntimeRequirementsCached(workflowData *WorkflowData) []RuntimeRequirement { + if workflowData == nil { + return nil + } + if workflowData.CachedRuntimeRequirementsSet { + return cloneRuntimeRequirements(workflowData.CachedRuntimeRequirements) + } + requirements := DetectRuntimeRequirements(workflowData) + workflowData.CachedRuntimeRequirements = cloneRuntimeRequirements(requirements) + workflowData.CachedRuntimeRequirementsSet = true + return cloneRuntimeRequirements(workflowData.CachedRuntimeRequirements) +} + // DetectRuntimeRequirements analyzes workflow data to detect required runtimes func DetectRuntimeRequirements(workflowData *WorkflowData) []RuntimeRequirement { runtimeSetupLog.Print("Detecting runtime requirements from workflow data") diff --git a/pkg/workflow/runtime_detection_cache_test.go b/pkg/workflow/runtime_detection_cache_test.go new file mode 100644 index 00000000000..4d3a4b32d21 --- /dev/null +++ b/pkg/workflow/runtime_detection_cache_test.go @@ -0,0 +1,69 @@ +//go:build !integration + +package workflow + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestDetectRuntimeRequirementsCached_NilWorkflowData(t *testing.T) { + require.Nil(t, detectRuntimeRequirementsCached(nil), "nil workflow data should produce no runtime requirements") +} + +func TestDetectRuntimeRequirementsCached_EmptyRequirements(t *testing.T) { + workflowData := &WorkflowData{} + + result := detectRuntimeRequirementsCached(workflowData) + + require.Empty(t, result, "workflow without runtime inputs should produce no runtime requirements") + require.True(t, workflowData.CachedRuntimeRequirementsSet, "empty detection result should still mark the cache as populated") + require.Nil(t, workflowData.CachedRuntimeRequirements, "empty detection result should be cached as nil requirements") +} + +func TestDetectRuntimeRequirementsCached_CacheMissReturnsIndependentCopy(t *testing.T) { + workflowData := &WorkflowData{ + CustomSteps: "run: node --version", + } + + first := detectRuntimeRequirementsCached(workflowData) + require.NotEmpty(t, first, "cache miss should still return detected runtime requirements") + require.True(t, workflowData.CachedRuntimeRequirementsSet, "cache miss should populate the runtime requirement cache") + + first[0].Version = "mutated-version" + require.NotEqual(t, "mutated-version", workflowData.CachedRuntimeRequirements[0].Version, "mutating the first cache-miss result must not corrupt the cached requirements") + + second := detectRuntimeRequirementsCached(workflowData) + require.NotEmpty(t, second, "subsequent cache hits should still return runtime requirements") + require.NotEqual(t, "mutated-version", second[0].Version, "cache-hit result should not observe mutation from the cache-miss return value") +} + +func TestDetectRuntimeRequirementsCached_CacheHitReturnsDeepCopy(t *testing.T) { + workflowData := &WorkflowData{ + CachedRuntimeRequirements: []RuntimeRequirement{ + { + Runtime: findRuntimeByID("node"), + Version: "24", + ExtraFields: map[string]any{ + "cache": "npm", + }, + }, + }, + CachedRuntimeRequirementsSet: true, + } + + first := detectRuntimeRequirementsCached(workflowData) + require.Len(t, first, 1, "cache hit should return the cached runtime requirements") + + first[0].Version = "mutated-version" + first[0].ExtraFields["cache"] = "mutated-cache" + + require.Equal(t, "24", workflowData.CachedRuntimeRequirements[0].Version, "mutating a cache-hit result must not change the cached version") + require.Equal(t, "npm", workflowData.CachedRuntimeRequirements[0].ExtraFields["cache"], "mutating a cache-hit result must not change cached extra fields") + + second := detectRuntimeRequirementsCached(workflowData) + require.Len(t, second, 1, "subsequent cache hits should continue to return one cached runtime requirement") + require.Equal(t, "24", second[0].Version, "subsequent cache hits should return the original cached version") + require.Equal(t, "npm", second[0].ExtraFields["cache"], "subsequent cache hits should return a deep-cloned extra-fields map") +} diff --git a/pkg/workflow/runtime_validation.go b/pkg/workflow/runtime_validation.go index c12b1784009..ef01b4251ef 100644 --- a/pkg/workflow/runtime_validation.go +++ b/pkg/workflow/runtime_validation.go @@ -251,7 +251,7 @@ func (c *Compiler) validateContainerImages(workflowData *WorkflowData) error { // validateRuntimePackages validates that packages required by npx, pip, and uv are available func (c *Compiler) validateRuntimePackages(workflowData *WorkflowData) error { // Detect runtime requirements - requirements := DetectRuntimeRequirements(workflowData) + requirements := detectRuntimeRequirementsCached(workflowData) runtimeValidationLog.Printf("Validating runtime packages: found %d runtime requirements", len(requirements)) var errors []string diff --git a/pkg/workflow/workflow_data.go b/pkg/workflow/workflow_data.go index d8c4839868e..488f9c96cb0 100644 --- a/pkg/workflow/workflow_data.go +++ b/pkg/workflow/workflow_data.go @@ -182,6 +182,8 @@ type WorkflowData struct { CachedParsedToolsets []string // cached result of ParseGitHubToolsets for the GitHub tool (for performance optimization); populated by applyDefaults CachedAllowedDomainsStr string // cached allowed-domains string for sanitization (for performance optimization); computed once and reused across multiple compilation steps CachedAllowedDomainsComputed bool // true once CachedAllowedDomainsStr has been set; distinguishes "computed empty" from "not yet computed" + CachedRuntimeRequirements []RuntimeRequirement // cached runtime requirements derived from DetectRuntimeRequirements; reused by validation and YAML generation within one compilation + CachedRuntimeRequirementsSet bool // true once CachedRuntimeRequirements is populated; distinguishes "computed empty" from "not yet computed" KnownActionCredentialEnvVars map[string]struct{} // env vars for clean_known_action_credentials.sh; keyed by GH_AW_CLEAN_* names; nil when no known credential-leaking actions are detected ModelMappings map[string][]string // merged model alias map (builtins + imported workflow aliases + main frontmatter overrides, in priority order); NOT yet emitted to AWF config JSON — pending AWF firewall support (config.models) ModelCosts map[string]any // model pricing data from frontmatter `models` field (providers structure); merged with built-in models.json at runtime by generate_aw_info.cjs