Skip to content

Commit 22350bb

Browse files
Copilotpelikhan
andauthored
Cache action pin resolutions in memory across workflow compilations (#3688)
* Initial plan * Add in-memory caching for action pin resolver across workflows - Add actionCache and actionResolver fields to Compiler struct - Implement getSharedActionResolver() to lazily initialize shared cache - Update ParseWorkflowFile to use shared cache instead of creating new ones - Add tests to verify cache is shared across multiple workflow compilations This change ensures that when compiling multiple workflows, the same action@version is resolved only once, with subsequent lookups using the in-memory cache. This significantly reduces GitHub API calls during compilation. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * Fix: Use shared cache for action SHA validation Add GetSharedActionCache() method to expose compiler's shared cache. Update validation functions to use shared cache instead of creating new ActionCache instances for each workflow. This ensures action SHA validation benefits from cached resolutions, eliminating redundant GitHub API queries during make recompile. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> Co-authored-by: Peli de Halleux <pelikhan@users.noreply.github.com>
1 parent 61b1d87 commit 22350bb

4 files changed

Lines changed: 191 additions & 33 deletions

File tree

.github/workflows/technical-doc-writer.lock.yml

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/cli/compile_command.go

Lines changed: 10 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -49,18 +49,11 @@ func CompileWorkflowWithValidation(compiler *workflow.Compiler, filePath string,
4949
// Validate action SHAs if requested
5050
if validateActionSHAs {
5151
compileLog.Print("Validating action SHAs in lock file")
52-
// Find git root for action cache
53-
gitRoot, err := findGitRoot()
54-
if err != nil {
55-
compileLog.Printf("Unable to find git root for action cache: %v", err)
56-
// Continue without validation if we can't find git root
57-
} else {
58-
// Create action cache for validation
59-
actionCache := workflow.NewActionCache(gitRoot)
60-
if err := workflow.ValidateActionSHAsInLockFile(lockFile, actionCache, verbose); err != nil {
61-
// Action SHA validation warnings are non-fatal
62-
compileLog.Printf("Action SHA validation completed with warnings: %v", err)
63-
}
52+
// Use the compiler's shared action cache to benefit from cached resolutions
53+
actionCache := compiler.GetSharedActionCache()
54+
if err := workflow.ValidateActionSHAsInLockFile(lockFile, actionCache, verbose); err != nil {
55+
// Action SHA validation warnings are non-fatal
56+
compileLog.Printf("Action SHA validation completed with warnings: %v", err)
6457
}
6558
}
6659

@@ -119,18 +112,11 @@ func CompileWorkflowDataWithValidation(compiler *workflow.Compiler, workflowData
119112
// Validate action SHAs if requested
120113
if validateActionSHAs {
121114
compileLog.Print("Validating action SHAs in lock file")
122-
// Find git root for action cache
123-
gitRoot, err := findGitRoot()
124-
if err != nil {
125-
compileLog.Printf("Unable to find git root for action cache: %v", err)
126-
// Continue without validation if we can't find git root
127-
} else {
128-
// Create action cache for validation
129-
actionCache := workflow.NewActionCache(gitRoot)
130-
if err := workflow.ValidateActionSHAsInLockFile(lockFile, actionCache, verbose); err != nil {
131-
// Action SHA validation warnings are non-fatal
132-
compileLog.Printf("Action SHA validation completed with warnings: %v", err)
133-
}
115+
// Use the compiler's shared action cache to benefit from cached resolutions
116+
actionCache := compiler.GetSharedActionCache()
117+
if err := workflow.ValidateActionSHAsInLockFile(lockFile, actionCache, verbose); err != nil {
118+
// Action SHA validation warnings are non-fatal
119+
compileLog.Printf("Action SHA validation completed with warnings: %v", err)
134120
}
135121
}
136122

pkg/workflow/compiler.go

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,8 @@ type Compiler struct {
6161
fileTracker FileTracker // Optional file tracker for tracking created files
6262
warningCount int // Number of warnings encountered during compilation
6363
stepOrderTracker *StepOrderTracker // Tracks step ordering for validation
64+
actionCache *ActionCache // Shared cache for action pin resolutions across all workflows
65+
actionResolver *ActionResolver // Shared resolver for action pins across all workflows
6466
}
6567

6668
// NewCompiler creates a new workflow compiler with optional configuration
@@ -123,6 +125,31 @@ func (c *Compiler) ResetWarningCount() {
123125
c.warningCount = 0
124126
}
125127

128+
// getSharedActionResolver returns the shared action resolver, initializing it on first use
129+
// This ensures all workflows compiled by this compiler instance share the same in-memory cache
130+
func (c *Compiler) getSharedActionResolver() (*ActionCache, *ActionResolver) {
131+
if c.actionCache == nil {
132+
// Initialize cache and resolver on first use
133+
cwd, err := os.Getwd()
134+
if err != nil {
135+
cwd = "."
136+
}
137+
c.actionCache = NewActionCache(cwd)
138+
_ = c.actionCache.Load() // Ignore errors if cache doesn't exist
139+
c.actionResolver = NewActionResolver(c.actionCache)
140+
log.Print("Initialized shared action cache and resolver for compiler")
141+
}
142+
return c.actionCache, c.actionResolver
143+
}
144+
145+
// GetSharedActionCache returns the shared action cache used by this compiler instance.
146+
// The cache is lazily initialized on first access and shared across all workflows.
147+
// This allows action SHA validation and other operations to reuse cached resolutions.
148+
func (c *Compiler) GetSharedActionCache() *ActionCache {
149+
cache, _ := c.getSharedActionResolver()
150+
return cache
151+
}
152+
126153
// NewCompilerWithCustomOutput creates a new workflow compiler with custom output path
127154
func NewCompilerWithCustomOutput(verbose bool, engineOverride string, customOutput string, version string) *Compiler {
128155
c := &Compiler{
@@ -957,14 +984,11 @@ func (c *Compiler) ParseWorkflowFile(markdownPath string) (*WorkflowData, error)
957984
SecretMasking: secretMasking,
958985
}
959986

960-
// Initialize action cache and resolver
961-
cwd, err := os.Getwd()
962-
if err != nil {
963-
cwd = "."
964-
}
965-
workflowData.ActionCache = NewActionCache(cwd)
966-
_ = workflowData.ActionCache.Load() // Ignore errors if cache doesn't exist
967-
workflowData.ActionResolver = NewActionResolver(workflowData.ActionCache)
987+
// Use shared action cache and resolver from the compiler
988+
// This ensures cache is shared across all workflows during compilation
989+
actionCache, actionResolver := c.getSharedActionResolver()
990+
workflowData.ActionCache = actionCache
991+
workflowData.ActionResolver = actionResolver
968992

969993
// Extract YAML sections from frontmatter - use direct frontmatter map extraction
970994
// to avoid issues with nested keys (e.g., tools.mcps.*.env being confused with top-level env)
Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
package workflow
2+
3+
import (
4+
"os"
5+
"path/filepath"
6+
"testing"
7+
)
8+
9+
func TestCompilerSharedActionCache(t *testing.T) {
10+
// Create a temporary directory for test workflows
11+
tmpDir := t.TempDir()
12+
13+
// Change to the temp directory so the cache path is consistent
14+
origDir, err := os.Getwd()
15+
if err != nil {
16+
t.Fatalf("Failed to get current directory: %v", err)
17+
}
18+
defer func() {
19+
if err := os.Chdir(origDir); err != nil {
20+
t.Errorf("Failed to restore directory: %v", err)
21+
}
22+
}()
23+
24+
if err := os.Chdir(tmpDir); err != nil {
25+
t.Fatalf("Failed to change to temp directory: %v", err)
26+
}
27+
28+
// Create a compiler instance
29+
compiler := NewCompiler(false, "", "test")
30+
31+
// Get the shared action resolver (first time - should initialize)
32+
cache1, resolver1 := compiler.getSharedActionResolver()
33+
if cache1 == nil {
34+
t.Error("Expected cache to be initialized")
35+
}
36+
if resolver1 == nil {
37+
t.Error("Expected resolver to be initialized")
38+
}
39+
40+
// Add an entry to the cache
41+
cache1.Set("actions/checkout", "v5", "test-sha-abc")
42+
43+
// Get the shared action resolver again (should be same instance)
44+
cache2, resolver2 := compiler.getSharedActionResolver()
45+
46+
// Verify it's the same instance
47+
if cache1 != cache2 {
48+
t.Error("Expected same cache instance to be returned")
49+
}
50+
if resolver1 != resolver2 {
51+
t.Error("Expected same resolver instance to be returned")
52+
}
53+
54+
// Verify the cache entry is still there (proves it's shared)
55+
sha, found := cache2.Get("actions/checkout", "v5")
56+
if !found {
57+
t.Error("Expected to find cached entry")
58+
}
59+
if sha != "test-sha-abc" {
60+
t.Errorf("Expected SHA 'test-sha-abc', got '%s'", sha)
61+
}
62+
}
63+
64+
func TestCompilerSharedCacheAcrossWorkflows(t *testing.T) {
65+
// Create a temporary directory for test
66+
tmpDir := t.TempDir()
67+
68+
// Change to the temp directory
69+
origDir, err := os.Getwd()
70+
if err != nil {
71+
t.Fatalf("Failed to get current directory: %v", err)
72+
}
73+
defer func() {
74+
if err := os.Chdir(origDir); err != nil {
75+
t.Errorf("Failed to restore directory: %v", err)
76+
}
77+
}()
78+
79+
if err := os.Chdir(tmpDir); err != nil {
80+
t.Fatalf("Failed to change to temp directory: %v", err)
81+
}
82+
83+
// Create test workflow files
84+
workflowsDir := filepath.Join(tmpDir, ".github", "workflows")
85+
if err := os.MkdirAll(workflowsDir, 0755); err != nil {
86+
t.Fatalf("Failed to create workflows directory: %v", err)
87+
}
88+
89+
workflow1Content := `---
90+
on: push
91+
engine: copilot
92+
---
93+
# Test Workflow 1
94+
Test content
95+
`
96+
97+
workflow2Content := `---
98+
on: pull_request
99+
engine: copilot
100+
---
101+
# Test Workflow 2
102+
Test content
103+
`
104+
105+
workflow1Path := filepath.Join(workflowsDir, "workflow1.md")
106+
workflow2Path := filepath.Join(workflowsDir, "workflow2.md")
107+
108+
if err := os.WriteFile(workflow1Path, []byte(workflow1Content), 0644); err != nil {
109+
t.Fatalf("Failed to write workflow1: %v", err)
110+
}
111+
if err := os.WriteFile(workflow2Path, []byte(workflow2Content), 0644); err != nil {
112+
t.Fatalf("Failed to write workflow2: %v", err)
113+
}
114+
115+
// Create a compiler
116+
compiler := NewCompiler(false, "", "test")
117+
compiler.SetSkipValidation(true)
118+
compiler.SetNoEmit(true)
119+
120+
// Parse the first workflow
121+
data1, err := compiler.ParseWorkflowFile(workflow1Path)
122+
if err != nil {
123+
t.Fatalf("Failed to parse workflow1: %v", err)
124+
}
125+
126+
// Manually add a cache entry via the first workflow's cache
127+
data1.ActionCache.Set("actions/checkout", "v5", "shared-sha-123")
128+
129+
// Parse the second workflow
130+
data2, err := compiler.ParseWorkflowFile(workflow2Path)
131+
if err != nil {
132+
t.Fatalf("Failed to parse workflow2: %v", err)
133+
}
134+
135+
// Verify the second workflow uses the same cache instance
136+
if data1.ActionCache != data2.ActionCache {
137+
t.Error("Expected both workflows to share the same cache instance")
138+
}
139+
140+
// Verify the cache entry is available in the second workflow
141+
sha, found := data2.ActionCache.Get("actions/checkout", "v5")
142+
if !found {
143+
t.Error("Expected to find cached entry in second workflow")
144+
}
145+
if sha != "shared-sha-123" {
146+
t.Errorf("Expected SHA 'shared-sha-123', got '%s'", sha)
147+
}
148+
}

0 commit comments

Comments
 (0)