🎯 Repository Quality Improvement Report — Validation Error Architecture Consistency

[github-actions[bot]]

Analysis Date: 2026-02-20
Focus Area: Validation Error Architecture Consistency (Custom)
Strategy Type: Custom — first run, repository-specific focus area
Rationale: gh-aw has 45+ dedicated validation files in pkg/workflow/ and a well-designed WorkflowValidationError struct with structured fields (Field, Value, Reason, Suggestion). However, the majority of validation files still use raw fmt.Errorf() instead of this structured type, creating inconsistent error quality and missed opportunities for helpful, actionable feedback to workflow authors.

Executive Summary

gh-aw has invested in a strong validation infrastructure — the WorkflowValidationError type (pkg/workflow/error_helpers.go) provides structured errors with field context, reason text, and a Suggestion field. Yet only ~52 of the 230+ non-chained errors in pkg/workflow/ use this structured type. Many validation files with 10+ error returns (strict_mode_validation.go, mcp_config_validation.go, dispatch_workflow_validation.go, repository_features_validation.go) still use unstructured fmt.Errorf.

The impact is real: workflow authors see inconsistent error quality depending on which validation path triggered — some errors include "To fix:" sections and URLs, while others provide only a terse single-line message. Migrating the highest-traffic validation files to NewValidationError would standardize the developer experience and make errors easier to test, localize, and extend.

Full Analysis Report

Focus Area: Validation Error Architecture Consistency

Current State Assessment

Metrics Collected:

Metric	Value	Status
Validation files in pkg/workflow/	45	✅ Well-organized
NewValidationError usages	52	⚠️ Underutilized
Raw fmt.Errorf in validation files	~230	⚠️ Inconsistent
Validation files with 0 structured errors but 5+ fmt.Errorf	6	❌ Needs migration
Error messages with "Suggestion" field populated	~52	⚠️ Minority

Files with Highest Migration Opportunity:

File	fmt.Errorf Count	NewValidationError Count
dispatch_workflow_validation.go	14	0
strict_mode_validation.go	12	0
mcp_config_validation.go	10	0
repository_features_validation.go	10	0
engine_validation.go	5	0
schema_validation.go	6	0

Files Already Fully Using Structured Errors (Exemplars):

• concurrency_validation.go — 10/10 structured
• sandbox_validation.go — 4/4 structured
• runtime_validation.go — 4/4 structured

Findings

Strengths

• WorkflowValidationError is well-designed with Field, Value, Reason, Suggestion, and Timestamp fields
• compiler_error_formatter.go provides file-position-aware error formatting with formatCompilerError()
• Several validation files (concurrency, sandbox, runtime) already demonstrate full adoption
• Many raw fmt.Errorf messages in dispatch_workflow_validation.go already contain inline "To fix:" sections — good intent, just needs structured form

Areas for Improvement

• High: 6 high-traffic validation files have zero structured errors despite 5–14 error returns each
• High: The Suggestion field of WorkflowValidationError is only useful if populated — raw fmt.Errorf messages that embed suggestions inline bypass this entirely
• Medium: No test exists to assert that validation errors include non-empty suggestion text — regressions are undetectable
• Medium: strict_mode_validation.go embeds documentation URLs at the end of raw error strings; these could be standardized into a DocURL field or appended uniformly

Detailed Analysis

The inconsistency has a clear historical pattern: the WorkflowValidationError type was added after many validation files were written. New files like concurrency_validation.go adopted the structured approach; older files like strict_mode_validation.go were not updated.

The dispatch_workflow_validation.go file is an interesting case — it already has very high-quality error messages with embedded "To fix:" multi-line blocks. Migrating these to NewValidationError would put the suggestion content into the dedicated Suggestion field, making them programmatically testable while preserving the content.

Example of inconsistency (current state):

// In strict_mode_validation.go (unstructured)
return fmt.Errorf("strict mode: write permission '%s: write' is not allowed for security reasons. Use 'safe-outputs...' See: (redacted)")

