🎯 Repository Quality Improvement Report — Validation Architecture Compliance

Analysis Date: 2026-04-03
Focus Area: Validation Architecture Compliance
Strategy Type: Custom (first run — no previous history)
Custom Area: Yes — This focus area was chosen because gh-aw has an unusually large and growing validation subsystem (54+ files, 10,296 LOC) with explicit architectural guidelines in AGENTS.md. Verifying compliance with those guidelines is uniquely valuable to this codebase.

Executive Summary

The pkg/workflow/ validation subsystem is one of the most critical parts of gh-aw — it ensures workflows compile correctly and safely. AGENTS.md defines clear architectural rules for these files: a 300-line hard limit, a minimum 30% comment coverage, and a separate test file for each validator. This analysis found that 9 of 54 validators exceed the hard line limit, 24 validators have no dedicated test file, and comment coverage falls well below 30% in the largest files. These gaps represent real maintainability debt and should be addressed incrementally.

The overall testing investment is impressive — 65 validation-related test files exist, and the test-to-source LOC ratio is 2.18:1 across the whole codebase. But the specific validators that are missing tests tend to be the larger, more complex ones (e.g., permissions_validation.go at 351 lines, expression_safety_validation.go at 304 lines), which is the highest-risk gap.

🤖 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.

Task 1: Refactor safe_outputs_validation_config.go (407 lines) into separate files

Priority: High
Estimated Effort: Medium
Focus Area: Validation Architecture Compliance

Description:
pkg/workflow/safe_outputs_validation_config.go is 407 lines with only ~1% comment coverage and a single exported function. Per AGENTS.md, the hard limit is 300 lines. The file contains two distinct concerns: (1) type definitions (FieldValidation, TypeValidationConfig) and (2) the large ValidationConfig map. These should be separated.

Acceptance Criteria:

• File is split so no resulting file exceeds 300 lines
• Type definitions extracted to a clearly named file (e.g., safe_outputs_validation_types.go)
• All existing tests continue to pass: go test ./pkg/workflow/... -run ".*safe_output.*"
• Comment coverage in new files is at least 30%
• make agent-finish passes without errors

Code Region: pkg/workflow/safe_outputs_validation_config.go

Refactor `pkg/workflow/safe_outputs_validation_config.go` (currently 407 lines, exceeding the 300-line hard limit defined in AGENTS.md).

The file has two distinct concerns:
1. Type definitions: `FieldValidation`, `TypeValidationConfig`
2. The `ValidationConfig` map (the bulk of the file)

Split the file into:
- `pkg/workflow/safe_outputs_validation_types.go` — type definitions only, well-commented
- `pkg/workflow/safe_outputs_validation_config.go` — the `ValidationConfig` map, trimmed to under 300 lines with comments

Requirements:
- Both files must stay under 300 lines
- Comment coverage must reach ≥30% per file
- Run `make agent-finish` to validate (all tests must pass)

Task 2: Add a dedicated test file for expression_safety_validation.go

Priority: High
Estimated Effort: Medium
Focus Area: Validation Architecture Compliance

Description:
pkg/workflow/expression_safety_validation.go (304 lines, exceeds 300-line limit) has no dedicated test file. It is responsible for blocking template injection attacks — a critical security validator. Given its security significance and size, it must have comprehensive unit tests.

Acceptance Criteria:

• pkg/workflow/expression_safety_validation_test.go created with //go:build !integration tag
• Tests cover: allowed expressions pass, unauthorized expressions blocked, multi-line expression rejection, fuzzy match suggestions, edge cases (empty content, nested expressions)
• go test -v -run "TestExpression" ./pkg/workflow/ passes
• make lint passes with no new warnings

Code Region: pkg/workflow/expression_safety_validation.go

Create `pkg/workflow/expression_safety_validation_test.go` to add test coverage for the critical expression safety validator.

The validator (`validateExpressionSafety`) checks GitHub Actions expressions in workflow markdown for allowlist compliance to prevent injection attacks.

Tests to write (table-driven style, following existing test conventions):
1. Valid/allowed expressions pass without error (e.g., `$\{\{ steps.foo.outputs.bar }}`, `$\{\{ github.aw.inputs.myInput }}`)
2. Unauthorized expressions return an error (e.g., `$\{\{ github.token }}`, `$\{\{ secrets.MY_SECRET }}`)
3. Multi-line expressions inside `$\{\{ }}` are rejected
4. Fuzzy match suggestions appear in error messages when expression is close to an allowed form
5. Empty/nil content is handled gracefully
6. The `or` fallback pattern (`$\{\{ expr || 'default' }}`) is validated correctly

Add the `//go:build !integration` build tag at the top of the file. Follow the testing conventions in `scratchpad/testing.md`.

Run `go test -v -run "TestExpression" ./pkg/workflow/` and `make lint` to verify.

Task 3: Add a dedicated test file for permissions_validation.go

Priority: High
Estimated Effort: Medium
Focus Area: Validation Architecture Compliance

Description:
pkg/workflow/permissions_validation.go (351 lines, exceeds 300-line limit) has no dedicated test file. It validates GitHub Actions permissions in workflow frontmatter — incorrect permission handling can lead to over-permissioned or broken workflows. The existing permissions_validator_test.go tests only the JSON-schema-based validator; this file's logic needs direct unit tests.

Acceptance Criteria:

• pkg/workflow/permissions_validation_test.go created with //go:build !integration tag
• Tests cover: valid permissions configurations, invalid permission values, permission shorthand validation, missing required permissions, dangerous permissions combinations
• go test -v -run ".*[Pp]ermission.*" ./pkg/workflow/ passes
• make lint passes with no new warnings

Code Region: pkg/workflow/permissions_validation.go

