From 3d167f6f1fabd93ed0d1dbcb3d0ab679d580725a Mon Sep 17 00:00:00 2001 From: Montana Date: Wed, 1 Jul 2026 08:07:22 -0700 Subject: [PATCH] feat: warn when src-cli is older than the instance's recommended version Before running a command, src does a best-effort check of the running version against the version recommended by the configured instance (/.api/src-cli/version) and prints a single stderr warning when behind. Fail-open, 3s timeout, stderr only so --json is unaffected. Skips version/help/no-arg, dev builds, and SRC_SKIP_VERSION_CHECK. Compares major/minor/patch only. Adds unit tests. Closes #930. Supersedes #1339, #1343. --- cmd/src/main.go | 4 + cmd/src/version_check.go | 135 +++++++++++++++++++++++++++++++++ cmd/src/version_check_test.go | 139 ++++++++++++++++++++++++++++++++++ 3 files changed, 278 insertions(+) create mode 100644 cmd/src/version_check.go create mode 100644 cmd/src/version_check_test.go diff --git a/cmd/src/main.go b/cmd/src/main.go index fd217ba51f..f69d72461a 100644 --- a/cmd/src/main.go +++ b/cmd/src/main.go @@ -96,6 +96,10 @@ func main() { log.SetFlags(0) log.SetPrefix("") + // Best-effort warning if src-cli is behind the version recommended by the + // configured instance. Fail-open, bounded by a short timeout, stderr only. + maybeWarnVersion(os.Args[1:]) + ranMigratedCmd, exitCode, err := maybeRunMigratedCommand() if ranMigratedCmd { if err != nil { diff --git a/cmd/src/version_check.go b/cmd/src/version_check.go new file mode 100644 index 0000000000..820a823f31 --- /dev/null +++ b/cmd/src/version_check.go @@ -0,0 +1,135 @@ +package main + +import ( + "context" + "fmt" + "os" + "strconv" + "strings" + "time" + + "github.com/sourcegraph/src-cli/internal/api" + "github.com/sourcegraph/src-cli/internal/version" +) + +// versionCheckTimeout bounds the best-effort recommended-version lookup so it +// never noticeably delays a command. +const versionCheckTimeout = 3 * time.Second + +// maybeWarnVersion performs a best-effort check of the running src-cli version +// against the version recommended by the configured Sourcegraph instance and +// prints a single warning to stderr if src-cli is behind. +// +// It is entirely fail-open: any error (missing config, network failure, +// unreachable instance, unparseable version) results in no output. It writes +// only to stderr, so --json output on stdout is unaffected. +func maybeWarnVersion(args []string) { + if skipVersionCheck(args) { + return + } + + // Read config independently of command dispatch. Note: this runs before + // flag parsing, so -endpoint/-config flags are not honored here; the + // endpoint is resolved from SRC_ENDPOINT, the config file, or the default. + cfg, err := readConfig() + if err != nil { + return // fail-open: missing or invalid config + } + + ctx, cancel := context.WithTimeout(context.Background(), versionCheckTimeout) + defer cancel() + + client := cfg.apiClient(&api.Flags{}, os.Stderr) + recommended, err := getRecommendedVersion(ctx, client) + if err != nil || recommended == "" { + return // fail-open: network error, unreachable, or unsupported instance + } + + if isOlderVersion(version.BuildTag, recommended) { + fmt.Fprintf(os.Stderr, + "warning: src-cli v%s is older than the version recommended by your Sourcegraph instance (v%s). "+ + "Run `src version` for details, or set SRC_SKIP_VERSION_CHECK=1 to silence this.\n", + version.BuildTag, recommended) + } +} + +// skipVersionCheck reports whether the version check should be skipped for this +// invocation. Dev builds, an explicit opt-out env var, and +// version/help/no-arg invocations are all skipped. +func skipVersionCheck(args []string) bool { + // Dev builds have no meaningful version to compare. + if version.BuildTag == version.DefaultBuildTag { + return true + } + + // Explicit opt-out. + if _, ok := os.LookupEnv("SRC_SKIP_VERSION_CHECK"); ok { + return true + } + + // No subcommand: `src` alone just prints usage. + if len(args) == 0 { + return true + } + + // version and help invocations already surface version info / need no nag. + for _, arg := range args { + switch arg { + case "version", "help", "-h", "-help", "--help": + return true + } + } + + return false +} + +// isOlderVersion reports whether current is strictly older than recommended, +// comparing only major/minor/patch so prereleases do not trigger spurious +// warnings. If either version is unparseable it returns false (fail-open). +func isOlderVersion(current, recommended string) bool { + cMaj, cMin, cPatch, ok := parseVersion(current) + if !ok { + return false + } + rMaj, rMin, rPatch, ok := parseVersion(recommended) + if !ok { + return false + } + + if cMaj != rMaj { + return cMaj < rMaj + } + if cMin != rMin { + return cMin < rMin + } + return cPatch < rPatch +} + +// parseVersion extracts the major, minor, and patch components from a semver +// string, tolerating a leading "v" and ignoring any prerelease or build +// metadata suffix (e.g. "v1.2.3-rc1+meta" -> 1, 2, 3). +func parseVersion(s string) (major, minor, patch int, ok bool) { + s = strings.TrimPrefix(strings.TrimSpace(s), "v") + + // Drop prerelease and build metadata: 1.2.3-rc1+meta -> 1.2.3 + if i := strings.IndexAny(s, "-+"); i >= 0 { + s = s[:i] + } + + parts := strings.SplitN(s, ".", 3) + if len(parts) != 3 { + return 0, 0, 0, false + } + + var err error + if major, err = strconv.Atoi(parts[0]); err != nil { + return 0, 0, 0, false + } + if minor, err = strconv.Atoi(parts[1]); err != nil { + return 0, 0, 0, false + } + if patch, err = strconv.Atoi(parts[2]); err != nil { + return 0, 0, 0, false + } + return major, minor, patch, true +} diff --git a/cmd/src/version_check_test.go b/cmd/src/version_check_test.go new file mode 100644 index 0000000000..83b8d1fea1 --- /dev/null +++ b/cmd/src/version_check_test.go @@ -0,0 +1,139 @@ +package main + +import ( + "os" + "testing" + + "github.com/sourcegraph/src-cli/internal/version" +) + +func TestParseVersion(t *testing.T) { + tests := []struct { + name string + in string + major, minor, patch int + ok bool + }{ + {"plain", "1.2.3", 1, 2, 3, true}, + {"leading v", "v1.2.3", 1, 2, 3, true}, + {"surrounding whitespace", " 1.2.3\n", 1, 2, 3, true}, + {"prerelease suffix ignored", "1.2.3-rc1", 1, 2, 3, true}, + {"build metadata ignored", "1.2.3+abc123", 1, 2, 3, true}, + {"prerelease and metadata ignored", "v1.2.3-rc1+abc", 1, 2, 3, true}, + {"multi-digit components", "10.20.30", 10, 20, 30, true}, + {"missing patch", "1.2", 0, 0, 0, false}, + {"extra component unparseable", "1.2.3.4", 0, 0, 0, false}, + {"empty", "", 0, 0, 0, false}, + {"non-numeric", "a.b.c", 0, 0, 0, false}, + {"partially numeric", "1.x.3", 0, 0, 0, false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + maj, min, patch, ok := parseVersion(tt.in) + if ok != tt.ok { + t.Fatalf("parseVersion(%q) ok = %v, want %v", tt.in, ok, tt.ok) + } + if !tt.ok { + return // components are meaningless when ok is false + } + if maj != tt.major || min != tt.minor || patch != tt.patch { + t.Errorf("parseVersion(%q) = (%d, %d, %d), want (%d, %d, %d)", + tt.in, maj, min, patch, tt.major, tt.minor, tt.patch) + } + }) + } +} + +func TestIsOlderVersion(t *testing.T) { + tests := []struct { + name string + current string + recommended string + want bool + }{ + {"older patch", "1.2.3", "1.2.4", true}, + {"older minor", "1.2.3", "1.3.0", true}, + {"older major", "1.2.3", "2.0.0", true}, + {"equal", "1.2.3", "1.2.3", false}, + {"newer patch", "1.2.4", "1.2.3", false}, + {"newer major", "2.0.0", "1.9.9", false}, + {"prerelease on current, same core", "1.2.3-rc1", "1.2.3", false}, + {"prerelease on recommended, same core", "1.2.3", "1.2.3-rc1", false}, + {"leading v tolerated", "v1.2.3", "1.2.4", true}, + {"unparseable current fails open", "garbage", "1.2.3", false}, + {"unparseable recommended fails open", "1.2.3", "garbage", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := isOlderVersion(tt.current, tt.recommended); got != tt.want { + t.Errorf("isOlderVersion(%q, %q) = %v, want %v", + tt.current, tt.recommended, got, tt.want) + } + }) + } +} + +func TestSkipVersionCheck(t *testing.T) { + // Ensure the opt-out env var is absent for the non-env cases, restoring + // whatever the developer's environment had afterward. + if orig, ok := os.LookupEnv("SRC_SKIP_VERSION_CHECK"); ok { + os.Unsetenv("SRC_SKIP_VERSION_CHECK") + t.Cleanup(func() { os.Setenv("SRC_SKIP_VERSION_CHECK", orig) }) + } + + // Pretend this is a release build for everything except the dev-build case. + origTag := version.BuildTag + t.Cleanup(func() { version.BuildTag = origTag }) + version.BuildTag = "5.1.2" + + t.Run("dev build is skipped", func(t *testing.T) { + version.BuildTag = version.DefaultBuildTag + defer func() { version.BuildTag = "5.1.2" }() + + if !skipVersionCheck([]string{"search", "foo"}) { + t.Error("expected dev build to be skipped") + } + }) + + t.Run("opt-out env var is skipped", func(t *testing.T) { + t.Setenv("SRC_SKIP_VERSION_CHECK", "1") + + if !skipVersionCheck([]string{"search", "foo"}) { + t.Error("expected SRC_SKIP_VERSION_CHECK to skip the check") + } + }) + + t.Run("empty env value still counts as set", func(t *testing.T) { + t.Setenv("SRC_SKIP_VERSION_CHECK", "") + + if !skipVersionCheck([]string{"search", "foo"}) { + t.Error("expected a present-but-empty env var to skip the check") + } + }) + + argCases := []struct { + name string + args []string + want bool + }{ + {"no args", nil, true}, + {"version subcommand", []string{"version"}, true}, + {"help subcommand", []string{"help"}, true}, + {"-h flag", []string{"-h"}, true}, + {"-help flag", []string{"-help"}, true}, + {"--help flag", []string{"--help"}, true}, + {"version after global flag", []string{"-v", "version"}, true}, + {"normal command", []string{"search", "-json", "foo"}, false}, + {"normal multi-arg command", []string{"batch", "preview", "-f", "x.yaml"}, false}, + } + + for _, tt := range argCases { + t.Run(tt.name, func(t *testing.T) { + if got := skipVersionCheck(tt.args); got != tt.want { + t.Errorf("skipVersionCheck(%v) = %v, want %v", tt.args, got, tt.want) + } + }) + } +}