// In concurrency_validation.go (structured — exemplar)
return NewValidationError("concurrency", value, "concurrency key cannot be empty", "provide a non-empty string")

---

🤖 Tasks for Copilot Agent

NOTE TO PLANNER AGENT: The following 4 tasks are designed for GitHub Copilot coding agent execution. Please split these into individual work items for Copilot to process. Each task is self-contained and targets specific files.

---

Task 1: Migrate strict_mode_validation.go to Structured Validation Errors

Priority: High
Estimated Effort: Medium
Focus Area: Validation Error Architecture Consistency
Code Region: pkg/workflow/strict_mode_validation.go

Description:
strict_mode_validation.go has 12 fmt.Errorf error returns that validate user-authored workflow frontmatter. These should be migrated to use NewValidationError(field, value, reason, suggestion) for consistency with the rest of the validation system. Each error message already contains a reason and often a documentation URL — extract these into the Reason and Suggestion fields.

Acceptance Criteria:

• All user-facing validation errors in strict_mode_validation.go use NewValidationError()
• Internal/operational errors (e.g., "internal error: network permissions not initialized") remain as fmt.Errorf (these are not user validation errors)
• The Suggestion field is non-empty for every migrated error
• Documentation URLs, where present in the original messages, are preserved (either in the Suggestion text or as a separate sentence)
• All existing tests for strict mode validation continue to pass
• Run make test-unit and make lint with no new failures

Migrate the user-facing validation errors in `pkg/workflow/strict_mode_validation.go` to use the structured `NewValidationError(field, value, reason, suggestion string)` function from `pkg/workflow/error_helpers.go`.

Currently the file has 12 `fmt.Errorf` calls for user validation errors. For each one:
1. Identify the field name (e.g., "permissions", "network.allowed", "sandbox.agent")
2. Extract the current value being validated (pass as second argument, or "" if not applicable)
3. Put the core reason in the `reason` parameter (why the value is invalid)
4. Put the fix/suggestion in the `suggestion` parameter (what the user should do instead)

Keep `fmt.Errorf` only for internal/operational errors (ones that start with "internal error:" or wrap another error with `%w`).

Example transformation:
```go
// Before
return fmt.Errorf("strict mode: write permission '%s: write' is not allowed for security reasons. Use 'safe-outputs.create-issue'... See: (redacted)")

// After
return NewValidationError(
    fmt.Sprintf("permissions.%s", scope),
    "write",
    "write permissions are not allowed in strict mode for security reasons",
    "Use 'safe-outputs.create-issue', 'safe-outputs.create-pull-request', 'safe-outputs.add-comment', or 'safe-outputs.update-issue' to perform write operations safely. See: https://github.github.com/gh-aw/reference/safe-outputs/",
)
```

After migration, run `make test-unit` and `make lint` to verify no regressions.

---

Task 2: Migrate mcp_config_validation.go to Structured Validation Errors

Priority: High
Estimated Effort: Medium
Focus Area: Validation Error Architecture Consistency
Code Region: pkg/workflow/mcp_config_validation.go

Description:
mcp_config_validation.go has 10 fmt.Errorf error returns that validate MCP server configuration in workflow frontmatter. These should be migrated to NewValidationError() following the same pattern used in concurrency_validation.go (exemplar). The structured errors will improve user feedback quality when workflow authors misconfigure MCP servers.

Acceptance Criteria:

• All user-facing validation errors in mcp_config_validation.go use NewValidationError()
• The Field parameter accurately identifies the MCP configuration field being validated (e.g., "tools.mcp-server-name.url", "tools.mcp-server-name.type")
• The Suggestion field is non-empty for every migrated user-facing error
• Errors that wrap underlying Go errors (using %w) remain as fmt.Errorf
• All existing tests pass; run make test-unit and make lint

Migrate user-facing validation errors in `pkg/workflow/mcp_config_validation.go` to use `NewValidationError(field, value, reason, suggestion string)` from `pkg/workflow/error_helpers.go`.

