diff --git a/.github/workflows/technical-doc-writer.lock.yml b/.github/workflows/technical-doc-writer.lock.yml index 820d77b2004..7f4701e57f4 100644 --- a/.github/workflows/technical-doc-writer.lock.yml +++ b/.github/workflows/technical-doc-writer.lock.yml @@ -2271,7 +2271,7 @@ jobs: run: | set -o pipefail sudo -E awf --env-all --allow-domains '*.githubusercontent.com,api.enterprise.githubcopilot.com,api.github.com,api.snapcraft.io,archive.ubuntu.com,azure.archive.ubuntu.com,codeload.github.com,crl.geotrust.com,crl.globalsign.com,crl.identrust.com,crl.sectigo.com,crl.thawte.com,crl.usertrust.com,crl.verisign.com,crl3.digicert.com,crl4.digicert.com,crls.ssl.com,github-cloud.githubusercontent.com,github-cloud.s3.amazonaws.com,github.com,json-schema.org,json.schemastore.org,keyserver.ubuntu.com,lfs.github.com,objects.githubusercontent.com,ocsp.digicert.com,ocsp.geotrust.com,ocsp.globalsign.com,ocsp.identrust.com,ocsp.sectigo.com,ocsp.ssl.com,ocsp.thawte.com,ocsp.usertrust.com,ocsp.verisign.com,packagecloud.io,packages.cloud.google.com,packages.microsoft.com,ppa.launchpad.net,raw.githubusercontent.com,registry.npmjs.org,s.symcb.com,s.symcd.com,security.ubuntu.com,ts-crl.ws.symantec.com,ts-ocsp.ws.symantec.com' --log-level info \ - "npx -y @github/copilot@0.0.354 --add-dir /tmp/gh-aw/ --log-level all --disable-builtin-mcps --agent \"\${GITHUB_WORKSPACE}/.github/agents/technical-doc-writer.md\" --allow-tool github --allow-tool safeoutputs --allow-tool shell --allow-tool write --add-dir /tmp/gh-aw/cache-memory/ --allow-all-paths --prompt \"\$(cat /tmp/gh-aw/aw-prompts/prompt.txt)\"" \ + "npx -y @github/copilot@0.0.354 --add-dir /tmp/gh-aw/ --log-level all --disable-builtin-mcps --agent technical-doc-writer --allow-tool github --allow-tool safeoutputs --allow-tool shell --allow-tool write --add-dir /tmp/gh-aw/cache-memory/ --allow-all-paths --prompt \"\$(cat /tmp/gh-aw/aw-prompts/prompt.txt)\"" \ 2>&1 | tee /tmp/gh-aw/agent-stdio.log # Move preserved Copilot logs to expected location diff --git a/pkg/cli/compile_command.go b/pkg/cli/compile_command.go index a9b9dff0df7..994c5c3af2b 100644 --- a/pkg/cli/compile_command.go +++ b/pkg/cli/compile_command.go @@ -49,18 +49,11 @@ func CompileWorkflowWithValidation(compiler *workflow.Compiler, filePath string, // Validate action SHAs if requested if validateActionSHAs { compileLog.Print("Validating action SHAs in lock file") - // Find git root for action cache - gitRoot, err := findGitRoot() - if err != nil { - compileLog.Printf("Unable to find git root for action cache: %v", err) - // Continue without validation if we can't find git root - } else { - // Create action cache for validation - actionCache := workflow.NewActionCache(gitRoot) - if err := workflow.ValidateActionSHAsInLockFile(lockFile, actionCache, verbose); err != nil { - // Action SHA validation warnings are non-fatal - compileLog.Printf("Action SHA validation completed with warnings: %v", err) - } + // Use the compiler's shared action cache to benefit from cached resolutions + actionCache := compiler.GetSharedActionCache() + if err := workflow.ValidateActionSHAsInLockFile(lockFile, actionCache, verbose); err != nil { + // Action SHA validation warnings are non-fatal + compileLog.Printf("Action SHA validation completed with warnings: %v", err) } } @@ -119,18 +112,11 @@ func CompileWorkflowDataWithValidation(compiler *workflow.Compiler, workflowData // Validate action SHAs if requested if validateActionSHAs { compileLog.Print("Validating action SHAs in lock file") - // Find git root for action cache - gitRoot, err := findGitRoot() - if err != nil { - compileLog.Printf("Unable to find git root for action cache: %v", err) - // Continue without validation if we can't find git root - } else { - // Create action cache for validation - actionCache := workflow.NewActionCache(gitRoot) - if err := workflow.ValidateActionSHAsInLockFile(lockFile, actionCache, verbose); err != nil { - // Action SHA validation warnings are non-fatal - compileLog.Printf("Action SHA validation completed with warnings: %v", err) - } + // Use the compiler's shared action cache to benefit from cached resolutions + actionCache := compiler.GetSharedActionCache() + if err := workflow.ValidateActionSHAsInLockFile(lockFile, actionCache, verbose); err != nil { + // Action SHA validation warnings are non-fatal + compileLog.Printf("Action SHA validation completed with warnings: %v", err) } } diff --git a/pkg/workflow/compiler.go b/pkg/workflow/compiler.go index 8cfa8a66170..e09557fa560 100644 --- a/pkg/workflow/compiler.go +++ b/pkg/workflow/compiler.go @@ -61,6 +61,8 @@ type Compiler struct { fileTracker FileTracker // Optional file tracker for tracking created files warningCount int // Number of warnings encountered during compilation stepOrderTracker *StepOrderTracker // Tracks step ordering for validation + actionCache *ActionCache // Shared cache for action pin resolutions across all workflows + actionResolver *ActionResolver // Shared resolver for action pins across all workflows } // NewCompiler creates a new workflow compiler with optional configuration @@ -123,6 +125,31 @@ func (c *Compiler) ResetWarningCount() { c.warningCount = 0 } +// getSharedActionResolver returns the shared action resolver, initializing it on first use +// This ensures all workflows compiled by this compiler instance share the same in-memory cache +func (c *Compiler) getSharedActionResolver() (*ActionCache, *ActionResolver) { + if c.actionCache == nil { + // Initialize cache and resolver on first use + cwd, err := os.Getwd() + if err != nil { + cwd = "." + } + c.actionCache = NewActionCache(cwd) + _ = c.actionCache.Load() // Ignore errors if cache doesn't exist + c.actionResolver = NewActionResolver(c.actionCache) + log.Print("Initialized shared action cache and resolver for compiler") + } + return c.actionCache, c.actionResolver +} + +// GetSharedActionCache returns the shared action cache used by this compiler instance. +// The cache is lazily initialized on first access and shared across all workflows. +// This allows action SHA validation and other operations to reuse cached resolutions. +func (c *Compiler) GetSharedActionCache() *ActionCache { + cache, _ := c.getSharedActionResolver() + return cache +} + // NewCompilerWithCustomOutput creates a new workflow compiler with custom output path func NewCompilerWithCustomOutput(verbose bool, engineOverride string, customOutput string, version string) *Compiler { c := &Compiler{ @@ -957,14 +984,11 @@ func (c *Compiler) ParseWorkflowFile(markdownPath string) (*WorkflowData, error) SecretMasking: secretMasking, } - // Initialize action cache and resolver - cwd, err := os.Getwd() - if err != nil { - cwd = "." - } - workflowData.ActionCache = NewActionCache(cwd) - _ = workflowData.ActionCache.Load() // Ignore errors if cache doesn't exist - workflowData.ActionResolver = NewActionResolver(workflowData.ActionCache) + // Use shared action cache and resolver from the compiler + // This ensures cache is shared across all workflows during compilation + actionCache, actionResolver := c.getSharedActionResolver() + workflowData.ActionCache = actionCache + workflowData.ActionResolver = actionResolver // Extract YAML sections from frontmatter - use direct frontmatter map extraction // to avoid issues with nested keys (e.g., tools.mcps.*.env being confused with top-level env) diff --git a/pkg/workflow/compiler_shared_cache_test.go b/pkg/workflow/compiler_shared_cache_test.go new file mode 100644 index 00000000000..e9da9e5491e --- /dev/null +++ b/pkg/workflow/compiler_shared_cache_test.go @@ -0,0 +1,148 @@ +package workflow + +import ( + "os" + "path/filepath" + "testing" +) + +func TestCompilerSharedActionCache(t *testing.T) { + // Create a temporary directory for test workflows + tmpDir := t.TempDir() + + // Change to the temp directory so the cache path is consistent + origDir, err := os.Getwd() + if err != nil { + t.Fatalf("Failed to get current directory: %v", err) + } + defer func() { + if err := os.Chdir(origDir); err != nil { + t.Errorf("Failed to restore directory: %v", err) + } + }() + + if err := os.Chdir(tmpDir); err != nil { + t.Fatalf("Failed to change to temp directory: %v", err) + } + + // Create a compiler instance + compiler := NewCompiler(false, "", "test") + + // Get the shared action resolver (first time - should initialize) + cache1, resolver1 := compiler.getSharedActionResolver() + if cache1 == nil { + t.Error("Expected cache to be initialized") + } + if resolver1 == nil { + t.Error("Expected resolver to be initialized") + } + + // Add an entry to the cache + cache1.Set("actions/checkout", "v5", "test-sha-abc") + + // Get the shared action resolver again (should be same instance) + cache2, resolver2 := compiler.getSharedActionResolver() + + // Verify it's the same instance + if cache1 != cache2 { + t.Error("Expected same cache instance to be returned") + } + if resolver1 != resolver2 { + t.Error("Expected same resolver instance to be returned") + } + + // Verify the cache entry is still there (proves it's shared) + sha, found := cache2.Get("actions/checkout", "v5") + if !found { + t.Error("Expected to find cached entry") + } + if sha != "test-sha-abc" { + t.Errorf("Expected SHA 'test-sha-abc', got '%s'", sha) + } +} + +func TestCompilerSharedCacheAcrossWorkflows(t *testing.T) { + // Create a temporary directory for test + tmpDir := t.TempDir() + + // Change to the temp directory + origDir, err := os.Getwd() + if err != nil { + t.Fatalf("Failed to get current directory: %v", err) + } + defer func() { + if err := os.Chdir(origDir); err != nil { + t.Errorf("Failed to restore directory: %v", err) + } + }() + + if err := os.Chdir(tmpDir); err != nil { + t.Fatalf("Failed to change to temp directory: %v", err) + } + + // Create test workflow files + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + if err := os.MkdirAll(workflowsDir, 0755); err != nil { + t.Fatalf("Failed to create workflows directory: %v", err) + } + + workflow1Content := `--- +on: push +engine: copilot +--- +# Test Workflow 1 +Test content +` + + workflow2Content := `--- +on: pull_request +engine: copilot +--- +# Test Workflow 2 +Test content +` + + workflow1Path := filepath.Join(workflowsDir, "workflow1.md") + workflow2Path := filepath.Join(workflowsDir, "workflow2.md") + + if err := os.WriteFile(workflow1Path, []byte(workflow1Content), 0644); err != nil { + t.Fatalf("Failed to write workflow1: %v", err) + } + if err := os.WriteFile(workflow2Path, []byte(workflow2Content), 0644); err != nil { + t.Fatalf("Failed to write workflow2: %v", err) + } + + // Create a compiler + compiler := NewCompiler(false, "", "test") + compiler.SetSkipValidation(true) + compiler.SetNoEmit(true) + + // Parse the first workflow + data1, err := compiler.ParseWorkflowFile(workflow1Path) + if err != nil { + t.Fatalf("Failed to parse workflow1: %v", err) + } + + // Manually add a cache entry via the first workflow's cache + data1.ActionCache.Set("actions/checkout", "v5", "shared-sha-123") + + // Parse the second workflow + data2, err := compiler.ParseWorkflowFile(workflow2Path) + if err != nil { + t.Fatalf("Failed to parse workflow2: %v", err) + } + + // Verify the second workflow uses the same cache instance + if data1.ActionCache != data2.ActionCache { + t.Error("Expected both workflows to share the same cache instance") + } + + // Verify the cache entry is available in the second workflow + sha, found := data2.ActionCache.Get("actions/checkout", "v5") + if !found { + t.Error("Expected to find cached entry in second workflow") + } + if sha != "shared-sha-123" { + t.Errorf("Expected SHA 'shared-sha-123', got '%s'", sha) + } +}