[repository-quality] 🎯 Repository Quality Improvement Report - Error Message Actionability & Consistency #50563
Closed
Replies: 1 comment
|
This discussion was automatically closed because it expired on 2026-08-06T13:28:58.634Z.
|
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-08-05
Focus Area: Error Message Actionability & Consistency (custom)
Strategy Type: Custom
Custom Area: Yes — the repository already maintains a dedicated
.github/skills/error-messages/SKILL.mdstyle guide and a purpose-builtpkg/linters/errormessageanalyzer, but both are scoped only to changed files in CI. This leaves a large body of pre-existing errors unaudited. This area is highly repo-specific (tailored conventions + custom linter) rather than a generic "Code Quality" pass, so it was selected as a custom focus area.Executive Summary
gh-awenforces an actionable error-message style guide (what's wrong → what's expected → how to fix) and even ships a custom Go analyzer (pkg/linters/errormessage) that flags negative-only wording, unguardedfailed to X: %wwrappers, and missingNewValidationErrorsuggestions. However, the linter only runs against files changed in a given CI diff (--changed-filesflag), so the vast majority of the ~2,600 error-producing call sites inpkg/were never checked against the guide. A repo-widegrepsample shows ~1,129 instances of the genericfmt.Errorf("failed to ...: %w", err)pattern the guide explicitly says to avoid "unless you add recovery guidance," plus 51 error strings that violate the Go convention of lowercase, non-punctuated error text.The greatest opportunity is not writing more prose guidance — the guide is already good — but (1) extending linter coverage from diff-only to a full-repo audit mode usable in scheduled CI or
make lint, and (2) fixing the highest-traffic offending files (pkg/cli/pr_command.go,pkg/cli/trial_repository.go,pkg/workflow/compiler_custom_jobs.go) as exemplars other contributors can pattern-match against. Tackling these in small, file-scoped PRs keeps risk low while meaningfully raising the actionability bar for the CLI's user-facing errors.Full Analysis Report
Focus Area: Error Message Actionability & Consistency
Current State Assessment
The project has strong tooling already:
.github/skills/error-messages/SKILL.mddefines a three-part template (what's wrong / what's expected / how to fix), a preference forNewValidationError(field, value, reason, suggestion)in*_validation.gofiles, and an explicit rule against barefmt.Errorf("failed to X: %w", err)wrapping without recovery guidance.pkg/linters/errormessage/errormessage.goimplementscheckNegativeLanguage,checkFailedToErrorfWrap, andcheckNewValidationSuggestionchecks, but is gated by a--changed-filesflag and is a no-op with no changed files — meaning it never audits the historical codebase, only new diffs.Metrics Collected:
fmt.Errorf(...)calls inpkg/fmt.Errorfcalls without%wwrapping"failed to X: %w"patternerrors.New(...)callsfmt.Errorf) + 25 (errors.New) = 51NewValidationError(...)call sites (validation-specific errors)*_validation.go)pkg/linters/errormessage(diff-scoped only)panic(...)calls in non-test codeFindings
Strengths
error-messagesskill) with good/bad examples and a suggestion-text checklist.pkg/linters/errormessage) already codifies these rules as compile-time checks, integrated withnolintdirective support and generated-file skipping.NewValidationError) are reasonably concentrated in*_validation.gofiles (e.g.,sandbox_validation.gohas 25 uses,network_firewall_validation.gohas 15), showing the pattern is being followed where it matters most (user-facing YAML validation).Areas for Improvement
errormessagelinter is diff-scoped (--changed-filesflag) and silently no-ops without it — there is no full-repo/CI baseline audit mode, so ~1,100+ pre-existing unguarded"failed to X: %w"errors are invisible to tooling.fmt.Errorf/errors.Newstart with a capital letter (e.g.,pkg/workflow/awf_config.go:124,pkg/cli/mcp_registry.go:99-107,pkg/cli/pr_helpers.go:20), violating both Go's error-string convention (lowercase, no trailing punctuation) and readability when wrapped by callers.pkg/cli/pr_command.go(41Errorfcalls),pkg/cli/trial_repository.go(34 calls, several like"failed to clone host repository %s: %w (output: %s)"with no next-step hint), andpkg/workflow/compiler_custom_jobs.go(31 calls, e.g."failed to convert runs-on to YAML for job '%s': %w").panic(...)calls remain in non-test code; some (e.g.pkg/workflow/model_aliases.go:60) already include a good actionable message ("BUG: ... (try 'make build' to rebuild...)") — a pattern worth extending to the rest.Detailed Analysis
The
errormessagelinter'scheckFailedToErrorfWrapfunction already detects the exact anti-pattern found 1,129 times in the codebase, but its--changed-filesgate means it currently only prevents new regressions rather than remediating existing debt. Making a full-repo audit mode available (e.g.,--changed-files=allor a separatego vet/golangci-linttarget with--full-repo) would let maintainers track remediation progress as a metric over time, and could be wired intomake lintas a non-blocking report initially.Convention violations (uppercase-starting error strings) are concentrated in
pkg/cli/mcp_registry.go(5 occurrences in a 10-line span, lines 99-107) and are easy, mechanical, low-risk fixes — good candidates for a quick top-down pass with thego-codemodskill if a repeatable pattern emerges (e.g., "any string literal passed to errors.New/fmt.Errorf starting uppercase → lowercase first rune").🤖 Tasks for Copilot Agent
NOTE TO PLANNER AGENT: Split the following tasks into individual work items.
Improvement Tasks
Task 1: Add full-repo audit mode to the
errormessagelinterPriority: High
Estimated Effort: Medium
Focus Area: Error Message Actionability & Consistency
Description: The
pkg/linters/errormessageanalyzer only runs when a--changed-fileslist is provided (seerun()inpkg/linters/errormessage/errormessage.go), and is a silent no-op otherwise. Add a mode (e.g., a--changed-files=allsentinel or a new--full-repoflag) that scans every non-generated, non-test Go file inpkg/, so the existing checks (checkNegativeLanguage,checkFailedToErrorfWrap,checkNewValidationSuggestion) can be run as a repo-wide report (non-blocking initially) to establish a baseline and track remediation over time.Acceptance Criteria:
pkg/linters/errormessage/errormessage.gothat bypasses the "no changed files → no-op" early return and checks all applicable filesshouldCheckFileupdated or bypassed appropriately for full-repo mode without breaking the existing diff-scoped behaviormaketarget or CI step documented (non-blocking) to run the audit and print a summary count of violationspkg/linters/errormessage/errormessage_test.gocovering the new full-repo modego-lintersskill guidance followed for analyzer conventionsCode Region:
pkg/linters/errormessage/errormessage.go(functionsrun,parseChangedFiles,shouldCheckFile)Task 2: Fix uppercase-starting error strings in
pkg/cli/mcp_registry.goand related filesPriority: Medium
Estimated Effort: Small
Focus Area: Error Message Actionability & Consistency
Description: Multiple error strings violate Go's convention that error strings should not be capitalized (they get wrapped/concatenated by callers).
pkg/cli/mcp_registry.golines 99-107 have five consecutive violations ("MCP registry access forbidden (403): %s...","MCP registry access unauthorized (401): %s...", etc.). Similar issues exist inpkg/workflow/awf_config.go:124,pkg/workflow/schema_validation.go:108,pkg/cli/forecast_resolution.go:149,pkg/cli/mcp_validation.go:85,99,111,pkg/cli/pr_helpers.go:20,pkg/cli/run_workflow_execution.go:88,pkg/cli/enable.go:73, and others found viagrep -rnoP 'fmt\.Errorf\("\K[A-Z]' --include=*.go pkg/andgrep -rnP 'errors\.New\("\K[A-Z]' --include=*.go pkg/.Acceptance Criteria:
make fmtand existingerrormessage/golangci-lintchecks to confirm no regressionsnolintdirectives only if a proper noun exception genuinely can't be rewordedCode Region:
pkg/cli/mcp_registry.go:99-107,pkg/workflow/awf_config.go:124,pkg/cli/mcp_validation.go:85,99,111,pkg/cli/pr_helpers.go:20Find and fix Go error-string convention violations (error strings should start with a lowercase letter and have no trailing punctuation, per Go's official style guidance and this repo's error-messages skill). Focus first on pkg/cli/mcp_registry.go lines 99-107, which contain five fmt.Errorf calls starting with "MCP registry ..." — lowercase "MCP registry" only if not treated as a proper noun exception; if "MCP" must stay capitalized as an acronym, that's fine, but ensure the overall string still reads naturally when wrapped (e.g., "mcp registry access forbidden (403): %s..."). Also fix: pkg/workflow/awf_config.go:124 ("AWF config schema validation failed: %w"), pkg/workflow/schema_validation.go:108 ("GitHub Actions schema validation failed: %w"), pkg/cli/mcp_validation.go:85,99,111 ("GitHub token required..."), and pkg/cli/pr_helpers.go:20 ("GitHub CLI (gh) is required for PR creation but not available"). Use `grep -rnoP 'fmt\.Errorf\("\K[A-Z][^"]*' --include=*.go pkg/` and `grep -rnP 'errors\.New\("\K[A-Z]' --include=*.go pkg/` to find the complete list (51 total across fmt.Errorf and errors.New), excluding _test.go files, and fix all of them consistently. Run `go build ./...` and existing unit tests after changes to confirm no test assertions depended on the old casing.Task 3: Add recovery guidance to high-traffic bare
"failed to X: %w"errors inpkg/cli/trial_repository.goPriority: Medium
Estimated Effort: Medium
Focus Area: Error Message Actionability & Consistency
Description:
pkg/cli/trial_repository.gohas 34fmt.Errorfcalls, several of which are bare"failed to X: %w"wrappers around git/gh operations with no recovery guidance, violating the error-messages skill's rule to avoid generic wrappers "unless you add recovery guidance." Examples: line 108"failed to force delete existing host repository %s: %w (output: %s)", line 165"failed to create host repository: %w (output: %s)", line 214"failed to delete host repository: %w (output: %s)", line 243"failed to clone host repository %s: %w (output: %s)".Acceptance Criteria:
pkg/linters/errormessage(run in diff mode against this file) passes without new violationspkg/cli/trial_repository_test.go(if present) still pass; add/update test assertions on error text where tests check exact stringsCode Region:
pkg/cli/trial_repository.go:108,165,214,243,257(and otherfmt.Errorf("failed to ...calls in this file)Task 4: Add recovery guidance to
pkg/workflow/compiler_custom_jobs.goYAML-conversion errorsPriority: Low
Estimated Effort: Small
Focus Area: Error Message Actionability & Consistency
Description:
pkg/workflow/compiler_custom_jobs.gohas 31fmt.Errorfcalls, several following the pattern"failed to convert X to YAML for job '%s': %w"(e.g., lines 207, 258, 315, 357 forstrategy,runs-on,concurrency, andcontainerrespectively) with no guidance on what causes a YAML conversion failure or how a workflow author should fix their frontmatter.Acceptance Criteria:
NewValidationErrorinstead offmt.Errorfif the error is a genuine user-facing configuration mistake (per the skill's guidance:NewValidationErrorfor*_validation.go-style logic,fmt.Errorffor operational/wrapping errors)make recompilerun on any.github/workflows/*.mdfiles exercising custom jobs to confirm no regressionsCode Region:
pkg/workflow/compiler_custom_jobs.go:53,207,258,315,357📊 Historical Context
Previous Focus Areas
🎯 Recommendations
Immediate Actions (This Week)
pkg/linters/errormessage— Priority: HighShort-term Actions (This Month)
pkg/cli/trial_repository.gotop offenders — Priority: MediumLong-term Actions (This Quarter)
📈 Success Metrics
Next Steps
Generated by Repository Quality Improvement Agent
Next analysis: 2026-08-06 — Focus area selected by diversity algorithm
All reactions