rickub
rickub/clipublic Fork
⑂ aa87859
Branches
feat/10-repo-rename-transfer feat/7-edit-title-and-body feat/8-run-logs-job-filter feat/staleness-notice main test/create-body-dash-stdin
Tags
v0.1.0 v0.1.0-rc.11 v0.1.0-rc.14 v0.1.0-rc.16 v0.1.0-rc.20 v0.1.0-rc.26 v0.1.0-rc.36 v0.1.0-rc.43 v0.1.0-rc.50 v0.1.0-rc.64 v0.2.0 View all branches → View all tags →
Commits
⬇ Clone ▾
git clone https://git.rickub.com/rickub/cli.git
git clone ssh://git@rickub.com/rickub/cli.git

Host key fingerprint (ed25519): SHA256:iycHnxEyq0Q7uyVpB7JlznP0G7JrTPXLYRcAU5CSLhc — verify it before your first connect.

Download ZIP

Merge feat/staleness-notice into main

rickub committed 2026-10-11T17:31:29Z Browse files
aa87859 parents: a97c085 89ad491
modified cmd/body_test.go +74 -0
@@ -209,6 +209,80 @@ func TestMRCommentAnchoredBlankBodyIsRefused(t *testing.T) {
209209 }
210210 }
211211
212+// TestIssueCreateBodyDashReadsStdin and TestPRCreateBodyDashReadsStdin close
213+// rickub/rickub#279's acceptance criterion 3: "-b - reaches the API as the stdin
214+// content for BOTH issue create and pr create". The comment commands above
215+// already pin their halves; these pin the two CREATE halves — `pr create -b -`
216+// is the exact command that once stored a literal "-" and discarded an 8.9 KB
217+// description with a success line. Each test fails on a parser that uses the
218+// flag value verbatim (sent == "-"), which is how the pr-create half regressed
219+// before the shared resolver existed.
220+func TestIssueCreateBodyDashReadsStdin(t *testing.T) {
221+ var sent string
222+ defer withCreateBodyServer(t, &sent, "/issues")()
223+
224+ const text = "## Summary\n\nPins the portal gun back on the shelf.\n\n- one\n- two"
225+ if _, _, err := executeStdin(t, text+"\n", "issue", "create", "-R", "rick/portal",
226+ "-t", "the gun slips", "-b", "-"); err != nil {
227+ t.Fatalf("execute: %v", err)
228+ }
229+ if sent == "-" {
230+ t.Fatal(`issue create -b - posted the literal "-" — rickub/rickub#279 has regressed`)
231+ }
232+ if sent != text {
233+ t.Fatalf("posted body = %q, want the stdin text %q", sent, text)
234+ }
235+}
236+
237+func TestPRCreateBodyDashReadsStdin(t *testing.T) {
238+ var sent string
239+ defer withCreateBodyServer(t, &sent, "/merge-requests")()
240+
241+ const text = "## What\n\nA long description piped in from a file, the way MR bodies\nare actually written.\n"
242+ if _, _, err := executeStdin(t, text, "pr", "create", "-R", "rick/portal",
243+ "--base", "main", "--head", "fix/pin-the-gun", "-t", "fix: pin the gun", "-b", "-"); err != nil {
244+ t.Fatalf("execute: %v", err)
245+ }
246+ if sent == "-" {
247+ t.Fatal(`pr create -b - posted the literal "-" — rickub/rickub#279 has regressed`)
248+ }
249+ if sent != strings.TrimRight(text, "\r\n") {
250+ t.Fatalf("posted body = %q, want the stdin text %q", sent, text)
251+ }
252+}
253+
254+// withCreateBodyServer stands up a fake that answers a create endpoint (path
255+// suffix) and records the request JSON's body field, wiring the CLI at it. The
256+// suffix keeps the two tests honest about WHICH create they exercised; any
257+// other path fails the test rather than passing as an empty capture.
258+func withCreateBodyServer(t *testing.T, got *string, suffix string) func() {
259+ t.Helper()
260+ srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
261+ if r.Method != http.MethodPost || !strings.HasSuffix(r.URL.Path, suffix) {
262+ t.Errorf("create request used %s %s, want POST …%s", r.Method, r.URL.Path, suffix)
263+ }
264+ var body struct {
265+ Body string `json:"body"`
266+ }
267+ b, err := io.ReadAll(r.Body)
268+ if err != nil {
269+ t.Errorf("reading request body: %v", err)
270+ }
271+ if err := json.Unmarshal(b, &body); err != nil {
272+ t.Errorf("request body %q is not the JSON the CLI should send: %v", b, err)
273+ }
274+ *got = body.Body
275+ w.WriteHeader(http.StatusCreated)
276+ _ = json.NewEncoder(w).Encode(map[string]any{
277+ "number": 62, "title": "recorded", "state": "open",
278+ })
279+ }))
280+ t.Setenv("XDG_CONFIG_HOME", t.TempDir())
281+ t.Setenv("RICKUB_HOST", srv.URL)
282+ t.Setenv("RICKUB_TOKEN", "rickub_pat_test")
283+ return srv.Close
284+}
285+
212286 // TestBodyFlagHelpIsIdentical: both commands document the stdin behaviour in the
213287 // same words, because they read the same constant. Issue #94's criterion 4.
214288 func TestBodyFlagHelpIsIdentical(t *testing.T) {
@@ -209,6 +209,80 @@ func TestMRCommentAnchoredBlankBodyIsRefused(t *testing.T) {
209 }209 }
210 }210 }
211 211
212+// TestIssueCreateBodyDashReadsStdin and TestPRCreateBodyDashReadsStdin close
213+// rickub/rickub#279's acceptance criterion 3: "-b - reaches the API as the stdin
214+// content for BOTH issue create and pr create". The comment commands above
215+// already pin their halves; these pin the two CREATE halves — `pr create -b -`
216+// is the exact command that once stored a literal "-" and discarded an 8.9 KB
217+// description with a success line. Each test fails on a parser that uses the
218+// flag value verbatim (sent == "-"), which is how the pr-create half regressed
219+// before the shared resolver existed.
220+func TestIssueCreateBodyDashReadsStdin(t *testing.T) {
221+ var sent string
222+ defer withCreateBodyServer(t, &sent, "/issues")()
223+
224+ const text = "## Summary\n\nPins the portal gun back on the shelf.\n\n- one\n- two"
225+ if _, _, err := executeStdin(t, text+"\n", "issue", "create", "-R", "rick/portal",
226+ "-t", "the gun slips", "-b", "-"); err != nil {
227+ t.Fatalf("execute: %v", err)
228+ }
229+ if sent == "-" {
230+ t.Fatal(`issue create -b - posted the literal "-" — rickub/rickub#279 has regressed`)
231+ }
232+ if sent != text {
233+ t.Fatalf("posted body = %q, want the stdin text %q", sent, text)
234+ }
235+}
236+
237+func TestPRCreateBodyDashReadsStdin(t *testing.T) {
238+ var sent string
239+ defer withCreateBodyServer(t, &sent, "/merge-requests")()
240+
241+ const text = "## What\n\nA long description piped in from a file, the way MR bodies\nare actually written.\n"
242+ if _, _, err := executeStdin(t, text, "pr", "create", "-R", "rick/portal",
243+ "--base", "main", "--head", "fix/pin-the-gun", "-t", "fix: pin the gun", "-b", "-"); err != nil {
244+ t.Fatalf("execute: %v", err)
245+ }
246+ if sent == "-" {
247+ t.Fatal(`pr create -b - posted the literal "-" — rickub/rickub#279 has regressed`)
248+ }
249+ if sent != strings.TrimRight(text, "\r\n") {
250+ t.Fatalf("posted body = %q, want the stdin text %q", sent, text)
251+ }
252+}
253+
254+// withCreateBodyServer stands up a fake that answers a create endpoint (path
255+// suffix) and records the request JSON's body field, wiring the CLI at it. The
256+// suffix keeps the two tests honest about WHICH create they exercised; any
257+// other path fails the test rather than passing as an empty capture.
258+func withCreateBodyServer(t *testing.T, got *string, suffix string) func() {
259+ t.Helper()
260+ srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
261+ if r.Method != http.MethodPost || !strings.HasSuffix(r.URL.Path, suffix) {
262+ t.Errorf("create request used %s %s, want POST …%s", r.Method, r.URL.Path, suffix)
263+ }
264+ var body struct {
265+ Body string `json:"body"`
266+ }
267+ b, err := io.ReadAll(r.Body)
268+ if err != nil {
269+ t.Errorf("reading request body: %v", err)
270+ }
271+ if err := json.Unmarshal(b, &body); err != nil {
272+ t.Errorf("request body %q is not the JSON the CLI should send: %v", b, err)
273+ }
274+ *got = body.Body
275+ w.WriteHeader(http.StatusCreated)
276+ _ = json.NewEncoder(w).Encode(map[string]any{
277+ "number": 62, "title": "recorded", "state": "open",
278+ })
279+ }))
280+ t.Setenv("XDG_CONFIG_HOME", t.TempDir())
281+ t.Setenv("RICKUB_HOST", srv.URL)
282+ t.Setenv("RICKUB_TOKEN", "rickub_pat_test")
283+ return srv.Close
284+}
285+
212 // TestBodyFlagHelpIsIdentical: both commands document the stdin behaviour in the286 // TestBodyFlagHelpIsIdentical: both commands document the stdin behaviour in the
213 // same words, because they read the same constant. Issue #94's criterion 4.287 // same words, because they read the same constant. Issue #94's criterion 4.
214 func TestBodyFlagHelpIsIdentical(t *testing.T) {288 func TestBodyFlagHelpIsIdentical(t *testing.T) {
modified cmd/root.go +8 -0
@@ -37,6 +37,14 @@ Authenticate once with "rickub auth login", then drive repos, merge requests,
3737 CI runs, orgs, and code browsing from your terminal.`,
3838 SilenceUsage: true,
3939 SilenceErrors: true,
40+ // Every command starts with the (throttled, best-effort) staleness probe of
41+ // cmd/update_check.go — a stale CLI describes an old product as current
42+ // (rickub/rickub#305), and the notice has to appear on the commands people
43+ // actually run, not only on `rickub version`.
44+ PersistentPreRunE: func(cmd *cobra.Command, _ []string) error {
45+ maybeWarnAboutStaleCLI(cmd)
46+ return nil
47+ },
4048 }
4149
4250 // Execute runs the root command. main() maps the returned error to an exit code.
@@ -37,6 +37,14 @@ Authenticate once with "rickub auth login", then drive repos, merge requests,
37 CI runs, orgs, and code browsing from your terminal.`,37 CI runs, orgs, and code browsing from your terminal.`,
38 SilenceUsage: true,38 SilenceUsage: true,
39 SilenceErrors: true,39 SilenceErrors: true,
40+ // Every command starts with the (throttled, best-effort) staleness probe of
41+ // cmd/update_check.go — a stale CLI describes an old product as current
42+ // (rickub/rickub#305), and the notice has to appear on the commands people
43+ // actually run, not only on `rickub version`.
44+ PersistentPreRunE: func(cmd *cobra.Command, _ []string) error {
45+ maybeWarnAboutStaleCLI(cmd)
46+ return nil
47+ },
40 }48 }
41 49
42 // Execute runs the root command. main() maps the returned error to an exit code.50 // Execute runs the root command. main() maps the returned error to an exit code.
added cmd/update_check.go +301 -0
new file mode 100644
@@ -0,0 +1,301 @@
1+package cmd
2+
3+import (
4+ "context"
5+ "fmt"
6+ "net/http"
7+ "net/url"
8+ "os"
9+ "path/filepath"
10+ "strconv"
11+ "strings"
12+ "time"
13+
14+ "rickub.com/rickub/cli/internal/config"
15+
16+ "github.com/spf13/cobra"
17+)
18+
19+// --- CLI staleness detection (rickub/rickub issue 305) -------------------------
20+//
21+// A stale CLI does not fail loudly — it describes an OLD version of the product
22+// as if it were current, and everything downstream inherits that: issues filed
23+// against defects already fixed (#294, #279), agents concluding a shipped
24+// feature is missing, hours re-diagnosing what was diagnosed. It is also
25+// self-concealing: "does my CLI support X?" answers truthfully about the BINARY,
26+// which is the wrong question, and nothing prompts you to ask which build you
27+// are holding.
28+//
29+// The honest, minimal signal: the host the CLI already talks to names the newest
30+// rickub/cli release — GET /rickub/cli/releases/latest is a documented 302 whose
31+// Location carries the tag (the same redirect the install script resolves
32+// "latest" with; no token, no HTML scraping). When that release is strictly
33+// NEWER than the running binary, print a one-line notice naming both versions.
34+//
35+// The check is throttled to once per day per host (a stamp beside the config
36+// file), always best-effort — any failure is silent and never blocks the real
37+// command — and skipped entirely for dev builds (Version "dev"), which carry no
38+// release identity to compare.
39+
40+const (
41+ // updateCheckOwner/Repo: the CLI's own distribution repository, served by
42+ // every host that mirrors it. A host without it 404s and the check stays
43+ // quiet — nothing to compare, nothing to say.
44+ updateCheckOwner = "rickub"
45+ updateCheckRepo = "cli"
46+ // updateCheckTimeout bounds the probe; it runs BEFORE the real command, so
47+ // a slow host must not tax every invocation (once a day, this is the ceiling).
48+ updateCheckTimeout = 2 * time.Second
49+ // updateCheckInterval is how often the probe repeats per host.
50+ updateCheckInterval = 24 * time.Hour
51+)
52+
53+// maybeWarnAboutStaleCLI is hooked into the root command's PersistentPreRun:
54+// every command (version included) gets the notice at most once a day. It never
55+// returns an error and never writes to stdout — the notice goes to stderr, so
56+// --json output stays parseable and pipes stay clean.
57+func maybeWarnAboutStaleCLI(cmd *cobra.Command) {
58+ if Version == "dev" {
59+ return // a build from source has no release identity to compare
60+ }
61+ cfg, err := loadConfig()
62+ if err != nil {
63+ return
64+ }
65+ host := config.ResolveHost(flagHost, cfg)
66+ stampPath, err := updateStampPath(host)
67+ if err != nil {
68+ return
69+ }
70+ if stampIsFresh(stampPath) {
71+ return
72+ }
73+ // An attempted check (whatever the outcome) suppresses the next for a day:
74+ // a host that cannot answer today should not be probed on every command.
75+ writeUpdateStamp(stampPath)
76+ tag := resolveLatestCLITag(cmd.Context(), host)
77+ if !newerThan(tag, Version) {
78+ return
79+ }
80+ base := strings.TrimSuffix(config.NormalizeHost(host), "/")
81+ fmt.Fprintf(cmd.ErrOrStderr(),
82+ "note: this rickub CLI is %s but the newest release on %s is %s — some bugs you see may already be fixed; upgrade: %s/%s/%s/releases/%s\n",
83+ Version, base, tag, base, updateCheckOwner, updateCheckRepo, tag)
84+}
85+
86+// resolveLatestCLITag asks host for the newest rickub/cli release tag via the
87+// documented /releases/latest redirect, returning "" on ANY failure (no
88+// redirect, no releases, unreachable host, unparsable Location). Redirects are
89+// not followed: the Location header is the answer.
90+func resolveLatestCLITag(ctx context.Context, host string) string {
91+ base := strings.TrimSuffix(config.NormalizeHost(host), "/")
92+ if base == "" {
93+ return ""
94+ }
95+ req, err := http.NewRequestWithContext(ctx, http.MethodGet,
96+ base+"/"+updateCheckOwner+"/"+updateCheckRepo+"/releases/latest", nil)
97+ if err != nil {
98+ return ""
99+ }
100+ client := &http.Client{
101+ Timeout: updateCheckTimeout,
102+ CheckRedirect: func(_ *http.Request, _ []*http.Request) error {
103+ return http.ErrUseLastResponse
104+ },
105+ }
106+ resp, err := client.Do(req)
107+ if err != nil {
108+ return ""
109+ }
110+ defer func() { _ = resp.Body.Close() }()
111+ if resp.StatusCode != http.StatusFound && resp.StatusCode != http.StatusMovedPermanently {
112+ return ""
113+ }
114+ return tagFromReleaseLocation(resp.Header.Get("Location"))
115+}
116+
117+// tagFromReleaseLocation extracts the release tag from a /releases/latest
118+// Location value: "/rickub/cli/releases/v0.2.0" (relative, what the host
119+// writes) or "https://host/rickub/cli/releases/v0.1.0-rc.9" (absolute). The tag
120+// is the whole tail after "/releases/" — tags may contain slashes, and the
121+// redirect builds the path from the tag verbatim. Anything else returns "".
122+func tagFromReleaseLocation(loc string) string {
123+ if loc == "" {
124+ return ""
125+ }
126+ u, err := url.Parse(loc)
127+ if err != nil {
128+ return ""
129+ }
130+ parts := strings.Split(strings.Trim(u.Path, "/"), "/")
131+ const min = 4 // owner, repo, "releases", tag...
132+ if len(parts) < min || parts[2] != "releases" {
133+ return ""
134+ }
135+ tag := strings.Join(parts[3:], "/")
136+ if tag == "" || !looksLikeTag(tag) {
137+ return ""
138+ }
139+ decoded, err := url.PathUnescape(tag)
140+ if err != nil {
141+ return ""
142+ }
143+ return decoded
144+}
145+
146+// looksLikeTag guards the tail-is-the-tag reading against a Location pointing
147+// at a different page under /releases/ (the list page, "latest" itself, an
148+// asset download). A release tag names a version; it may carry slashes, but its
149+// first segment is never one of the reserved page names.
150+func looksLikeTag(tag string) bool {
151+ switch strings.Split(tag, "/")[0] {
152+ case "new", "latest", "download", "edit", "release-assets":
153+ return false
154+ }
155+ return true
156+}
157+
158+// ---- version comparison -------------------------------------------------------
159+//
160+// Release tags are v-prefixed semver ("v0.2.0", "v0.1.0-rc.11"); the stamped
161+// Version is bare ("0.2.0", "0.1.0-rc.11"). Comparison follows semver ordering:
162+// numeric major.minor.patch (missing components are 0), then prerelease — where
163+// a version WITHOUT a prerelease is newer than the same version WITH one, and
164+// prerelease identifiers compare numerically when both are numeric, otherwise
165+// byte-wise (semver spec rule 12, the parts rc.N needs).
166+
167+// newerThan reports whether latest is a strictly newer version than current.
168+// Anything unparsable is NOT newer: an exotic tag never produces a notice.
169+func newerThan(latest, current string) bool {
170+ return compareVersions(latest, current) > 0
171+}
172+
173+// compareVersions returns -1, 0 or 1 comparing two v-prefixed-or-bare semver
174+// strings. Two unparsable, or an unparsable against a parsable, compare equal
175+// (0) — the caller's contract is "strictly newer means warn", so unknowns never
176+// warn.
177+func compareVersions(a, b string) int {
178+ ac, ap, aok := splitVersion(a)
179+ bc, bp, bok := splitVersion(b)
180+ if !aok || !bok {
181+ return 0
182+ }
183+ for i := range 3 {
184+ if ac[i] != bc[i] {
185+ if ac[i] < bc[i] {
186+ return -1
187+ }
188+ return 1
189+ }
190+ }
191+ // Same numbers: no prerelease wins over any prerelease.
192+ if ap == "" && bp == "" {
193+ return 0
194+ }
195+ if ap == "" {
196+ return 1
197+ }
198+ if bp == "" {
199+ return -1
200+ }
201+ return comparePrerelease(ap, bp)
202+}
203+
204+// splitVersion parses "v1.2.3-rc.4" into ([3]int{1,2,3}, "rc.4"); ok is false
205+// for a version it does not understand.
206+func splitVersion(v string) (core [3]int, pre string, ok bool) {
207+ v = strings.TrimPrefix(strings.TrimSpace(v), "v")
208+ coreStr, pre, _ := strings.Cut(v, "-")
209+ parts := strings.Split(coreStr, ".")
210+ if len(parts) == 0 || len(parts) > 3 {
211+ return [3]int{}, "", false
212+ }
213+ for i, p := range parts {
214+ n, err := strconv.Atoi(p)
215+ if err != nil || n < 0 || (len(p) > 1 && p[0] == '0') {
216+ return [3]int{}, "", false // leading zeros / non-numeric: not semver
217+ }
218+ core[i] = n
219+ }
220+ return core, pre, true
221+}
222+
223+// comparePrerelease orders two dot-separated prerelease strings per semver: each
224+// identifier numerically when both are all-digits, else byte-wise; a shorter
225+// list is LOWER when every shared identifier is equal.
226+func comparePrerelease(a, b string) int {
227+ as, bs := strings.Split(a, "."), strings.Split(b, ".")
228+ for i := 0; i < len(as) && i < len(bs); i++ {
229+ an, aerr := strconv.Atoi(as[i])
230+ bn, berr := strconv.Atoi(bs[i])
231+ if aerr == nil && berr == nil {
232+ if an != bn {
233+ if an < bn {
234+ return -1
235+ }
236+ return 1
237+ }
238+ continue
239+ }
240+ if as[i] != bs[i] {
241+ if as[i] < bs[i] {
242+ return -1
243+ }
244+ return 1
245+ }
246+ }
247+ switch {
248+ case len(as) < len(bs):
249+ return -1
250+ case len(as) > len(bs):
251+ return 1
252+ }
253+ return 0
254+}
255+
256+// ---- the once-a-day stamp -----------------------------------------------------
257+
258+// updateStampPath names the stamp file beside the config file, one per host:
259+// <configdir>/rickub/update-check/<normalized host>. The host is normalized and
260+// path-safe (slashes and colons folded to underscores) so two hosts never share
261+// a stamp.
262+func updateStampPath(host string) (string, error) {
263+ cfgPath, err := config.Path()
264+ if err != nil {
265+ return "", err
266+ }
267+ key := config.NormalizeHost(host)
268+ if key == "" {
269+ key = "default"
270+ }
271+ key = strings.NewReplacer("/", "_", ":", "_").Replace(key)
272+ return filepath.Join(filepath.Dir(cfgPath), "update-check", key), nil
273+}
274+
275+// stampIsFresh reports whether the stamp at path was written within the check
276+// interval. A missing or unreadable stamp is stale (check again); a future
277+// stamp (clock moved back) is treated as fresh so it cannot pinball the probe.
278+func stampIsFresh(path string) bool {
279+ // #nosec G304 -- path is the CLI's own stamp under the config directory
280+ // (config.Path + a sanitized host key), the same class as the config read
281+ // in internal/config; it is never assembled from request or command input.
282+ data, err := os.ReadFile(path)
283+ if err != nil {
284+ return false
285+ }
286+ stamped, err := time.Parse(time.RFC3339, strings.TrimSpace(string(data)))
287+ if err != nil {
288+ return false
289+ }
290+ return time.Since(stamped) < updateCheckInterval || stamped.After(time.Now())
291+}
292+
293+// writeUpdateStamp records a successful attempt, best-effort: a stamp that
294+// cannot be written simply means the check may repeat — never a user-visible
295+// error.
296+func writeUpdateStamp(path string) {
297+ if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil {
298+ return
299+ }
300+ _ = os.WriteFile(path, []byte(time.Now().UTC().Format(time.RFC3339)), 0o600)
301+}
new file mode 100644
@@ -0,0 +1,301 @@
1+package cmd
2+
3+import (
4+ "context"
5+ "fmt"
6+ "net/http"
7+ "net/url"
8+ "os"
9+ "path/filepath"
10+ "strconv"
11+ "strings"
12+ "time"
13+
14+ "rickub.com/rickub/cli/internal/config"
15+
16+ "github.com/spf13/cobra"
17+)
18+
19+// --- CLI staleness detection (rickub/rickub issue 305) -------------------------
20+//
21+// A stale CLI does not fail loudly — it describes an OLD version of the product
22+// as if it were current, and everything downstream inherits that: issues filed
23+// against defects already fixed (#294, #279), agents concluding a shipped
24+// feature is missing, hours re-diagnosing what was diagnosed. It is also
25+// self-concealing: "does my CLI support X?" answers truthfully about the BINARY,
26+// which is the wrong question, and nothing prompts you to ask which build you
27+// are holding.
28+//
29+// The honest, minimal signal: the host the CLI already talks to names the newest
30+// rickub/cli release — GET /rickub/cli/releases/latest is a documented 302 whose
31+// Location carries the tag (the same redirect the install script resolves
32+// "latest" with; no token, no HTML scraping). When that release is strictly
33+// NEWER than the running binary, print a one-line notice naming both versions.
34+//
35+// The check is throttled to once per day per host (a stamp beside the config
36+// file), always best-effort — any failure is silent and never blocks the real
37+// command — and skipped entirely for dev builds (Version "dev"), which carry no
38+// release identity to compare.
39+
40+const (
41+ // updateCheckOwner/Repo: the CLI's own distribution repository, served by
42+ // every host that mirrors it. A host without it 404s and the check stays
43+ // quiet — nothing to compare, nothing to say.
44+ updateCheckOwner = "rickub"
45+ updateCheckRepo = "cli"
46+ // updateCheckTimeout bounds the probe; it runs BEFORE the real command, so
47+ // a slow host must not tax every invocation (once a day, this is the ceiling).
48+ updateCheckTimeout = 2 * time.Second
49+ // updateCheckInterval is how often the probe repeats per host.
50+ updateCheckInterval = 24 * time.Hour
51+)
52+
53+// maybeWarnAboutStaleCLI is hooked into the root command's PersistentPreRun:
54+// every command (version included) gets the notice at most once a day. It never
55+// returns an error and never writes to stdout — the notice goes to stderr, so
56+// --json output stays parseable and pipes stay clean.
57+func maybeWarnAboutStaleCLI(cmd *cobra.Command) {
58+ if Version == "dev" {
59+ return // a build from source has no release identity to compare
60+ }
61+ cfg, err := loadConfig()
62+ if err != nil {
63+ return
64+ }
65+ host := config.ResolveHost(flagHost, cfg)
66+ stampPath, err := updateStampPath(host)
67+ if err != nil {
68+ return
69+ }
70+ if stampIsFresh(stampPath) {
71+ return
72+ }
73+ // An attempted check (whatever the outcome) suppresses the next for a day:
74+ // a host that cannot answer today should not be probed on every command.
75+ writeUpdateStamp(stampPath)
76+ tag := resolveLatestCLITag(cmd.Context(), host)
77+ if !newerThan(tag, Version) {
78+ return
79+ }
80+ base := strings.TrimSuffix(config.NormalizeHost(host), "/")
81+ fmt.Fprintf(cmd.ErrOrStderr(),
82+ "note: this rickub CLI is %s but the newest release on %s is %s — some bugs you see may already be fixed; upgrade: %s/%s/%s/releases/%s\n",
83+ Version, base, tag, base, updateCheckOwner, updateCheckRepo, tag)
84+}
85+
86+// resolveLatestCLITag asks host for the newest rickub/cli release tag via the
87+// documented /releases/latest redirect, returning "" on ANY failure (no
88+// redirect, no releases, unreachable host, unparsable Location). Redirects are
89+// not followed: the Location header is the answer.
90+func resolveLatestCLITag(ctx context.Context, host string) string {
91+ base := strings.TrimSuffix(config.NormalizeHost(host), "/")
92+ if base == "" {
93+ return ""
94+ }
95+ req, err := http.NewRequestWithContext(ctx, http.MethodGet,
96+ base+"/"+updateCheckOwner+"/"+updateCheckRepo+"/releases/latest", nil)
97+ if err != nil {
98+ return ""
99+ }
100+ client := &http.Client{
101+ Timeout: updateCheckTimeout,
102+ CheckRedirect: func(_ *http.Request, _ []*http.Request) error {
103+ return http.ErrUseLastResponse
104+ },
105+ }
106+ resp, err := client.Do(req)
107+ if err != nil {
108+ return ""
109+ }
110+ defer func() { _ = resp.Body.Close() }()
111+ if resp.StatusCode != http.StatusFound && resp.StatusCode != http.StatusMovedPermanently {
112+ return ""
113+ }
114+ return tagFromReleaseLocation(resp.Header.Get("Location"))
115+}
116+
117+// tagFromReleaseLocation extracts the release tag from a /releases/latest
118+// Location value: "/rickub/cli/releases/v0.2.0" (relative, what the host
119+// writes) or "https://host/rickub/cli/releases/v0.1.0-rc.9" (absolute). The tag
120+// is the whole tail after "/releases/" — tags may contain slashes, and the
121+// redirect builds the path from the tag verbatim. Anything else returns "".
122+func tagFromReleaseLocation(loc string) string {
123+ if loc == "" {
124+ return ""
125+ }
126+ u, err := url.Parse(loc)
127+ if err != nil {
128+ return ""
129+ }
130+ parts := strings.Split(strings.Trim(u.Path, "/"), "/")
131+ const min = 4 // owner, repo, "releases", tag...
132+ if len(parts) < min || parts[2] != "releases" {
133+ return ""
134+ }
135+ tag := strings.Join(parts[3:], "/")
136+ if tag == "" || !looksLikeTag(tag) {
137+ return ""
138+ }
139+ decoded, err := url.PathUnescape(tag)
140+ if err != nil {
141+ return ""
142+ }
143+ return decoded
144+}
145+
146+// looksLikeTag guards the tail-is-the-tag reading against a Location pointing
147+// at a different page under /releases/ (the list page, "latest" itself, an
148+// asset download). A release tag names a version; it may carry slashes, but its
149+// first segment is never one of the reserved page names.
150+func looksLikeTag(tag string) bool {
151+ switch strings.Split(tag, "/")[0] {
152+ case "new", "latest", "download", "edit", "release-assets":
153+ return false
154+ }
155+ return true
156+}
157+
158+// ---- version comparison -------------------------------------------------------
159+//
160+// Release tags are v-prefixed semver ("v0.2.0", "v0.1.0-rc.11"); the stamped
161+// Version is bare ("0.2.0", "0.1.0-rc.11"). Comparison follows semver ordering:
162+// numeric major.minor.patch (missing components are 0), then prerelease — where
163+// a version WITHOUT a prerelease is newer than the same version WITH one, and
164+// prerelease identifiers compare numerically when both are numeric, otherwise
165+// byte-wise (semver spec rule 12, the parts rc.N needs).
166+
167+// newerThan reports whether latest is a strictly newer version than current.
168+// Anything unparsable is NOT newer: an exotic tag never produces a notice.
169+func newerThan(latest, current string) bool {
170+ return compareVersions(latest, current) > 0
171+}
172+
173+// compareVersions returns -1, 0 or 1 comparing two v-prefixed-or-bare semver
174+// strings. Two unparsable, or an unparsable against a parsable, compare equal
175+// (0) — the caller's contract is "strictly newer means warn", so unknowns never
176+// warn.
177+func compareVersions(a, b string) int {
178+ ac, ap, aok := splitVersion(a)
179+ bc, bp, bok := splitVersion(b)
180+ if !aok || !bok {
181+ return 0
182+ }
183+ for i := range 3 {
184+ if ac[i] != bc[i] {
185+ if ac[i] < bc[i] {
186+ return -1
187+ }
188+ return 1
189+ }
190+ }
191+ // Same numbers: no prerelease wins over any prerelease.
192+ if ap == "" && bp == "" {
193+ return 0
194+ }
195+ if ap == "" {
196+ return 1
197+ }
198+ if bp == "" {
199+ return -1
200+ }
201+ return comparePrerelease(ap, bp)
202+}
203+
204+// splitVersion parses "v1.2.3-rc.4" into ([3]int{1,2,3}, "rc.4"); ok is false
205+// for a version it does not understand.
206+func splitVersion(v string) (core [3]int, pre string, ok bool) {
207+ v = strings.TrimPrefix(strings.TrimSpace(v), "v")
208+ coreStr, pre, _ := strings.Cut(v, "-")
209+ parts := strings.Split(coreStr, ".")
210+ if len(parts) == 0 || len(parts) > 3 {
211+ return [3]int{}, "", false
212+ }
213+ for i, p := range parts {
214+ n, err := strconv.Atoi(p)
215+ if err != nil || n < 0 || (len(p) > 1 && p[0] == '0') {
216+ return [3]int{}, "", false // leading zeros / non-numeric: not semver
217+ }
218+ core[i] = n
219+ }
220+ return core, pre, true
221+}
222+
223+// comparePrerelease orders two dot-separated prerelease strings per semver: each
224+// identifier numerically when both are all-digits, else byte-wise; a shorter
225+// list is LOWER when every shared identifier is equal.
226+func comparePrerelease(a, b string) int {
227+ as, bs := strings.Split(a, "."), strings.Split(b, ".")
228+ for i := 0; i < len(as) && i < len(bs); i++ {
229+ an, aerr := strconv.Atoi(as[i])
230+ bn, berr := strconv.Atoi(bs[i])
231+ if aerr == nil && berr == nil {
232+ if an != bn {
233+ if an < bn {
234+ return -1
235+ }
236+ return 1
237+ }
238+ continue
239+ }
240+ if as[i] != bs[i] {
241+ if as[i] < bs[i] {
242+ return -1
243+ }
244+ return 1
245+ }
246+ }
247+ switch {
248+ case len(as) < len(bs):
249+ return -1
250+ case len(as) > len(bs):
251+ return 1
252+ }
253+ return 0
254+}
255+
256+// ---- the once-a-day stamp -----------------------------------------------------
257+
258+// updateStampPath names the stamp file beside the config file, one per host:
259+// <configdir>/rickub/update-check/<normalized host>. The host is normalized and
260+// path-safe (slashes and colons folded to underscores) so two hosts never share
261+// a stamp.
262+func updateStampPath(host string) (string, error) {
263+ cfgPath, err := config.Path()
264+ if err != nil {
265+ return "", err
266+ }
267+ key := config.NormalizeHost(host)
268+ if key == "" {
269+ key = "default"
270+ }
271+ key = strings.NewReplacer("/", "_", ":", "_").Replace(key)
272+ return filepath.Join(filepath.Dir(cfgPath), "update-check", key), nil
273+}
274+
275+// stampIsFresh reports whether the stamp at path was written within the check
276+// interval. A missing or unreadable stamp is stale (check again); a future
277+// stamp (clock moved back) is treated as fresh so it cannot pinball the probe.
278+func stampIsFresh(path string) bool {
279+ // #nosec G304 -- path is the CLI's own stamp under the config directory
280+ // (config.Path + a sanitized host key), the same class as the config read
281+ // in internal/config; it is never assembled from request or command input.
282+ data, err := os.ReadFile(path)
283+ if err != nil {
284+ return false
285+ }
286+ stamped, err := time.Parse(time.RFC3339, strings.TrimSpace(string(data)))
287+ if err != nil {
288+ return false
289+ }
290+ return time.Since(stamped) < updateCheckInterval || stamped.After(time.Now())
291+}
292+
293+// writeUpdateStamp records a successful attempt, best-effort: a stamp that
294+// cannot be written simply means the check may repeat — never a user-visible
295+// error.
296+func writeUpdateStamp(path string) {
297+ if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil {
298+ return
299+ }
300+ _ = os.WriteFile(path, []byte(time.Now().UTC().Format(time.RFC3339)), 0o600)
301+}
added cmd/update_check_test.go +372 -0
new file mode 100644
@@ -0,0 +1,372 @@
1+package cmd
2+
3+import (
4+ "net/http"
5+ "net/http/httptest"
6+ "os"
7+ "path/filepath"
8+ "strings"
9+ "testing"
10+ "time"
11+
12+ "rickub.com/rickub/cli/internal/config"
13+)
14+
15+// Tests for the CLI staleness notice (rickub/rickub#305).
16+//
17+// The notice is the one-line signal that a binary older than the newest release
18+// on its host keeps describing an old product as current — the failure mode that
19+// manufactured the false bug reports #294 and #279. Each behavior test below
20+// fails on the pre-change code (no notice, no probe) and on the two plausible
21+// regressions: the comparator flipping (nagging on fresh binaries, silence on
22+// stale ones) and the probe losing its throttle (a request on every command).
23+
24+// stampAge rewinds the stamp for host by d, so a test can simulate "checked
25+// yesterday" without sleeping. It fails the test when no stamp exists — a probe
26+// that never wrote its stamp is itself a bug (it would hammer the host).
27+func stampAge(t *testing.T, host string, d time.Duration) {
28+ t.Helper()
29+ path, err := updateStampPath(host)
30+ if err != nil {
31+ t.Fatalf("stamp path: %v", err)
32+ }
33+ if err := os.WriteFile(path, []byte(time.Now().UTC().Add(-d).Format(time.RFC3339)), 0o600); err != nil {
34+ t.Fatalf("rewind stamp: %v", err)
35+ }
36+}
37+
38+// updateServer stands up a host whose /rickub/cli/releases/latest redirects to
39+// tag, counting probes. It answers nothing else — a probe that wanders off its
40+// endpoint shows up as a failed assertion or an empty tag, never as a pass.
41+func updateServer(t *testing.T, tag string, probes *int) *httptest.Server {
42+ t.Helper()
43+ srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
44+ if r.URL.Path == "/"+updateCheckOwner+"/"+updateCheckRepo+"/releases/latest" {
45+ if probes != nil {
46+ *probes++
47+ }
48+ http.Redirect(w, r, "/"+updateCheckOwner+"/"+updateCheckRepo+"/releases/"+tag, http.StatusFound)
49+ return
50+ }
51+ http.NotFound(w, r)
52+ }))
53+ t.Setenv("XDG_CONFIG_HOME", t.TempDir())
54+ t.Setenv("RICKUB_HOST", srv.URL)
55+ t.Setenv("RICKUB_TOKEN", "rickub_pat_test")
56+ return srv
57+}
58+
59+// withVersion pins cmd.Version for one test and restores it after.
60+func withVersion(t *testing.T, v string) {
61+ t.Helper()
62+ old := Version
63+ Version = v
64+ t.Cleanup(func() { Version = old })
65+}
66+
67+// TestStaleCLINoticeNamesBothVersions is the acceptance core of rickub/rickub#305:
68+// a CLI older than the newest release on its host prints a visible one-line
69+// warning naming BOTH versions, on stderr (stdout stays machine-parseable).
70+func TestStaleCLINoticeNamesBothVersions(t *testing.T) {
71+ withVersion(t, "0.1.0")
72+ srv := updateServer(t, "v0.2.0", nil)
73+ defer srv.Close()
74+
75+ out, errOut, err := execute(t, "version")
76+ if err != nil {
77+ t.Fatalf("execute: %v", err)
78+ }
79+ if !strings.Contains(errOut, "0.1.0") || !strings.Contains(errOut, "v0.2.0") {
80+ t.Fatalf("notice must name both versions (running 0.1.0, available v0.2.0); stderr = %q", errOut)
81+ }
82+ if !strings.Contains(errOut, "upgrade") {
83+ t.Fatalf("notice must say how to move on; stderr = %q", errOut)
84+ }
85+ if !strings.Contains(out, "rickub 0.1.0") {
86+ t.Fatalf("stdout must keep the plain version line, got %q", out)
87+ }
88+ if strings.Contains(out, "note:") {
89+ t.Fatalf("notice leaked into stdout: %q", out)
90+ }
91+}
92+
93+// TestFreshCLIIsQuiet: a binary at (or past) the newest release prints nothing —
94+// the notice is a staleness signal, not a check-in.
95+func TestFreshCLIIsQuiet(t *testing.T) {
96+ withVersion(t, "0.2.0")
97+ srv := updateServer(t, "v0.2.0", nil)
98+ defer srv.Close()
99+
100+ _, errOut, err := execute(t, "version")
101+ if err != nil {
102+ t.Fatalf("execute: %v", err)
103+ }
104+ if strings.Contains(errOut, "note:") {
105+ t.Fatalf("fresh binary was nagged: %q", errOut)
106+ }
107+}
108+
109+// TestRCOfCurrentVersionIsNotNewer: the host's "latest" includes prereleases
110+// (most-recently-created), and v0.2.0-rc.7 is NOT newer than a running 0.2.0 —
111+// semver orders the prerelease below its release. Nagging every stable user
112+// about each rc would teach people to ignore the notice.
113+func TestRCOfCurrentVersionIsNotNewer(t *testing.T) {
114+ withVersion(t, "0.2.0")
115+ srv := updateServer(t, "v0.2.0-rc.7", nil)
116+ defer srv.Close()
117+
118+ _, errOut, err := execute(t, "version")
119+ if err != nil {
120+ t.Fatalf("execute: %v", err)
121+ }
122+ if strings.Contains(errOut, "note:") {
123+ t.Fatalf("rc-of-running-version produced a notice: %q", errOut)
124+ }
125+}
126+
127+// TestProbeIsThrottledPerHost: two commands in one day issue ONE probe; a day
128+// later the probe repeats. The stamp is what keeps the check off every command —
129+// without it the CLI adds a network round-trip to every invocation.
130+func TestProbeIsThrottledPerHost(t *testing.T) {
131+ withVersion(t, "0.1.0")
132+ var probes int
133+ srv := updateServer(t, "v0.2.0", &probes)
134+ defer srv.Close()
135+
136+ if _, _, err := execute(t, "version"); err != nil {
137+ t.Fatalf("execute: %v", err)
138+ }
139+ if _, _, err := execute(t, "version"); err != nil {
140+ t.Fatalf("execute: %v", err)
141+ }
142+ if probes != 1 {
143+ t.Fatalf("probes = %d after two same-day commands, want 1", probes)
144+ }
145+ stampAge(t, srv.URL, 25*time.Hour) // yesterday's check has aged out
146+ if _, _, err := execute(t, "version"); err != nil {
147+ t.Fatalf("execute: %v", err)
148+ }
149+ if probes != 2 {
150+ t.Fatalf("probes = %d after the stamp aged out, want 2", probes)
151+ }
152+}
153+
154+// TestDevBuildSkipsTheProbe: Version "dev" (a build from source, every `go
155+// test`/`go run`) never probes — it has no release identity, and the check must
156+// not add requests to test runs and local development.
157+func TestDevBuildSkipsTheProbe(t *testing.T) {
158+ withVersion(t, "dev")
159+ var probes int
160+ srv := updateServer(t, "v9.9.9", &probes)
161+ defer srv.Close()
162+
163+ if _, _, err := execute(t, "version"); err != nil {
164+ t.Fatalf("execute: %v", err)
165+ }
166+ if probes != 0 {
167+ t.Fatalf("dev build probed the host %d times, want 0", probes)
168+ }
169+}
170+
171+// TestProbeFailuresAreSilent: a host without the CLI repo (404), a non-redirect
172+// answer, a Location that is not a release tag, and an unreachable host each
173+// leave the command succeeding with no notice — best-effort means a failed check
174+// never becomes the user's problem.
175+func TestProbeFailuresAreSilent(t *testing.T) {
176+ withVersion(t, "0.1.0")
177+
178+ cases := []struct {
179+ name string
180+ handler http.HandlerFunc
181+ }{
182+ {"no releases on host", func(w http.ResponseWriter, r *http.Request) {
183+ http.NotFound(w, r)
184+ }},
185+ {"not a redirect", func(w http.ResponseWriter, _ *http.Request) {
186+ _, _ = w.Write([]byte(`{"ok":true}`))
187+ }},
188+ {"location is the releases list page", func(w http.ResponseWriter, r *http.Request) {
189+ http.Redirect(w, r, "/"+updateCheckOwner+"/"+updateCheckRepo+"/releases", http.StatusFound)
190+ }},
191+ {"location names a non-version tag", func(w http.ResponseWriter, r *http.Request) {
192+ http.Redirect(w, r, "/"+updateCheckOwner+"/"+updateCheckRepo+"/releases/runner-abc123", http.StatusFound)
193+ }},
194+ }
195+ for _, tc := range cases {
196+ t.Run(tc.name, func(t *testing.T) {
197+ srv := httptest.NewServer(tc.handler)
198+ defer srv.Close()
199+ t.Setenv("XDG_CONFIG_HOME", t.TempDir())
200+ t.Setenv("RICKUB_HOST", srv.URL)
201+
202+ _, errOut, err := execute(t, "version")
203+ if err != nil {
204+ t.Fatalf("execute: %v", err)
205+ }
206+ if strings.Contains(errOut, "note:") {
207+ t.Fatalf("failed probe produced a notice: %q", errOut)
208+ }
209+ })
210+ }
211+
212+ // An unreachable host: point at a port that is closed.
213+ t.Run("unreachable host", func(t *testing.T) {
214+ t.Setenv("XDG_CONFIG_HOME", t.TempDir())
215+ t.Setenv("RICKUB_HOST", "http://127.0.0.1:1")
216+ _, errOut, err := execute(t, "version")
217+ if err != nil {
218+ t.Fatalf("execute: %v", err)
219+ }
220+ if strings.Contains(errOut, "note:") {
221+ t.Fatalf("unreachable host produced a notice: %q", errOut)
222+ }
223+ })
224+}
225+
226+// TestStampIsPerHost: a stamp for one host must not silence the check against
227+// another — switching --host (or RICKUB_HOST) re-checks the host you actually
228+// asked about.
229+func TestStampIsPerHost(t *testing.T) {
230+ withVersion(t, "0.1.0")
231+ var probesA, probesB int
232+ srvA := updateServer(t, "v0.2.0", &probesA)
233+ defer srvA.Close()
234+ if _, _, err := execute(t, "version"); err != nil {
235+ t.Fatalf("execute: %v", err)
236+ }
237+ if probesA != 1 {
238+ t.Fatalf("probesA = %d, want 1", probesA)
239+ }
240+
241+ srvB := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
242+ if r.URL.Path == "/"+updateCheckOwner+"/"+updateCheckRepo+"/releases/latest" {
243+ probesB++
244+ http.Redirect(w, r, "/"+updateCheckOwner+"/"+updateCheckRepo+"/releases/v0.3.0", http.StatusFound)
245+ return
246+ }
247+ http.NotFound(w, r)
248+ }))
249+ defer srvB.Close()
250+
251+ _, errOut, err := execute(t, "--host", srvB.URL, "version")
252+ if err != nil {
253+ t.Fatalf("execute: %v", err)
254+ }
255+ if probesB != 1 {
256+ t.Fatalf("probesB = %d, want 1 (host B must not inherit host A's stamp)", probesB)
257+ }
258+ if !strings.Contains(errOut, "v0.3.0") {
259+ t.Fatalf("notice must reflect host B's release; stderr = %q", errOut)
260+ }
261+}
262+
263+// TestCompareVersions pins the ordering rules the notice depends on.
264+func TestCompareVersions(t *testing.T) {
265+ cases := []struct {
266+ a, b string
267+ want int
268+ }{
269+ {"v0.2.0", "0.1.0", 1},
270+ {"0.1.0", "v0.2.0", -1},
271+ {"v0.2.0", "v0.2.0", 0},
272+ {"0.2", "0.1.9", 1}, // missing components are zero
273+ {"0.2.0", "0.2.0-rc.1", 1}, // release > its prerelease
274+ {"0.2.0-rc.11", "0.2.0-rc.9", 1}, // numeric identifiers, not lexical
275+ {"0.2.0-rc.2", "0.2.0-rc.2", 0}, //
276+ {"0.2.0-rc.1", "0.2.0-beta", 1}, // rc > beta byte-wise
277+ {"0.2.0-rc.1", "0.2.0-rc.1.1", -1}, //
278+ {"runner-abc123", "0.1.0", 0}, // unparsable never compares
279+ {"0.1.0", "runner-abc123", 0}, //
280+ {"", "0.1.0", 0}, //
281+ }
282+ for _, tc := range cases {
283+ if got := compareVersions(tc.a, tc.b); got != tc.want {
284+ t.Errorf("compareVersions(%q, %q) = %d, want %d", tc.a, tc.b, got, tc.want)
285+ }
286+ }
287+}
288+
289+// TestTagFromReleaseLocation pins the Location shapes the host writes and the
290+// ones it must not be confused by.
291+func TestTagFromReleaseLocation(t *testing.T) {
292+ cases := []struct {
293+ loc string
294+ want string
295+ }{
296+ {"/rickub/cli/releases/v0.2.0", "v0.2.0"},
297+ {"https://host.example/rickub/cli/releases/v0.1.0-rc.9", "v0.1.0-rc.9"},
298+ {"/rickub/cli/releases/latest", ""}, // redirect to itself
299+ {"/rickub/cli/releases", ""}, // the list page
300+ {"/rickub/cli/releases/download/v0.2.0/rickub_linux_amd64.tar.gz", ""},
301+ {"/somewhere/else entirely", ""},
302+ {"", ""},
303+ }
304+ for _, tc := range cases {
305+ if got := tagFromReleaseLocation(tc.loc); got != tc.want {
306+ t.Errorf("tagFromReleaseLocation(%q) = %q, want %q", tc.loc, got, tc.want)
307+ }
308+ }
309+}
310+
311+// TestUpdateStampPathIsPerHostAndSane: the stamp lands in its own directory
312+// beside the config file, with a path-safe host key — two hosts, two stamps, no
313+// clobbering, and never inside the config file itself.
314+func TestUpdateStampPathIsPerHostAndSane(t *testing.T) {
315+ t.Setenv("XDG_CONFIG_HOME", t.TempDir())
316+ a, err := updateStampPath("https://rickub.com")
317+ if err != nil {
318+ t.Fatalf("stamp path: %v", err)
319+ }
320+ b, err := updateStampPath("https://dev.rickub.com/")
321+ if err != nil {
322+ t.Fatalf("stamp path: %v", err)
323+ }
324+ if a == b {
325+ t.Fatalf("two hosts share a stamp: %q", a)
326+ }
327+ cfgPath, err := config.Path()
328+ if err != nil {
329+ t.Fatalf("config path: %v", err)
330+ }
331+ if a == cfgPath || filepath.Dir(a) == cfgPath {
332+ t.Fatalf("stamp %q must not be (or sit inside) the config file %q", a, cfgPath)
333+ }
334+ if base := filepath.Base(a); !strings.HasSuffix(base, "rickub.com") {
335+ t.Errorf("stamp base %q loses the host identity", base)
336+ }
337+ if dir := filepath.Dir(a); !strings.HasSuffix(filepath.Base(dir), "update-check") {
338+ t.Errorf("stamp directory %q is not the dedicated update-check dir", dir)
339+ }
340+}
341+
342+// TestStampFreshnessWindow pins the window: fresh inside the interval, stale
343+// after it, and a stamp in the future (clock jumped back) reads fresh so the
344+// probe cannot rapid-fire.
345+func TestStampFreshnessWindow(t *testing.T) {
346+ path := filepath.Join(t.TempDir(), "stamp")
347+ writeUpdateStamp(path)
348+ if !stampIsFresh(path) {
349+ t.Fatal("a just-written stamp must be fresh")
350+ }
351+ if err := os.WriteFile(path, []byte(time.Now().UTC().Add(-updateCheckInterval-time.Hour).Format(time.RFC3339)), 0o600); err != nil {
352+ t.Fatalf("age stamp: %v", err)
353+ }
354+ if stampIsFresh(path) {
355+ t.Fatal("a stamp older than the interval must be stale")
356+ }
357+ if err := os.WriteFile(path, []byte(time.Now().UTC().Add(time.Hour).Format(time.RFC3339)), 0o600); err != nil {
358+ t.Fatalf("future stamp: %v", err)
359+ }
360+ if !stampIsFresh(path) {
361+ t.Fatal("a future stamp (clock moved back) must read fresh, not pinball the probe")
362+ }
363+ if stampIsFresh(filepath.Join(filepath.Dir(path), "absent")) {
364+ t.Fatal("an absent stamp must read stale")
365+ }
366+ if err := os.WriteFile(path, []byte("not a timestamp"), 0o600); err != nil {
367+ t.Fatalf("garbage stamp: %v", err)
368+ }
369+ if stampIsFresh(path) {
370+ t.Fatal("an unparsable stamp must read stale")
371+ }
372+}
new file mode 100644
@@ -0,0 +1,372 @@
1+package cmd
2+
3+import (
4+ "net/http"
5+ "net/http/httptest"
6+ "os"
7+ "path/filepath"
8+ "strings"
9+ "testing"
10+ "time"
11+
12+ "rickub.com/rickub/cli/internal/config"
13+)
14+
15+// Tests for the CLI staleness notice (rickub/rickub#305).
16+//
17+// The notice is the one-line signal that a binary older than the newest release
18+// on its host keeps describing an old product as current — the failure mode that
19+// manufactured the false bug reports #294 and #279. Each behavior test below
20+// fails on the pre-change code (no notice, no probe) and on the two plausible
21+// regressions: the comparator flipping (nagging on fresh binaries, silence on
22+// stale ones) and the probe losing its throttle (a request on every command).
23+
24+// stampAge rewinds the stamp for host by d, so a test can simulate "checked
25+// yesterday" without sleeping. It fails the test when no stamp exists — a probe
26+// that never wrote its stamp is itself a bug (it would hammer the host).
27+func stampAge(t *testing.T, host string, d time.Duration) {
28+ t.Helper()
29+ path, err := updateStampPath(host)
30+ if err != nil {
31+ t.Fatalf("stamp path: %v", err)
32+ }
33+ if err := os.WriteFile(path, []byte(time.Now().UTC().Add(-d).Format(time.RFC3339)), 0o600); err != nil {
34+ t.Fatalf("rewind stamp: %v", err)
35+ }
36+}
37+
38+// updateServer stands up a host whose /rickub/cli/releases/latest redirects to
39+// tag, counting probes. It answers nothing else — a probe that wanders off its
40+// endpoint shows up as a failed assertion or an empty tag, never as a pass.
41+func updateServer(t *testing.T, tag string, probes *int) *httptest.Server {
42+ t.Helper()
43+ srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
44+ if r.URL.Path == "/"+updateCheckOwner+"/"+updateCheckRepo+"/releases/latest" {
45+ if probes != nil {
46+ *probes++
47+ }
48+ http.Redirect(w, r, "/"+updateCheckOwner+"/"+updateCheckRepo+"/releases/"+tag, http.StatusFound)
49+ return
50+ }
51+ http.NotFound(w, r)
52+ }))
53+ t.Setenv("XDG_CONFIG_HOME", t.TempDir())
54+ t.Setenv("RICKUB_HOST", srv.URL)
55+ t.Setenv("RICKUB_TOKEN", "rickub_pat_test")
56+ return srv
57+}
58+
59+// withVersion pins cmd.Version for one test and restores it after.
60+func withVersion(t *testing.T, v string) {
61+ t.Helper()
62+ old := Version
63+ Version = v
64+ t.Cleanup(func() { Version = old })
65+}
66+
67+// TestStaleCLINoticeNamesBothVersions is the acceptance core of rickub/rickub#305:
68+// a CLI older than the newest release on its host prints a visible one-line
69+// warning naming BOTH versions, on stderr (stdout stays machine-parseable).
70+func TestStaleCLINoticeNamesBothVersions(t *testing.T) {
71+ withVersion(t, "0.1.0")
72+ srv := updateServer(t, "v0.2.0", nil)
73+ defer srv.Close()
74+
75+ out, errOut, err := execute(t, "version")
76+ if err != nil {
77+ t.Fatalf("execute: %v", err)
78+ }
79+ if !strings.Contains(errOut, "0.1.0") || !strings.Contains(errOut, "v0.2.0") {
80+ t.Fatalf("notice must name both versions (running 0.1.0, available v0.2.0); stderr = %q", errOut)
81+ }
82+ if !strings.Contains(errOut, "upgrade") {
83+ t.Fatalf("notice must say how to move on; stderr = %q", errOut)
84+ }
85+ if !strings.Contains(out, "rickub 0.1.0") {
86+ t.Fatalf("stdout must keep the plain version line, got %q", out)
87+ }
88+ if strings.Contains(out, "note:") {
89+ t.Fatalf("notice leaked into stdout: %q", out)
90+ }
91+}
92+
93+// TestFreshCLIIsQuiet: a binary at (or past) the newest release prints nothing —
94+// the notice is a staleness signal, not a check-in.
95+func TestFreshCLIIsQuiet(t *testing.T) {
96+ withVersion(t, "0.2.0")
97+ srv := updateServer(t, "v0.2.0", nil)
98+ defer srv.Close()
99+
100+ _, errOut, err := execute(t, "version")
101+ if err != nil {
102+ t.Fatalf("execute: %v", err)
103+ }
104+ if strings.Contains(errOut, "note:") {
105+ t.Fatalf("fresh binary was nagged: %q", errOut)
106+ }
107+}
108+
109+// TestRCOfCurrentVersionIsNotNewer: the host's "latest" includes prereleases
110+// (most-recently-created), and v0.2.0-rc.7 is NOT newer than a running 0.2.0 —
111+// semver orders the prerelease below its release. Nagging every stable user
112+// about each rc would teach people to ignore the notice.
113+func TestRCOfCurrentVersionIsNotNewer(t *testing.T) {
114+ withVersion(t, "0.2.0")
115+ srv := updateServer(t, "v0.2.0-rc.7", nil)
116+ defer srv.Close()
117+
118+ _, errOut, err := execute(t, "version")
119+ if err != nil {
120+ t.Fatalf("execute: %v", err)
121+ }
122+ if strings.Contains(errOut, "note:") {
123+ t.Fatalf("rc-of-running-version produced a notice: %q", errOut)
124+ }
125+}
126+
127+// TestProbeIsThrottledPerHost: two commands in one day issue ONE probe; a day
128+// later the probe repeats. The stamp is what keeps the check off every command —
129+// without it the CLI adds a network round-trip to every invocation.
130+func TestProbeIsThrottledPerHost(t *testing.T) {
131+ withVersion(t, "0.1.0")
132+ var probes int
133+ srv := updateServer(t, "v0.2.0", &probes)
134+ defer srv.Close()
135+
136+ if _, _, err := execute(t, "version"); err != nil {
137+ t.Fatalf("execute: %v", err)
138+ }
139+ if _, _, err := execute(t, "version"); err != nil {
140+ t.Fatalf("execute: %v", err)
141+ }
142+ if probes != 1 {
143+ t.Fatalf("probes = %d after two same-day commands, want 1", probes)
144+ }
145+ stampAge(t, srv.URL, 25*time.Hour) // yesterday's check has aged out
146+ if _, _, err := execute(t, "version"); err != nil {
147+ t.Fatalf("execute: %v", err)
148+ }
149+ if probes != 2 {
150+ t.Fatalf("probes = %d after the stamp aged out, want 2", probes)
151+ }
152+}
153+
154+// TestDevBuildSkipsTheProbe: Version "dev" (a build from source, every `go
155+// test`/`go run`) never probes — it has no release identity, and the check must
156+// not add requests to test runs and local development.
157+func TestDevBuildSkipsTheProbe(t *testing.T) {
158+ withVersion(t, "dev")
159+ var probes int
160+ srv := updateServer(t, "v9.9.9", &probes)
161+ defer srv.Close()
162+
163+ if _, _, err := execute(t, "version"); err != nil {
164+ t.Fatalf("execute: %v", err)
165+ }
166+ if probes != 0 {
167+ t.Fatalf("dev build probed the host %d times, want 0", probes)
168+ }
169+}
170+
171+// TestProbeFailuresAreSilent: a host without the CLI repo (404), a non-redirect
172+// answer, a Location that is not a release tag, and an unreachable host each
173+// leave the command succeeding with no notice — best-effort means a failed check
174+// never becomes the user's problem.
175+func TestProbeFailuresAreSilent(t *testing.T) {
176+ withVersion(t, "0.1.0")
177+
178+ cases := []struct {
179+ name string
180+ handler http.HandlerFunc
181+ }{
182+ {"no releases on host", func(w http.ResponseWriter, r *http.Request) {
183+ http.NotFound(w, r)
184+ }},
185+ {"not a redirect", func(w http.ResponseWriter, _ *http.Request) {
186+ _, _ = w.Write([]byte(`{"ok":true}`))
187+ }},
188+ {"location is the releases list page", func(w http.ResponseWriter, r *http.Request) {
189+ http.Redirect(w, r, "/"+updateCheckOwner+"/"+updateCheckRepo+"/releases", http.StatusFound)
190+ }},
191+ {"location names a non-version tag", func(w http.ResponseWriter, r *http.Request) {
192+ http.Redirect(w, r, "/"+updateCheckOwner+"/"+updateCheckRepo+"/releases/runner-abc123", http.StatusFound)
193+ }},
194+ }
195+ for _, tc := range cases {
196+ t.Run(tc.name, func(t *testing.T) {
197+ srv := httptest.NewServer(tc.handler)
198+ defer srv.Close()
199+ t.Setenv("XDG_CONFIG_HOME", t.TempDir())
200+ t.Setenv("RICKUB_HOST", srv.URL)
201+
202+ _, errOut, err := execute(t, "version")
203+ if err != nil {
204+ t.Fatalf("execute: %v", err)
205+ }
206+ if strings.Contains(errOut, "note:") {
207+ t.Fatalf("failed probe produced a notice: %q", errOut)
208+ }
209+ })
210+ }
211+
212+ // An unreachable host: point at a port that is closed.
213+ t.Run("unreachable host", func(t *testing.T) {
214+ t.Setenv("XDG_CONFIG_HOME", t.TempDir())
215+ t.Setenv("RICKUB_HOST", "http://127.0.0.1:1")
216+ _, errOut, err := execute(t, "version")
217+ if err != nil {
218+ t.Fatalf("execute: %v", err)
219+ }
220+ if strings.Contains(errOut, "note:") {
221+ t.Fatalf("unreachable host produced a notice: %q", errOut)
222+ }
223+ })
224+}
225+
226+// TestStampIsPerHost: a stamp for one host must not silence the check against
227+// another — switching --host (or RICKUB_HOST) re-checks the host you actually
228+// asked about.
229+func TestStampIsPerHost(t *testing.T) {
230+ withVersion(t, "0.1.0")
231+ var probesA, probesB int
232+ srvA := updateServer(t, "v0.2.0", &probesA)
233+ defer srvA.Close()
234+ if _, _, err := execute(t, "version"); err != nil {
235+ t.Fatalf("execute: %v", err)
236+ }
237+ if probesA != 1 {
238+ t.Fatalf("probesA = %d, want 1", probesA)
239+ }
240+
241+ srvB := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
242+ if r.URL.Path == "/"+updateCheckOwner+"/"+updateCheckRepo+"/releases/latest" {
243+ probesB++
244+ http.Redirect(w, r, "/"+updateCheckOwner+"/"+updateCheckRepo+"/releases/v0.3.0", http.StatusFound)
245+ return
246+ }
247+ http.NotFound(w, r)
248+ }))
249+ defer srvB.Close()
250+
251+ _, errOut, err := execute(t, "--host", srvB.URL, "version")
252+ if err != nil {
253+ t.Fatalf("execute: %v", err)
254+ }
255+ if probesB != 1 {
256+ t.Fatalf("probesB = %d, want 1 (host B must not inherit host A's stamp)", probesB)
257+ }
258+ if !strings.Contains(errOut, "v0.3.0") {
259+ t.Fatalf("notice must reflect host B's release; stderr = %q", errOut)
260+ }
261+}
262+
263+// TestCompareVersions pins the ordering rules the notice depends on.
264+func TestCompareVersions(t *testing.T) {
265+ cases := []struct {
266+ a, b string
267+ want int
268+ }{
269+ {"v0.2.0", "0.1.0", 1},
270+ {"0.1.0", "v0.2.0", -1},
271+ {"v0.2.0", "v0.2.0", 0},
272+ {"0.2", "0.1.9", 1}, // missing components are zero
273+ {"0.2.0", "0.2.0-rc.1", 1}, // release > its prerelease
274+ {"0.2.0-rc.11", "0.2.0-rc.9", 1}, // numeric identifiers, not lexical
275+ {"0.2.0-rc.2", "0.2.0-rc.2", 0}, //
276+ {"0.2.0-rc.1", "0.2.0-beta", 1}, // rc > beta byte-wise
277+ {"0.2.0-rc.1", "0.2.0-rc.1.1", -1}, //
278+ {"runner-abc123", "0.1.0", 0}, // unparsable never compares
279+ {"0.1.0", "runner-abc123", 0}, //
280+ {"", "0.1.0", 0}, //
281+ }
282+ for _, tc := range cases {
283+ if got := compareVersions(tc.a, tc.b); got != tc.want {
284+ t.Errorf("compareVersions(%q, %q) = %d, want %d", tc.a, tc.b, got, tc.want)
285+ }
286+ }
287+}
288+
289+// TestTagFromReleaseLocation pins the Location shapes the host writes and the
290+// ones it must not be confused by.
291+func TestTagFromReleaseLocation(t *testing.T) {
292+ cases := []struct {
293+ loc string
294+ want string
295+ }{
296+ {"/rickub/cli/releases/v0.2.0", "v0.2.0"},
297+ {"https://host.example/rickub/cli/releases/v0.1.0-rc.9", "v0.1.0-rc.9"},
298+ {"/rickub/cli/releases/latest", ""}, // redirect to itself
299+ {"/rickub/cli/releases", ""}, // the list page
300+ {"/rickub/cli/releases/download/v0.2.0/rickub_linux_amd64.tar.gz", ""},
301+ {"/somewhere/else entirely", ""},
302+ {"", ""},
303+ }
304+ for _, tc := range cases {
305+ if got := tagFromReleaseLocation(tc.loc); got != tc.want {
306+ t.Errorf("tagFromReleaseLocation(%q) = %q, want %q", tc.loc, got, tc.want)
307+ }
308+ }
309+}
310+
311+// TestUpdateStampPathIsPerHostAndSane: the stamp lands in its own directory
312+// beside the config file, with a path-safe host key — two hosts, two stamps, no
313+// clobbering, and never inside the config file itself.
314+func TestUpdateStampPathIsPerHostAndSane(t *testing.T) {
315+ t.Setenv("XDG_CONFIG_HOME", t.TempDir())
316+ a, err := updateStampPath("https://rickub.com")
317+ if err != nil {
318+ t.Fatalf("stamp path: %v", err)
319+ }
320+ b, err := updateStampPath("https://dev.rickub.com/")
321+ if err != nil {
322+ t.Fatalf("stamp path: %v", err)
323+ }
324+ if a == b {
325+ t.Fatalf("two hosts share a stamp: %q", a)
326+ }
327+ cfgPath, err := config.Path()
328+ if err != nil {
329+ t.Fatalf("config path: %v", err)
330+ }
331+ if a == cfgPath || filepath.Dir(a) == cfgPath {
332+ t.Fatalf("stamp %q must not be (or sit inside) the config file %q", a, cfgPath)
333+ }
334+ if base := filepath.Base(a); !strings.HasSuffix(base, "rickub.com") {
335+ t.Errorf("stamp base %q loses the host identity", base)
336+ }
337+ if dir := filepath.Dir(a); !strings.HasSuffix(filepath.Base(dir), "update-check") {
338+ t.Errorf("stamp directory %q is not the dedicated update-check dir", dir)
339+ }
340+}
341+
342+// TestStampFreshnessWindow pins the window: fresh inside the interval, stale
343+// after it, and a stamp in the future (clock jumped back) reads fresh so the
344+// probe cannot rapid-fire.
345+func TestStampFreshnessWindow(t *testing.T) {
346+ path := filepath.Join(t.TempDir(), "stamp")
347+ writeUpdateStamp(path)
348+ if !stampIsFresh(path) {
349+ t.Fatal("a just-written stamp must be fresh")
350+ }
351+ if err := os.WriteFile(path, []byte(time.Now().UTC().Add(-updateCheckInterval-time.Hour).Format(time.RFC3339)), 0o600); err != nil {
352+ t.Fatalf("age stamp: %v", err)
353+ }
354+ if stampIsFresh(path) {
355+ t.Fatal("a stamp older than the interval must be stale")
356+ }
357+ if err := os.WriteFile(path, []byte(time.Now().UTC().Add(time.Hour).Format(time.RFC3339)), 0o600); err != nil {
358+ t.Fatalf("future stamp: %v", err)
359+ }
360+ if !stampIsFresh(path) {
361+ t.Fatal("a future stamp (clock moved back) must read fresh, not pinball the probe")
362+ }
363+ if stampIsFresh(filepath.Join(filepath.Dir(path), "absent")) {
364+ t.Fatal("an absent stamp must read stale")
365+ }
366+ if err := os.WriteFile(path, []byte("not a timestamp"), 0o600); err != nil {
367+ t.Fatalf("garbage stamp: %v", err)
368+ }
369+ if stampIsFresh(path) {
370+ t.Fatal("an unparsable stamp must read stale")
371+ }
372+}