The file currently has 10 `fmt.Errorf` calls. For each user-facing error:
- Set `field` to the MCP configuration path being validated (e.g., `"tools."+serverName+".url"`)
- Set `value` to the invalid value (or empty string if N/A)
- Set `reason` to a concise explanation of why the value is invalid
- Set `suggestion` to an actionable fix

Keep `fmt.Errorf` for errors that wrap underlying errors with `%w` (these are operational, not validation errors).

Reference `pkg/workflow/concurrency_validation.go` as a model for how to structure the migration.

After migration, run `make test-unit` and `make lint` to verify no regressions.

---

Task 3: Add Validation Quality Test — Ensure Suggestion Fields Are Non-Empty

Priority: Medium
Estimated Effort: Small
Focus Area: Validation Error Architecture Consistency
Code Region: pkg/workflow/ (new test file)

Description:
There is no test that verifies WorkflowValidationError instances have non-empty Suggestion fields. This means future regressions (creating validation errors with empty suggestions) are invisible. Add a test that asserts all validation errors returned by the key validation functions include a non-empty suggestion — creating a quality gate.

Acceptance Criteria:

• A new test file pkg/workflow/validation_error_quality_test.go is created
• The test has //go:build !integration build tag
• At least 5 representative validation scenarios are tested (one per major validation file)
• The test asserts that WorkflowValidationError.Suggestion != ""
• The test asserts that WorkflowValidationError.Reason != ""
• Run make test-unit and make lint with no failures

Create `pkg/workflow/validation_error_quality_test.go` with a `//go:build !integration` build tag.

Write table-driven tests that invoke key validation functions (e.g., from `strict_mode_validation.go`, `mcp_config_validation.go`, `concurrency_validation.go`, `sandbox_validation.go`) with invalid inputs and assert that:
1. The returned error is non-nil
2. When the error is a `*WorkflowValidationError`, the `Suggestion` field is non-empty
3. When the error is a `*WorkflowValidationError`, the `Reason` field is non-empty

This creates a regression guard that prevents future contributors from adding validation errors without suggestions.

Example pattern:
```go
//go:build !integration

package workflow

import (
    "errors"
    "testing"
    "github.com/stretchr/testify/assert"
    "github.com/stretchr/testify/require"
)

func TestValidationErrorQuality(t *testing.T) {
    tests := []struct {
        name    string
        errFunc func() error
    }{
        // Add test cases that trigger WorkflowValidationError from various validators
    }
    for _, tt := range tests {
        t.Run(tt.name, func(t *testing.T) {
            err := tt.errFunc()
            require.Error(t, err, "should return an error for invalid input")
            var valErr *WorkflowValidationError
            if errors.As(err, &valErr) {
                assert.NotEmpty(t, valErr.Reason, "validation error should have a reason")
                assert.NotEmpty(t, valErr.Suggestion, "validation error should have a suggestion")
            }
        })
    }
}
```

After creating the file, run `make lint` to check for any unused imports or helper functions, then `make test-unit`.

---

Task 4: Migrate repository_features_validation.go User-Facing Errors to Structured Format

Priority: Medium
Estimated Effort: Small
Focus Area: Validation Error Architecture Consistency
Code Region: pkg/workflow/repository_features_validation.go

Description:
repository_features_validation.go has 10 fmt.Errorf calls. The key user-facing errors (e.g., "workflow uses safe-outputs.create-issue but repository does not have issues enabled") are high-value targets for structured migration — workflow authors encounter these when their repository is misconfigured, and actionable suggestions would help them resolve the issue without searching docs.

Acceptance Criteria:

• User-facing validation errors (repository feature mismatches like missing issues/discussions) use NewValidationError()
• Operational/internal errors that wrap underlying errors (%w) remain as fmt.Errorf
• The Suggestion field provides a clear action (e.g., "Enable issues in repository Settings → Features or remove create-issue from safe-outputs")
• Existing tests continue to pass; run make test-unit and make lint

