[repository-quality] Repository Quality Improvement — Validation Architecture Compliance (2026-04-27) #28724
Closed
Replies: 1 comment
-
|
This discussion has been marked as outdated by Repository Quality Improvement Agent. A newer discussion is available at Discussion #28945. |
Beta Was this translation helpful? Give feedback.
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
-
Analysis Date: 2026-04-27
Focus Area: Validation Architecture Compliance
Strategy Type: Custom (repository-specific)
Custom Area: Yes — gh-aw's
AGENTS.mddefines explicit size constraints for validator files (target 100–200 lines, hard limit 300 lines). This run audits compliance across all*_validation*.gofiles.Executive Summary
The codebase has clear, documented architectural rules for validator files: a 100–200 line target and a 300-line hard limit, with a decision tree for when to split. However, 8 of ~20 validator files currently exceed the hard limit, ranging from 302 to 440 lines. This creates maintenance burden, reduces test isolation, and silently erodes the architecture guidelines that contributors are expected to follow.
The most critical violation is
safe_outputs_validation_config.go(440 lines), which is almost entirely a large data declaration with one function — a clear candidate to split into a pure config/data file. Other violations likepermissions_validation.goandrepository_features_validation.gomix multiple distinct concerns within a single file.Full Analysis Report
Focus Area: Validation Architecture Compliance
Current State Assessment
AGENTS.md Section "Validation Complexity Guidelines" states:
Metrics Collected:
safe_outputs_validation_config.go(440 lines)Violating Files (sorted by severity):
pkg/workflow/safe_outputs_validation_config.gopkg/workflow/permissions_validation.gopkg/cli/run_workflow_validation.gopkg/workflow/repository_features_validation.gopkg/cli/mcp_validation.gopkg/workflow/engine_validation.gopkg/workflow/network_firewall_validation.gopkg/workflow/expression_safety_validation.goFindings
Strengths
AGENTS.md){domain}_{subdomain}_validation.go) is consistently appliedAreas for Improvement
safe_outputs_validation_config.go: 370+ lines are a largevar ValidationConfigdata map — this is config/data, not validation logic, and should live in its own filepermissions_validation.go: Contains both permission validation and message formatting (FormatValidationMessage,formatMissingPermissionsMessage) — 2 distinct concernsrepository_features_validation.go: Contains GitHub API client calls (getRepositoryFeatures,checkRepositoryHasDiscussions, etc.) mixed with the validation orchestrator — the repository API layer should be separatedengine_validation.go: Contains engine registration (registerInlineEngineDefinition) alongside validation — registration is a side-effect, not validationDetailed Analysis
The split decision tree from AGENTS.md is clear: "File > 300 lines?" → Should split. All 8 files above should have been refactored by this guideline. The root cause is likely organic growth — validators gain complexity as new engine types, tools, and features are added.
The most impactful quick win is
safe_outputs_validation_config.go: the hugeValidationConfigvar (lines 46–416) is pure data and has nothing in common with validation logic. Extracting it tosafe_outputs_types_config.goor similar would immediately bring the file into compliance and improve navigability.For
permissions_validation.go, message formatting is a presentation concern that doesn't belong in validation logic — it should live in apermissions_validation_messages.goor be moved to the console package.🤖 Tasks for Copilot Agent
NOTE TO PLANNER AGENT: The following tasks are designed for GitHub Copilot coding agent execution. Please split these into individual work items for Copilot to process one at a time.
Improvement Tasks
Task 1: Extract
ValidationConfigdata fromsafe_outputs_validation_config.goPriority: High
Estimated Effort: Small
Focus Area: Validation Architecture Compliance
Description:
pkg/workflow/safe_outputs_validation_config.gois 440 lines but has only 1 function. Lines 46–416 are a largevar ValidationConfig = map[string]TypeValidationConfig{...}data declaration. The type definitions (FieldValidation,TypeValidationConfig) and the data variable should be moved to a new file, leaving only theGetValidationConfigJSONfunction (and its logger) in the original file.Acceptance Criteria:
pkg/workflow/safe_outputs_validation_types.gocontaining theFieldValidationandTypeValidationConfigtype definitions and theValidationConfigvariablesafe_outputs_validation_config.goretains only the logger, constants, andGetValidationConfigJSONfunctionsafe_outputs_validation_config.gois under 100 lines after the splitmake buildandmake test-unitpass without errorsmake fmtbefore committingCode Region:
pkg/workflow/safe_outputs_validation_config.goTask 2: Split
permissions_validation.go— separate message formattingPriority: High
Estimated Effort: Small
Focus Area: Validation Architecture Compliance
Description:
pkg/workflow/permissions_validation.go(351 lines) contains both validation logic and message formatting functions (FormatValidationMessage,formatMissingPermissionsMessage). Per AGENTS.md, files with 2+ unrelated validation domains should be split. Message formatting is a presentation concern separate from validation logic.Acceptance Criteria:
pkg/workflow/permissions_validation_messages.gocontainingFormatValidationMessageandformatMissingPermissionsMessagepermissions_validation.goretainsValidatePermissions,checkMissingPermissions, andValidateIncludedPermissionsmake buildandmake test-unitpassmake fmtbefore committingCode Region:
pkg/workflow/permissions_validation.goTask 3: Split
repository_features_validation.go— separate API layer from validationPriority: Medium
Estimated Effort: Medium
Focus Area: Validation Architecture Compliance
Description:
pkg/workflow/repository_features_validation.go(333 lines, 8 functions) mixes two distinct concerns: (1) GitHub API calls to check repository features (getRepositoryFeatures,checkRepositoryHasDiscussions,checkRepositoryHasIssues, and their uncached variants, plusgetCurrentRepository), and (2) the validation orchestrator (validateRepositoryFeatures). The API layer should be separated into its own file.Acceptance Criteria:
pkg/workflow/repository_features_client.gocontaining the GitHub API helper functions (getCurrentRepository,getCurrentRepositoryUncached,getRepositoryFeatures,checkRepositoryHasDiscussions,checkRepositoryHasDiscussionsUncached,checkRepositoryHasIssues,checkRepositoryHasIssuesUncached)repository_features_validation.goretains onlyvalidateRepositoryFeaturesand theRepositoryFeaturestypemake buildandmake test-unitpassmake fmtbefore committingCode Region:
pkg/workflow/repository_features_validation.goTask 4: Split
engine_validation.go— separate engine registration from validationPriority: Medium
Estimated Effort: Small
Focus Area: Validation Architecture Compliance
Description:
pkg/workflow/engine_validation.go(316 lines, 6 functions) includesregisterInlineEngineDefinition, which is a side-effect registration operation, not a validation function. This mixes registration and validation concerns. Extracting registration to its own file will bringengine_validation.gounder the 300-line limit and clarify responsibilities.Acceptance Criteria:
registerInlineEngineDefinition(and any closely related registration helpers) topkg/workflow/engine_registration.goengine_validation.gois under 300 linesmake buildandmake test-unitpassmake fmtbefore committingCode Region:
pkg/workflow/engine_validation.go📊 Historical Context
Previous Focus Areas
🎯 Recommendations
Immediate Actions (This Week)
ValidationConfigdata fromsafe_outputs_validation_config.go— Priority: High (easiest win, purely mechanical)permissions_validation.go— Priority: High (clear domain boundary exists)Short-term Actions (This Month)
repository_features_validation.go— Priority: Medium (higher impact but needs careful API boundary review)engine_validation.go— Priority: Medium (small change, removes side-effect from validator)Long-term Actions (This Quarter)
*_validation*.gofile exceeds 300 lines — prevents regressionnetwork_firewall_validation.goat 305,expression_safety_validation.goat 302) for potential minor refactors📈 Success Metrics
Track these metrics to measure improvement in Validation Architecture Compliance:
Next Steps
References:
Beta Was this translation helpful? Give feedback.
All reactions