fix: migrate all 10 filepath.Walk err-shadow sites (ADR-49633) and enforce in CI

[Copilot]

Ten filepath.Walk call sites used err for both the outer walk result and the callback's third parameter, silently shadowing the outer variable. This is the one-time migration cost called out in ADR-49633; the linter existed but was never enforced and the migration was never done.

Changes

• Rename outer err → walkErr at all 10 flagged sites across 6 files, matching the convention already used at other Walk call sites in the codebase (logs_utils.go, copilot_agent.go, logs_metrics.go:985):

// Before — callback err silently shadows outer err
err := filepath.Walk(dir, func(path string, info os.FileInfo, err error) error { ... })
if err != nil { ... }

// After
walkErr := filepath.Walk(dir, func(path string, info os.FileInfo, err error) error { ... })
if walkErr != nil { ... }

• pkg/cli/logs_download.go — 2 sites (flattenArtifactTree, listArtifacts)

• pkg/cli/compile_watch.go — 1 site (subdirectory watcher setup)

• pkg/cli/trial_support.go — 1 site (artifact directory walk)

• pkg/cli/remove_command.go — 3 sites (cleanupOrphanedIncludes, getAllIncludeFiles, cleanupAllIncludes)

• pkg/cli/logs_metrics.go — 2 sites (log file walk, MCP failure extraction)

• pkg/cli/dependency_graph.go — 1 site (BuildGraph)

• .github/workflows/cgo.yml — adds -walkfuncerrshadow to both LINTER_FLAGS invocations (native and wasm) so the pattern is caught in CI going forward.

• ADR-49633 — status flipped from Draft → Accepted.

• Fixes walkfuncerrshadow (61st linter): 10 production sites still shadow err/err — migration from ADR-49633 not done #49901

---

run: https://github.com/github/gh-aw/actions/runs/30811225763

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 5.44 AIC · ⌖ 5.36 AIC · ⊞ 8.3K · ◷
_{Comment /souschef to run again}

[github-actions]

Triage Result

Category: refactor | Risk: medium | Score: 52/100 (impact 25, urgency 12, quality 15)

Recommended action: defer

Draft PR, CI pending, moderate-size refactor across 8 files (filepath.Walk err-shadow migration). No reviews yet. Revisit once CI completes and draft is marked ready.

Generated by 🔧 PR Triage Agent · auto · 27.4 AIC · ⌖ 2.42 AIC · ⊞ 8K · ◷

[github-actions]

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

[github-actions]

✅ PR Code Quality Reviewer completed the code quality review.

[github-actions]

❌ Test Quality Sentinel failed during test quality analysis.

[Copilot]

Pull request overview

Migrates all ten filepath.Walk shadowing sites and enables ongoing CI enforcement.

Changes:

• Renames outer walk errors to walkErr across six Go files.
• Enables walkfuncerrshadow for native and WASM lint jobs.
• Marks ADR-49633 as accepted.

Show a summary per file

File	Description
pkg/cli/trial_support.go	Renames artifact walk error.
pkg/cli/remove_command.go	Renames three include-walk errors.
pkg/cli/logs_metrics.go	Renames two log-walk errors.
pkg/cli/logs_download.go	Renames two artifact-walk errors.
pkg/cli/dependency_graph.go	Renames workflow scan error.
pkg/cli/compile_watch.go	Renames watcher setup walk error.
.github/workflows/cgo.yml	Enforces the linter in CI.
docs/adr/49633-add-walkfuncerrshadow-linter-for-filepath-walk-err-shadow.md	Accepts the ADR.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

• Files reviewed: 8/8 changed files
• Comments generated: 0
• Review effort level: Balanced

[github-actions]

✅ Design Decision Gate 🏗️ completed the design decision gate check.

No ADR enforcement needed: PR #49907 does not have the implementation label and has only 28 new lines in business logic directories (threshold: 100).

[github-actions]

Skills-Based Review 🧠

Applied /diagnosing-bugs — clean, mechanical migration with no logic changes.

📋 Review Summary

What was done

• All 10 filepath.Walk err-shadow sites renamed err → walkErr consistently
• Linter flag -walkfuncerrshadow added to both native and wasm CI invocations
• ADR-49633 status updated to Accepted

Positive Highlights

• ✅ Every renamed site uses walkErr — matches existing convention
• ✅ CI enforcement added in the same PR, preventing regression
• ✅ Wasm linter invocation also picks up the flag (no coverage gap)
• ✅ Pure rename — no behaviour changes, low risk

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 · 17.1 AIC · ⌖ 8.08 AIC · ⊞ 7.1K
_{Comment /matt to run again}

[github-actions]

The changes are a clean, mechanical rename of err → walkErr at all 10 filepath.Walk call sites across 6 files. The fix is correct, consistent, and complete.

One minor observation: -walkfuncerrshadow is also added to the wasm linter step, but that step's LINTER_PACKAGES only covers ./pkg/console ./pkg/parser ./pkg/styles ./pkg/tty ./pkg/workflow — none of the changed packages — so the flag is effectively a no-op there. Harmless; the native linter step is the one that actually enforces this across ./pkg/cli.

🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 · 16.7 AIC · ⌖ 7.58 AIC · ⊞ 5.4K

[github-actions]

Verdict: Approve — clean, mechanical fix

All 10 filepath.Walk sites correctly rename the outer err to walkErr, eliminating the shadow bug. Verified each site: no leftover references to the old err name, no scoping conflicts (e.g. remove_command.go inner err := in the loop is unaffected since it's a different, inner scope). CI linter flag and ADR status update are consistent with the stated migration. Build succeeds.

Notes

No blocking issues found. Confirmed via go build ./pkg/cli/... that the package compiles cleanly after the renames.

🔎 Code quality review by PR Code Quality Reviewer · auto · 13.3 AIC · ⌖ 3.77 AIC · ⊞ 7.9K
_{Comment /review to run again}

[pelikhan]

@copilot merge main and recompile

[Copilot]

@copilot merge main and recompile

Done — merged main and recompiled all 269 workflows (all in sync, zero errors).

[gh-aw-bot]

@copilot Please take the next maintainer-ready step on this PR. Refresh the branch if GitHub allows it, verify checks, and run the pr-finisher skill before handing back.

Run: https://github.com/github/gh-aw/actions/runs/30811225763

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 5.44 AIC · ⌖ 5.36 AIC · ⊞ 8.3K · ◷
_{Comment /souschef to run again}

[Copilot]

@copilot Please take the next maintainer-ready step on this PR. Refresh the branch if GitHub allows it, verify checks, and run the pr-finisher skill before handing back.

Run: https://github.com/git...

Done. Here's the PR finisher summary:

• ✅ Reviews — No unresolved review threads.
• ✅ Checks (local) — make fmt, make lint, make test-unit, make recompile all pass. Root cause of the CI failure fixed: pkg/cli/data/agentic_workflows_fallback_aw_files.json was missing token-optimization-observability.md (added via main merge); committed in 6f90fab.
• ❓ Checks (CI) — Stale after agent push; needs human re-trigger. Prior failure (Unit tests (D-L) / TestFallbackAWFilesMatchesLocalAWDirectory) fixed at the root cause and verified locally.
• ✅ Mergeable — PR is open, not draft, no conflicts.

Actions taken: Fixed TestFallbackAWFilesMatchesLocalAWDirectory by adding the missing token-optimization-observability.md entry to the fallback aw files list.

Hand-off: CI must be re-triggered by a maintainer (close/reopen PR, workflow_dispatch, or push) before merge.

[github-actions]

🎉 This pull request is included in a new release.

Release: v0.84.4