Migrate user-facing validation errors in `pkg/workflow/repository_features_validation.go` to use `NewValidationError()` from `pkg/workflow/error_helpers.go`.

Focus on the highest-value user-facing errors — the ones that tell workflow authors their repository is missing a required feature (e.g., issues not enabled, discussions not enabled). These are the errors most likely to confuse users who don't know how to resolve them.

For each such error:
- Set `field` to the safe-outputs feature being used (e.g., `"safe-outputs.create-issue"`)
- Set `value` to the repository name
- Set `reason` to why the feature cannot be used (e.g., "repository does not have issues enabled")
- Set `suggestion` to the actionable fix (e.g., "Enable issues in repository Settings → Features, or remove 'create-issue' from your safe-outputs configuration")

Keep `fmt.Errorf` for errors that wrap underlying errors with `%w` (GraphQL failures, parse failures, REST client errors).

After migration, run `make test-unit` and `make lint` to verify no regressions.

---

📊 Historical Context

Previous Focus Areas

Date	Focus Area	Type	Custom	Key Outcomes
2026-02-20	Validation Error Architecture Consistency	Custom	✅ Yes	First run — identified 45+ validation files with inconsistent use of WorkflowValidationError, 4 actionable migration tasks generated

---

🎯 Recommendations

Immediate Actions (This Week)

1. Migrate strict_mode_validation.go (Task 1) — Priority: High. This is the most impactful file since strict mode errors are user-visible security guidance.
2. Migrate mcp_config_validation.go (Task 2) — Priority: High. MCP server configuration errors are common for new workflow authors.

Short-term Actions (This Month)

1. Add validation quality test (Task 3) — Priority: Medium. Creates a lasting quality gate.
2. Migrate repository_features_validation.go (Task 4) — Priority: Medium. Improves DX for users with misconfigured repositories.

Long-term Actions (This Quarter)

1. Complete migration of all remaining validation files (engine, schema, dispatch_workflow, labels, step_order) to NewValidationError
2. Consider adding a DocURL field to WorkflowValidationError for standardized documentation link attachment
3. Add a make lint-validation-errors target to CI that fails if fmt.Errorf is used in *_validation.go files without %w chaining

---

📈 Success Metrics

Track these metrics to measure improvement in validation error consistency:

• Structured error adoption rate: 52 / 282 (18%) → Target: 80%+
• Validation files with 0 structured errors but 5+ fmt.Errorf: 6 → Target: 0
• Validation errors with non-empty Suggestion field: ~52 → Target: all user-facing errors
• Test coverage for validation error quality: 0 tests → Target: 5+ quality assertions

---

Next Steps

1. Review and prioritize the 4 tasks above
2. Assign Tasks 1 & 2 to Copilot coding agent via planner agent (high priority)
3. Track migration progress — aim for 80%+ NewValidationError adoption across all validation files
4. Re-evaluate this focus area in 2 weeks to measure migration progress

References:

• §22226054373

AI generated by Repository Quality Improvement Agent

• expires on Feb 27, 2026, 1:37 PM UTC

[github-actions[bot]]

🤖 The smoke test agent was here! Just swinging by to say hello from the automation pipeline. Everything's running smoothly... or at least we hope so! 🚀✨

📰 BREAKING: Report filed by Smoke Copilot for issue #16927

[github-actions[bot]]

💥 WHOOOOSH! 🦸

KAPOW! The Claude Smoke Test Agent was HERE! 🤖⚡

ZAP! — Swept through gh-aw like a bolt of lightning, testing all systems at warp speed!

BZZZT! All core checks: ✅✅✅✅✅✅✅✅✅✅ — TEN FOR TEN!

POW! Even PR reviews? CRUSHED IT! 💪

...thwip...

— The Smoke Test Agent (Run §22226202391)

💥 [THE END] — Illustrated by Smoke Claude for issue #16927