refactor: read the instance version through api.ServerVersion

The merge brought in `api.ServerVersion`, added by #86 for the same
purpose, so the version lookup added here was a second one in the same
package. Uses the shared function instead, which also stops going through
the 301 that `/version` (no trailing slash) answers with.

Adopts its rule about the tag, too: an instance reporting "latest" or a
commit sha now has its tag left out of the message rather than quoted
back as though it were a version. Nothing here rests on the tag — the 404
already established the endpoint is absent — so it only decides whether
the version can be named, where `login` has to stay silent entirely
because it is inferring age.

Dropped the `package_versions` fallback with it: it would be a change to
what `login` gates on, which wants deciding on its own terms.

Author: khvn26

internal/api/client.go

@@ -13,8 +13,9 @@ import (
 	"strconv"
 	"strings"
 
-	"github.com/Flagsmith/flagsmith-cli/v2/internal/bug"
+	"github.com/blang/semver/v4"
 
+	"github.com/Flagsmith/flagsmith-cli/v2/internal/bug"
 	"github.com/Flagsmith/flagsmith-cli/v2/internal/httpx"
 	"github.com/Flagsmith/flagsmith-cli/v2/internal/version"
 )
@@ -990,36 +991,28 @@ func (c *Client) classifyFlagWrite(ctx context.Context, err error) error {
 
 // unsupportedFlagWrite names the instance's version in the error when it can be
 // read, since "upgrade" is easier to act on knowing what you're upgrading from.
-// The lookup only happens on this failure path, and its own failure just leaves
-// the version out.
+// The lookup only happens on this failure path.
+//
+// A tag that is not a version ("latest", a commit sha) is left out rather than
+// quoted back: unlike the inference in `login`, nothing here rests on it — the
+// 404 already established the endpoint is absent — so it only decides whether
+// the version can be named.
 func (c *Client) unsupportedFlagWrite(ctx context.Context) error {
-	if version, err := c.ServerVersion(ctx); err == nil && version != "" {
-		return fmt.Errorf("%w: this instance runs %s", ErrFlagWritesUnsupported, version)
+	tag, err := ServerVersion(ctx, c.httpClient, c.baseURL)
+	if err != nil {
+		return ErrFlagWritesUnsupported
+	}
+	if _, err := semver.ParseTolerant(tag); err != nil {
+		return ErrFlagWritesUnsupported
 	}
-	return ErrFlagWritesUnsupported
+	return fmt.Errorf("%w: this instance runs %s", ErrFlagWritesUnsupported, tag)
 }
 
 // ErrFlagWritesUnsupported marks an instance without the update-flag endpoints,
 // which every flag mutation needs. Reads are unaffected: they use endpoints that
 // have been there all along.
 var ErrFlagWritesUnsupported = fmt.Errorf("changing flags needs Flagsmith %s or newer", MinFlagWriteVersion)
 
-// ServerVersion reports the instance's version, as /version advertises it. The
-// endpoint is unauthenticated and predates everything the CLI uses.
-func (c *Client) ServerVersion(ctx context.Context) (string, error) {
-	var info struct {
-		ImageTag        string            `json:"image_tag"`
-		PackageVersions map[string]string `json:"package_versions"`
-	}
-	if err := c.get(ctx, "/version", &info); err != nil {
-		return "", err
-	}
-	if info.ImageTag != "" {
-		return info.ImageTag, nil
-	}
-	return info.PackageVersions["."], nil
-}
-
 // ErrWorkflowGated is returned when update-flag refuses because the environment
 // has change-request workflows enabled.
 var ErrWorkflowGated = fmt.Errorf("this environment uses change-request workflows; direct updates are disabled")

internal/api/client_test.go

@@ -583,7 +583,7 @@ func TestUpdateFlag(t *testing.T) {
 	t.Run("a 404 with no detail is an instance too old, and names its version", func(t *testing.T) {
 		// Given
 		mux := http.NewServeMux()
-		mux.HandleFunc("/version", func(w http.ResponseWriter, r *http.Request) {
+		mux.HandleFunc("/version/", func(w http.ResponseWriter, r *http.Request) {
 			fmt.Fprint(w, `{"image_tag": "2.262.0", "package_versions": {".": "2.262.0"}}`)
 		})
 		mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
@@ -609,6 +609,32 @@ func TestUpdateFlag(t *testing.T) {
 		}
 	})
 
+	t.Run("a tag that is not a version is left out rather than quoted back", func(t *testing.T) {
+		// Given: what a self-hosted instance tracking a branch reports.
+		mux := http.NewServeMux()
+		mux.HandleFunc("/version/", func(w http.ResponseWriter, r *http.Request) {
+			fmt.Fprint(w, `{"image_tag": "latest"}`)
+		})
+		mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
+			w.WriteHeader(http.StatusNotFound)
+			fmt.Fprint(w, "<h1>Not Found</h1>")
+		})
+		srv := httptest.NewServer(mux)
+		defer srv.Close()
+
+		// When
+		_, err := testClient(srv.URL, APIKey("k.s"), srv).UpdateFlag(context.Background(), "envkey", 42,
+			UpdateFlagRequest{EnvironmentDefault: &FlagStateUpdate{Enabled: &enabled}})
+
+		// Then
+		if !errors.Is(err, ErrFlagWritesUnsupported) {
+			t.Fatalf("err = %v, want ErrFlagWritesUnsupported", err)
+		}
+		if strings.Contains(err.Error(), "latest") {
+			t.Errorf("err = %v, want the unparseable tag left out", err)
+		}
+	})
+
 	t.Run("an unreadable version still reports the requirement", func(t *testing.T) {
 		// Given: no /version to read, as a proxy in front of an old instance
 		// might leave it.

internal/cmd/cmd_test.go

@@ -591,7 +591,7 @@ func newFakeInstance(t *testing.T) *fakeInstance {
 		w.WriteHeader(http.StatusCreated)
 		json.NewEncoder(w).Encode(clone)
 	})
-	mux.HandleFunc("GET /version", func(w http.ResponseWriter, r *http.Request) {
+	mux.HandleFunc("GET /version/", func(w http.ResponseWriter, r *http.Request) {
 		json.NewEncoder(w).Encode(map[string]any{
 			"image_tag": "2.262.0", "package_versions": map[string]string{".": "2.262.0"},
 		})