diff --git a/CHANGELOG.md b/CHANGELOG.md index 7e021c6713..0d928db61c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,12 @@ +## [Unreleased] + +### Added + +- feat(cli): warn when a newer spec-kit release is available (opt-in via `SPECIFY_ENABLE_UPDATE_CHECK=1`; suppressed when `CI` is set or stdout is not a TTY; result cached for 24h) (#1320) + ## [0.10.2] - 2026-06-11 ### Changed diff --git a/docs/installation.md b/docs/installation.md index 3ee2f67b0e..df43363a9d 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -101,6 +101,23 @@ Scripts are installed into a variant subdirectory matching the chosen script typ - `.specify/scripts/bash/` — contains `.sh` scripts (default on Linux/macOS) - `.specify/scripts/powershell/` — contains `.ps1` scripts (default on Windows) +### Update Notifications + +`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: + +- 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 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 ### Enterprise / Air-Gapped Installation diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 186593000c..935ead7c5d 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -77,6 +77,7 @@ self_app as _self_app, self_check as self_check, self_upgrade as self_upgrade, + _check_for_updates, ) from ._agent_config import ( AGENT_CONFIG as AGENT_CONFIG, @@ -113,6 +114,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 not in {"version", "self"} + 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 e634a4f286..0cc164e135 100644 --- a/src/specify_cli/_version.py +++ b/src/specify_cli/_version.py @@ -18,6 +18,7 @@ import shutil import subprocess import sys +import time import urllib.error import urllib.parse import urllib.request @@ -1427,3 +1428,177 @@ def self_upgrade( f"Upgraded specify-cli: {pre_upgrade_display} → {verified_display}", soft_wrap=True, ) + + +# ===== 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 +# upgrade guidance 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 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 + + +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")) + if not isinstance(data, dict): + return None + + checked_at = float(data.get("checked_at", 0)) + now = time.time() + if not math.isfinite(checked_at) or checked_at > now: + return None + if now - checked_at > _UPDATE_CHECK_CACHE_TTL_SECONDS: + return None + + latest = data.get("latest") + if latest is not None and not isinstance(latest, str): + return None + return data + except Exception: + return None + + +def _create_cache_dir_no_symlinks(directory: Path) -> bool: + """Create *directory* (with missing parents) without following symlinks. + + ``mkdir(parents=True)`` silently traverses symlinked ancestors, so a planted + symlink anywhere on the way could redirect the cache write to an arbitrary + location. Instead we walk one component at a time: the deepest pre-existing + ancestor must be a real directory (not a symlink), and every component we + create is re-checked after ``mkdir``. Everything at/above the first existing + directory is trusted, mirroring the project's other safe-write helpers. + + Returns False (and writes nothing) when any managed component is a symlink. + """ + missing: list[Path] = [] + current = directory + while not current.exists(): + missing.append(current) + parent = current.parent + if parent == current: # reached the filesystem root + break + current = parent + # Deepest pre-existing ancestor: refuse if it's a symlink or not a directory. + if current.is_symlink() or not current.is_dir(): + return False + # Create the missing tail top-down, one component at a time, re-checking each + # so a concurrently-planted symlink can't slip in under a TOCTOU race. + for component in reversed(missing): + try: + component.mkdir() + except FileExistsError: + pass # created concurrently; validated immediately below + if component.is_symlink() or not component.is_dir(): + return False + return True + + +def _write_update_check_cache(path: Path, latest: str | None) -> None: + try: + # Refuse to follow symlinks anywhere on the way to the cache file: an + # attacker (or misconfigured XDG_CACHE_HOME) could otherwise have us + # overwrite an arbitrary file the current user can write. + if not _create_cache_dir_no_symlinks(path.parent): + return + if path.is_symlink(): + return + + payload = json.dumps({"checked_at": time.time(), "latest": latest}).encode("utf-8") + flags = os.O_WRONLY | os.O_CREAT | os.O_TRUNC | getattr(os, "O_NOFOLLOW", 0) + fd = os.open(str(path), flags, 0o600) + try: + os.write(fd, payload) + finally: + os.close(fd) + 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 upgrade guidance 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 failed. + """ + 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: + 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. + _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@{latest_tag}[/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 new file mode 100644 index 0000000000..6ba42abc20 --- /dev/null +++ b/tests/test_update_check.py @@ -0,0 +1,380 @@ +"""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 +failing any command when offline or rate-limited. +""" + +import json +import os +import time +from io import StringIO +from typing import Any, cast + +from specify_cli._version import ( + _check_for_updates, + _read_update_check_cache, + _write_update_check_cache, +) + + +class _TtyStdout(StringIO): + def isatty(self) -> bool: + return True + + +class _CaptureConsole: + def __init__(self) -> None: + self._output = StringIO() + + def print(self, *objects: object, sep: str = " ", end: str = "\n", **_: object) -> None: + self._output.write(sep.join(str(obj) for obj in objects)) + self._output.write(end) + + def getvalue(self) -> str: + return self._output.getvalue() + + +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_invalid_latest_cache_type_returns_none(self, tmp_path): + cache_file = tmp_path / "version_check.json" + cache_file.write_text(json.dumps({"checked_at": time.time(), "latest": ["v0.7.0"]})) + assert _read_update_check_cache(cache_file) is None + + def test_non_dict_cache_returns_none(self, tmp_path): + cache_file = tmp_path / "version_check.json" + cache_file.write_text(json.dumps([{"checked_at": time.time(), "latest": "v0.7.0"}])) + assert _read_update_check_cache(cache_file) is None + + def test_non_finite_checked_at_returns_none(self, tmp_path): + cache_file = tmp_path / "version_check.json" + for checked_at in ("nan", "inf", "-inf"): + cache_file.write_text(json.dumps({"checked_at": checked_at, "latest": "v0.7.0"})) + assert _read_update_check_cache(cache_file) is None + + def test_future_checked_at_returns_none(self, tmp_path): + cache_file = tmp_path / "version_check.json" + cache_file.write_text(json.dumps({"checked_at": time.time() + 60, "latest": "v0.7.0"})) + 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() + + 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() + + def test_write_refuses_symlinked_cache_file(self, tmp_path): + # If the cache file itself is a symlink, refuse to follow it: an + # attacker could otherwise have us overwrite an arbitrary file the + # current user can write to. + target = tmp_path / "victim.txt" + target.write_text("untouched") + cache_file = tmp_path / "version_check.json" + os.symlink(target, cache_file) + + _write_update_check_cache(cache_file, "v9.9.9") + + assert target.read_text() == "untouched" + + def test_write_refuses_symlinked_parent_dir(self, tmp_path): + # Same idea, but the cache *directory* is the symlink. We must not + # write through it into the linked-to location. + real_target_dir = tmp_path / "victim_dir" + real_target_dir.mkdir() + victim = real_target_dir / "version_check.json" + + link_parent = tmp_path / "cache_dir" + os.symlink(real_target_dir, link_parent) + cache_file = link_parent / "version_check.json" + + _write_update_check_cache(cache_file, "v9.9.9") + + assert not victim.exists() + + def test_write_refuses_symlinked_ancestor_dir(self, tmp_path): + # An ancestor *above* the immediate parent is the symlink, and the + # cache directory itself does not exist yet. mkdir(parents=True) would + # happily create the tail through the symlinked ancestor; the + # component-by-component walk must refuse instead. + real_target_dir = tmp_path / "victim_dir" + real_target_dir.mkdir() + + link_ancestor = tmp_path / "cache_root" + os.symlink(real_target_dir, link_ancestor) + cache_file = link_ancestor / "specify-cli" / "version_check.json" + + _write_update_check_cache(cache_file, "v9.9.9") + + assert not (real_target_dir / "specify-cli").exists() + assert not cache_file.exists() + + + +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._version._should_skip_update_check", lambda: False) + import specify_cli._version + captured = _CaptureConsole() + monkeypatch.setattr(specify_cli._version, "console", captured) + _check_for_updates() + return captured.getvalue() + + def test_prints_warning_when_newer_release_available(self, monkeypatch, tmp_path): + monkeypatch.setattr("specify_cli._version._get_installed_version", lambda: "0.6.2") + monkeypatch.setattr( + "specify_cli._version._update_check_cache_path", lambda: tmp_path / "vc.json" + ) + monkeypatch.setattr("specify_cli._version._fetch_latest_release_tag", lambda: ("v0.7.0", None)) + + 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_upgrade_command_uses_exact_release_tag(self, monkeypatch, tmp_path): + monkeypatch.setattr("specify_cli._version._get_installed_version", lambda: "0.6.2") + monkeypatch.setattr( + "specify_cli._version._update_check_cache_path", lambda: tmp_path / "vc.json" + ) + monkeypatch.setattr("specify_cli._version._fetch_latest_release_tag", lambda: ("0.7.0", None)) + + out = self._run_and_capture(monkeypatch) + + assert "git+https://github.com/github/spec-kit.git@0.7.0" in out + assert "git+https://github.com/github/spec-kit.git@v0.7.0" not in out + + def test_no_output_when_up_to_date(self, monkeypatch, tmp_path): + monkeypatch.setattr("specify_cli._version._get_installed_version", lambda: "0.7.0") + monkeypatch.setattr( + "specify_cli._version._update_check_cache_path", lambda: tmp_path / "vc.json" + ) + monkeypatch.setattr("specify_cli._version._fetch_latest_release_tag", lambda: ("v0.7.0", None)) + + 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._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() -> tuple[str | None, str | None]: + call_counter["n"] += 1 + return (None, None) + + monkeypatch.setattr("specify_cli._version._fetch_latest_release_tag", _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._version._get_installed_version", lambda: "0.6.2") + monkeypatch.setattr( + "specify_cli._version._update_check_cache_path", lambda: tmp_path / "vc.json" + ) + monkeypatch.setattr("specify_cli._version._fetch_latest_release_tag", lambda: (None, "offline or timeout")) + + out = self._run_and_capture(monkeypatch) + + assert out == "" + + def test_network_failure_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) + monkeypatch.setattr("specify_cli._version._fetch_latest_release_tag", lambda: (None, "offline or timeout")) + + 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_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})) + + 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() -> tuple[str | None, str | None]: + call_counter["n"] += 1 + return (None, None) + + monkeypatch.setattr("specify_cli._version._fetch_latest_release_tag", _should_not_be_called) + + out = self._run_and_capture(monkeypatch) + + assert call_counter["n"] == 0 + assert out == "" + + def test_invalid_latest_cache_type_is_treated_as_miss(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._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 _fetch() -> tuple[str | None, str | None]: + call_counter["n"] += 1 + return ("v0.7.0", None) + + monkeypatch.setattr("specify_cli._version._fetch_latest_release_tag", _fetch) + + out = self._run_and_capture(monkeypatch) + + assert call_counter["n"] == 1 + assert "v0.7.0" in 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) + + fetched = {"called": False} + + def _fetch(): + fetched["called"] = True + return ("v99.0.0", None) + + 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._version._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", _TtyStdout()) + + fetched = {"called": False} + + def _fetch(): + fetched["called"] = True + return ("v99.0.0", None) + + 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._version._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. + + 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", _TtyStdout()) + + fetched = {"called": False} + + def _fetch(): + fetched["called"] = True + return ("v99.0.0", None) + + 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._version._update_check_cache_path", lambda: tmp_path / "vc.json" + ) + + _check_for_updates() + + assert fetched["called"] is False + + def test_callback_skips_startup_update_check_for_self_subcommands(self, monkeypatch): + """`specify self check` does its own fetch; startup check should not run too.""" + import specify_cli + + calls = {"n": 0} + + def _should_not_be_called() -> None: + calls["n"] += 1 + + monkeypatch.setattr("sys.argv", ["specify", "self", "check"]) + monkeypatch.setattr(specify_cli, "_check_for_updates", _should_not_be_called) + ctx = type("ContextStub", (), {"invoked_subcommand": "self"})() + + specify_cli.callback(cast(Any, ctx)) + + assert calls["n"] == 0