Repository Quality Improvement Report - Validator File Size Compliance #21398
Replies: 4 comments 1 reply
-
|
👋 The smoke test agent was here! Passing through on a quality verification mission — checking that all systems are go. 🚀✨
|
Beta Was this translation helpful? Give feedback.
-
|
🎉 The smoke test agent returns with a second message — fully caffeinated and reporting for duty! All systems verified operational. The haiku has been dispatched, discussions commented, builds compiled, pages fetched, and files written. The Copilot was here... and it was glorious. 🤖✨🚀
|
Beta Was this translation helpful? Give feedback.
-
|
/plan |
Beta Was this translation helpful? Give feedback.
-
|
This discussion has been marked as outdated by Repository Quality Improvement Agent. A newer discussion is available at Discussion #21596. |
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
-
🎯 Repository Quality Improvement Report - Validator File Size Compliance
Analysis Date: 2026-03-17
Focus Area: Validator File Size Compliance
Strategy Type: Custom (first run — no history to guide selection)
Custom Area: Yes — The AGENTS.md explicitly documents a 300-line hard limit and 100-200 line target for validators in
pkg/workflow/. Analysis found 10 of 50 validator files violating this guideline, with the worst case at 715 lines — 2.4× the hard limit.Executive Summary
The repository enforces a well-documented validator file size policy (100–200 lines target, 300-line hard limit) that has drifted out of compliance in 10 of the 50 validator files. The average validator file sits at 186 lines — well within the target range — demonstrating that most validators are well-managed. However, the 10 oversize files concentrate significant complexity in ways that reduce readability, increase test surface complexity, and make future contributors less confident about where to add new validation logic.
The primary violation is
expression_validation.goat 715 lines (2.4× the limit), which bundles five conceptually distinct concerns: main safety orchestration, dangerous-property checking, runtime import path handling, expression syntax analysis, and quote/brace balancing. Splitting this file would improve clarity with essentially no risk of regression.Two other high-priority targets are
strict_mode_validation.go(546 lines, mixing permissions/network/environment-secret validation concerns) andpermissions_validation.go(477 lines, combining toolset-permissions data loading, validation logic, and message formatting).Full Analysis Report
Focus Area: Validator File Size Compliance
Current State Assessment
Metrics Collected:
expression_validation.go)All Oversized Validators (> 300 lines):
pkg/workflow/expression_validation.gopkg/workflow/strict_mode_validation.gopkg/workflow/permissions_validation.gopkg/workflow/safe_outputs_validation_config.gopkg/workflow/safe_outputs_validation.gopkg/workflow/repository_features_validation.gopkg/workflow/dispatch_workflow_validation.gopkg/workflow/runtime_validation.gopkg/cli/run_workflow_validation.gopkg/workflow/tools_validation.goFindings
Strengths
*_validation.goconventionscache_validation.go39 lines,secrets_validation.go59 lines)validation-refactoring.md) provides clear guidance for splittingAreas for Improvement
expression_validation.go(715 lines): Contains 5 distinct concerns mixed together — expression safety orchestration, dangerous-property logic, runtime import extraction/validation, expression syntax checking, and balanced-brace/quote helpersstrict_mode_validation.go(546 lines): Combines permissions checks, network policy checks, environment secrets validation, and the top-level orchestrator in a single filepermissions_validation.go(477 lines): Mixes toolset data initialization (embedded JSON loading,init()), permissions mapping, validation logic, and message formatting across 10 functionssafe_outputs_validation_config.goandsafe_outputs_validation.goare each 407 lines — their two-file structure suggests a split was attempted but didn't fully resolve the size problemDetailed Analysis
expression_validation.go(715 lines, 11 functions)Natural split into 3 files:
expression_safety_validation.go—validateExpressionSafety(),validateExpressionForDangerousProps(),validateSingleExpression(),containsExpression(),containsLogicalOperators()(~300 lines)expression_syntax_validation.go—validateBalancedBraces(),validateExpressionSyntax(),validateExpressionContent(),validateBalancedQuotes()(~200 lines)runtime_import_validation.go—extractRuntimeImportPaths(),validateRuntimeImportFiles()(~215 lines)strict_mode_validation.go(546 lines, 10 functions)Natural split into 3 files:
strict_mode_permissions_validation.go—validateStrictPermissions(),validateStrictDeprecatedFields(),validateStrictFirewall()(~150 lines)strict_mode_network_validation.go—validateStrictNetwork(),validateStrictMCPNetwork(),validateStrictTools()(~140 lines)strict_mode_env_validation.go—validateEnvSecrets(),getEngineBaseEnvVarKeys(),validateEnvSecretsSection(),validateStrictMode()(orchestrator) (~260 lines)permissions_validation.go(477 lines, 10 functions)Natural split into 2 files:
permissions_toolset_data.go—init(),GitHubToolsetPermissionstype,GitHubToolsetsDatatype,collectRequiredPermissions(),isPermissionSufficient()— data and lookup logic (~200 lines)permissions_validation.go—ValidatePermissions(),checkMissingPermissions(),FormatValidationMessage(),formatMissingPermissionsMessage(),ValidateIncludedPermissions()— active validation logic (~280 lines) → still slightly over; could trim to 250 with formatting helpers moved🤖 Tasks for Copilot Agent
NOTE TO PLANNER AGENT: The following tasks are designed for GitHub Copilot coding agent execution. Each is independently actionable. Please split these into individual work items for processing.
Improvement Tasks
Task 1: Split
expression_validation.gointo three focused filesPriority: High
Estimated Effort: Medium
Focus Area: Validator File Size Compliance
Description:
pkg/workflow/expression_validation.gois 715 lines — 2.4× the 300-line hard limit documented in AGENTS.md. It mixes expression safety orchestration, dangerous-property detection, runtime import path handling, and expression syntax/quote/brace validation. Split it into three files with clear responsibilities.Acceptance Criteria:
expression_safety_validation.gocontains orchestration and allowlist safety checks (validateExpressionSafety,validateExpressionForDangerousProps,validateSingleExpression,containsExpression,containsLogicalOperators)expression_syntax_validation.gocontains syntax-level helpers (validateBalancedBraces,validateExpressionSyntax,validateExpressionContent,validateBalancedQuotes)runtime_import_validation.gocontains import path logic (extractRuntimeImportPaths,validateRuntimeImportFiles)expression_validation.gois removed or reduced to package-level type declarations onlymake test-unitpasses after the splitmake fmt && make lintpassCode Region:
pkg/workflow/expression_validation.goTask 2: Split
strict_mode_validation.gointo focused concern-specific filesPriority: High
Estimated Effort: Medium
Focus Area: Validator File Size Compliance
Description:
pkg/workflow/strict_mode_validation.gois 546 lines — 1.8× the hard limit. It contains four distinct validation concerns: permissions enforcement, network policy, MCP network requirements, tools checking, deprecated fields, and environment secrets validation. These can be cleanly separated.Acceptance Criteria:
strict_mode_permissions_validation.gocontainsvalidateStrictPermissions,validateStrictDeprecatedFields,validateStrictFirewallstrict_mode_network_validation.gocontainsvalidateStrictNetwork,validateStrictMCPNetwork,validateStrictToolsstrict_mode_env_validation.gocontainsvalidateEnvSecrets,getEngineBaseEnvVarKeys,validateEnvSecretsSectionstrict_mode_validation.gois reduced to the orchestratorvalidateStrictModeonly (and any shared types/vars)make test-unitpassesmake fmt && make lintpassCode Region:
pkg/workflow/strict_mode_validation.goTask 3: Split
permissions_validation.go— separate data loading from validation logicPriority: Medium
Estimated Effort: Medium
Focus Area: Validator File Size Compliance
Description:
pkg/workflow/permissions_validation.gois 477 lines — 59% over the limit. It mixes two distinct responsibilities: loading and indexing the embeddedgithub_toolsets_permissions.jsondata, and the active validation/formatting logic. Separating these concerns also makes the data-loading layer independently testable.Acceptance Criteria:
permissions_toolset_data.gocontainsinit(),GitHubToolsetPermissionstype,GitHubToolsetsDatatype,toolsetPermissionsMapvar,GitHubToolConfigmethods,collectRequiredPermissions,isPermissionSufficient— the data layerpermissions_validation.goretainsValidatePermissions,checkMissingPermissions,FormatValidationMessage,formatMissingPermissionsMessage,ValidateIncludedPermissions— the active validation layermake test-unitpassesmake fmt && make lintpassCode Region:
pkg/workflow/permissions_validation.goTask 4: Add compliance check for validator file sizes to CI
Priority: Medium
Estimated Effort: Small
Focus Area: Validator File Size Compliance
Description:
The 300-line hard limit for validators is documented in AGENTS.md but has no automated enforcement. A lightweight shell check in the CI workflow (or Makefile) would prevent future regressions after the current violations are fixed. This could be a simple
find | awkcheck added tomake lintor as a dedicatedmake check-validator-sizestarget.Acceptance Criteria:
check-validator-sizesfinds all*valid*.gofiles inpkg/and fails if any exceed 300 linesmake lintormake agent-finishchainCode Region:
MakefileThen add
check-validator-sizesto thelinttarget's dependencies (or toagent-finish) so it runs automatically.Test by running
make check-validator-sizes. It should currently fail (showing the 10 violations). After Tasks 1-3 are completed, it should pass.Beta Was this translation helpful? Give feedback.
All reactions