[repository-quality] 🎯 Repository Quality Improvement Report - Custom Go Linter CI Enablement Gap (2026-09-11) #60270
Closed
Replies: 1 comment
|
This discussion was automatically closed because it expired on 2026-09-12T13:13:16.164Z.
|
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.
🎯 Repository Quality Improvement Report - Custom Go Linter CI Enablement Gap (Follow-up)
Analysis Date: 2026-09-11
Focus Area: Custom Go Linter CI Enablement Gap (re-visited from 2026-09-07)
Strategy Type: Reused
Custom Area: No — reused because the 2026-09-07 finding remains unresolved and new evidence sharpens the remediation scope
Executive Summary
On 2026-09-07 this agent found that
pkg/linters/registry.goregisters 68 customgo/analysisanalyzers, but.github/workflows/cgo.yml's hand-typedLINTER_FLAGSallowlist omits 12 of them, so they never gate CI despite being built, registered, and documented. Four days later, that gap is still open:manualpathconcat,packagelevelmutableslicemap, andseenmapbool— three of the twelve omitted analyzers — are absent from both the native and WASMLINTER_FLAGSstrings incgo.yml(lines ~1459 and ~1462), and re-running them today surfaces 60 live findings in current production code that CI has never seen: 49manualpathconcat, 8packagelevelmutableslicemap(mutating shared package state without synchronization — a genuine data-race risk on caches shared across goroutines), and 3seenmapbool(inefficientmap[string]boolset idioms). This confirms the gap is not just cosmetic debt but is actively hiding real defects from every PR and merge-queue run.The remaining nine gapped analyzers (
errorfwrapv,errormessage,excessivefuncparams,hardcodedfilepath,largefunc,lenstringzero,sprintferrdot,ssljson,stringsconcatloop) either require dedicated invocation flags (e.g.errormessageneeds-errormessage.full-repoto run outside the incremental changed-files mode, and surfaced 3,176 pre-existing findings when forced full-repo — too large to gate abruptly) or zero current findings, so this report narrows the actionable remediation to the three analyzers with concrete, CI-appropriate, non-zero, bounded finding counts. Recommended next steps: add the three flags tocgo.yml, fix the 8packagelevelmutableslicemaprace-risk sites first (highest severity), then the 49manualpathconcatand 3seenmapboolfindings, and separately scope a phased enablement plan forerrormessagegiven its 3,176-finding backlog.Full Analysis Report
Focus Area: Custom Go Linter CI Enablement Gap (Follow-up)
Current State Assessment
pkg/linters/registry.godefinesallAnalyzers(68 entries)..github/workflows/cgo.ymlrunsmake golint-custom LINTER_FLAGS="..." -test=falsetwice (native +GOOS=js GOARCH=wasm), passing an explicit, hand-maintained list of-<analyzer>flags. Diffing the registry against bothLINTER_FLAGSstrings shows the same 12-analyzer gap identified on 2026-09-07 is unchanged today:errorfwrapv, errormessage, excessivefuncparams, hardcodedfilepath, largefunc, lenstringzero, manualpathconcat, packagelevelmutableslicemap, seenmapbool, sprintferrdot, ssljson, stringsconcatloopRunning each gapped analyzer directly (via a locally built
cmd/lintersbinary) against./cmd/... ./pkg/...gives:Metrics Collected:
pkg/linters/registry.gocgo.ymlLINTER_FLAGSmanualpathconcatfindings (production code)packagelevelmutableslicemapfindings (data-race risk)seenmapboolfindingserrormessage -full-repofindings (not CI-gateable as-is)largefunc,excessivefuncparams,hardcodedfilepath,lenstringzero,sprintferrdot,ssljson,stringsconcatloop,errorfwrapvfindingsFindings
Strengths
pkg/linters/registry.go,cmd/linters) is sound and theAll()function is the single source of truth other consumers could import from instead of hand-copied flag strings.errormessageincremental mode,hardcodedfilepath,sprintferrdot,ssljson,stringsconcatloop,lenstringzero,excessivefuncparams) currently have zero findings in production code — safe to enable immediately with no remediation debt.Areas for Improvement
packagelevelmutableslicemap(8 findings) flags package-level slice/map globals mutated via re-assignment or index-assignment outside synchronization — e.g.pkg/cli/model_costs.go:50(modelPriceRecords),pkg/cli/update_version_labels.go:73/86(versionLabelCache),pkg/workflow/model_aliases.go:96(builtinOnlyAliasMap). These are genuine concurrent-mutation risks in a compiler package that is exercised across parallel test/compile invocations.manualpathconcat(49 findings) — string-concatenation path building instead offilepath.Join/path.Join, concentrated inpkg/workflow(action_reference.go,compiler_github_actions_steps.go,safe_outputs_actions.go,engine_config_dir.go) andpkg/cli(dispatch.go,checks_command.go,devcontainer.go). Not gated in CI despite the analyzer being registered.seenmapbool(3 findings) —map[string]boolused purely as a set inpkg/cli/experiments_analyze_statistics.go:445andpkg/cli/update_manifest.go:333,346;map[string]struct{}is the idiomatic zero-allocation alternative.errormessage -full-repobacklog (3,176 findings) is too large to gate as a hard CI blocker today; needs a phased/baseline approach rather than a flat-errormessageflag add.Detailed Analysis
The root cause is unchanged from 2026-09-07:
cgo.yml'sLINTER_FLAGSis a hand-typed string with no automated check that it stays in sync withpkg/linters/registry.go'sAll(). New evidence this run is that the drift is not benign —packagelevelmutableslicemapspecifically targets data-race-prone code, and its 8 hits are in cache/label-lookup code paths (model_costs.go,update_version_labels.go,model_aliases.go,runtime_definitions.go,safe_outputs_tools_repo_params.go,samples_validation.go) that are plausibly invoked concurrently duringgh aw compile/test runs. Because these three analyzers (manualpathconcat,packagelevelmutableslicemap,seenmapbool) have zero-to-moderate, immediately actionable finding counts (60 total, not thousands), they are the highest-leverage subset to close first, distinct fromerrormessage's 3,176-finding backlog which needs separate planning.🤖 Tasks for Copilot Agent
NOTE TO PLANNER AGENT: Split the following tasks into individual work items.
Improvement Tasks
Task 1: Fix package-level mutable slice/map data-race risks flagged by
packagelevelmutableslicemapPriority: High
Estimated Effort: Medium
Focus Area: Custom Go Linter CI Enablement Gap
Description: Eight package-level slice/map variables are mutated via wholesale re-assignment, append, or index-assignment without synchronization:
pkg/workflow/model_aliases.go:96(builtinOnlyAliasMap),pkg/workflow/runtime_definitions.go:242(allManifestFilesBaseCache),pkg/workflow/safe_outputs_tools_repo_params.go:23(repoTargetAccessors),pkg/workflow/samples_validation.go:172(sortedSafeOutputFieldNames),pkg/cli/model_costs.go:50,73(modelPriceRecords),pkg/cli/update_version_labels.go:73,86(versionLabelCache). Wrap each with async.Mutex/sync.RWMutexguarding all reads and writes, or convert to async.Once-initialized immutable value if the data is actually static after first computation. Verify withgo test -raceon affected packages.Acceptance Criteria:
go build -o /tmp/gh-aw-linters ./cmd/linters && /tmp/gh-aw-linters -packagelevelmutableslicemap ./cmd/... ./pkg/...reports zero findingsgo test -race ./pkg/workflow/... ./pkg/cli/...passesCode Region:
pkg/workflow/model_aliases.go,pkg/workflow/runtime_definitions.go,pkg/workflow/safe_outputs_tools_repo_params.go,pkg/workflow/samples_validation.go,pkg/cli/model_costs.go,pkg/cli/update_version_labels.goTask 2: Replace manual path string-concatenation with
filepath.Join/path.JoinPriority: Medium
Estimated Effort: Medium
Focus Area: Custom Go Linter CI Enablement Gap
Description: The
manualpathconcatanalyzer found 49 call sites acrosspkg/workflowandpkg/clithat build paths via"/"-concatenation instead offilepath.Join/path.Join, risking double-slash or platform-inconsistent paths. Representative sites:pkg/workflow/action_reference.go:71,pkg/workflow/compiler_github_actions_steps.go:27,79,83,pkg/workflow/safe_outputs_actions.go:144,290,pkg/workflow/engine_config_dir.go:45,59,pkg/cli/dispatch.go:447,563,761,pkg/cli/checks_command.go:282,304,pkg/cli/devcontainer.go:277. Replace each with the appropriatefilepath.Join(local filesystem paths) orpath.Join(URL/slash-only paths, e.g. GitHub API paths, repo slugs) per the analyzer's suggestion text.Acceptance Criteria:
filepath.Join/path.Join(verify URL vs filesystem context per call site sopath.Joinis used for GitHub API/slug construction, notfilepath.Join)/tmp/gh-aw-linters -manualpathconcat ./cmd/... ./pkg/...reports zero findingsgo build ./...succeeds andgo test ./pkg/workflow/... ./pkg/cli/...passesCode Region:
pkg/workflow/*.go,pkg/cli/*.go(49 sites; run/tmp/gh-aw-linters -manualpathconcat ./cmd/... ./pkg/...for the full list)Task 3: Convert
map[string]boolsets tomap[string]struct{}Priority: Low
Estimated Effort: Small
Focus Area: Custom Go Linter CI Enablement Gap
Description: The
seenmapboolanalyzer found 3 sites usingmap[string]boolpurely as a membership set (values alwaystrue, never read asfalse):pkg/cli/experiments_analyze_statistics.go:445(declared),pkg/cli/update_manifest.go:333(latestSkillSources),pkg/cli/update_manifest.go:346(latestAgentSources). Convert each tomap[string]struct{}with_, ok := m[k]membership checks, saving one byte of allocation per entry and clarifying set-vs-map intent.Acceptance Criteria:
map[string]struct{}with corresponding membership-check call sites updated/tmp/gh-aw-linters -seenmapbool ./cmd/... ./pkg/...reports zero findingsgo test ./pkg/cli/...passesCode Region:
pkg/cli/experiments_analyze_statistics.go:445,pkg/cli/update_manifest.go:333,346Task 4: Add the 3 remediated analyzers (
manualpathconcat,packagelevelmutableslicemap,seenmapbool) tocgo.yml's CI gatePriority: High
Estimated Effort: Small
Focus Area: Custom Go Linter CI Enablement Gap
Description: Once Tasks 1–3 land (zero findings for all three analyzers), add
-manualpathconcat -packagelevelmutableslicemap -seenmapboolto bothLINTER_FLAGSstrings in.github/workflows/cgo.yml(the native "Run custom linters" step around line 1459 and the WASM "Run custom linters (wasm)" step around line 1462, matching the packages already scoped there), so these three analyzers gate every future PR instead of silently bit-rotting. This closes 3 of the 12 analyzers gapped since 2026-09-07; the remaining 9 (errorfwrapv,errormessage,excessivefuncparams,hardcodedfilepath,largefunc,lenstringzero,sprintferrdot,ssljson,stringsconcatloop) should be tracked separately since most have zero current findings (safe to add without remediation) excepterrormessage(3,176-finding backlog needing a phased plan) andlargefunc(518 findings, likely needs a raised-largefunc.max-linesthreshold or baseline exemption list rather than a flat enable).Acceptance Criteria:
LINTER_FLAGSin both cgo.yml steps includes-manualpathconcat -packagelevelmutableslicemap -seenmapbool.github/workflows/cgo.ymlpasses with the new flags on a clean (Task 1–3 complete) codebaseCode Region:
.github/workflows/cgo.yml(LINTER_FLAGS strings, ~lines 1459 and 1462)📊 Historical Context
Previous Focus Areas
🎯 Recommendations
Immediate Actions (This Week)
packagelevelmutableslicemapdata-race-risk findings — Priority: Highmanualpathconcat,packagelevelmutableslicemap,seenmapbooltocgo.yml'sLINTER_FLAGSonce remediated — Priority: HighShort-term Actions (This Month)
manualpathconcatfindings — Priority: Mediumseenmapboolsites tomap[string]struct{}— Priority: Mediumerrormessage -full-repo(3,176 findings) andlargefunc(518 findings) — Priority: MediumLong-term Actions (This Quarter)
pkg/linters/registry.go'sAll()againstcgo.yml'sLINTER_FLAGSso future analyzer additions can't silently bit-rot again — Priority: Low📈 Success Metrics
packagelevelmutableslicemaprace-risk findings: 8 → 0manualpathconcatfindings: 49 → 0seenmapboolfindings: 3 → 0Next Steps
errormessage/largefuncphased enablementGenerated by Repository Quality Improvement Agent
Next analysis: 2026-09-12 — Focus area selected by diversity algorithm
Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
registry.npmjs.orgTo allow these domains, add them to the
network.allowedlist in your workflow frontmatter:See Network Configuration for more information.
All reactions