Problem
walkfuncerrshadow (ADR-49633, status: Draft, merged 2026-08-01) flags filepath.Walk/WalkDir calls where the outer result variable and the callback's third parameter are both named err. The ADR itself calls this out as a known cost ("Any existing code that uses the flagged pattern must be reviewed and potentially renamed; this is a one-time migration cost") — but that migration was never done. The linter is also absent from cgo.yml's LINTER_FLAGS enforcement list, so nothing currently catches these in CI.
Evidence — 10 unmigrated production sites (exact err/err shadow)
pkg/cli/logs_download.go:192 err := filepath.Walk(sourceDir, func(path string, info os.FileInfo, err error) error {
pkg/cli/logs_download.go:505 err := filepath.Walk(outputDir, func(path string, info os.FileInfo, err error) error {
pkg/cli/compile_watch.go:82 err = filepath.Walk(workflowsDir, func(path string, info os.FileInfo, err error) error {
pkg/cli/trial_support.go:58 err = filepath.Walk(tempDir, func(path string, info os.FileInfo, err error) error {
pkg/cli/remove_command.go:222 err = filepath.Walk(workflowsDir, func(path string, info os.FileInfo, err error) error {
pkg/cli/remove_command.go:342 err := filepath.Walk(workflowsDir, func(path string, info os.FileInfo, err error) error {
pkg/cli/remove_command.go:370 err := filepath.Walk(workflowsDir, func(path string, info os.FileInfo, err error) error {
pkg/cli/logs_metrics.go:188 err = filepath.Walk(logDir, func(path string, info os.FileInfo, err error) error {
pkg/cli/logs_metrics.go:681 err := filepath.Walk(runDir, func(path string, info os.FileInfo, err error) error {
pkg/cli/dependency_graph.go:66 err := filepath.Walk(g.workflowsDir, func(path string, info os.FileInfo, err error) error {
For contrast, most other filepath.Walk call sites in the same files (and package) already use walkErr for the outer variable specifically to avoid this ambiguity (e.g. pkg/cli/logs_utils.go, pkg/cli/copilot_agent.go:113, pkg/cli/logs_metrics.go:985), confirming the shadow pattern above is inconsistent with the codebase's own established convention, not an accepted idiom.
Impact
Each flagged site has an inner callback err that shadows the outer err used to carry the walk's overall result. Today the shadowing happens to be benign in all 10 spots (each callback branch either returns immediately or is followed by the outer if err != nil check right after Walk returns), but this is exactly the fragile pattern the ADR warns about: a future refactor that adds an early return referencing the outer err inside the callback, or that renames only one side, will silently do the wrong thing — the compiler gives no warning because both errs are valid, distinct variables.
Recommendation
- Rename the outer variable to
walkErr (or the callback param, though outer rename matches the codebase's existing convention) at all 10 sites — mechanical, low-risk.
- Add
-walkfuncerrshadow to both LINTER_FLAGS invocations in .github/workflows/cgo.yml (~line 1362 and ~line 1365) once the sites are clean, so this stays enforced going forward.
- Flip the ADR status from Draft to Accepted once landed.
Validation checklist
Effort
Small - mechanical rename across 6 files (10 call sites) plus one CI config line change.
Generated by 🤖 Sergo - Serena Go Expert · agent · 167.6 AIC · ⌖ 33.2 AIC · ⊞ 6K · ◷
Problem
walkfuncerrshadow(ADR-49633, status: Draft, merged 2026-08-01) flagsfilepath.Walk/WalkDircalls where the outer result variable and the callback's third parameter are both namederr. The ADR itself calls this out as a known cost ("Any existing code that uses the flagged pattern must be reviewed and potentially renamed; this is a one-time migration cost") — but that migration was never done. The linter is also absent fromcgo.yml'sLINTER_FLAGSenforcement list, so nothing currently catches these in CI.Evidence — 10 unmigrated production sites (exact
err/errshadow)For contrast, most other
filepath.Walkcall sites in the same files (and package) already usewalkErrfor the outer variable specifically to avoid this ambiguity (e.g.pkg/cli/logs_utils.go,pkg/cli/copilot_agent.go:113,pkg/cli/logs_metrics.go:985), confirming the shadow pattern above is inconsistent with the codebase's own established convention, not an accepted idiom.Impact
Each flagged site has an inner callback
errthat shadows the outererrused to carry the walk's overall result. Today the shadowing happens to be benign in all 10 spots (each callback branch either returns immediately or is followed by the outerif err != nilcheck right afterWalkreturns), but this is exactly the fragile pattern the ADR warns about: a future refactor that adds an earlyreturnreferencing the outererrinside the callback, or that renames only one side, will silently do the wrong thing — the compiler gives no warning because botherrs are valid, distinct variables.Recommendation
walkErr(or the callback param, though outer rename matches the codebase's existing convention) at all 10 sites — mechanical, low-risk.-walkfuncerrshadowto bothLINTER_FLAGSinvocations in.github/workflows/cgo.yml(~line 1362 and ~line 1365) once the sites are clean, so this stays enforced going forward.Validation checklist
err->walkErr, or equivalent)go build ./...and existing tests still pass (pure rename, no behavior change)walkfuncerrshadowadded tocgo.ymlLINTER_FLAGS(both the default and theGOOS=js GOARCH=wasminvocation)make golint-custom LINTER_FLAGS="-walkfuncerrshadow"reports zero violations after renameEffort
Small - mechanical rename across 6 files (10 call sites) plus one CI config line change.