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

scripts/install_go.sh

@@ -6,7 +6,6 @@
 
 have() { command -v "$1" >/dev/null 2>&1; }
 
-TOOL="go"
 ACTION="${1:-install}"
 
 # Support version-specific installation via GO_VERSION env var
@@ -22,16 +21,33 @@
   DISPLAY_NAME="go"
 fi
 
-# Get current version of the specific binary
+# GOBIN (default GOPATH/bin) — where `go install` puts version wrappers.
+# May be off PATH, so never rely on `have` alone for these binaries.
+go_bin_dir() {
+  have go || return 0
+  local dir
+  dir="$(go env GOBIN 2>/dev/null || true)"
+  [ -z "$dir" ] && dir="$(go env GOPATH 2>/dev/null || true)/bin"
+  echo "$dir"
+}
+
+# Get current version of the specific binary (PATH first, then GOBIN)
 get_go_version() {
   local bin="$1"
   if have "$bin"; then
     "$bin" version 2>/dev/null | head -1 || true
+    return 0
+  fi
+  local gobin
+  gobin="$(go_bin_dir)"
+  if [ -n "$gobin" ] && [ -x "$gobin/$bin" ]; then
+    "$gobin/$bin" version 2>/dev/null | head -1 || true
   fi
 }
 
 install_go() {
-  local before="$(get_go_version "$BINARY")"
+  local before
+  before="$(get_go_version "$BINARY")"
 
   # Version-specific Go installation (e.g., go1.24 alongside go1.25)
   if [ -n "$TARGET_CYCLE" ]; then
@@ -65,17 +81,34 @@
 
     echo "Installing ${FULL_BINARY} via go install golang.org/dl/${FULL_BINARY}@latest..."
     if go install "golang.org/dl/${FULL_BINARY}@latest"; then
-      # The go1.XX.YY command needs to download its SDK on first run
-      if have "$FULL_BINARY"; then
+      # go install lands in GOBIN, which may be off PATH — resolve explicitly
+      local gobin full_bin
+      gobin="$(go_bin_dir)"
+      full_bin="$gobin/$FULL_BINARY"
+      if [ -x "$full_bin" ]; then
+        # The go1.XX.YY command needs to download its SDK on first run
         echo "Downloading Go ${FULL_VERSION} SDK..."
-        "$FULL_BINARY" download || true
+        "$full_bin" download || true
 
         # Update symlink from go1.24 -> go1.24.12
-        GOBIN="$(go env GOPATH)/bin"
-        if [ -x "$GOBIN/$FULL_BINARY" ]; then
-          ln -sf "$FULL_BINARY" "$GOBIN/$BINARY" 2>/dev/null || true
-          echo "Updated symlink: $BINARY -> $FULL_BINARY"
+        ln -sf "$FULL_BINARY" "$gobin/$BINARY" 2>/dev/null || true
+        echo "Updated symlink: $BINARY -> $FULL_BINARY"
+
+        # Remove superseded wrappers (and SDKs) of the same cycle so
+        # repeated updates don't accumulate go1.26.0, go1.26.2, ...
+        if [[ "$TARGET_CYCLE" =~ ^[0-9]+\.[0-9]+$ ]]; then
+          local stale stale_name
+          for stale in "$gobin/go${TARGET_CYCLE}".*; do
+            [ -e "$stale" ] || continue
+            stale_name="$(basename "$stale")"
+            [ "$stale_name" = "$FULL_BINARY" ] && continue
+            rm -f "$stale"
+            rm -rf "$HOME/sdk/$stale_name" 2>/dev/null || true
+            echo "Removed superseded wrapper: $stale_name"
+          done
         fi
+      else
+        echo "Error: ${FULL_BINARY} not found in $gobin after go install" >&2
       fi
     else
       echo "Failed to install ${FULL_BINARY}" >&2
@@ -140,8 +173,11 @@
     rm -rf "$TMP" 2>/dev/null || true
   fi
 
-  local after="$(get_go_version "$BINARY")"
-  local path="$(command -v "$BINARY" 2>/dev/null || true)"
+  local after
+
+  after="$(get_go_version "$BINARY")"
+  local path
+  path="$(command -v "$BINARY" 2>/dev/null || true)"
   printf "[%s] before: %s\n" "$DISPLAY_NAME" "${before:-<none>}"
   printf "[%s] after:  %s\n"  "$DISPLAY_NAME" "${after:-<none>}"
   if [ -n "$path" ]; then printf "[%s] path:   %s\n" "$DISPLAY_NAME" "$path"; fi

tests/test_go_multiversion.py

@@ -0,0 +1,146 @@
+"""Tests for multi-version go installs (GO_VERSION=1.26 install_go.sh).
+
+After `go install golang.org/dl/go1.26.5@latest`, the script verified the
+wrapper via PATH (`have go1.26.5`) — but go installs into GOBIN
+(default GOPATH/bin), which may be off PATH. The SDK download and the
+go1.26 cycle symlink were then silently skipped, before/after reported
+`<none>`, and every auto-update run left another orphaned goX.Y.Z wrapper.
+"""
+
+from __future__ import annotations
+
+import os
+import subprocess
+import sys
+from pathlib import Path
+
+import pytest
+
+skip_on_windows = pytest.mark.skipif(
+    sys.platform == "win32", reason="Shell script tests require POSIX shell"
+)
+
+SCRIPT = Path(__file__).parent.parent / "scripts" / "install_go.sh"
+
+# Template for the goX.Y.Z wrapper the `go` stub "installs" into GOPATH/bin
+WRAPPER_TEMPLATE = """#!/usr/bin/env bash
+# real golang.org/dl wrappers report their own version even when invoked
+# through the cycle symlink (go1.26 -> go1.26.5)
+me="$(basename "$(readlink -f "$0")")"
+echo "$me $*" >> "$GO_STUB_LOG"
+case "$1" in
+  download) exit 0 ;;
+  version) echo "go version $me linux/amd64" ;;
+esac
+"""
+
+GO_STUB = """#!/usr/bin/env bash
+echo "go $*" >> "$GO_STUB_LOG"
+case "$1" in
+  env)
+    case "$2" in
+      GOBIN) echo "" ;;
+      GOPATH) echo "$FAKE_GOPATH" ;;
+    esac
+    ;;
+  install)
+    pkg="${2#golang.org/dl/}"
+    pkg="${pkg%@latest}"
+    mkdir -p "$FAKE_GOPATH/bin"
+    cp "$WRAPPER_TEMPLATE_PATH" "$FAKE_GOPATH/bin/$pkg"
+    chmod +x "$FAKE_GOPATH/bin/$pkg"
+    ;;
+  version)
+    echo "go version go1.26.1 linux/amd64"
+    ;;
+esac
+exit 0
+"""
+
+
+@skip_on_windows
+class TestGoMultiVersionInstall:
+    def _setup(self, tmp_path: Path) -> tuple[dict, Path, Path]:
+        stub_dir = tmp_path / "stubs"
+        stub_dir.mkdir()
+        gopath = tmp_path / "gopath"
+        (gopath / "bin").mkdir(parents=True)
+        home = tmp_path / "home"
+        home.mkdir()
+        log = tmp_path / "stub.log"
+        log.touch()
+
+        template = tmp_path / "wrapper.template"
+        template.write_text(WRAPPER_TEMPLATE)
+
+        for name, body in (
+            ("go", GO_STUB),
+            ("curl", '#!/usr/bin/env bash\necho \'[{"version":"go1.26.5"}]\'\n'),
+            ("python3", "#!/usr/bin/env bash\nexit 0\n"),
+        ):
+            stub = stub_dir / name
+            stub.write_text(body)
+            stub.chmod(0o755)
+
+        env = {
+            "HOME": str(home),
+            # GOPATH/bin deliberately NOT on PATH — the bug's trigger
+            "PATH": f"{stub_dir}:/usr/bin:/bin",
+            "GO_VERSION": "1.26",
+            "FAKE_GOPATH": str(gopath),
+            "GO_STUB_LOG": str(log),
+            "WRAPPER_TEMPLATE_PATH": str(template),
+        }
+        return env, gopath, log
+
+    def _run(self, env: dict) -> subprocess.CompletedProcess:
+        return subprocess.run(
+            ["bash", str(SCRIPT), "install"],
+            capture_output=True, text=True, timeout=60, env=env,
+        )
+
+    def test_sdk_download_and_symlink_despite_gobin_off_path(self, tmp_path):
+        env, gopath, log = self._setup(tmp_path)
+        result = self._run(env)
+        assert result.returncode == 0, result.stderr
+
+        assert "go1.26.5 download" in log.read_text(), (
+            "SDK download must run even when GOPATH/bin is off PATH"
+        )
+        cycle_link = gopath / "bin" / "go1.26"
+        assert cycle_link.is_symlink(), "go1.26 cycle symlink must be created"
+        assert os.readlink(cycle_link) == "go1.26.5"
+
+    def test_before_after_report_not_none(self, tmp_path):
+        env, _, _ = self._setup(tmp_path)
+        result = self._run(env)
+        out = result.stdout + result.stderr
+        # "before: <none>" is correct on a fresh install — the bug was the
+        # after-probe returning <none> because GOPATH/bin is off PATH
+        assert "[go@1.26] after:  go version go1.26.5" in out, (
+            f"after-version must probe GOPATH/bin, got: {out}"
+        )
+
+    def test_superseded_wrappers_of_same_cycle_are_removed(self, tmp_path):
+        env, gopath, _ = self._setup(tmp_path)
+        # Orphans from earlier runs (the wrapper zoo)
+        for old in ("go1.26.0", "go1.26.2"):
+            stale = gopath / "bin" / old
+            stale.write_text("#!/bin/sh\n")
+            stale.chmod(0o755)
+            (Path(env["HOME"]) / "sdk" / old).mkdir(parents=True)
+        # A different cycle must be untouched
+        other = gopath / "bin" / "go1.25.11"
+        other.write_text("#!/bin/sh\n")
+        other.chmod(0o755)
+
+        result = self._run(env)
+        assert result.returncode == 0, result.stderr
+
+        assert not (gopath / "bin" / "go1.26.0").exists()
+        assert not (gopath / "bin" / "go1.26.2").exists()
+        assert not (Path(env["HOME"]) / "sdk" / "go1.26.0").exists()
+        assert (gopath / "bin" / "go1.26.5").exists()
+        assert (gopath / "bin" / "go1.25.11").exists(), (
+            "other cycles must not be touched"
+        )