From fb4110652011924c76af58ef36a7aec07c6e873b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 3 Mar 2026 11:02:25 +0000 Subject: [PATCH 1/4] Initial plan From 4334c9d378bbed2282b0ddae10f4ad10d3e490b2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 3 Mar 2026 11:19:10 +0000 Subject: [PATCH 2/4] Add cache-memory scope enum validation to parser Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/cache.go | 5 + pkg/workflow/cache_scope_validation_test.go | 146 ++++++++++++++++++++ 2 files changed, 151 insertions(+) create mode 100644 pkg/workflow/cache_scope_validation_test.go diff --git a/pkg/workflow/cache.go b/pkg/workflow/cache.go index fef75712bc6..f34aa09a05f 100644 --- a/pkg/workflow/cache.go +++ b/pkg/workflow/cache.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" "os" + "slices" "sort" "strings" @@ -112,6 +113,10 @@ func parseCacheMemoryEntry(cacheMap map[string]any, defaultID string) (CacheMemo if entry.Scope == "" { entry.Scope = "workflow" } + // Validate scope value + if !slices.Contains([]string{"workflow", "repo"}, entry.Scope) { + return entry, fmt.Errorf("invalid cache-memory scope %q: must be one of [workflow, repo]", entry.Scope) + } // Parse allowed-extensions field if allowedExts, exists := cacheMap["allowed-extensions"]; exists { diff --git a/pkg/workflow/cache_scope_validation_test.go b/pkg/workflow/cache_scope_validation_test.go new file mode 100644 index 00000000000..26c3ecf5bc3 --- /dev/null +++ b/pkg/workflow/cache_scope_validation_test.go @@ -0,0 +1,146 @@ +//go:build !integration + +package workflow + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestCacheMemoryScopeValidationObject tests scope validation with object notation +func TestCacheMemoryScopeValidationObject(t *testing.T) { + tests := []struct { + name string + scope string + wantError bool + errorText string + }{ + { + name: "valid workflow scope", + scope: "workflow", + wantError: false, + }, + { + name: "valid repo scope", + scope: "repo", + wantError: false, + }, + { + name: "invalid organization scope", + scope: "organization", + wantError: true, + errorText: `invalid cache-memory scope "organization": must be one of [workflow, repo]`, + }, + { + name: "invalid global scope", + scope: "global", + wantError: true, + errorText: `invalid cache-memory scope "global": must be one of [workflow, repo]`, + }, + { + name: "invalid empty string scope", + scope: " ", + wantError: true, + errorText: `invalid cache-memory scope " ": must be one of [workflow, repo]`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + toolsMap := map[string]any{ + "cache-memory": map[string]any{ + "scope": tt.scope, + }, + } + + toolsConfig, err := ParseToolsConfig(toolsMap) + require.NoError(t, err, "Should parse tools config") + + compiler := NewCompiler() + _, err = compiler.extractCacheMemoryConfig(toolsConfig) + + if tt.wantError { + require.Error(t, err, "Should return error for invalid scope") + assert.ErrorContains(t, err, tt.errorText, "Error should contain expected message") + } else { + assert.NoError(t, err, "Should not return error for valid scope") + } + }) + } +} + +// TestCacheMemoryScopeValidationArray tests scope validation with array notation +func TestCacheMemoryScopeValidationArray(t *testing.T) { + tests := []struct { + name string + scope string + wantError bool + errorText string + }{ + { + name: "valid workflow scope in array", + scope: "workflow", + wantError: false, + }, + { + name: "valid repo scope in array", + scope: "repo", + wantError: false, + }, + { + name: "invalid scope in array", + scope: "global", + wantError: true, + errorText: `invalid cache-memory scope "global": must be one of [workflow, repo]`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + toolsMap := map[string]any{ + "cache-memory": []any{ + map[string]any{ + "id": "test-cache", + "scope": tt.scope, + }, + }, + } + + toolsConfig, err := ParseToolsConfig(toolsMap) + require.NoError(t, err, "Should parse tools config") + + compiler := NewCompiler() + _, err = compiler.extractCacheMemoryConfig(toolsConfig) + + if tt.wantError { + require.Error(t, err, "Should return error for invalid scope") + assert.ErrorContains(t, err, tt.errorText, "Error should contain expected message") + } else { + assert.NoError(t, err, "Should not return error for valid scope") + } + }) + } +} + +// TestCacheMemoryScopeDefault tests that omitting scope defaults to "workflow" +func TestCacheMemoryScopeDefault(t *testing.T) { + toolsMap := map[string]any{ + "cache-memory": map[string]any{ + "key": "my-cache-key", + }, + } + + toolsConfig, err := ParseToolsConfig(toolsMap) + require.NoError(t, err, "Should parse tools config") + + compiler := NewCompiler() + config, err := compiler.extractCacheMemoryConfig(toolsConfig) + + require.NoError(t, err, "Should not error when scope is omitted") + require.NotNil(t, config, "Config should not be nil") + require.Len(t, config.Caches, 1, "Should have exactly one cache entry") + + assert.Equal(t, "workflow", config.Caches[0].Scope, "Default scope should be 'workflow'") +} From 7deef609f3c6dd75f2d6a917a0012eefcc25e269 Mon Sep 17 00:00:00 2001 From: Peli de Halleux Date: Tue, 3 Mar 2026 11:27:00 +0000 Subject: [PATCH 3/4] Update cache_scope_validation_test.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- pkg/workflow/cache_scope_validation_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/workflow/cache_scope_validation_test.go b/pkg/workflow/cache_scope_validation_test.go index 26c3ecf5bc3..2e6e8dc964f 100644 --- a/pkg/workflow/cache_scope_validation_test.go +++ b/pkg/workflow/cache_scope_validation_test.go @@ -40,7 +40,7 @@ func TestCacheMemoryScopeValidationObject(t *testing.T) { errorText: `invalid cache-memory scope "global": must be one of [workflow, repo]`, }, { - name: "invalid empty string scope", + name: "invalid whitespace scope", scope: " ", wantError: true, errorText: `invalid cache-memory scope " ": must be one of [workflow, repo]`, From 0d62b4f19f998b0ffa27040ca14a0dd8d020c3c2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 3 Mar 2026 11:34:02 +0000 Subject: [PATCH 4/4] Refactor: define validCacheMemoryScopes once for validation and error formatting Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/cache.go | 7 +++++-- pkg/workflow/cache_scope_validation_test.go | 8 ++++---- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/pkg/workflow/cache.go b/pkg/workflow/cache.go index f34aa09a05f..dd7c709570d 100644 --- a/pkg/workflow/cache.go +++ b/pkg/workflow/cache.go @@ -15,6 +15,9 @@ import ( var cacheLog = logger.New("workflow:cache") +// validCacheMemoryScopes defines the allowed values for cache-memory scope +var validCacheMemoryScopes = []string{"workflow", "repo"} + // CacheMemoryConfig holds configuration for cache-memory functionality type CacheMemoryConfig struct { Caches []CacheMemoryEntry `yaml:"caches,omitempty"` // cache configurations @@ -114,8 +117,8 @@ func parseCacheMemoryEntry(cacheMap map[string]any, defaultID string) (CacheMemo entry.Scope = "workflow" } // Validate scope value - if !slices.Contains([]string{"workflow", "repo"}, entry.Scope) { - return entry, fmt.Errorf("invalid cache-memory scope %q: must be one of [workflow, repo]", entry.Scope) + if !slices.Contains(validCacheMemoryScopes, entry.Scope) { + return entry, fmt.Errorf("invalid cache-memory scope %q: must be one of %v", entry.Scope, validCacheMemoryScopes) } // Parse allowed-extensions field diff --git a/pkg/workflow/cache_scope_validation_test.go b/pkg/workflow/cache_scope_validation_test.go index 2e6e8dc964f..345bff2a5e8 100644 --- a/pkg/workflow/cache_scope_validation_test.go +++ b/pkg/workflow/cache_scope_validation_test.go @@ -31,19 +31,19 @@ func TestCacheMemoryScopeValidationObject(t *testing.T) { name: "invalid organization scope", scope: "organization", wantError: true, - errorText: `invalid cache-memory scope "organization": must be one of [workflow, repo]`, + errorText: `invalid cache-memory scope "organization": must be one of [workflow repo]`, }, { name: "invalid global scope", scope: "global", wantError: true, - errorText: `invalid cache-memory scope "global": must be one of [workflow, repo]`, + errorText: `invalid cache-memory scope "global": must be one of [workflow repo]`, }, { name: "invalid whitespace scope", scope: " ", wantError: true, - errorText: `invalid cache-memory scope " ": must be one of [workflow, repo]`, + errorText: `invalid cache-memory scope " ": must be one of [workflow repo]`, }, } @@ -93,7 +93,7 @@ func TestCacheMemoryScopeValidationArray(t *testing.T) { name: "invalid scope in array", scope: "global", wantError: true, - errorText: `invalid cache-memory scope "global": must be one of [workflow, repo]`, + errorText: `invalid cache-memory scope "global": must be one of [workflow repo]`, }, }