From 1ffcbf96fbcc0e41f9340d867e4101bfba543300 Mon Sep 17 00:00:00 2001 From: arystan Date: Tue, 14 Apr 2026 12:55:12 +0500 Subject: [PATCH 1/6] feat(cli): warn when a newer spec-kit release is available (#1320) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Print a one-line upgrade hint on every launch when the installed CLI is older than the latest GitHub release. Cached for 24h and suppressed when SPECIFY_SKIP_UPDATE_CHECK is set, CI=1 is set, or stdout is not a TTY. Any network / parse failure is swallowed — the command the user invoked is never blocked. Closes #1320. --- CHANGELOG.md | 6 + docs/installation.md | 10 ++ src/specify_cli/__init__.py | 141 +++++++++++++++++++++++ tests/test_update_check.py | 221 ++++++++++++++++++++++++++++++++++++ 4 files changed, 378 insertions(+) create mode 100644 tests/test_update_check.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 928bc74b9b..517cc53f4f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,12 @@ +## [Unreleased] + +### Added + +- feat(cli): warn on launch when a newer spec-kit release is available; cached for 24h and suppressed with `SPECIFY_SKIP_UPDATE_CHECK=1`, non-interactive shells, or `CI=1` (#1320) + ## [0.6.2] - 2026-04-13 ### Changed diff --git a/docs/installation.md b/docs/installation.md index 5d560b6e33..a00aa30c7e 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -77,6 +77,16 @@ After initialization, you should see the following commands available in your AI The `.specify/scripts` directory will contain both `.sh` and `.ps1` scripts. +### Update Notifications + +On each launch, `specify` checks once per 24 hours whether a newer release is available on GitHub and prints an upgrade hint if so. The check is silent when: + +- `SPECIFY_SKIP_UPDATE_CHECK=1` (or `true`/`yes`/`on`) is set +- stdout is not a TTY (piped output, redirected to a file, etc.) +- the `CI` environment variable is set + +Network failures and rate-limit responses are swallowed — the check never blocks the command you ran. + ## Troubleshooting ### Enterprise / Air-Gapped Installation diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 0bbf42ad5a..ab9799ff71 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -327,6 +327,10 @@ def callback(ctx: typer.Context): show_banner() console.print(Align.center("[dim]Run 'specify --help' for usage information[/dim]")) console.print() + # Addresses #1320: nudge users running outdated CLIs. The `version` subcommand + # already surfaces the version, so skip there to avoid double-printing. + if ctx.invoked_subcommand not in (None, "version"): + _check_for_updates() def run_command(cmd: list[str], check_return: bool = True, capture: bool = False, shell: bool = False) -> Optional[str]: """Run a shell command and optionally capture output.""" @@ -1586,6 +1590,143 @@ def get_speckit_version() -> str: return "unknown" +# ===== Update check (addresses #1320) ===== +# +# Cached once per 24h in the platform user-cache dir. Triggered from the top-level +# callback. Never blocks the user — every failure path swallows the exception. + +_UPDATE_CHECK_URL = "https://api.github.com/repos/github/spec-kit/releases/latest" +_UPDATE_CHECK_CACHE_TTL_SECONDS = 24 * 60 * 60 +_UPDATE_CHECK_TIMEOUT_SECONDS = 2.0 + + +def _parse_version_tuple(version: str) -> tuple[int, ...] | None: + """Parse `v0.6.2` / `0.6.2` / `0.6.2.dev0` → tuple of ints. Returns None if unparseable.""" + if not version: + return None + s = version.strip().lstrip("vV") + # Drop PEP 440 pre/post/dev/local segments; we only compare release numbers. + for sep in ("-", "+", "a", "b", "rc", ".dev", ".post"): + idx = s.find(sep) + if idx != -1: + s = s[:idx] + parts: list[int] = [] + for piece in s.split("."): + if not piece.isdigit(): + return None + parts.append(int(piece)) + return tuple(parts) if parts else None + + +def _update_check_cache_path() -> Path | None: + try: + from platformdirs import user_cache_dir + return Path(user_cache_dir("specify-cli")) / "version_check.json" + except Exception: + return None + + +def _read_update_check_cache(path: Path) -> dict | None: + try: + import time + if not path.exists(): + return None + data = json.loads(path.read_text()) + checked_at = float(data.get("checked_at", 0)) + if time.time() - checked_at > _UPDATE_CHECK_CACHE_TTL_SECONDS: + return None + return data + except Exception: + return None + + +def _write_update_check_cache(path: Path, latest: str) -> None: + try: + import time + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(json.dumps({"checked_at": time.time(), "latest": latest})) + except Exception: + # Cache write failures are non-fatal. + pass + + +def _fetch_latest_version() -> str | None: + """Query GitHub for the latest release tag. Returns None on any failure.""" + try: + import urllib.request + req = urllib.request.Request( + _UPDATE_CHECK_URL, + headers={"Accept": "application/vnd.github+json", "User-Agent": "specify-cli"}, + ) + with urllib.request.urlopen(req, timeout=_UPDATE_CHECK_TIMEOUT_SECONDS) as resp: + payload = json.loads(resp.read().decode("utf-8")) + tag = payload.get("tag_name") + return tag if isinstance(tag, str) and tag else None + except Exception: + return None + + +def _should_skip_update_check() -> bool: + if os.environ.get("SPECIFY_SKIP_UPDATE_CHECK", "").strip().lower() in ("1", "true", "yes", "on"): + return True + if os.environ.get("CI"): + return True + try: + if not sys.stdout.isatty(): + return True + except Exception: + return True + return False + + +def _check_for_updates() -> None: + """Print a one-line upgrade hint when a newer spec-kit release is available. + + Fully best-effort — any error (offline, rate-limited, parse failure) is + swallowed so the command the user actually invoked is never blocked. + """ + if _should_skip_update_check(): + return + try: + current_str = get_speckit_version() + current = _parse_version_tuple(current_str) + if current is None: + return + + cache_path = _update_check_cache_path() + latest_str: str | None = None + if cache_path is not None: + cached = _read_update_check_cache(cache_path) + if cached: + latest_str = cached.get("latest") + + if latest_str is None: + latest_str = _fetch_latest_version() + if latest_str and cache_path is not None: + _write_update_check_cache(cache_path, latest_str) + + latest = _parse_version_tuple(latest_str) if latest_str else None + if latest is None or latest <= current: + return + + current_display = current_str.lstrip("vV") + latest_display = latest_str.lstrip("vV") + console.print( + f"[yellow]⚠ A new spec-kit version is available: " + f"v{latest_display} (you have v{current_display})[/yellow]" + ) + console.print( + f"[dim] Upgrade: uv tool install specify-cli --force " + f"--from git+https://github.com/github/spec-kit.git@v{latest_display}[/dim]" + ) + console.print( + "[dim] (set SPECIFY_SKIP_UPDATE_CHECK=1 to silence this check)[/dim]" + ) + except Exception: + # Update check must never surface an error to the user. + return + + # ===== Integration Commands ===== integration_app = typer.Typer( diff --git a/tests/test_update_check.py b/tests/test_update_check.py new file mode 100644 index 0000000000..adf8a156b7 --- /dev/null +++ b/tests/test_update_check.py @@ -0,0 +1,221 @@ +"""Tests for the update-check helper in specify_cli.__init__. + +Covers issue https://github.com/github/spec-kit/issues/1320 — the CLI should +nudge users who are running outdated releases toward an upgrade, without +blocking any command when offline or rate-limited. +""" + +import json +import time +import urllib.error +from io import StringIO +from unittest.mock import patch + +import pytest + +from specify_cli import ( + _check_for_updates, + _fetch_latest_version, + _parse_version_tuple, + _read_update_check_cache, + _write_update_check_cache, +) + + +class TestParseVersionTuple: + @pytest.mark.parametrize( + "raw,expected", + [ + ("v0.6.2", (0, 6, 2)), + ("0.6.2", (0, 6, 2)), + ("V1.2.3.4", (1, 2, 3, 4)), + ("0.6.2.dev0", (0, 6, 2)), + ("1.0.0-rc.1", (1, 0, 0)), + ("1.0.0+meta", (1, 0, 0)), + ], + ) + def test_parses_common_version_strings(self, raw, expected): + assert _parse_version_tuple(raw) == expected + + @pytest.mark.parametrize("raw", ["", "abc", "v.", None]) + def test_returns_none_on_unparseable(self, raw): + assert _parse_version_tuple(raw) is None + + def test_ordering_matches_semver_intuition(self): + assert _parse_version_tuple("v0.6.2") < _parse_version_tuple("v0.6.3") + assert _parse_version_tuple("v0.6.2") < _parse_version_tuple("v0.7.0") + assert _parse_version_tuple("v0.6.2") == _parse_version_tuple("0.6.2") + + +class TestCache: + def test_fresh_cache_is_returned(self, tmp_path): + cache_file = tmp_path / "version_check.json" + cache_file.write_text(json.dumps({"checked_at": time.time(), "latest": "v0.7.0"})) + data = _read_update_check_cache(cache_file) + assert data is not None + assert data["latest"] == "v0.7.0" + + def test_stale_cache_is_ignored(self, tmp_path): + cache_file = tmp_path / "version_check.json" + very_old = time.time() - (48 * 60 * 60) + cache_file.write_text(json.dumps({"checked_at": very_old, "latest": "v0.5.0"})) + assert _read_update_check_cache(cache_file) is None + + def test_missing_cache_returns_none(self, tmp_path): + assert _read_update_check_cache(tmp_path / "missing.json") is None + + def test_corrupt_cache_returns_none(self, tmp_path): + cache_file = tmp_path / "version_check.json" + cache_file.write_text("{not json") + assert _read_update_check_cache(cache_file) is None + + def test_write_round_trips(self, tmp_path): + cache_file = tmp_path / "nested" / "version_check.json" + _write_update_check_cache(cache_file, "v0.9.9") + assert cache_file.exists() + data = json.loads(cache_file.read_text()) + assert data["latest"] == "v0.9.9" + assert float(data["checked_at"]) <= time.time() + + +class TestFetchLatestVersion: + def test_returns_tag_on_success(self): + payload = json.dumps({"tag_name": "v0.6.3"}).encode("utf-8") + + class FakeResp: + def __enter__(self): + return self + + def __exit__(self, *a): + return False + + def read(self): + return payload + + with patch("urllib.request.urlopen", return_value=FakeResp()): + assert _fetch_latest_version() == "v0.6.3" + + def test_returns_none_on_network_error(self): + with patch("urllib.request.urlopen", side_effect=urllib.error.URLError("offline")): + assert _fetch_latest_version() is None + + def test_returns_none_on_malformed_json(self): + class FakeResp: + def __enter__(self): + return self + + def __exit__(self, *a): + return False + + def read(self): + return b"not json" + + with patch("urllib.request.urlopen", return_value=FakeResp()): + assert _fetch_latest_version() is None + + def test_returns_none_when_tag_missing(self): + payload = json.dumps({"name": "unnamed"}).encode("utf-8") + + class FakeResp: + def __enter__(self): + return self + + def __exit__(self, *a): + return False + + def read(self): + return payload + + with patch("urllib.request.urlopen", return_value=FakeResp()): + assert _fetch_latest_version() is None + + +class TestCheckForUpdates: + """End-to-end-ish checks on `_check_for_updates` with skip conditions patched off.""" + + def _run_and_capture(self, monkeypatch) -> str: + """Force the skip-guard off so the helper runs, then capture console output.""" + # Guard returns False → helper proceeds. + monkeypatch.setattr("specify_cli._should_skip_update_check", lambda: False) + buf = StringIO() + import specify_cli + from rich.console import Console + captured = Console(file=buf, force_terminal=False, width=200) + monkeypatch.setattr(specify_cli, "console", captured) + _check_for_updates() + return buf.getvalue() + + def test_prints_warning_when_newer_release_available(self, monkeypatch, tmp_path): + monkeypatch.setattr("specify_cli.get_speckit_version", lambda: "0.6.2") + monkeypatch.setattr( + "specify_cli._update_check_cache_path", lambda: tmp_path / "vc.json" + ) + monkeypatch.setattr("specify_cli._fetch_latest_version", lambda: "v0.7.0") + + out = self._run_and_capture(monkeypatch) + + assert "new spec-kit version is available" in out + assert "v0.7.0" in out + assert "v0.6.2" in out + assert "uv tool install specify-cli" in out + + def test_no_output_when_up_to_date(self, monkeypatch, tmp_path): + monkeypatch.setattr("specify_cli.get_speckit_version", lambda: "0.7.0") + monkeypatch.setattr( + "specify_cli._update_check_cache_path", lambda: tmp_path / "vc.json" + ) + monkeypatch.setattr("specify_cli._fetch_latest_version", lambda: "v0.7.0") + + out = self._run_and_capture(monkeypatch) + + assert out == "" + + def test_uses_cache_when_fresh(self, monkeypatch, tmp_path): + cache_file = tmp_path / "vc.json" + cache_file.write_text(json.dumps({"checked_at": time.time(), "latest": "v0.7.0"})) + + monkeypatch.setattr("specify_cli.get_speckit_version", lambda: "0.6.2") + monkeypatch.setattr("specify_cli._update_check_cache_path", lambda: cache_file) + + call_counter = {"n": 0} + + def _should_not_be_called() -> str | None: + call_counter["n"] += 1 + return None + + monkeypatch.setattr("specify_cli._fetch_latest_version", _should_not_be_called) + + out = self._run_and_capture(monkeypatch) + + assert call_counter["n"] == 0 + assert "v0.7.0" in out + + def test_network_failure_is_silent(self, monkeypatch, tmp_path): + monkeypatch.setattr("specify_cli.get_speckit_version", lambda: "0.6.2") + monkeypatch.setattr( + "specify_cli._update_check_cache_path", lambda: tmp_path / "vc.json" + ) + monkeypatch.setattr("specify_cli._fetch_latest_version", lambda: None) + + out = self._run_and_capture(monkeypatch) + + assert out == "" + + def test_skip_env_var_short_circuits(self, monkeypatch, tmp_path): + monkeypatch.setenv("SPECIFY_SKIP_UPDATE_CHECK", "1") + + fetched = {"called": False} + + def _fetch(): + fetched["called"] = True + return "v99.0.0" + + monkeypatch.setattr("specify_cli._fetch_latest_version", _fetch) + monkeypatch.setattr("specify_cli.get_speckit_version", lambda: "0.0.1") + monkeypatch.setattr( + "specify_cli._update_check_cache_path", lambda: tmp_path / "vc.json" + ) + + _check_for_updates() + + assert fetched["called"] is False From 81a7418c8cacc46729e17496b996f688a505fcee Mon Sep 17 00:00:00 2001 From: arystan Date: Tue, 21 Apr 2026 12:27:19 +0500 Subject: [PATCH 2/6] fix(cli): make update check opt-in; address review feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses CHANGES_REQUESTED on #2212. The update check now only runs when SPECIFY_ENABLE_UPDATE_CHECK=1 (or true/yes/on) is set, so air-gapped and network-constrained environments never attempt to reach GitHub by default. Also addresses the Copilot review findings: - Widen `_parse_version_tuple(version: str | None)` signature and guard with `isinstance` (matches what the tests were already passing). - Use explicit `encoding="utf-8"` for the update-check cache read and write, consistent with the rest of the module. - Reword the "never blocks" claim in the module comment and in docs/installation.md to "never fails the command", and note the possible small startup delay on cache miss. - Include the `None` `invoked_subcommand` case (bare `specify` launch) so the check runs alongside the banner when opted in. Tests: - Replace the opt-out short-circuit test with an opt-in default-off test. - Add tests asserting `SPECIFY_ENABLE_UPDATE_CHECK=1` allows the fetch and that `CI=1` still suppresses it. - `uv run pytest tests/test_update_check.py` → 27 passed. - Full suite: 1301 passed, 20 skipped, 1 pre-existing unrelated failure (`test_without_force_errors_on_existing_dir`, Rich panel-wrap on `already exists`). --- CHANGELOG.md | 2 +- docs/installation.md | 13 +++++++--- src/specify_cli/__init__.py | 40 +++++++++++++++++++++++-------- tests/test_update_check.py | 48 +++++++++++++++++++++++++++++++++++-- 4 files changed, 87 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 517cc53f4f..ef1dff66d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ ### Added -- feat(cli): warn on launch when a newer spec-kit release is available; cached for 24h and suppressed with `SPECIFY_SKIP_UPDATE_CHECK=1`, non-interactive shells, or `CI=1` (#1320) +- feat(cli): opt-in launch warning when a newer spec-kit release is available; enable with `SPECIFY_ENABLE_UPDATE_CHECK=1` (or `true`/`yes`/`on`), cached for 24h, and suppressed in non-interactive shells and `CI=1` (#1320) ## [0.6.2] - 2026-04-13 diff --git a/docs/installation.md b/docs/installation.md index a00aa30c7e..abe3f93b82 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -79,13 +79,20 @@ The `.specify/scripts` directory will contain both `.sh` and `.ps1` scripts. ### Update Notifications -On each launch, `specify` checks once per 24 hours whether a newer release is available on GitHub and prints an upgrade hint if so. The check is silent when: +`specify` can check once per 24 hours whether a newer release is available on GitHub and print an upgrade hint. This is **opt-in**: the check is off by default because air-gapped and network-constrained environments cannot reach GitHub. + +To enable it, set: + +```bash +export SPECIFY_ENABLE_UPDATE_CHECK=1 # or true / yes / on +``` + +Even when enabled, the check stays silent when: -- `SPECIFY_SKIP_UPDATE_CHECK=1` (or `true`/`yes`/`on`) is set - stdout is not a TTY (piped output, redirected to a file, etc.) - the `CI` environment variable is set -Network failures and rate-limit responses are swallowed — the check never blocks the command you ran. +Network failures and rate-limit responses are swallowed — the check never fails the command you ran, though a cache miss may add a small startup delay (bounded by a 2-second fetch timeout) while contacting GitHub. ## Troubleshooting diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index ab9799ff71..236dea5331 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -328,8 +328,14 @@ def callback(ctx: typer.Context): console.print(Align.center("[dim]Run 'specify --help' for usage information[/dim]")) console.print() # Addresses #1320: nudge users running outdated CLIs. The `version` subcommand - # already surfaces the version, so skip there to avoid double-printing. - if ctx.invoked_subcommand not in (None, "version"): + # already surfaces the version, so skip there to avoid double-printing; also + # skip help invocations. Runs on bare `specify` too so the banner launch + # benefits from the nudge when the user has opted in. + if ( + ctx.invoked_subcommand != "version" + and "--help" not in sys.argv + and "-h" not in sys.argv + ): _check_for_updates() def run_command(cmd: list[str], check_return: bool = True, capture: bool = False, shell: bool = False) -> Optional[str]: @@ -1592,17 +1598,21 @@ def get_speckit_version() -> str: # ===== Update check (addresses #1320) ===== # -# Cached once per 24h in the platform user-cache dir. Triggered from the top-level -# callback. Never blocks the user — every failure path swallows the exception. +# Opt-in only (set SPECIFY_ENABLE_UPDATE_CHECK=1). Air-gapped / network-constrained +# environments never reach GitHub, so the check is off by default. When enabled, +# it is cached once per 24h in the platform user-cache dir and triggered from the +# top-level callback. Best-effort: every failure path swallows the exception so +# the check never fails the command, though cache misses may add a small startup +# delay (bounded by the fetch timeout) while contacting GitHub. _UPDATE_CHECK_URL = "https://api.github.com/repos/github/spec-kit/releases/latest" _UPDATE_CHECK_CACHE_TTL_SECONDS = 24 * 60 * 60 _UPDATE_CHECK_TIMEOUT_SECONDS = 2.0 -def _parse_version_tuple(version: str) -> tuple[int, ...] | None: +def _parse_version_tuple(version: str | None) -> tuple[int, ...] | None: """Parse `v0.6.2` / `0.6.2` / `0.6.2.dev0` → tuple of ints. Returns None if unparseable.""" - if not version: + if not isinstance(version, str) or not version: return None s = version.strip().lstrip("vV") # Drop PEP 440 pre/post/dev/local segments; we only compare release numbers. @@ -1631,7 +1641,7 @@ def _read_update_check_cache(path: Path) -> dict | None: import time if not path.exists(): return None - data = json.loads(path.read_text()) + data = json.loads(path.read_text(encoding="utf-8")) checked_at = float(data.get("checked_at", 0)) if time.time() - checked_at > _UPDATE_CHECK_CACHE_TTL_SECONDS: return None @@ -1644,7 +1654,10 @@ def _write_update_check_cache(path: Path, latest: str) -> None: try: import time path.parent.mkdir(parents=True, exist_ok=True) - path.write_text(json.dumps({"checked_at": time.time(), "latest": latest})) + path.write_text( + json.dumps({"checked_at": time.time(), "latest": latest}), + encoding="utf-8", + ) except Exception: # Cache write failures are non-fatal. pass @@ -1667,8 +1680,15 @@ def _fetch_latest_version() -> str | None: def _should_skip_update_check() -> bool: - if os.environ.get("SPECIFY_SKIP_UPDATE_CHECK", "").strip().lower() in ("1", "true", "yes", "on"): + # Opt-in only: skip unless the user has explicitly enabled the check. + # Air-gapped / network-constrained environments cannot reach GitHub, so a + # default-on network call is a non-starter; keeping this off by default + # also means users never pay the fetch latency unless they asked for it. + if os.environ.get("SPECIFY_ENABLE_UPDATE_CHECK", "").strip().lower() not in ("1", "true", "yes", "on"): return True + # Belt-and-suspenders: even when opted in, suppress in CI and when the + # caller isn't a TTY (piped output, redirected logs, etc.) so we don't + # dirty machine-readable output with a human-facing warning. if os.environ.get("CI"): return True try: @@ -1720,7 +1740,7 @@ def _check_for_updates() -> None: f"--from git+https://github.com/github/spec-kit.git@v{latest_display}[/dim]" ) console.print( - "[dim] (set SPECIFY_SKIP_UPDATE_CHECK=1 to silence this check)[/dim]" + "[dim] (unset SPECIFY_ENABLE_UPDATE_CHECK to disable this check)[/dim]" ) except Exception: # Update check must never surface an error to the user. diff --git a/tests/test_update_check.py b/tests/test_update_check.py index adf8a156b7..5db693c0df 100644 --- a/tests/test_update_check.py +++ b/tests/test_update_check.py @@ -201,8 +201,52 @@ def test_network_failure_is_silent(self, monkeypatch, tmp_path): assert out == "" - def test_skip_env_var_short_circuits(self, monkeypatch, tmp_path): - monkeypatch.setenv("SPECIFY_SKIP_UPDATE_CHECK", "1") + def test_opt_in_default_off_short_circuits(self, monkeypatch, tmp_path): + """Without SPECIFY_ENABLE_UPDATE_CHECK the helper must not hit the network.""" + monkeypatch.delenv("SPECIFY_ENABLE_UPDATE_CHECK", raising=False) + + fetched = {"called": False} + + def _fetch(): + fetched["called"] = True + return "v99.0.0" + + monkeypatch.setattr("specify_cli._fetch_latest_version", _fetch) + monkeypatch.setattr("specify_cli.get_speckit_version", lambda: "0.0.1") + monkeypatch.setattr( + "specify_cli._update_check_cache_path", lambda: tmp_path / "vc.json" + ) + + _check_for_updates() + + assert fetched["called"] is False + + def test_opt_in_env_var_allows_check(self, monkeypatch, tmp_path): + """With SPECIFY_ENABLE_UPDATE_CHECK=1 and a TTY, the helper proceeds.""" + monkeypatch.setenv("SPECIFY_ENABLE_UPDATE_CHECK", "1") + monkeypatch.delenv("CI", raising=False) + monkeypatch.setattr("sys.stdout.isatty", lambda: True) + + fetched = {"called": False} + + def _fetch(): + fetched["called"] = True + return "v99.0.0" + + monkeypatch.setattr("specify_cli._fetch_latest_version", _fetch) + monkeypatch.setattr("specify_cli.get_speckit_version", lambda: "0.0.1") + monkeypatch.setattr( + "specify_cli._update_check_cache_path", lambda: tmp_path / "vc.json" + ) + + _check_for_updates() + + assert fetched["called"] is True + + def test_ci_suppresses_even_when_opted_in(self, monkeypatch, tmp_path): + """Belt-and-suspenders: CI=1 wins over the opt-in flag.""" + monkeypatch.setenv("SPECIFY_ENABLE_UPDATE_CHECK", "1") + monkeypatch.setenv("CI", "1") fetched = {"called": False} From b75e55fd6f361f8f1d232f871dc956406c06e706 Mon Sep 17 00:00:00 2001 From: arystan Date: Tue, 28 Apr 2026 13:16:53 +0500 Subject: [PATCH 3/6] test(update-check): pin isatty in CI-suppression test Without this, pytest's stdout capture makes sys.stdout.isatty() return False under pytest, so the TTY guard alone would suppress the fetch and the assertion would still pass even if the CI guard were removed. Pinning isatty()=True ensures CI=1 is what's actually being verified. Addresses Copilot feedback on PR #2212. --- tests/test_update_check.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/test_update_check.py b/tests/test_update_check.py index 5db693c0df..8c1db66bef 100644 --- a/tests/test_update_check.py +++ b/tests/test_update_check.py @@ -244,9 +244,15 @@ def _fetch(): assert fetched["called"] is True def test_ci_suppresses_even_when_opted_in(self, monkeypatch, tmp_path): - """Belt-and-suspenders: CI=1 wins over the opt-in flag.""" + """Belt-and-suspenders: CI=1 wins over the opt-in flag. + + Pin isatty()=True so this test fails if the CI guard is removed — + otherwise pytest's stdout capture makes isatty False and the TTY + guard alone would suppress the fetch, masking a regression. + """ monkeypatch.setenv("SPECIFY_ENABLE_UPDATE_CHECK", "1") monkeypatch.setenv("CI", "1") + monkeypatch.setattr("sys.stdout.isatty", lambda: True) fetched = {"called": False} From 53e20e2d9658c565c3b849bb6ec9604456b5d90f Mon Sep 17 00:00:00 2001 From: arystan Date: Sat, 16 May 2026 15:31:21 +0500 Subject: [PATCH 4/6] fix(cli): cache update-check failures to honor <=1 fetch/24h MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the user has opted in via SPECIFY_ENABLE_UPDATE_CHECK=1 but the GitHub fetch fails (offline, rate-limited, blocked), record a negative cache entry (latest=null) and key the next decision off "was there a fresh cache hit" rather than "is latest_str None" — so the CLI does not retry the network on every invocation until the 24h TTL expires. Addresses Copilot review feedback on PR #2212. --- CHANGELOG.md | 1 + docs/installation.md | 2 +- src/specify_cli/__init__.py | 21 +++++++++++-------- tests/test_update_check.py | 41 +++++++++++++++++++++++++++++++++++++ 4 files changed, 55 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 786c9127a8..3f2b79586f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ ### Added - feat(cli): opt-in launch warning when a newer spec-kit release is available; enable with `SPECIFY_ENABLE_UPDATE_CHECK=1` (or `true`/`yes`/`on`), cached for 24h, and suppressed in non-interactive shells and `CI=1` (#1320) +- fix(cli): cache update-check failures so transient outages don't trigger a network call on every CLI invocation (#1320) ## [0.8.1] - 2026-04-24 diff --git a/docs/installation.md b/docs/installation.md index aac7456f9d..fe4b2bd4bc 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -109,7 +109,7 @@ Even when enabled, the check stays silent when: - stdout is not a TTY (piped output, redirected to a file, etc.) - the `CI` environment variable is set -Network failures and rate-limit responses are swallowed — the check never fails the command you ran, though a cache miss may add a small startup delay (bounded by a 2-second fetch timeout) while contacting GitHub. +Network failures and rate-limit responses are swallowed — the check never fails the command you ran, though a cache miss may add a small startup delay (bounded by a 2-second fetch timeout) while contacting GitHub. Failures are also cached for the same 24h window, so a transient outage or block won't cause the CLI to retry on every invocation. ## Troubleshooting diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index a211787448..c4d5aaacdd 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1895,6 +1895,8 @@ def get_speckit_version() -> str: # top-level callback. Best-effort: every failure path swallows the exception so # the check never fails the command, though cache misses may add a small startup # delay (bounded by the fetch timeout) while contacting GitHub. +# Fetch failures are cached too (with `latest=null`) so a transient outage +# doesn't cause a retry on every CLI invocation until the TTL expires. _UPDATE_CHECK_URL = "https://api.github.com/repos/github/spec-kit/releases/latest" _UPDATE_CHECK_CACHE_TTL_SECONDS = 24 * 60 * 60 @@ -1941,7 +1943,7 @@ def _read_update_check_cache(path: Path) -> dict | None: return None -def _write_update_check_cache(path: Path, latest: str) -> None: +def _write_update_check_cache(path: Path, latest: str | None) -> None: try: import time path.parent.mkdir(parents=True, exist_ok=True) @@ -2005,15 +2007,16 @@ def _check_for_updates() -> None: return cache_path = _update_check_cache_path() - latest_str: str | None = None - if cache_path is not None: - cached = _read_update_check_cache(cache_path) - if cached: - latest_str = cached.get("latest") - - if latest_str is None: + cached = _read_update_check_cache(cache_path) if cache_path is not None else None + if cached is not None: + # Fresh cache hit — may be a positive (`latest=v…`) or + # negative (`latest=null`) entry; either way, no fetch. + latest_str = cached.get("latest") + else: latest_str = _fetch_latest_version() - if latest_str and cache_path is not None: + if cache_path is not None: + # Cache the attempt even on failure so transient outages + # don't trigger a network call on every CLI invocation. _write_update_check_cache(cache_path, latest_str) latest = _parse_version_tuple(latest_str) if latest_str else None diff --git a/tests/test_update_check.py b/tests/test_update_check.py index 8c1db66bef..e268ad30ce 100644 --- a/tests/test_update_check.py +++ b/tests/test_update_check.py @@ -77,6 +77,14 @@ def test_write_round_trips(self, tmp_path): assert data["latest"] == "v0.9.9" assert float(data["checked_at"]) <= time.time() + def test_write_round_trips_negative_entry(self, tmp_path): + cache_file = tmp_path / "nested" / "version_check.json" + _write_update_check_cache(cache_file, None) + assert cache_file.exists() + data = json.loads(cache_file.read_text()) + assert data["latest"] is None + assert float(data["checked_at"]) <= time.time() + class TestFetchLatestVersion: def test_returns_tag_on_success(self): @@ -201,6 +209,39 @@ def test_network_failure_is_silent(self, monkeypatch, tmp_path): assert out == "" + def test_network_failure_writes_negative_cache(self, monkeypatch, tmp_path): + cache_file = tmp_path / "vc.json" + monkeypatch.setattr("specify_cli.get_speckit_version", lambda: "0.6.2") + monkeypatch.setattr("specify_cli._update_check_cache_path", lambda: cache_file) + monkeypatch.setattr("specify_cli._fetch_latest_version", lambda: None) + + self._run_and_capture(monkeypatch) + + assert cache_file.exists() + data = json.loads(cache_file.read_text()) + assert data["latest"] is None + assert time.time() - float(data["checked_at"]) < 5 + + def test_negative_cache_skips_fetch_within_ttl(self, monkeypatch, tmp_path): + cache_file = tmp_path / "vc.json" + cache_file.write_text(json.dumps({"checked_at": time.time(), "latest": None})) + + monkeypatch.setattr("specify_cli.get_speckit_version", lambda: "0.6.2") + monkeypatch.setattr("specify_cli._update_check_cache_path", lambda: cache_file) + + call_counter = {"n": 0} + + def _should_not_be_called() -> str | None: + call_counter["n"] += 1 + return None + + monkeypatch.setattr("specify_cli._fetch_latest_version", _should_not_be_called) + + out = self._run_and_capture(monkeypatch) + + assert call_counter["n"] == 0 + assert out == "" + def test_opt_in_default_off_short_circuits(self, monkeypatch, tmp_path): """Without SPECIFY_ENABLE_UPDATE_CHECK the helper must not hit the network.""" monkeypatch.delenv("SPECIFY_ENABLE_UPDATE_CHECK", raising=False) From 5185ebf81532c3c611869c2fbf22cf456823ba5d Mon Sep 17 00:00:00 2001 From: arystan Date: Sat, 16 May 2026 16:30:46 +0500 Subject: [PATCH 5/6] refactor(cli): move update-check into _version.py, reuse upstream primitives Following PR #2550 which extracted version handling into _version.py, move the opt-in startup update-check helpers there too and replace our duplicates with upstream's: - _get_installed_version (was: local get_speckit_version with pyproject fallback) - _fetch_latest_release_tag (was: local _fetch_latest_version; gains auth via open_url) - _is_newer (was: local _parse_version_tuple; proper PEP 440 via packaging.Version) Behavior preserved: same opt-in env var, same 24h TTL, same negative caching, same CI/TTY skip guards. Cache file format unchanged. --- CHANGELOG.md | 1 + src/specify_cli/__init__.py | 11 +++ src/specify_cli/_version.py | 116 +++++++++++++++++++++++++++ tests/test_update_check.py | 155 +++++++++--------------------------- 4 files changed, 166 insertions(+), 117 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b9c0a4598b..fc7e6afd9c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ - feat(cli): opt-in launch warning when a newer spec-kit release is available; enable with `SPECIFY_ENABLE_UPDATE_CHECK=1` (or `true`/`yes`/`on`), cached for 24h, and suppressed in non-interactive shells and `CI=1` (#1320) - fix(cli): cache update-check failures so transient outages don't trigger a network call on every CLI invocation (#1320) +- refactor(cli): move update-check helpers into `_version.py` and reuse the existing `_get_installed_version` / `_fetch_latest_release_tag` / `_is_newer` primitives from #2550 (#1320) ## [0.8.11] - 2026-05-15 diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 41fb994726..939bd59b08 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -97,6 +97,7 @@ self_app as _self_app, self_check as self_check, self_upgrade as self_upgrade, + _check_for_updates, ) def _build_agent_config() -> dict[str, dict[str, Any]]: @@ -200,6 +201,16 @@ def callback( show_banner() console.print(Align.center("[dim]Run 'specify --help' for usage information[/dim]")) console.print() + # Addresses #1320: nudge users running outdated CLIs. The `version` subcommand + # already surfaces the version, so skip there to avoid double-printing; also + # skip help invocations. Runs on bare `specify` too so the banner launch + # benefits from the nudge when the user has opted in. + if ( + ctx.invoked_subcommand != "version" + and "--help" not in sys.argv + and "-h" not in sys.argv + ): + _check_for_updates() def _refresh_shared_templates( project_path: Path, diff --git a/src/specify_cli/_version.py b/src/specify_cli/_version.py index 0a52ac7e80..dd1d49499e 100644 --- a/src/specify_cli/_version.py +++ b/src/specify_cli/_version.py @@ -171,3 +171,119 @@ def self_upgrade() -> None: console.print("specify self upgrade is not implemented yet.") console.print("Run 'specify self check' to see whether a newer release is available.") console.print("Actual self-upgrade is planned as follow-up work.") + + +# ===== Opt-in startup update check (addresses #1320) ===== +# +# Silent companion to `specify self check`: when SPECIFY_ENABLE_UPDATE_CHECK=1 +# is set in an interactive non-CI shell, the top-level Typer callback prints a +# one-line upgrade hint if a newer release is available. Result is cached for +# 24h in the platform user-cache dir; cache misses are written even on fetch +# failure (`latest=null`) so a transient outage doesn't trigger a network call +# on every CLI invocation. Best-effort: every error path swallows the exception +# so the helper never blocks the command the user actually invoked. + +import os +import sys +import time +from pathlib import Path + +_UPDATE_CHECK_CACHE_TTL_SECONDS = 24 * 60 * 60 + + +def _update_check_cache_path() -> Path | None: + try: + from platformdirs import user_cache_dir + return Path(user_cache_dir("specify-cli")) / "version_check.json" + except Exception: + return None + + +def _read_update_check_cache(path: Path) -> dict | None: + try: + if not path.exists(): + return None + data = json.loads(path.read_text(encoding="utf-8")) + checked_at = float(data.get("checked_at", 0)) + if time.time() - checked_at > _UPDATE_CHECK_CACHE_TTL_SECONDS: + return None + return data + except Exception: + return None + + +def _write_update_check_cache(path: Path, latest: str | None) -> None: + try: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text( + json.dumps({"checked_at": time.time(), "latest": latest}), + encoding="utf-8", + ) + except Exception: + # Cache write failures are non-fatal. + pass + + +def _should_skip_update_check() -> bool: + # Opt-in only: air-gapped / network-constrained environments cannot reach + # GitHub, so the check is off by default. + if os.environ.get("SPECIFY_ENABLE_UPDATE_CHECK", "").strip().lower() not in ("1", "true", "yes", "on"): + return True + # Belt-and-suspenders: even when opted in, suppress in CI and when the + # caller isn't a TTY so we don't dirty machine-readable output. + if os.environ.get("CI"): + return True + try: + if not sys.stdout.isatty(): + return True + except Exception: + return True + return False + + +def _check_for_updates() -> None: + """Print a one-line upgrade hint when a newer spec-kit release is available. + + Fully best-effort — any error (offline, rate-limited, parse failure) is + swallowed so the command the user actually invoked is never blocked. + """ + if _should_skip_update_check(): + return + try: + current = _get_installed_version() + if current == "unknown": + return + + cache_path = _update_check_cache_path() + cached = _read_update_check_cache(cache_path) if cache_path is not None else None + if cached is not None: + # Fresh cache hit — may be a positive (`latest=v…`) or + # negative (`latest=null`) entry; either way, no fetch. + latest_tag = cached.get("latest") + else: + latest_tag, _reason = _fetch_latest_release_tag() + if cache_path is not None: + # Cache the attempt even on failure so transient outages + # don't trigger a network call on every CLI invocation. + _write_update_check_cache(cache_path, latest_tag) + + if not latest_tag: + return + latest_display = _normalize_tag(latest_tag) + if not _is_newer(latest_display, current): + return + + console.print( + f"[yellow]⚠ A new spec-kit version is available: " + f"v{latest_display} (you have v{current})[/yellow]" + ) + console.print( + f"[dim] Upgrade: uv tool install specify-cli --force " + f"--from git+https://github.com/github/spec-kit.git@v{latest_display}[/dim]" + ) + console.print( + "[dim] (unset SPECIFY_ENABLE_UPDATE_CHECK to disable this check)[/dim]" + ) + except Exception: + # Update check must never surface an error to the user. + return diff --git a/tests/test_update_check.py b/tests/test_update_check.py index e268ad30ce..1031b55dfb 100644 --- a/tests/test_update_check.py +++ b/tests/test_update_check.py @@ -7,45 +7,17 @@ import json import time -import urllib.error from io import StringIO -from unittest.mock import patch import pytest -from specify_cli import ( +from specify_cli._version import ( _check_for_updates, - _fetch_latest_version, - _parse_version_tuple, _read_update_check_cache, _write_update_check_cache, ) -class TestParseVersionTuple: - @pytest.mark.parametrize( - "raw,expected", - [ - ("v0.6.2", (0, 6, 2)), - ("0.6.2", (0, 6, 2)), - ("V1.2.3.4", (1, 2, 3, 4)), - ("0.6.2.dev0", (0, 6, 2)), - ("1.0.0-rc.1", (1, 0, 0)), - ("1.0.0+meta", (1, 0, 0)), - ], - ) - def test_parses_common_version_strings(self, raw, expected): - assert _parse_version_tuple(raw) == expected - - @pytest.mark.parametrize("raw", ["", "abc", "v.", None]) - def test_returns_none_on_unparseable(self, raw): - assert _parse_version_tuple(raw) is None - - def test_ordering_matches_semver_intuition(self): - assert _parse_version_tuple("v0.6.2") < _parse_version_tuple("v0.6.3") - assert _parse_version_tuple("v0.6.2") < _parse_version_tuple("v0.7.0") - assert _parse_version_tuple("v0.6.2") == _parse_version_tuple("0.6.2") - class TestCache: def test_fresh_cache_is_returned(self, tmp_path): @@ -86,57 +58,6 @@ def test_write_round_trips_negative_entry(self, tmp_path): assert float(data["checked_at"]) <= time.time() -class TestFetchLatestVersion: - def test_returns_tag_on_success(self): - payload = json.dumps({"tag_name": "v0.6.3"}).encode("utf-8") - - class FakeResp: - def __enter__(self): - return self - - def __exit__(self, *a): - return False - - def read(self): - return payload - - with patch("urllib.request.urlopen", return_value=FakeResp()): - assert _fetch_latest_version() == "v0.6.3" - - def test_returns_none_on_network_error(self): - with patch("urllib.request.urlopen", side_effect=urllib.error.URLError("offline")): - assert _fetch_latest_version() is None - - def test_returns_none_on_malformed_json(self): - class FakeResp: - def __enter__(self): - return self - - def __exit__(self, *a): - return False - - def read(self): - return b"not json" - - with patch("urllib.request.urlopen", return_value=FakeResp()): - assert _fetch_latest_version() is None - - def test_returns_none_when_tag_missing(self): - payload = json.dumps({"name": "unnamed"}).encode("utf-8") - - class FakeResp: - def __enter__(self): - return self - - def __exit__(self, *a): - return False - - def read(self): - return payload - - with patch("urllib.request.urlopen", return_value=FakeResp()): - assert _fetch_latest_version() is None - class TestCheckForUpdates: """End-to-end-ish checks on `_check_for_updates` with skip conditions patched off.""" @@ -144,21 +65,21 @@ class TestCheckForUpdates: def _run_and_capture(self, monkeypatch) -> str: """Force the skip-guard off so the helper runs, then capture console output.""" # Guard returns False → helper proceeds. - monkeypatch.setattr("specify_cli._should_skip_update_check", lambda: False) + monkeypatch.setattr("specify_cli._version._should_skip_update_check", lambda: False) buf = StringIO() - import specify_cli + import specify_cli._version from rich.console import Console captured = Console(file=buf, force_terminal=False, width=200) - monkeypatch.setattr(specify_cli, "console", captured) + monkeypatch.setattr(specify_cli._version, "console", captured) _check_for_updates() return buf.getvalue() def test_prints_warning_when_newer_release_available(self, monkeypatch, tmp_path): - monkeypatch.setattr("specify_cli.get_speckit_version", lambda: "0.6.2") + monkeypatch.setattr("specify_cli._version._get_installed_version", lambda: "0.6.2") monkeypatch.setattr( - "specify_cli._update_check_cache_path", lambda: tmp_path / "vc.json" + "specify_cli._version._update_check_cache_path", lambda: tmp_path / "vc.json" ) - monkeypatch.setattr("specify_cli._fetch_latest_version", lambda: "v0.7.0") + monkeypatch.setattr("specify_cli._version._fetch_latest_release_tag", lambda: ("v0.7.0", None)) out = self._run_and_capture(monkeypatch) @@ -168,11 +89,11 @@ def test_prints_warning_when_newer_release_available(self, monkeypatch, tmp_path assert "uv tool install specify-cli" in out def test_no_output_when_up_to_date(self, monkeypatch, tmp_path): - monkeypatch.setattr("specify_cli.get_speckit_version", lambda: "0.7.0") + monkeypatch.setattr("specify_cli._version._get_installed_version", lambda: "0.7.0") monkeypatch.setattr( - "specify_cli._update_check_cache_path", lambda: tmp_path / "vc.json" + "specify_cli._version._update_check_cache_path", lambda: tmp_path / "vc.json" ) - monkeypatch.setattr("specify_cli._fetch_latest_version", lambda: "v0.7.0") + monkeypatch.setattr("specify_cli._version._fetch_latest_release_tag", lambda: ("v0.7.0", None)) out = self._run_and_capture(monkeypatch) @@ -182,16 +103,16 @@ def test_uses_cache_when_fresh(self, monkeypatch, tmp_path): cache_file = tmp_path / "vc.json" cache_file.write_text(json.dumps({"checked_at": time.time(), "latest": "v0.7.0"})) - monkeypatch.setattr("specify_cli.get_speckit_version", lambda: "0.6.2") - monkeypatch.setattr("specify_cli._update_check_cache_path", lambda: cache_file) + monkeypatch.setattr("specify_cli._version._get_installed_version", lambda: "0.6.2") + monkeypatch.setattr("specify_cli._version._update_check_cache_path", lambda: cache_file) call_counter = {"n": 0} - def _should_not_be_called() -> str | None: + def _should_not_be_called() -> tuple[str | None, str | None]: call_counter["n"] += 1 - return None + return (None, None) - monkeypatch.setattr("specify_cli._fetch_latest_version", _should_not_be_called) + monkeypatch.setattr("specify_cli._version._fetch_latest_release_tag", _should_not_be_called) out = self._run_and_capture(monkeypatch) @@ -199,11 +120,11 @@ def _should_not_be_called() -> str | None: assert "v0.7.0" in out def test_network_failure_is_silent(self, monkeypatch, tmp_path): - monkeypatch.setattr("specify_cli.get_speckit_version", lambda: "0.6.2") + monkeypatch.setattr("specify_cli._version._get_installed_version", lambda: "0.6.2") monkeypatch.setattr( - "specify_cli._update_check_cache_path", lambda: tmp_path / "vc.json" + "specify_cli._version._update_check_cache_path", lambda: tmp_path / "vc.json" ) - monkeypatch.setattr("specify_cli._fetch_latest_version", lambda: None) + monkeypatch.setattr("specify_cli._version._fetch_latest_release_tag", lambda: (None, "offline or timeout")) out = self._run_and_capture(monkeypatch) @@ -211,9 +132,9 @@ def test_network_failure_is_silent(self, monkeypatch, tmp_path): def test_network_failure_writes_negative_cache(self, monkeypatch, tmp_path): cache_file = tmp_path / "vc.json" - monkeypatch.setattr("specify_cli.get_speckit_version", lambda: "0.6.2") - monkeypatch.setattr("specify_cli._update_check_cache_path", lambda: cache_file) - monkeypatch.setattr("specify_cli._fetch_latest_version", lambda: None) + monkeypatch.setattr("specify_cli._version._get_installed_version", lambda: "0.6.2") + monkeypatch.setattr("specify_cli._version._update_check_cache_path", lambda: cache_file) + monkeypatch.setattr("specify_cli._version._fetch_latest_release_tag", lambda: (None, "offline or timeout")) self._run_and_capture(monkeypatch) @@ -226,16 +147,16 @@ def test_negative_cache_skips_fetch_within_ttl(self, monkeypatch, tmp_path): cache_file = tmp_path / "vc.json" cache_file.write_text(json.dumps({"checked_at": time.time(), "latest": None})) - monkeypatch.setattr("specify_cli.get_speckit_version", lambda: "0.6.2") - monkeypatch.setattr("specify_cli._update_check_cache_path", lambda: cache_file) + monkeypatch.setattr("specify_cli._version._get_installed_version", lambda: "0.6.2") + monkeypatch.setattr("specify_cli._version._update_check_cache_path", lambda: cache_file) call_counter = {"n": 0} - def _should_not_be_called() -> str | None: + def _should_not_be_called() -> tuple[str | None, str | None]: call_counter["n"] += 1 - return None + return (None, None) - monkeypatch.setattr("specify_cli._fetch_latest_version", _should_not_be_called) + monkeypatch.setattr("specify_cli._version._fetch_latest_release_tag", _should_not_be_called) out = self._run_and_capture(monkeypatch) @@ -250,12 +171,12 @@ def test_opt_in_default_off_short_circuits(self, monkeypatch, tmp_path): def _fetch(): fetched["called"] = True - return "v99.0.0" + return ("v99.0.0", None) - monkeypatch.setattr("specify_cli._fetch_latest_version", _fetch) - monkeypatch.setattr("specify_cli.get_speckit_version", lambda: "0.0.1") + monkeypatch.setattr("specify_cli._version._fetch_latest_release_tag", _fetch) + monkeypatch.setattr("specify_cli._version._get_installed_version", lambda: "0.0.1") monkeypatch.setattr( - "specify_cli._update_check_cache_path", lambda: tmp_path / "vc.json" + "specify_cli._version._update_check_cache_path", lambda: tmp_path / "vc.json" ) _check_for_updates() @@ -272,12 +193,12 @@ def test_opt_in_env_var_allows_check(self, monkeypatch, tmp_path): def _fetch(): fetched["called"] = True - return "v99.0.0" + return ("v99.0.0", None) - monkeypatch.setattr("specify_cli._fetch_latest_version", _fetch) - monkeypatch.setattr("specify_cli.get_speckit_version", lambda: "0.0.1") + monkeypatch.setattr("specify_cli._version._fetch_latest_release_tag", _fetch) + monkeypatch.setattr("specify_cli._version._get_installed_version", lambda: "0.0.1") monkeypatch.setattr( - "specify_cli._update_check_cache_path", lambda: tmp_path / "vc.json" + "specify_cli._version._update_check_cache_path", lambda: tmp_path / "vc.json" ) _check_for_updates() @@ -299,12 +220,12 @@ def test_ci_suppresses_even_when_opted_in(self, monkeypatch, tmp_path): def _fetch(): fetched["called"] = True - return "v99.0.0" + return ("v99.0.0", None) - monkeypatch.setattr("specify_cli._fetch_latest_version", _fetch) - monkeypatch.setattr("specify_cli.get_speckit_version", lambda: "0.0.1") + monkeypatch.setattr("specify_cli._version._fetch_latest_release_tag", _fetch) + monkeypatch.setattr("specify_cli._version._get_installed_version", lambda: "0.0.1") monkeypatch.setattr( - "specify_cli._update_check_cache_path", lambda: tmp_path / "vc.json" + "specify_cli._version._update_check_cache_path", lambda: tmp_path / "vc.json" ) _check_for_updates() From c3e063d1d45b4b47f3011abebbb325f8a52db8ac Mon Sep 17 00:00:00 2001 From: arystan Date: Wed, 20 May 2026 18:22:29 +0500 Subject: [PATCH 6/6] fix(cli): address update-check review feedback --- docs/installation.md | 2 +- src/specify_cli/_version.py | 23 +++++++++++++++-------- tests/test_update_check.py | 24 ++++++++++++++++++++---- 3 files changed, 36 insertions(+), 13 deletions(-) diff --git a/docs/installation.md b/docs/installation.md index 5977f2b376..be295d5eb5 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -114,7 +114,7 @@ Even when enabled, the check stays silent when: - stdout is not a TTY (piped output, redirected to a file, etc.) - the `CI` environment variable is set -Network failures and rate-limit responses are swallowed — the check never fails the command you ran, though a cache miss may add a small startup delay (bounded by a 2-second fetch timeout) while contacting GitHub. Failures are also cached for the same 24h window, so a transient outage or block won't cause the CLI to retry on every invocation. +Network failures and rate-limit responses are swallowed — the check never fails the command you ran, though a cache miss may add a small startup delay (bounded by a 5-second fetch timeout) while contacting GitHub. Failures are also cached for the same 24h window, so a transient outage or block won't cause the CLI to retry on every invocation. ## Troubleshooting diff --git a/src/specify_cli/_version.py b/src/specify_cli/_version.py index dd1d49499e..d9f7a0abef 100644 --- a/src/specify_cli/_version.py +++ b/src/specify_cli/_version.py @@ -10,7 +10,11 @@ from __future__ import annotations import json +import os +import sys +import time import urllib.error +from pathlib import Path import typer from packaging.version import InvalidVersion, Version @@ -181,12 +185,8 @@ def self_upgrade() -> None: # 24h in the platform user-cache dir; cache misses are written even on fetch # failure (`latest=null`) so a transient outage doesn't trigger a network call # on every CLI invocation. Best-effort: every error path swallows the exception -# so the helper never blocks the command the user actually invoked. - -import os -import sys -import time -from pathlib import Path +# so the helper never fails the command the user actually invoked, though cache +# misses may add a bounded startup delay while contacting GitHub. _UPDATE_CHECK_CACHE_TTL_SECONDS = 24 * 60 * 60 @@ -245,7 +245,7 @@ def _check_for_updates() -> None: """Print a one-line upgrade hint when a newer spec-kit release is available. Fully best-effort — any error (offline, rate-limited, parse failure) is - swallowed so the command the user actually invoked is never blocked. + swallowed so the command the user actually invoked is never failed. """ if _should_skip_update_check(): return @@ -261,7 +261,14 @@ def _check_for_updates() -> None: # negative (`latest=null`) entry; either way, no fetch. latest_tag = cached.get("latest") else: - latest_tag, _reason = _fetch_latest_release_tag() + try: + latest_tag, _reason = _fetch_latest_release_tag() + except Exception: + if cache_path is not None: + # Cache malformed/unexpected fetch failures too, so they + # don't trigger a network call on every CLI invocation. + _write_update_check_cache(cache_path, None) + return if cache_path is not None: # Cache the attempt even on failure so transient outages # don't trigger a network call on every CLI invocation. diff --git a/tests/test_update_check.py b/tests/test_update_check.py index 1031b55dfb..8742a0d098 100644 --- a/tests/test_update_check.py +++ b/tests/test_update_check.py @@ -1,16 +1,14 @@ -"""Tests for the update-check helper in specify_cli.__init__. +"""Tests for the update-check helper in specify_cli._version. Covers issue https://github.com/github/spec-kit/issues/1320 — the CLI should nudge users who are running outdated releases toward an upgrade, without -blocking any command when offline or rate-limited. +failing any command when offline or rate-limited. """ import json import time from io import StringIO -import pytest - from specify_cli._version import ( _check_for_updates, _read_update_check_cache, @@ -143,6 +141,24 @@ def test_network_failure_writes_negative_cache(self, monkeypatch, tmp_path): assert data["latest"] is None assert time.time() - float(data["checked_at"]) < 5 + def test_fetch_exception_writes_negative_cache(self, monkeypatch, tmp_path): + cache_file = tmp_path / "vc.json" + monkeypatch.setattr("specify_cli._version._get_installed_version", lambda: "0.6.2") + monkeypatch.setattr("specify_cli._version._update_check_cache_path", lambda: cache_file) + + def _fetch_raises() -> tuple[str | None, str | None]: + raise ValueError("GitHub API response missing valid tag_name") + + monkeypatch.setattr("specify_cli._version._fetch_latest_release_tag", _fetch_raises) + + out = self._run_and_capture(monkeypatch) + + assert out == "" + assert cache_file.exists() + data = json.loads(cache_file.read_text()) + assert data["latest"] is None + assert time.time() - float(data["checked_at"]) < 5 + def test_negative_cache_skips_fetch_within_ttl(self, monkeypatch, tmp_path): cache_file = tmp_path / "vc.json" cache_file.write_text(json.dumps({"checked_at": time.time(), "latest": None}))