diff --git a/internal/cli/update_check.go b/internal/cli/update_check.go index e4d319e..8f317c4 100644 --- a/internal/cli/update_check.go +++ b/internal/cli/update_check.go @@ -80,6 +80,16 @@ func latestReleaseVersion() string { if c, ok := readUpdateCache(path); ok && time.Since(c.CheckedAt) < updateCheckInterval { return c.Latest } + // No fresh cache. If the config dir is absent, the throttle can't be persisted + // (writeUpdateCache won't recreate it — Bugbot #404), so fetching here would + // repeat on EVERY command and burn updateCheckTimeout each time. A missing dir + // also means the CLI isn't set up (fresh install) or was offboarded — nothing + // to nudge about. Skip the network check entirely. The dir-present but + // stale/unreadable case still falls through to the throttled fetch below, so + // this doesn't defeat the normal path (Bugbot #397). + if !configDirExists(path) { + return "" + } latest, err := fetchLatestRelease(latestReleaseURL, updateCheckTimeout) if err != nil { if c, ok := readUpdateCache(path); ok { @@ -134,6 +144,22 @@ func updateCachePath() string { return filepath.Join(dir, updateCacheFile) } +// configDirExists reports whether the tracebloc config dir (the parent of the +// update cache) is present — the single gate for "can the update-check throttle +// be persisted?". When it's absent (a fresh install, or a wiped/offboarded +// ~/.tracebloc) the cache can neither be read nor written, so the caller must +// no-op: latestReleaseVersion skips the network check (so a fetch isn't repeated +// unthrottled on every command — Bugbot #397) and writeUpdateCache skips the +// write (so a throttle cache never resurrects a just-wiped dir — Bugbot #404). +// An empty path (config.Dir() failed) counts as absent. +func configDirExists(cachePath string) bool { + if cachePath == "" { + return false + } + _, err := os.Stat(filepath.Dir(cachePath)) + return err == nil +} + func readUpdateCache(path string) (updateCache, bool) { if path == "" { return updateCache{}, false @@ -158,10 +184,7 @@ func readUpdateCache(path string) (updateCache, bool) { // login/client-create and delete, never by a throttle cache. A missing dir is a // silent no-op (the throttle simply isn't persisted until the dir exists again). func writeUpdateCache(path string, c updateCache) error { - if path == "" { - return nil - } - if _, err := os.Stat(filepath.Dir(path)); err != nil { + if !configDirExists(path) { return nil // dir gone (fresh machine, or just-offboarded) — don't recreate it } raw, err := json.Marshal(c) diff --git a/internal/cli/update_check_test.go b/internal/cli/update_check_test.go index 3a5bb3f..989d95e 100644 --- a/internal/cli/update_check_test.go +++ b/internal/cli/update_check_test.go @@ -118,6 +118,50 @@ func TestLatestReleaseVersion_FreshCacheSkipsNetwork(t *testing.T) { } } +// An ABSENT config dir (fresh install / offboarded) must SKIP the network check +// entirely — not hit GitHub on every command. writeUpdateCache can't persist the +// throttle without the dir (Bugbot #404), so an unconditional fetch would repeat +// forever and burn updateCheckTimeout each time (Bugbot #397). Distinct from the +// dir-present-but-stale case, which still fetches (throttled) below. +func TestLatestReleaseVersion_MissingConfigDirSkipsNetwork(t *testing.T) { + absent := filepath.Join(t.TempDir(), "nope") // deliberately never created + t.Setenv("TRACEBLOC_CONFIG_DIR", absent) + // A server that fails the test if it's ever hit — proves no fetch is attempted. + srv := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { + t.Error("network hit despite an absent config dir — the update check must be skipped") + })) + defer srv.Close() + swapURL(t, srv.URL) + + if got := latestReleaseVersion(); got != "" { + t.Errorf("latestReleaseVersion = %q, want \"\" (skipped: no config dir)", got) + } + // The skipped check must not have resurrected the dir either (reconciles #404). + if _, err := os.Stat(absent); !os.IsNotExist(err) { + t.Errorf("update check must not create the missing config dir %s (err=%v)", absent, err) + } +} + +// The dir-present-but-no-cache case (e.g. right after login created ~/.tracebloc) +// must still fetch and then persist the throttle — the missing-dir skip must NOT +// bleed into the normal path, or the once-a-day throttle would never arm. +func TestLatestReleaseVersion_DirPresentNoCacheFetchesAndPersists(t *testing.T) { + t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) // dir exists; no cache file yet + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte(`{"tag_name":"v2.0.0"}`)) + })) + defer srv.Close() + swapURL(t, srv.URL) + + if got := latestReleaseVersion(); got != "2.0.0" { + t.Errorf("latestReleaseVersion = %q, want 2.0.0 (dir present, no cache → fetch)", got) + } + // The fetch must have persisted the throttle so the next call is served from cache. + if c, ok := readUpdateCache(updateCachePath()); !ok || c.Latest != "2.0.0" { + t.Errorf("throttle not persisted after a dir-present fetch: %+v ok=%v", c, ok) + } +} + func TestLatestReleaseVersion_StaleCacheFetchesAndRewrites(t *testing.T) { t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {