From 23e6f1e3f5ceb9f19221637906c5e271cf61ce95 Mon Sep 17 00:00:00 2001 From: Gustavo Bertoi Date: Mon, 29 Jun 2026 12:14:30 -0300 Subject: [PATCH] =?UTF-8?q?feat(selfupdate):=20X8=20=E2=80=94=20throttled,?= =?UTF-8?q?=20fail-silent=20update=20notifier=20(spec=2014)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit selfupdate.Notifier prints a one-line "new release available" notice after a command, wired into the CLI root's PersistentPostRun (stderr). It is: - throttled — hits GitHub at most once per 24h, caching {checkedAt,latest} under the XDG cache dir; within the TTL it reads the cache (no network). - fail-silent — any error (offline, rate limit, bad cache) yields no notice and never affects the command's exit. - quiet by default where it should be — skipped for --json/--quiet output, for dev/dirty builds (no clean semver to compare), and when DEVSTACK_NO_UPDATE_NOTIFIER is set. - bounded — the refresh check runs under a 2s context timeout. Unit-tested with an injected clock + check fn + temp cache: notice-when-newer, silent-when-current, cache throttle (1 network call within TTL, refresh after), fail-silent on error, dev-build skip, env opt-out. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/cli/root.go | 11 +++ internal/selfupdate/notify.go | 127 +++++++++++++++++++++++++++++ internal/selfupdate/notify_test.go | 99 ++++++++++++++++++++++ 3 files changed, 237 insertions(+) create mode 100644 internal/selfupdate/notify.go create mode 100644 internal/selfupdate/notify_test.go diff --git a/internal/cli/root.go b/internal/cli/root.go index 54ee813..777d028 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -14,7 +14,9 @@ import ( "github.com/spf13/cobra" "github.com/open-source-cloud/devstack/internal/alias" + "github.com/open-source-cloud/devstack/internal/selfupdate" "github.com/open-source-cloud/devstack/internal/version" + "github.com/open-source-cloud/devstack/internal/xdg" ) // GlobalOpts holds the global flags every command renders through. @@ -54,6 +56,15 @@ func NewRootCmd(opts Options) *cobra.Command { setupLogging(cmd.ErrOrStderr(), g) return nil }, + // Fail-silent, throttled update notice on stderr (spec 14). Skipped for + // --json/--quiet and dev builds; at most one network check per day. + PersistentPostRun: func(cmd *cobra.Command, _ []string) { + if g.JSON || g.Quiet { + return + } + selfupdate.Notifier{Current: version.Version, CacheDir: xdg.CacheHome()}. + Notify(cmd.Context(), cmd.ErrOrStderr()) + }, } pf := root.PersistentFlags() diff --git a/internal/selfupdate/notify.go b/internal/selfupdate/notify.go new file mode 100644 index 0000000..815e3ae --- /dev/null +++ b/internal/selfupdate/notify.go @@ -0,0 +1,127 @@ +package selfupdate + +import ( + "context" + "encoding/json" + "fmt" + "io" + "os" + "path/filepath" + "time" + + "golang.org/x/mod/semver" +) + +// This file is the background update notifier (spec 14): a throttled, fail-silent +// check that prints a one-line notice when a newer release exists. It hits the +// network at most once per TTL (cached under the XDG cache dir), never blocks a +// command for long, never errors out, and stays silent for dev builds, --json/ +// --quiet output, or when DEVSTACK_NO_UPDATE_NOTIFIER is set. + +const ( + notifierTTL = 24 * time.Hour + notifierTimeout = 2 * time.Second + notifierDisableEnv = "DEVSTACK_NO_UPDATE_NOTIFIER" + notifierCacheFile = "update-check.json" +) + +type notifyCache struct { + CheckedAt time.Time `json:"checkedAt"` + Latest string `json:"latest"` +} + +// Notifier performs a throttled, fail-silent "is there a newer release?" check. +type Notifier struct { + Current string // the running version + CacheDir string // the devstack cache dir (cache file written directly inside) + Now func() time.Time // injectable clock (default time.Now) + CheckFn func(ctx context.Context) (string, error) // returns the latest tag (default LatestTag) +} + +// Notify writes an update notice to w if a newer release is available. It is +// best-effort: any error (no network, bad cache, rate limit) silently yields no +// notice. Returns true iff a notice was written (for tests). +func (n Notifier) Notify(ctx context.Context, w io.Writer) bool { + if os.Getenv(notifierDisableEnv) != "" { + return false + } + if n.Current == "" || IsDevBuild(n.Current) { + return false // dev/dirty builds have no clean version to compare + } + latest := n.latest(ctx) + if latest == "" { + return false + } + if semver.IsValid(n.Current) && semver.IsValid(latest) && semver.Compare(latest, n.Current) > 0 { + fmt.Fprintf(w, "\nA new devstack release is available: %s → %s (run `devstack self update`)\n", n.Current, latest) + return true + } + return false +} + +// latest returns the latest tag from a fresh cache, or refreshes via CheckFn +// (bounded + fail-silent) when the cache is stale/absent. +func (n Notifier) latest(ctx context.Context) string { + now := n.now() + if c, ok := n.readCache(); ok && !c.CheckedAt.IsZero() && now.Sub(c.CheckedAt) < notifierTTL { + return c.Latest // fresh cache → no network + } + cctx, cancel := context.WithTimeout(ctx, notifierTimeout) + defer cancel() + latest, err := n.check(cctx) + if err != nil || latest == "" { + return "" + } + n.writeCache(notifyCache{CheckedAt: now, Latest: latest}) + return latest +} + +func (n Notifier) now() time.Time { + if n.Now != nil { + return n.Now() + } + return time.Now() +} + +func (n Notifier) check(ctx context.Context) (string, error) { + if n.CheckFn != nil { + return n.CheckFn(ctx) + } + return LatestTag(ctx) +} + +func (n Notifier) cachePath() string { + if n.CacheDir == "" { + return "" + } + return filepath.Join(n.CacheDir, notifierCacheFile) +} + +func (n Notifier) readCache() (notifyCache, bool) { + p := n.cachePath() + if p == "" { + return notifyCache{}, false + } + b, err := os.ReadFile(p) + if err != nil { + return notifyCache{}, false + } + var c notifyCache + if err := json.Unmarshal(b, &c); err != nil { + return notifyCache{}, false + } + return c, true +} + +func (n Notifier) writeCache(c notifyCache) { + p := n.cachePath() + if p == "" { + return + } + if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { + return // best-effort + } + if b, err := json.Marshal(c); err == nil { + _ = os.WriteFile(p, b, 0o644) + } +} diff --git a/internal/selfupdate/notify_test.go b/internal/selfupdate/notify_test.go new file mode 100644 index 0000000..76db1be --- /dev/null +++ b/internal/selfupdate/notify_test.go @@ -0,0 +1,99 @@ +package selfupdate + +import ( + "context" + "errors" + "strings" + "testing" + "time" +) + +func TestNotifyAvailable(t *testing.T) { + calls := 0 + n := Notifier{ + Current: "v0.1.0", + CacheDir: t.TempDir(), + Now: func() time.Time { return time.Unix(1000, 0) }, + CheckFn: func(context.Context) (string, error) { calls++; return "v0.2.0", nil }, + } + var w strings.Builder + if !n.Notify(context.Background(), &w) { + t.Fatal("expected an update notice") + } + if !strings.Contains(w.String(), "v0.1.0 → v0.2.0") { + t.Errorf("notice = %q", w.String()) + } + if calls != 1 { + t.Errorf("checked %d times, want 1", calls) + } +} + +func TestNotifyUpToDate(t *testing.T) { + n := Notifier{ + Current: "v0.2.0", + CacheDir: t.TempDir(), + Now: func() time.Time { return time.Unix(1000, 0) }, + CheckFn: func(context.Context) (string, error) { return "v0.2.0", nil }, + } + var w strings.Builder + if n.Notify(context.Background(), &w) || w.Len() != 0 { + t.Errorf("no notice when current == latest, got %q", w.String()) + } +} + +func TestNotifyThrottledByCache(t *testing.T) { + dir := t.TempDir() + calls := 0 + mk := func(now time.Time) Notifier { + return Notifier{ + Current: "v0.1.0", CacheDir: dir, + Now: func() time.Time { return now }, + CheckFn: func(context.Context) (string, error) { calls++; return "v0.2.0", nil }, + } + } + // First call hits the network and caches. + mk(time.Unix(0, 0)).Notify(context.Background(), &strings.Builder{}) + // Second call within the TTL uses the cache (no network). + mk(time.Unix(int64((notifierTTL-time.Minute).Seconds()), 0)).Notify(context.Background(), &strings.Builder{}) + if calls != 1 { + t.Errorf("checked %d times within TTL, want 1 (cached)", calls) + } + // After the TTL it refreshes. + mk(time.Unix(int64((notifierTTL+time.Hour).Seconds()), 0)).Notify(context.Background(), &strings.Builder{}) + if calls != 2 { + t.Errorf("checked %d times after TTL, want 2 (refreshed)", calls) + } +} + +func TestNotifyFailSilent(t *testing.T) { + n := Notifier{ + Current: "v0.1.0", CacheDir: t.TempDir(), + Now: func() time.Time { return time.Unix(1000, 0) }, + CheckFn: func(context.Context) (string, error) { return "", errors.New("no network") }, + } + var w strings.Builder + if n.Notify(context.Background(), &w) || w.Len() != 0 { + t.Errorf("network failure must be silent, got %q", w.String()) + } +} + +func TestNotifyDevBuildSilent(t *testing.T) { + n := Notifier{ + Current: "v0.1.0-11-gabc1234", CacheDir: t.TempDir(), + CheckFn: func(context.Context) (string, error) { t.Fatal("dev build must not check"); return "", nil }, + } + if n.Notify(context.Background(), &strings.Builder{}) { + t.Error("dev build should never notify") + } +} + +func TestNotifyDisabledByEnv(t *testing.T) { + t.Setenv(notifierDisableEnv, "1") + n := Notifier{ + Current: "v0.1.0", CacheDir: t.TempDir(), + CheckFn: func(context.Context) (string, error) { t.Fatal("must not check when disabled"); return "", nil }, + } + if n.Notify(context.Background(), &strings.Builder{}) { + t.Error("notifier disabled by env should do nothing") + } +}