🎯 Repository Quality Improvement Report — Validation Ecosystem Health

Analysis Date: 2026-04-02
Focus Area: Validation Ecosystem Health
Strategy Type: Custom
Custom Area: Yes — gh-aw has an unusually large validation subsystem (47 files in pkg/workflow/) that is central to its correctness guarantees. Periodic health checks on this subsystem catch structural drift, documentation gaps, and test coverage holes before they compound.

Executive Summary

The pkg/workflow/ validation subsystem is one of the most critical parts of gh-aw: it protects users from invalid or dangerous workflow configurations. With 47 dedicated validator files, the ecosystem is mature and well-structured. However, several validators have grown beyond the project's documented hard limit of 300 lines, and a significant number of large validators lack dedicated unit test files. Comment coverage also falls below the 30% minimum threshold in several files.

The good news: the overall test infrastructure is healthy — there are 48 test files covering validation logic, and the naming conventions are consistent. The issues are concentrated in a small number of oversized files and gaps in standalone unit tests for important validator functions. Addressing these will improve maintainability and make the validator logic easier for contributors to navigate and extend.

🤖 Tasks for Copilot Agent

NOTE TO PLANNER AGENT: The following 5 tasks are designed for GitHub Copilot coding agent execution. Please split these into individual work items for processing. Each task is self-contained with clear acceptance criteria and a specific code region.

Improvement Tasks

Task 1: Split safe_outputs_validation.go into two focused files

Priority: High
Estimated Effort: Small
Focus Area: Validation file size / code organization

Description:
safe_outputs_validation.go is 407 lines and contains two distinct concerns: (1) network/domain validation (validateNetworkAllowedDomains, validateSafeOutputsAllowedDomains, validateDomainPattern, isEcosystemIdentifier) and (2) safe-outputs target validation (validateSafeOutputsTarget, validateTargetValue, isGitHubExpression). Per AGENTS.md guidelines, the hard limit is 300 lines and files with 2+ distinct domains should be split.

Acceptance Criteria:

