fix(cli): complete version protocol follow-ups

cmd/volcano/main_test.go

@@ -7,6 +7,7 @@ import (
 	"net/http/httptest"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -17,15 +18,20 @@ import (
 	cliruntime "github.com/Kong/volcano-cli/internal/runtime"
 )
 
+func resetInstructions(t *testing.T) {
+	t.Helper()
+	api.ResetLastInstructionsForTest()
+	t.Cleanup(api.ResetLastInstructionsForTest)
+}
+
 // withInstructions drives api.LastInstructions() through a real api.Client
 // call against a test server, mirroring how production populates it from
 // response headers (VOL-180).
 func withInstructions(t *testing.T, latest, deviceInstruction string) {
 	t.Helper()
-	// recordInstructions is sticky (VOL-180): a field a response omits doesn't
-	// clear a value recorded by an earlier test. Reset explicitly so each test
-	// starts from the zero value regardless of execution order.
-	api.ResetLastInstructionsForTest()
+	// recordInstructions is sticky (VOL-180), so isolate both the beginning and
+	// end of each test from the process-global state.
+	resetInstructions(t)
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
 		if latest != "" {
 			// The server never sends X-Volcano-CLI-Latest-Version without a
@@ -125,7 +131,7 @@ func runDeps(server *httptest.Server) cliruntime.Deps {
 }
 
 func TestRun_SuccessNoNotice(t *testing.T) {
-	api.ResetLastInstructionsForTest()
+	resetInstructions(t)
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
 		w.Header().Set("Content-Type", "application/json")
 		w.WriteHeader(http.StatusOK)
@@ -147,7 +153,7 @@ func TestRun_SuccessNoNotice(t *testing.T) {
 }
 
 func TestRun_SuccessWithSuggestionNotice(t *testing.T) {
-	api.ResetLastInstructionsForTest()
+	resetInstructions(t)
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
 		w.Header().Set("X-Volcano-CLI-Instruction", api.CLIInstructionSuggestionVersionUpgrade)
 		w.Header().Set("X-Volcano-CLI-Latest-Version", "v1.5.0")
@@ -170,15 +176,10 @@ func TestRun_SuccessWithSuggestionNotice(t *testing.T) {
 	assert.Contains(t, stderr.String(), "A newer Volcano CLI version is available: v1.5.0")
 }
 
-func TestRun_SuccessWithDeprecationNoticeOnExemptRoute(t *testing.T) {
-	// The exact scenario the notice-printing path exists for (per
-	// printDeprecationWarning's doc comment): a deprecated CLI's request lands
-	// on an exempt route (e.g. `login`) and succeeds — the server sets the
-	// require_version_upgrade header but doesn't 426 it. The user still needs
-	// to learn their CLI is deprecated even though this command was let
-	// through. Only the deprecation-error (426) path had run()-level coverage
-	// before this; this is the success-path counterpart.
-	api.ResetLastInstructionsForTest()
+func TestRun_SuccessWithDeprecationNotice(t *testing.T) {
+	// A successful API response can still carry a deprecation instruction. The
+	// command must warn without changing its successful exit status.
+	resetInstructions(t)
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
 		w.Header().Set("X-Volcano-CLI-Instruction", api.CLIInstructionRequireVersionUpgrade)
 		w.Header().Set("X-Volcano-CLI-Latest-Version", "v1.5.0")
@@ -205,8 +206,69 @@ func TestRun_SuccessWithDeprecationNoticeOnExemptRoute(t *testing.T) {
 	assert.Contains(t, stderr.String(), "is no longer supported. Upgrade to v1.5.0 or later:")
 }
 
+func TestRun_SuccessWithDeprecationNoticeOnExemptAuthRoute(t *testing.T) {
+	resetInstructions(t)
+	t.Setenv("HOME", t.TempDir())
+	t.Setenv("VOLCANO_TOKEN", "")
+	t.Setenv("VOLCANO_API_URL", "")
+	t.Setenv("VOLCANO_FIRST_PARTY_DEVICE_CLIENT_ID", "")
+
+	pollTicker := newMainTestTicker()
+	dotTicker := newMainTestTicker()
+	timeoutTimer := newMainTestTicker()
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("X-Volcano-CLI-Instruction", api.CLIInstructionRequireVersionUpgrade)
+		w.Header().Set("X-Volcano-CLI-Latest-Version", "v1.5.0")
+		w.Header().Set("Content-Type", "application/json")
+		switch r.URL.Path {
+		case "/auth/device/authorize":
+			_, _ = w.Write([]byte(`{"device_code":"device-code","user_code":"ABCD-EFGH","verification_uri":"https://volcano.dev/device","verification_uri_complete":"https://volcano.dev/device?user_code=ABCD-EFGH","expires_in":120,"interval":1}`))
+		case "/auth/device/token":
+			_, _ = w.Write([]byte(`{"access_token":"auth-access-token"}`))
+		case "/auth/platform/exchange":
+			_, _ = w.Write([]byte(`{"token":"platform-token","user_id":"platform-user-1","token_id":"33333333-3333-4333-8333-333333333333","expires_at":"2030-01-01T00:00:00Z"}`))
+		default:
+			http.NotFound(w, r)
+		}
+	}))
+	defer server.Close()
+
+	var tickerCalls int
+	deps := cliruntime.Deps{
+		HTTPClient:  server.Client(),
+		APIBaseURL:  server.URL,
+		OpenBrowser: func(string) error { return nil },
+		NewTimer:    func(time.Duration) cliruntime.Timer { return timeoutTimer },
+		NewTicker: func(time.Duration) cliruntime.Ticker {
+			tickerCalls++
+			if tickerCalls == 1 {
+				return pollTicker
+			}
+			return dotTicker
+		},
+	}
+	root := rootcmd.New(deps)
+	var stdout, stderr bytes.Buffer
+	root.SetOut(&stdout)
+	root.SetErr(&stderr)
+	root.SetArgs([]string{"login"})
+
+	done := make(chan int, 1)
+	go func() { done <- run(root, deps) }()
+	pollTicker.tick()
+	select {
+	case code := <-done:
+		assert.Equal(t, 0, code)
+	case <-time.After(2 * time.Second):
+		t.Fatal("login did not complete")
+	}
+	assert.Contains(t, stderr.String(), "Volcano CLI")
+	assert.Contains(t, stderr.String(), "is no longer supported. Upgrade to v1.5.0 or later:")
+	assert.Equal(t, 1, strings.Count(stderr.String(), "Volcano CLI"), "the early auth-route notice must not be repeated after Execute returns")
+}
+
 func TestRun_DeprecationErrorShortCircuitsWithoutDuplicateNotice(t *testing.T) {
-	api.ResetLastInstructionsForTest()
+	resetInstructions(t)
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
 		w.Header().Set("X-Volcano-CLI-Instruction", api.CLIInstructionRequireVersionUpgrade)
 		w.Header().Set("X-Volcano-CLI-Latest-Version", "v1.5.0")
@@ -238,7 +300,7 @@ func TestRun_DeprecationErrorShortCircuitsWithoutDuplicateNotice(t *testing.T) {
 func TestRun_NonBlockingErrorPrintsErrorBeforeNotice(t *testing.T) {
 	// A command that fails for an unrelated reason (404 here) while also
 	// carrying a pending suggestion notice must print the actual error first.
-	api.ResetLastInstructionsForTest()
+	resetInstructions(t)
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
 		w.Header().Set("X-Volcano-CLI-Instruction", api.CLIInstructionSuggestionVersionUpgrade)
 		w.Header().Set("X-Volcano-CLI-Latest-Version", "v1.5.0")
@@ -266,7 +328,7 @@ func TestRun_NonBlockingErrorPrintsErrorBeforeNotice(t *testing.T) {
 }
 
 func TestRun_NonBlockingErrorWithReauthHint(t *testing.T) {
-	api.ResetLastInstructionsForTest()
+	resetInstructions(t)
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
 		w.Header().Set("X-Volcano-Device-Instruction", api.DeviceInstructionReauth)
 		w.Header().Set("Content-Type", "application/json")
@@ -291,3 +353,16 @@ func TestRun_NonBlockingErrorWithReauthHint(t *testing.T) {
 	assert.Less(t, strings.Index(text, "Error:"), strings.Index(text, "Run `volcano login`"),
 		"the error line must come before the reauth hint: %q", text)
 }
+
+type mainTestTicker struct {
+	ch chan time.Time
+}
+
+func newMainTestTicker() *mainTestTicker {
+	return &mainTestTicker{ch: make(chan time.Time, 1)}
+}
+
+func (t *mainTestTicker) C() <-chan time.Time { return t.ch }
+func (t *mainTestTicker) Stop()               {}
+func (t *mainTestTicker) Reset(time.Duration) {}
+func (t *mainTestTicker) tick()               { t.ch <- time.Now() }

internal/api/log_stream_test.go

@@ -11,9 +11,12 @@ import (
 	"github.com/google/uuid"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+
+	"github.com/Kong/volcano-cli/internal/version"
 )
 
 func TestStreamProjectLogsRequestAndEvents(t *testing.T) {
+	resetInstructions(t)
 	projectID := uuid.MustParse("11111111-1111-4111-8111-111111111111")
 	functionID := uuid.MustParse("22222222-2222-4222-8222-222222222222")
 	deploymentID := uuid.MustParse("33333333-3333-4333-8333-333333333333")
@@ -25,7 +28,11 @@ func TestStreamProjectLogsRequestAndEvents(t *testing.T) {
 		assert.Equal(t, "/volcano-api/projects/"+projectID.String()+"/logs/stream", r.URL.Path)
 		assert.Equal(t, "Bearer token", r.Header.Get("Authorization"))
 		assert.Equal(t, "text/event-stream", r.Header.Get("Accept"))
+		assert.Equal(t, reportedCLIVersion(), r.Header.Get(headerCLIVersion))
+		assert.Contains(t, r.Header.Get("User-Agent"), "volcano-cli/"+version.Version)
 		lastEventID = r.Header.Get("Last-Event-ID")
+		w.Header().Set(headerCLIInstruction, CLIInstructionSuggestionVersionUpgrade)
+		w.Header().Set(headerCLILatestVersion, "v1.5.0")
 		require.NoError(t, json.NewDecoder(r.Body).Decode(&body))
 
 		w.Header().Set("Content-Type", "text/event-stream")
@@ -46,6 +53,10 @@ func TestStreamProjectLogsRequestAndEvents(t *testing.T) {
 	defer stream.Close()
 
 	assert.Equal(t, "previous-id", lastEventID)
+	assert.Equal(t, Instructions{
+		CLIInstruction: CLIInstructionSuggestionVersionUpgrade,
+		LatestVersion:  "v1.5.0",
+	}, LastInstructions())
 	resource, ok := body["resource"].(map[string]any)
 	require.True(t, ok)
 	assert.Equal(t, "function", resource["type"])

internal/api/version_protocol.go

@@ -10,14 +10,11 @@ import (
 	"github.com/Kong/volcano-cli/internal/version"
 )
 
-// Header names and instruction values for the VOL-180 CLI version protocol.
-// These MUST match volcano-hosting's internal/constants/http.go and
-// docs/cli/version-gating.md. Duplicated here because this is a separate
-// repository with no shared Go module; there is intentionally no
-// cryptographic binding on the request header — see "Security model" in that
-// doc for why the claimed version is advisory input only and never a
-// privilege (the CLI cannot make this header trustworthy, and doesn't need
-// to: the API never grants anything based on it).
+// Header names and instruction values for the CLI version protocol. This is
+// the CLI's HTTP wire contract and is duplicated here because this repository
+// has no shared Go module with the API. The claimed version is advisory input
+// only and never a privilege: the CLI cannot make this request header
+// trustworthy, and it does not use the value to grant authority.
 const (
 	headerCLIVersion        = "X-Volcano-CLI-Version"
 	headerCLIInstruction    = "X-Volcano-CLI-Instruction"
@@ -52,14 +49,22 @@ type Instructions struct {
 	DeviceInstruction string
 }
 
+type cliInstructionPair struct {
+	instruction   string
+	latestVersion string
+}
+
 var (
-	instructionsMu   sync.RWMutex
-	lastInstructions Instructions
+	instructionsMu          sync.RWMutex
+	lastInstructions        Instructions
+	consumedCLIInstructions map[cliInstructionPair]struct{}
 )
 
 // LastInstructions returns the most recently observed VOL-180 instructions
-// from any API response in this process. It is the zero value if no API call
-// has completed yet — e.g. local-only commands (init, help, version,
+// from any API response in this process. It retains the latest observed values
+// after ConsumeCLIInstructions renders their one-shot notice so error paths can
+// still use response metadata such as LatestVersion. It is the zero value if no
+// API call has completed yet — e.g. local-only commands (init, help, version,
 // completion) never populate this, since they never make a request.
 //
 // A CLI process runs exactly one command per invocation, so "most recent" is
@@ -71,6 +76,35 @@ func LastInstructions() Instructions {
 	return lastInstructions
 }
 
+// ConsumeCLIInstructions returns each observed CLI instruction/latest-version
+// pair once. Repeated API responses commonly carry the same pair, so retaining
+// the last consumed pair prevents a long-running command from rendering the
+// same notice again after a poll or stream reconnect. DeviceInstruction is
+// returned unchanged because callers render the reauthentication hint from
+// LastInstructions alongside command errors.
+func ConsumeCLIInstructions() Instructions {
+	instructionsMu.Lock()
+	defer instructionsMu.Unlock()
+
+	instructions := lastInstructions
+	pair := cliInstructionPair{
+		instruction:   instructions.CLIInstruction,
+		latestVersion: instructions.LatestVersion,
+	}
+	_, consumed := consumedCLIInstructions[pair]
+	if pair.instruction == "" || consumed {
+		instructions.CLIInstruction = ""
+		instructions.LatestVersion = ""
+		return instructions
+	}
+
+	if consumedCLIInstructions == nil {
+		consumedCLIInstructions = make(map[cliInstructionPair]struct{})
+	}
+	consumedCLIInstructions[pair] = struct{}{}
+	return instructions
+}
+
 // recordInstructions updates the two instruction fields independently, and
 // only when a response actually carries a value for that field — a response
 // with no X-Volcano-CLI-Instruction header does not clear a CLIInstruction
@@ -106,6 +140,57 @@ func userAgent() string {
 	return "volcano-cli/" + version.Version + " (" + runtime.GOOS + "/" + runtime.GOARCH + ")"
 }
 
+// reportedCLIVersion avoids accidentally presenting a source build as a
+// prerelease of its nearest tag. `git describe --tags --always --dirty` emits
+// values like v1.4.2-6-gabcdef0 (or a -dirty suffix), which semver correctly
+// orders below v1.4.2 even though the source is newer. Report source-build
+// identifiers as dev while retaining the full build identifier in User-Agent
+// for support diagnostics.
+func reportedCLIVersion() string {
+	value := strings.TrimSpace(version.Version)
+	if isGitDescribeBuild(value) || isGitSHA(value) {
+		return "dev"
+	}
+	return value
+}
+
+func isGitDescribeBuild(value string) bool {
+	if strings.HasSuffix(value, "-dirty") {
+		return true
+	}
+	marker := strings.LastIndex(value, "-g")
+	if marker < 1 {
+		return false
+	}
+	sha := value[marker+2:]
+	if len(sha) < 7 || !isHex(sha) {
+		return false
+	}
+	distanceStart := strings.LastIndex(value[:marker], "-")
+	if distanceStart < 1 {
+		return false
+	}
+	for _, r := range value[distanceStart+1 : marker] {
+		if r < '0' || r > '9' {
+			return false
+		}
+	}
+	return true
+}
+
+func isGitSHA(value string) bool {
+	return len(value) >= 7 && isHex(value)
+}
+
+func isHex(value string) bool {
+	for _, r := range value {
+		if (r < '0' || r > '9') && (r < 'a' || r > 'f') && (r < 'A' || r > 'F') {
+			return false
+		}
+	}
+	return true
+}
+
 // versionProtocolDoer wraps an apiclient.HttpRequestDoer to speak the VOL-180
 // CLI version protocol on every request: it reports this CLI's version and
 // identity, and records whatever instructions the API returns so callers can
@@ -117,7 +202,7 @@ type versionProtocolDoer struct {
 }
 
 func (d versionProtocolDoer) Do(req *http.Request) (*http.Response, error) {
-	req.Header.Set(headerCLIVersion, version.Version)
+	req.Header.Set(headerCLIVersion, reportedCLIVersion())
 	req.Header.Set("User-Agent", userAgent())
 	resp, err := d.next.Do(req)
 	if resp != nil {
@@ -136,5 +221,6 @@ func (d versionProtocolDoer) Do(req *http.Request) (*http.Response, error) {
 func ResetLastInstructionsForTest() {
 	instructionsMu.Lock()
 	lastInstructions = Instructions{}
+	consumedCLIInstructions = nil
 	instructionsMu.Unlock()
 }