Create `pkg/workflow/permissions_validation_test.go` with unit tests for the permissions validator in `pkg/workflow/permissions_validation.go`.

The validator enforces correct GitHub Actions permission values in workflow frontmatter (e.g., `read`, `write`, `none`).

Write table-driven tests covering:
1. Valid permission configurations pass (`read`, `write`, `none` for various scopes)
2. Invalid permission values return descriptive errors
3. Shorthand permissions (`permissions: read-all`, `permissions: write-all`) are accepted
4. Permission combinations that are flagged as dangerous are caught
5. Empty permissions map is handled correctly

Add the `//go:build !integration` build tag at the top of the file.
Run `go test -v -run ".*[Pp]ermission.*" ./pkg/workflow/` and `make lint` to verify.

Task 4: Increase comment coverage in the 3 largest over-limit validators

Priority: Medium
Estimated Effort: Small
Focus Area: Validation Architecture Compliance

Description:
tools_validation.go (368 lines, 15% comments), dispatch_workflow_validation.go (363 lines, 13% comments), and mcp_config_validation.go (325 lines) all exceed the 300-line limit AND fall well below the 30% comment coverage minimum defined in AGENTS.md. Comments should explain why validations exist, not just what they do.

Acceptance Criteria:

• Each file reaches ≥30% comment coverage (measured as comment lines / total lines)
• Comments explain business rules, security rationale, and non-obvious constraints
• make agent-finish passes without errors
• No functional code changes — documentation only

Code Region: pkg/workflow/tools_validation.go, pkg/workflow/dispatch_workflow_validation.go, pkg/workflow/mcp_config_validation.go

Add documentation comments to three validation files that exceed the 300-line hard limit but currently have very low comment coverage (below the 30% minimum required by AGENTS.md):

1. `pkg/workflow/tools_validation.go` (368 lines, ~15% comments)
2. `pkg/workflow/dispatch_workflow_validation.go` (363 lines, ~13% comments)
3. `pkg/workflow/mcp_config_validation.go` (325 lines)

For each file:
- Add file-level doc comment explaining the validator's purpose and scope
- Add function-level comments for all exported and complex functions
- Add inline comments explaining non-obvious validation rules (security constraints, edge cases, business rules)
- Target: ≥30% comment coverage

No functional changes — documentation only. Run `make agent-finish` to verify tests still pass.

Task 5: Add test files for the strict_mode_* validators

Priority: Medium
Estimated Effort: Small
Focus Area: Validation Architecture Compliance

Description:
Three strict-mode validators have no dedicated test files:

• strict_mode_env_validation.go (147 lines)
• strict_mode_permissions_validation.go (190 lines)
• strict_mode_network_validation.go (133 lines)

Strict mode is a security feature — regressions in these validators could silently weaken security guarantees for users.

Acceptance Criteria:

• Three test files created: strict_mode_env_validation_test.go, strict_mode_permissions_validation_test.go, strict_mode_network_validation_test.go
• Each has //go:build !integration build tag
• Tests cover: strict mode enabled + valid config, strict mode enabled + invalid config, strict mode disabled (no errors), edge cases
• go test -v -run ".*[Ss]trict.*" ./pkg/workflow/ passes
• make lint passes with no new warnings

Code Region: pkg/workflow/strict_mode_env_validation.go, pkg/workflow/strict_mode_permissions_validation.go, pkg/workflow/strict_mode_network_validation.go

Create unit test files for the three untested strict-mode validators in `pkg/workflow/`:

1. `strict_mode_env_validation.go` → `strict_mode_env_validation_test.go`
2. `strict_mode_permissions_validation.go` → `strict_mode_permissions_validation_test.go`
3. `strict_mode_network_validation.go` → `strict_mode_network_validation_test.go`

For each test file:
- Add `//go:build !integration` build tag at the top
- Write table-driven tests covering: strict mode enabled + valid config (no error), strict mode enabled + invalid config (error returned), strict mode disabled (no error regardless of config)
- Use `assert.*` for validations and `require.*` for setup steps
- Include descriptive assertion messages

Run `go test -v -run ".*[Ss]trict.*" ./pkg/workflow/` and `make lint` to verify all tests pass and no unused helpers are flagged.

📊 Historical Context

🎯 Recommendations

Immediate Actions (This Week)

1. Refactor safe_outputs_validation_config.go into type file + config file — Priority: High
2. Add test file for expression_safety_validation.go (security-critical) — Priority: High

Short-term Actions (This Month)

1. Add test files for permissions_validation.go and the three strict_mode_* validators — Priority: High
2. Increase comment coverage in tools_validation.go, dispatch_workflow_validation.go, mcp_config_validation.go — Priority: Medium
3. Review the 12 files approaching 200–300 lines before they exceed the limit — Priority: Medium

Long-term Actions (This Quarter)

1. Establish a CI lint check for validation file sizes (fail if any *_validation.go exceeds 300 lines) — Priority: Low
2. Audit all 24 untested validators systematically, adding test files for each — Priority: Medium

📈 Success Metrics

Track these metrics to measure improvement in Validation Architecture Compliance:

• Files over 300-line limit: 9 → 0
• Validators without test files: 24 → ≤5
• Average comment coverage in validation files: ~10% → ≥30%
• Files approaching limit (200–300 lines): 12 (monitor for growth)

Next Steps

1. Review and prioritize the 5 tasks above
2. Assign Tasks 1–3 (High priority) to Copilot coding agent via planner agent
3. Track progress on improvement items
4. Re-evaluate validation architecture compliance in ~30 days

References:

• §23947382098

AI generated by Repository Quality Improvement Agent · history

• expires on Apr 4, 2026, 1:19 PM UTC