Aggregate validation errors across workflow compilation passes#13810
Aggregate validation errors across workflow compilation passes#13810
Conversation
- Updated bundler_runtime_validation.go with fail-fast comment (already uses recursive pattern) - Updated repository_features_validation.go to use ErrorCollector - Updated dispatch_workflow_validation.go to use ErrorCollector for multiple workflows - Updated template_validation.go to use errors.Join for template region errors Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
- Added TestDispatchWorkflowMultipleErrors to verify error aggregation - Added TestDispatchWorkflowMultipleErrorsFailFast to verify fail-fast mode - Added TestValidateNoIncludesInTemplateRegions_MultipleErrors for template validation - Added TestValidateNoIncludesInTemplateRegions_SingleError for single error case - Fixed missing closing braces in validation functions Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR implements multi-error aggregation for workflow validation functions to improve developer feedback loops. Instead of stopping at the first validation error, the compiler now collects all issues and presents them together using Go's errors.Join() and a new ErrorCollector utility.
Changes:
- Added error aggregation to dispatch workflow validation, repository features validation, and template validation
- Implemented
ErrorCollectorutility to handle fail-fast mode and formatted error output - Added comprehensive tests for multi-error scenarios and fail-fast behavior
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/template_validation.go | Aggregates import directive violations across template blocks |
| pkg/workflow/template_include_validation_test.go | Tests multi-error aggregation and single-error behavior for template validation |
| pkg/workflow/repository_features_validation.go | Collects repository capability errors (discussions/issues) using ErrorCollector |
| pkg/workflow/dispatch_workflow_validation.go | Aggregates errors across workflows in dispatch list (self-references, missing files, trigger issues) |
| pkg/workflow/dispatch_workflow_test.go | Tests multi-error and fail-fast modes for dispatch workflow validation |
| pkg/workflow/bundler_runtime_validation.go | Documentation update explaining fail-fast rationale for recursive dependency resolution |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| if !hasIssues { | ||
| return fmt.Errorf("workflow uses safe-outputs.create-issue but repository %s does not have issues enabled. Enable issues in repository settings or remove create-issue from safe-outputs", repo) | ||
| // Continue to return aggregated errors even if this check fails |
There was a problem hiding this comment.
Corrected spelling of 'to' to 'checking'; the comment should read 'Continue checking other features even if this check fails' to match the pattern established in line 134.
| // Continue to return aggregated errors even if this check fails | |
| // Continue checking other features even if this check fails |
| if returnErr := collector.Add(selfRefErr); returnErr != nil { | ||
| return returnErr // Fail-fast mode | ||
| } | ||
| continue // Skip further validation for this workflow |
There was a problem hiding this comment.
Multiple identical comments 'Skip further validation for this workflow' repeated throughout the function create noise. Consider reducing these comments or making them more specific to each validation step (e.g., 'Skip after self-reference check', 'Skip after file read error').
| if returnErr := collector.Add(findErr); returnErr != nil { | ||
| return returnErr // Fail-fast mode | ||
| } | ||
| continue // Skip further validation for this workflow |
There was a problem hiding this comment.
Multiple identical comments 'Skip further validation for this workflow' repeated throughout the function create noise. Consider reducing these comments or making them more specific to each validation step (e.g., 'Skip after self-reference check', 'Skip after file read error').
| if returnErr := collector.Add(notFoundErr); returnErr != nil { | ||
| return returnErr // Fail-fast mode | ||
| } | ||
| continue // Skip further validation for this workflow |
There was a problem hiding this comment.
Multiple identical comments 'Skip further validation for this workflow' repeated throughout the function create noise. Consider reducing these comments or making them more specific to each validation step (e.g., 'Skip after self-reference check', 'Skip after file read error').
| if returnErr := collector.Add(fileReadErr); returnErr != nil { | ||
| return returnErr // Fail-fast mode | ||
| } | ||
| continue // Skip further validation for this workflow |
There was a problem hiding this comment.
Multiple identical comments 'Skip further validation for this workflow' repeated throughout the function create noise. Consider reducing these comments or making them more specific to each validation step (e.g., 'Skip after self-reference check', 'Skip after file read error').
| if returnErr := collector.Add(fileReadErr); returnErr != nil { | ||
| return returnErr // Fail-fast mode | ||
| } | ||
| continue // Skip further validation for this workflow |
There was a problem hiding this comment.
Multiple identical comments 'Skip further validation for this workflow' repeated throughout the function create noise. Consider reducing these comments or making them more specific to each validation step (e.g., 'Skip after self-reference check', 'Skip after file read error').
| if returnErr := collector.Add(compileErr); returnErr != nil { | ||
| return returnErr // Fail-fast mode | ||
| } | ||
| continue // Skip further validation for this workflow |
There was a problem hiding this comment.
Multiple identical comments 'Skip further validation for this workflow' repeated throughout the function create noise. Consider reducing these comments or making them more specific to each validation step (e.g., 'Skip after self-reference check', 'Skip after file read error').
| if returnErr := collector.Add(parseErr); returnErr != nil { | ||
| return returnErr // Fail-fast mode | ||
| } | ||
| continue // Skip further validation for this workflow |
There was a problem hiding this comment.
Multiple identical comments 'Skip further validation for this workflow' repeated throughout the function create noise. Consider reducing these comments or making them more specific to each validation step (e.g., 'Skip after self-reference check', 'Skip after file read error').
| if returnErr := collector.Add(onSectionErr); returnErr != nil { | ||
| return returnErr // Fail-fast mode | ||
| } | ||
| continue // Skip further validation for this workflow |
There was a problem hiding this comment.
Multiple identical comments 'Skip further validation for this workflow' repeated throughout the function create noise. Consider reducing these comments or making them more specific to each validation step (e.g., 'Skip after self-reference check', 'Skip after file read error').
| if returnErr := collector.Add(dispatchErr); returnErr != nil { | ||
| return returnErr // Fail-fast mode | ||
| } | ||
| continue // Skip further validation for this workflow |
There was a problem hiding this comment.
Multiple identical comments 'Skip further validation for this workflow' repeated throughout the function create noise. Consider reducing these comments or making them more specific to each validation step (e.g., 'Skip after self-reference check', 'Skip after file read error').
Validation functions were returning on first error, requiring users to fix issues sequentially. This implements multi-error aggregation using
errors.Join()to surface all validation issues in a single compilation pass.Changes
repository_features_validation.go
ErrorCollectordispatch_workflow_validation.go
template_validation.go
errors.Join()for import directives in template regionsbundler_runtime_validation.go
Example Output
Before:
After:
All implementations respect the existing
--fail-fastflag for users preferring immediate error feedback.Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.