• Create safe_outputs_network_validation.go with network/domain validation functions
• Create safe_outputs_target_validation.go (or extend existing safe_outputs_target_validation_test.go's source) with target validation functions
• All existing tests continue to pass (make test-unit)
• Both resulting files are under 250 lines
• Both files have at least 30% comment coverage

Code Region: pkg/workflow/safe_outputs_validation.go

Split `pkg/workflow/safe_outputs_validation.go` (407 lines) into two files following the project's validation refactoring guidelines in AGENTS.md.

The file contains two distinct concerns:
1. **Network/domain validation**: `validateNetworkAllowedDomains`, `validateSafeOutputsAllowedDomains`, `validateDomainPattern`, `isEcosystemIdentifier` — move to `safe_outputs_network_validation.go`
2. **Target validation**: `validateSafeOutputsTarget`, `validateTargetValue`, `isGitHubExpression` — move to a new or existing target validation file (check if `safe_outputs_target_validation_test.go` exists and align with it)

Requirements:
- Keep `package workflow` in both files
- Ensure all existing tests pass: `go test ./pkg/workflow/... -run "TestSafeOutputs" -v`
- Each file should be under 250 lines
- Add section comments describing the purpose of each file
- Run `make fmt` after changes
- Follow the naming convention `{domain}_{subdomain}_validation.go`

Task 2: Split dispatch_workflow_validation.go into dispatch logic and file-discovery helpers

Priority: High
Estimated Effort: Small
Focus Area: Validation file size / code organization

Description:
dispatch_workflow_validation.go is 363 lines with two clear concerns: (1) the high-level validation logic (validateDispatchWorkflow) and (2) workflow file discovery utilities (extractWorkflowDispatchInputs, getCurrentWorkflowName, isPathWithinDir, findWorkflowFile, mdHasWorkflowDispatch, extractMDWorkflowDispatchInputs, containsWorkflowDispatch). The file discovery helpers are generic enough to warrant their own file.

Acceptance Criteria:

• Create dispatch_workflow_helpers.go containing workflow file discovery helper functions
• dispatch_workflow_validation.go reduced to under 200 lines (pure validation logic)
• All existing tests pass (go test ./pkg/workflow/... -run "TestDispatch")
• Both files have at least 30% comment coverage

Code Region: pkg/workflow/dispatch_workflow_validation.go

Split `pkg/workflow/dispatch_workflow_validation.go` (363 lines) into two files:

1. Keep `dispatch_workflow_validation.go` with only the validation function `validateDispatchWorkflow` (lines 18-163) and the `containsWorkflowDispatch` helper (lines 361-end)
2. Create `dispatch_workflow_helpers.go` with the workflow file discovery utilities: `extractWorkflowDispatchInputs`, `getCurrentWorkflowName`, `isPathWithinDir`, `findWorkflowFile`, `mdHasWorkflowDispatch`, `extractMDWorkflowDispatchInputs`

Requirements:
- Both files use `package workflow`
- Add a file-level doc comment to each file explaining its purpose
- All existing tests must pass: `go test ./pkg/workflow/... -run "TestDispatch" -v`
- Run `make fmt` after changes

Task 3: Add dedicated unit tests for permissions_validation.go

Priority: Medium
Estimated Effort: Medium
Focus Area: Test coverage for critical validation logic

Description:
permissions_validation.go (351 lines) contains critical permission validation logic (ValidatePermissions, checkMissingPermissions, FormatValidationMessage, ValidateIncludedPermissions) but has no dedicated test file. While permissions are tested indirectly through compiler integration tests (e.g., permissions_validator_test.go), the standalone functions in permissions_validation.go would benefit from unit tests that directly exercise edge cases.

Acceptance Criteria:

• Create pkg/workflow/permissions_validation_test.go with //go:build !integration tag
• Tests cover ValidatePermissions with at least: nil permissions, missing required permissions, sufficient permissions
• Tests cover FormatValidationMessage with strict and non-strict mode
• Tests cover ValidateIncludedPermissions with valid and invalid YAML
• All new tests pass: go test ./pkg/workflow/... -run "TestValidatePermissions|TestFormatValidationMessage|TestValidateIncludedPermissions"

Code Region: pkg/workflow/permissions_validation.go

Add a dedicated unit test file `pkg/workflow/permissions_validation_test.go` for the standalone validation functions in `permissions_validation.go`.

The file exports these testable functions:
- `ValidatePermissions(permissions *Permissions, githubTool ValidatableTool) *PermissionsValidationResult`
- `FormatValidationMessage(result *PermissionsValidationResult, strict bool) string`
- `ValidateIncludedPermissions(topPermissionsYAML string, importedPermissionsJSON string) error` (on *Compiler)

Test requirements:
1. Add `//go:build !integration` as the FIRST LINE of the test file
2. Use table-driven tests with `t.Run()`
3. Use `require.*` for critical setup and `assert.*` for validations
4. Cover: nil inputs, missing permissions, sufficient permissions, strict vs non-strict mode
5. For `FormatValidationMessage`: test that the output contains expected permission names
6. Run `make lint` after to catch any unused helpers

Run validation: `go test ./pkg/workflow/... -run "TestValidatePermissions" -v`

Task 4: Improve comment coverage in expression_safety_validation.go

Priority: Medium
Estimated Effort: Small
Focus Area: Documentation quality

Description:
expression_safety_validation.go (304 lines, 12% comments) is one of the most comment-sparse validators in the codebase. It handles security-critical template injection and expression safety validation. Given its security importance and complexity, it should have at minimum 30% comment coverage as required by AGENTS.md, with clear explanations of why certain patterns are dangerous.

Acceptance Criteria:

• Comment coverage increased to at least 30%
• Each exported function has a godoc comment
• Each non-obvious regex pattern or security check has an explanatory comment
• Comments explain the security rationale (not just what the code does, but why)
• make lint passes after changes

Code Region: pkg/workflow/expression_safety_validation.go

Add comprehensive documentation comments to `pkg/workflow/expression_safety_validation.go` to bring comment coverage from 12% to at least 30%.

Focus on:
1. File-level package comment explaining the security purpose of this validator
2. Godoc comments for every exported function
3. Inline comments for non-obvious regex patterns — explain what attack vector they prevent
4. Comments for complex conditional logic explaining the security invariant being enforced
5. Section comments grouping related validation rules

Style requirements:
- Comments should explain the "why" (security rationale), not just the "what"
- Keep each comment concise (1-3 lines)
- Use imperative mood for function docs ("validates", "returns", "checks")
- Do NOT add comments to trivial lines (e.g., `return nil`)

After changes: run `make fmt && make lint` to verify no issues.

Task 5: Add dedicated unit tests for agent_validation.go

Priority: Low
Estimated Effort: Medium
Focus Area: Test coverage

Description:
agent_validation.go (255 lines) validates agent-specific workflow configuration including max turns, continuations, web search support, and workflow run branches. It has no dedicated test file. These validations guard important behavioral limits and engine capability differences, making direct unit testing valuable.

Acceptance Criteria:

• Create pkg/workflow/agent_validation_test.go with //go:build !integration tag
• Tests cover validateMaxTurnsSupport for engines that do/don't support it
• Tests cover validateMaxContinuationsSupport for engines that do/don't support it
• Tests cover validateWebSearchSupport with tools that do/don't include web search
• All tests pass: go test ./pkg/workflow/... -run "TestValidateAgent|TestValidateMaxTurns|TestValidateWebSearch"

Code Region: pkg/workflow/agent_validation.go

Add a dedicated unit test file `pkg/workflow/agent_validation_test.go` for the validation functions in `agent_validation.go`.

The *Compiler methods to test:
- `validateMaxTurnsSupport(frontmatter map[string]any, engine CodingAgentEngine) error`
- `validateMaxContinuationsSupport(frontmatter map[string]any, engine CodingAgentEngine) error`
- `validateWebSearchSupport(tools map[string]any, engine CodingAgentEngine)` (emits warnings)

Key test cases:
1. `validateMaxTurnsSupport`: engine supports max_turns + max_turns set → no error; engine doesn't support + max_turns set → error; max_turns not set → no error
2. `validateMaxContinuationsSupport`: similar pattern for continuations
3. `validateWebSearchSupport`: tools with web search + unsupported engine → warning emitted; supported engine → no warning

Requirements:
- `//go:build !integration` as FIRST LINE
- Use `NewCompiler()` to get a Compiler instance for testing
- Table-driven tests with descriptive names
- Run `make lint` after — catch any unused helper functions

📊 Historical Context

🎯 Recommendations

Immediate Actions (This Week)

1. Split safe_outputs_validation.go (407 lines → 2 files) — Priority: High
2. Split dispatch_workflow_validation.go (363 lines → 2 files) — Priority: High

Short-term Actions (This Month)

1. Add unit tests for permissions_validation.go — Priority: Medium
2. Improve comment coverage in expression_safety_validation.go — Priority: Medium

Long-term Actions (This Quarter)

1. Add unit tests for agent_validation.go — Priority: Low
2. Establish a CI check for validator file size (fail if any *_validation.go exceeds 300 lines)

📈 Success Metrics

Track these metrics to measure improvement in Validation Ecosystem Health:

• Files over 300-line limit: 8 → 0 (target)
• Large validators (>150 lines) without tests: 10 → ≤3 (target)
• Validators with <30% comment coverage: ~12 → 0 (target)
• Validation test file count: 48 → 55+ (target after adding 5+ new test files)

Next Steps

1. Review and prioritize the tasks above
2. Assign tasks 1–2 to Copilot coding agent immediately (high priority, small effort)
3. Schedule tasks 3–4 for this sprint (medium priority)
4. Re-evaluate this focus area in 2 weeks after refactoring

References:

• §23902915979

AI generated by Repository Quality Improvement Agent · history

• expires on Apr 3, 2026, 1:38 PM UTC