Skip to content

fix(scripts): resolve go version wrappers via GOBIN, prune stale ones#111

Merged
CybotTM merged 1 commit into
mainfrom
fix/go-multiversion-gobin
Jul 17, 2026
Merged

fix(scripts): resolve go version wrappers via GOBIN, prune stale ones#111
CybotTM merged 1 commit into
mainfrom
fix/go-multiversion-gobin

Conversation

@CybotTM

@CybotTM CybotTM commented Jul 16, 2026

Copy link
Copy Markdown
Member

Summary

make upgrade-all's go@1.26 step printed before: <none> / after: <none> while claiming success, and ~/go/bin accumulated 12 orphaned version wrappers (go1.25.5go1.26.5).

Root cause — same class as #107: after go install golang.org/dl/go1.26.5@latest the script checked have go1.26.5 (PATH lookup), but go install lands in GOBIN (default GOPATH/bin), which may be off PATH. The SDK download and go1.26 cycle-symlink update were then silently skipped, and each run left another orphaned wrapper.

  • resolve wrappers explicitly via go env GOBIN/GOPATH — install verification, SDK download, symlink update, and the before/after version probes
  • prune superseded same-cycle wrappers and their ~/sdk trees after a successful update (go1.26.0, go1.26.2 → gone once go1.26.5 is in place); other cycles are untouched
  • drop the unused TOOL var (pre-existing SC2034 that blocked the shellcheck hook on this file)

Tests

  • tests/test_go_multiversion.py (3 tests, red→green, go/curl/python3 PATH-stubbed with GOPATH/bin deliberately off PATH): SDK download + symlink happen despite GOBIN off PATH, after-version probes GOBIN instead of reporting <none>, superseded same-cycle wrappers and SDKs pruned while other cycles survive
  • Full suite: 692 passed, 1 skipped; shellcheck clean; smoke tests pass

GO_VERSION=1.26 install reported before/after <none> and skipped both
the SDK download and the go1.26 cycle symlink whenever GOPATH/bin was
off PATH: the post-install check used 'have go1.26.5' (a PATH lookup)
although go install always lands in GOBIN. Every auto-update run then
left another orphaned goX.Y.Z wrapper (12 accumulated in the field).

- resolve wrappers via go env GOBIN/GOPATH explicitly (install check,
  SDK download, symlink update, and the before/after version probes)
- prune superseded same-cycle wrappers and their ~/sdk trees after a
  successful update; other cycles untouched
- drop unused TOOL var (pre-existing SC2034 blocking the shellcheck
  hook on this file)

Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
Copilot AI review requested due to automatic review settings July 16, 2026 22:42
@gemini-code-assist

Copy link
Copy Markdown

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@CybotTM
CybotTM merged commit 4e581e8 into main Jul 17, 2026
19 of 20 checks passed
@CybotTM
CybotTM deleted the fix/go-multiversion-gobin branch July 17, 2026 05:57
CybotTM added a commit that referenced this pull request Jul 17, 2026
## Summary

Retro follow-ups from this week's bug chain (#106#111):

1. **`scripts/AGENTS.md` — four hard-won invariants**, each of which
caused a shipped bug: never verify installs via PATH lookup (resolve the
manager's bin dir; pass detected paths into removal code), environments
≠ installations (both detection layers must stay in sync), quiet command
wrappers detach stdin and surface failures, and the shellcheck hook
lints the whole changed file.
2. **CI `Shell Tests` job** running `tests/test_guide_multi_install.sh`
(52 assertions) and `tests/test_reconcile_dryrun.sh` (7 assertions) —
they were wired into no job, and a dormant assertion in the former
(`classify nvm path returns npm(...)`) predicted the pnpm/nvm
misclassification fixed in #106 long before it bit.

Both suites verified green locally before wiring.

Came from /retro: yes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants