From a0b9735acecc47b78e2a461f9467d9976570bb58 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 24 Dec 2025 22:54:27 +0000 Subject: [PATCH 1/2] Initial plan From 278fc7f18fb8609631263d07a76dc3ad761618d0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 24 Dec 2025 23:05:27 +0000 Subject: [PATCH 2/2] Add validation helpers and refactor constraint validations - Create pkg/workflow/validation_helpers.go with reusable validateIntRange function - Create comprehensive tests in validation_helpers_test.go (12 basic + 16 real-world test cases) - Refactor port validation in gateway.go to use helper - Refactor max-file-size validation in repo_memory.go to use helper (2 locations) - Refactor max-file-count validation in repo_memory.go to use helper (2 locations) - Refactor retention-days validation in cache.go to use helper (2 locations) - Update error messages in repo_memory_test.go to match new format - All refactored validations maintain same behavior with cleaner code Co-authored-by: mnkiefer <8320933+mnkiefer@users.noreply.github.com> --- .github/workflows/issue-classifier.lock.yml | 2 +- .github/workflows/release.lock.yml | 6 +- .../workflows/stale-repo-identifier.lock.yml | 2 +- .github/workflows/super-linter.lock.yml | 2 +- pkg/workflow/cache.go | 12 +- pkg/workflow/gateway.go | 4 +- pkg/workflow/repo_memory.go | 16 +- pkg/workflow/repo_memory_test.go | 12 +- pkg/workflow/validation_helpers.go | 38 +++ pkg/workflow/validation_helpers_test.go | 305 ++++++++++++++++++ 10 files changed, 373 insertions(+), 26 deletions(-) create mode 100644 pkg/workflow/validation_helpers.go create mode 100644 pkg/workflow/validation_helpers_test.go diff --git a/.github/workflows/issue-classifier.lock.yml b/.github/workflows/issue-classifier.lock.yml index 8d038aaf05..bbbbaa0e75 100644 --- a/.github/workflows/issue-classifier.lock.yml +++ b/.github/workflows/issue-classifier.lock.yml @@ -2206,7 +2206,7 @@ jobs: path: /tmp/gh-aw/aw_info.json if-no-files-found: warn - name: Run AI Inference - uses: actions/ai-inference@334892bb203895caaed82ec52d23c1ed9385151e # v1 + uses: actions/ai-inference@334892bb203895caaed82ec52d23c1ed9385151e # v2.0.4 env: GH_AW_MCP_CONFIG: /tmp/gh-aw/mcp-config/mcp-servers.json GH_AW_PROMPT: /tmp/gh-aw/aw-prompts/prompt.txt diff --git a/.github/workflows/release.lock.yml b/.github/workflows/release.lock.yml index 4cdc5646f7..da587dae86 100644 --- a/.github/workflows/release.lock.yml +++ b/.github/workflows/release.lock.yml @@ -6021,13 +6021,13 @@ jobs: - name: Download Go modules run: go mod download - name: Generate SBOM (SPDX format) - uses: anchore/sbom-action@43a17d6e7add2b5535efe4dcae9952337c479a93 # v0.20.10 + uses: anchore/sbom-action@43a17d6e7add2b5535efe4dcae9952337c479a93 # v0.20.11 with: artifact-name: sbom.spdx.json format: spdx-json output-file: sbom.spdx.json - name: Generate SBOM (CycloneDX format) - uses: anchore/sbom-action@43a17d6e7add2b5535efe4dcae9952337c479a93 # v0.20.10 + uses: anchore/sbom-action@43a17d6e7add2b5535efe4dcae9952337c479a93 # v0.20.11 with: artifact-name: sbom.cdx.json format: cyclonedx-json @@ -6234,7 +6234,7 @@ jobs: fetch-depth: 0 persist-credentials: false - name: Release with gh-extension-precompile - uses: cli/gh-extension-precompile@9e2237c30f869ad3bcaed6a4be2cd43564dd421b # v2 + uses: cli/gh-extension-precompile@9e2237c30f869ad3bcaed6a4be2cd43564dd421b # v2.1.0 with: build_script_override: scripts/build-release.sh go_version_file: go.mod diff --git a/.github/workflows/stale-repo-identifier.lock.yml b/.github/workflows/stale-repo-identifier.lock.yml index 3393e02f2e..f1fe7c40a4 100644 --- a/.github/workflows/stale-repo-identifier.lock.yml +++ b/.github/workflows/stale-repo-identifier.lock.yml @@ -176,7 +176,7 @@ jobs: ORGANIZATION: ${{ env.ORGANIZATION }} id: stale-repos name: Run stale_repos tool - uses: github/stale-repos@a21e55567b83cf3c3f3f9085d3038dc6cee02598 # v3 + uses: github/stale-repos@a21e55567b83cf3c3f3f9085d3038dc6cee02598 # v3.0.2 - env: INACTIVE_REPOS: ${{ steps.stale-repos.outputs.inactiveRepos }} name: Save stale repos output diff --git a/.github/workflows/super-linter.lock.yml b/.github/workflows/super-linter.lock.yml index 5824283b2c..129ba416e5 100644 --- a/.github/workflows/super-linter.lock.yml +++ b/.github/workflows/super-linter.lock.yml @@ -6151,7 +6151,7 @@ jobs: persist-credentials: false - name: Super-linter id: super-linter - uses: super-linter/super-linter@47984f49b4e87383eed97890fe2dca6063bbd9c3 # v8.2.1 + uses: super-linter/super-linter@47984f49b4e87383eed97890fe2dca6063bbd9c3 # v8.3.1 env: CREATE_LOG_FILE: "true" DEFAULT_BRANCH: main diff --git a/pkg/workflow/cache.go b/pkg/workflow/cache.go index 3f70c2c0e2..3132efdcc7 100644 --- a/pkg/workflow/cache.go +++ b/pkg/workflow/cache.go @@ -126,8 +126,10 @@ func (c *Compiler) extractCacheMemoryConfig(toolsConfig *ToolsConfig) (*CacheMem entry.RetentionDays = &retentionDaysIntValue } // Validate retention-days bounds - if entry.RetentionDays != nil && (*entry.RetentionDays < 1 || *entry.RetentionDays > 90) { - return nil, fmt.Errorf("retention-days must be between 1 and 90, got %d", *entry.RetentionDays) + if entry.RetentionDays != nil { + if err := validateIntRange(*entry.RetentionDays, 1, 90, "retention-days"); err != nil { + return nil, err + } } } @@ -189,8 +191,10 @@ func (c *Compiler) extractCacheMemoryConfig(toolsConfig *ToolsConfig) (*CacheMem entry.RetentionDays = &retentionDaysIntValue } // Validate retention-days bounds - if entry.RetentionDays != nil && (*entry.RetentionDays < 1 || *entry.RetentionDays > 90) { - return nil, fmt.Errorf("retention-days must be between 1 and 90, got %d", *entry.RetentionDays) + if entry.RetentionDays != nil { + if err := validateIntRange(*entry.RetentionDays, 1, 90, "retention-days"); err != nil { + return nil, err + } } } diff --git a/pkg/workflow/gateway.go b/pkg/workflow/gateway.go index e5c1e15a61..e31c95267f 100644 --- a/pkg/workflow/gateway.go +++ b/pkg/workflow/gateway.go @@ -77,8 +77,8 @@ func validateAndNormalizePort(port int) (int, error) { } // Validate port is in valid range (1-65535) - if port < 1 || port > 65535 { - return 0, fmt.Errorf("port must be between 1 and 65535, got %d", port) + if err := validateIntRange(port, 1, 65535, "port"); err != nil { + return 0, err } return port, nil diff --git a/pkg/workflow/repo_memory.go b/pkg/workflow/repo_memory.go index 8b674705ee..f6ab6e56af 100644 --- a/pkg/workflow/repo_memory.go +++ b/pkg/workflow/repo_memory.go @@ -166,8 +166,8 @@ func (c *Compiler) extractRepoMemoryConfig(toolsConfig *ToolsConfig) (*RepoMemor entry.MaxFileSize = int(sizeUint64) } // Validate max-file-size bounds - if entry.MaxFileSize < 1 || entry.MaxFileSize > 104857600 { - return nil, fmt.Errorf("max-file-size must be between 1 and 104857600 bytes, got %d", entry.MaxFileSize) + if err := validateIntRange(entry.MaxFileSize, 1, 104857600, "max-file-size"); err != nil { + return nil, err } } @@ -181,8 +181,8 @@ func (c *Compiler) extractRepoMemoryConfig(toolsConfig *ToolsConfig) (*RepoMemor entry.MaxFileCount = int(countUint64) } // Validate max-file-count bounds - if entry.MaxFileCount < 1 || entry.MaxFileCount > 1000 { - return nil, fmt.Errorf("max-file-count must be between 1 and 1000, got %d", entry.MaxFileCount) + if err := validateIntRange(entry.MaxFileCount, 1, 1000, "max-file-count"); err != nil { + return nil, err } } @@ -262,8 +262,8 @@ func (c *Compiler) extractRepoMemoryConfig(toolsConfig *ToolsConfig) (*RepoMemor entry.MaxFileSize = int(sizeUint64) } // Validate max-file-size bounds - if entry.MaxFileSize < 1 || entry.MaxFileSize > 104857600 { - return nil, fmt.Errorf("max-file-size must be between 1 and 104857600 bytes, got %d", entry.MaxFileSize) + if err := validateIntRange(entry.MaxFileSize, 1, 104857600, "max-file-size"); err != nil { + return nil, err } } @@ -277,8 +277,8 @@ func (c *Compiler) extractRepoMemoryConfig(toolsConfig *ToolsConfig) (*RepoMemor entry.MaxFileCount = int(countUint64) } // Validate max-file-count bounds - if entry.MaxFileCount < 1 || entry.MaxFileCount > 1000 { - return nil, fmt.Errorf("max-file-count must be between 1 and 1000, got %d", entry.MaxFileCount) + if err := validateIntRange(entry.MaxFileCount, 1, 1000, "max-file-count"); err != nil { + return nil, err } } diff --git a/pkg/workflow/repo_memory_test.go b/pkg/workflow/repo_memory_test.go index fcee298404..711542b036 100644 --- a/pkg/workflow/repo_memory_test.go +++ b/pkg/workflow/repo_memory_test.go @@ -366,25 +366,25 @@ func TestRepoMemoryMaxFileSizeValidation(t *testing.T) { name: "invalid zero size", maxFileSize: 0, wantError: true, - errorText: "max-file-size must be between 1 and 104857600 bytes, got 0", + errorText: "max-file-size must be between 1 and 104857600, got 0", }, { name: "invalid negative size", maxFileSize: -1, wantError: true, - errorText: "max-file-size must be between 1 and 104857600 bytes, got -1", + errorText: "max-file-size must be between 1 and 104857600, got -1", }, { name: "invalid size exceeds maximum", maxFileSize: 104857601, wantError: true, - errorText: "max-file-size must be between 1 and 104857600 bytes, got 104857601", + errorText: "max-file-size must be between 1 and 104857600, got 104857601", }, { name: "invalid large size", maxFileSize: 200000000, wantError: true, - errorText: "max-file-size must be between 1 and 104857600 bytes, got 200000000", + errorText: "max-file-size must be between 1 and 104857600, got 200000000", }, } @@ -445,13 +445,13 @@ func TestRepoMemoryMaxFileSizeValidationArray(t *testing.T) { name: "invalid size in array (zero)", maxFileSize: 0, wantError: true, - errorText: "max-file-size must be between 1 and 104857600 bytes, got 0", + errorText: "max-file-size must be between 1 and 104857600, got 0", }, { name: "invalid size in array (exceeds max)", maxFileSize: 104857601, wantError: true, - errorText: "max-file-size must be between 1 and 104857600 bytes, got 104857601", + errorText: "max-file-size must be between 1 and 104857600, got 104857601", }, } diff --git a/pkg/workflow/validation_helpers.go b/pkg/workflow/validation_helpers.go new file mode 100644 index 0000000000..d99bda2ac7 --- /dev/null +++ b/pkg/workflow/validation_helpers.go @@ -0,0 +1,38 @@ +// Package workflow provides validation helper functions for agentic workflow compilation. +// +// This file contains reusable validation helpers for common validation patterns +// such as integer range validation, used across multiple workflow configuration +// validation functions. +// +// For the validation architecture overview, see validation.go. +package workflow + +import "fmt" + +// validateIntRange validates that a value is within the specified inclusive range [min, max]. +// It returns an error if the value is outside the range, with a descriptive message +// including the field name and the actual value. +// +// Parameters: +// - value: The integer value to validate +// - min: The minimum allowed value (inclusive) +// - max: The maximum allowed value (inclusive) +// - fieldName: A human-readable name for the field being validated (used in error messages) +// +// Returns: +// - nil if the value is within range +// - error with a descriptive message if the value is outside the range +// +// Example: +// +// err := validateIntRange(port, 1, 65535, "port") +// if err != nil { +// return err +// } +func validateIntRange(value, min, max int, fieldName string) error { + if value < min || value > max { + return fmt.Errorf("%s must be between %d and %d, got %d", + fieldName, min, max, value) + } + return nil +} diff --git a/pkg/workflow/validation_helpers_test.go b/pkg/workflow/validation_helpers_test.go new file mode 100644 index 0000000000..3c1f9f519f --- /dev/null +++ b/pkg/workflow/validation_helpers_test.go @@ -0,0 +1,305 @@ +package workflow + +import ( + "strings" + "testing" +) + +// TestValidateIntRange tests the validateIntRange helper function with boundary values +func TestValidateIntRange(t *testing.T) { + tests := []struct { + name string + value int + min int + max int + fieldName string + wantError bool + errorText string + }{ + { + name: "value at minimum", + value: 1, + min: 1, + max: 100, + fieldName: "test-field", + wantError: false, + }, + { + name: "value at maximum", + value: 100, + min: 1, + max: 100, + fieldName: "test-field", + wantError: false, + }, + { + name: "value in middle of range", + value: 50, + min: 1, + max: 100, + fieldName: "test-field", + wantError: false, + }, + { + name: "value below minimum", + value: 0, + min: 1, + max: 100, + fieldName: "test-field", + wantError: true, + errorText: "test-field must be between 1 and 100, got 0", + }, + { + name: "value above maximum", + value: 101, + min: 1, + max: 100, + fieldName: "test-field", + wantError: true, + errorText: "test-field must be between 1 and 100, got 101", + }, + { + name: "negative value below minimum", + value: -1, + min: 1, + max: 100, + fieldName: "test-field", + wantError: true, + errorText: "test-field must be between 1 and 100, got -1", + }, + { + name: "zero when minimum is zero", + value: 0, + min: 0, + max: 100, + fieldName: "test-field", + wantError: false, + }, + { + name: "large negative value", + value: -9999, + min: 1, + max: 100, + fieldName: "test-field", + wantError: true, + errorText: "test-field must be between 1 and 100, got -9999", + }, + { + name: "large positive value exceeding maximum", + value: 999999, + min: 1, + max: 100, + fieldName: "test-field", + wantError: true, + errorText: "test-field must be between 1 and 100, got 999999", + }, + { + name: "single value range (min equals max)", + value: 42, + min: 42, + max: 42, + fieldName: "test-field", + wantError: false, + }, + { + name: "single value range - below", + value: 41, + min: 42, + max: 42, + fieldName: "test-field", + wantError: true, + errorText: "test-field must be between 42 and 42, got 41", + }, + { + name: "single value range - above", + value: 43, + min: 42, + max: 42, + fieldName: "test-field", + wantError: true, + errorText: "test-field must be between 42 and 42, got 43", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateIntRange(tt.value, tt.min, tt.max, tt.fieldName) + + if tt.wantError { + if err == nil { + t.Errorf("Expected error, got nil") + } else if !strings.Contains(err.Error(), tt.errorText) { + t.Errorf("Expected error containing '%s', got '%s'", tt.errorText, err.Error()) + } + } else { + if err != nil { + t.Errorf("Expected no error, got: %v", err) + } + } + }) + } +} + +// TestValidateIntRangeWithRealWorldValues tests validateIntRange with actual constraint values +func TestValidateIntRangeWithRealWorldValues(t *testing.T) { + tests := []struct { + name string + value int + min int + max int + fieldName string + wantError bool + }{ + // Port validation (1-65535) + { + name: "port - minimum valid", + value: 1, + min: 1, + max: 65535, + fieldName: "port", + wantError: false, + }, + { + name: "port - maximum valid", + value: 65535, + min: 1, + max: 65535, + fieldName: "port", + wantError: false, + }, + { + name: "port - zero invalid", + value: 0, + min: 1, + max: 65535, + fieldName: "port", + wantError: true, + }, + { + name: "port - above maximum", + value: 65536, + min: 1, + max: 65535, + fieldName: "port", + wantError: true, + }, + + // Max-file-size validation (1-104857600) + { + name: "max-file-size - minimum valid", + value: 1, + min: 1, + max: 104857600, + fieldName: "max-file-size", + wantError: false, + }, + { + name: "max-file-size - maximum valid", + value: 104857600, + min: 1, + max: 104857600, + fieldName: "max-file-size", + wantError: false, + }, + { + name: "max-file-size - zero invalid", + value: 0, + min: 1, + max: 104857600, + fieldName: "max-file-size", + wantError: true, + }, + { + name: "max-file-size - above maximum", + value: 104857601, + min: 1, + max: 104857600, + fieldName: "max-file-size", + wantError: true, + }, + + // Max-file-count validation (1-1000) + { + name: "max-file-count - minimum valid", + value: 1, + min: 1, + max: 1000, + fieldName: "max-file-count", + wantError: false, + }, + { + name: "max-file-count - maximum valid", + value: 1000, + min: 1, + max: 1000, + fieldName: "max-file-count", + wantError: false, + }, + { + name: "max-file-count - zero invalid", + value: 0, + min: 1, + max: 1000, + fieldName: "max-file-count", + wantError: true, + }, + { + name: "max-file-count - above maximum", + value: 1001, + min: 1, + max: 1000, + fieldName: "max-file-count", + wantError: true, + }, + + // Retention-days validation (1-90) + { + name: "retention-days - minimum valid", + value: 1, + min: 1, + max: 90, + fieldName: "retention-days", + wantError: false, + }, + { + name: "retention-days - maximum valid", + value: 90, + min: 1, + max: 90, + fieldName: "retention-days", + wantError: false, + }, + { + name: "retention-days - zero invalid", + value: 0, + min: 1, + max: 90, + fieldName: "retention-days", + wantError: true, + }, + { + name: "retention-days - above maximum", + value: 91, + min: 1, + max: 90, + fieldName: "retention-days", + wantError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateIntRange(tt.value, tt.min, tt.max, tt.fieldName) + + if tt.wantError { + if err == nil { + t.Errorf("Expected error for %s=%d, got nil", tt.fieldName, tt.value) + } + } else { + if err != nil { + t.Errorf("Expected no error for %s=%d, got: %v", tt.fieldName, tt.value, err) + } + } + }) + } +}