feat(cli): warn when a newer spec-kit release is available (#1320)#2212
feat(cli): warn when a newer spec-kit release is available (#1320)#2212ATelbay wants to merge 1 commit intogithub:mainfrom
Conversation
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 github#1320.
There was a problem hiding this comment.
Pull request overview
Adds a best-effort “new version available” notice to the specify CLI startup flow to help users discover they need to upgrade when running an outdated specify-cli release.
Changes:
- Implement a cached (24h) GitHub release check in
specify_cli.__init__and print an upgrade hint when a newer tag is available. - Add a new
tests/test_update_check.pysuite covering version parsing, cache behavior, network/JSON failure swallowing, and end-to-end output behavior. - Document update notifications in
docs/installation.mdand add an Unreleased changelog entry.
Show a summary per file
| File | Description |
|---|---|
src/specify_cli/__init__.py |
Adds update-check helpers and invokes the check from the Typer callback. |
tests/test_update_check.py |
New tests validating parsing/caching/network handling and printed warning behavior. |
docs/installation.md |
Documents update-check behavior and opt-out/skip conditions. |
CHANGELOG.md |
Adds an Unreleased entry describing the new update warning behavior. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
src/specify_cli/init.py:1648
_write_update_check_cacheusespath.write_text(...)without an explicit encoding. For consistency with other file writes/reads in this module and to avoid platform default-encoding issues, specifyencoding="utf-8"here as well.
path.parent.mkdir(parents=True, exist_ok=True)
path.write_text(json.dumps({"checked_at": time.time(), "latest": latest}))
except Exception:
- Files reviewed: 4/4 changed files
- Comments generated: 5
| 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: |
There was a problem hiding this comment.
_parse_version_tuple is annotated as taking version: str, but the implementation handles falsey/non-str inputs (e.g., tests pass None). To keep annotations accurate (and avoid future type-checking surprises), either update the signature to str | None and guard with isinstance(version, str), or change callers/tests to never pass None.
| 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: | |
| 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 isinstance(version, str) or not version: |
| return None | ||
| data = json.loads(path.read_text()) | ||
| checked_at = float(data.get("checked_at", 0)) |
There was a problem hiding this comment.
_read_update_check_cache uses path.read_text() without an explicit encoding, while other JSON reads in this module use encoding="utf-8" (e.g., _read_integration_json). For consistency and to avoid platform default-encoding surprises, specify UTF-8 explicitly here.
This issue also appears on line 1646 of the same file.
| - 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. |
There was a problem hiding this comment.
This doc claims the update check “never blocks the command you ran,” but the current implementation performs a synchronous network request during the CLI callback on cache misses, which can add latency before the command executes. Either adjust the implementation to be non-blocking, or tweak this wording to match the behavior (e.g., “never aborts/fails the command” rather than “never blocks”).
| 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 aborts or fails the command you ran. |
| # 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() |
There was a problem hiding this comment.
callback() only triggers _check_for_updates() when ctx.invoked_subcommand is neither None nor version. That means running specify with no subcommand (the banner/launch case) will never perform the update check, which conflicts with the docs text (“On each launch…”) and the PR summary. Consider either (a) including the None case (while still skipping --help/-h), or (b) adjusting the docs/PR wording to match the actual behavior (only runs for subcommands).
| # ===== 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. |
There was a problem hiding this comment.
The comment says the update check “Never blocks the user”, but _check_for_updates() runs synchronously in the CLI callback and can delay startup by up to _UPDATE_CHECK_TIMEOUT_SECONDS (and potentially longer in practice due to DNS/socket behavior). If the intent is truly non-blocking, this should run asynchronously / in a background thread, or be deferred until after the command completes; otherwise, please reword the comment/docs to clarify that it never fails the command, but may add a small delay on cache misses.
| # callback. Never blocks the user — every failure path swallows the exception. | |
| # callback. This is best-effort: every failure path swallows the exception so it | |
| # never fails the command, but cache misses may add a small startup delay while | |
| # performing the network check. |
mnriem
left a comment
mnriem
left a comment
There was a problem hiding this comment.
Please address Copilot feedback and be aware that this MUST be an opt-in and NOT an opt-out as air-gapped / network-constrained environments will not have access to GitHub perse
Summary
Addresses #1320 — the CLI now prints a one-line upgrade hint on launch when a newer release is available. Suppressed for non-interactive shells,
CI=1, orSPECIFY_SKIP_UPDATE_CHECK=1. Cached for 24h in the platform user-cache dir. Every network / parse failure is swallowed — the user's command is never blocked.Motivation
Observed in the wild: users running older CLIs (for example v0.3.0 still installed from PyPI, or v0.4.2 as in #2185) hit
No matching release asset found for claudewhen they tryspecify init --ai claude. The legacy asset-download path was removed in the Stage 6 migration (#2063) and the release workflow stopped producing those assets starting v0.4.5, so old clients have no recovery path and no signal that the fix is to upgrade the CLI. A launch-time update warning turns this silent failure into actionable guidance.This PR implements the spec in #1320:
Changes
src/specify_cli/__init__.pyget_speckit_version()):_parse_version_tuple()— tolerant parser (drops PEP 440 pre/post/dev/local segments)_update_check_cache_path()/_read_update_check_cache()/_write_update_check_cache()— JSON cache inplatformdirs.user_cache_dir(\"specify-cli\")_fetch_latest_version()—urllib.requestGET with 2s timeout; never raises_should_skip_update_check()— env-var / CI / non-TTY guard_check_for_updates()— top-level wrapper; all errors swallowedcallback()invokes_check_for_updates()for any non-versionsubcommand (versionalready surfaces the installed version, so we skip to avoid double-printing).platformdirsis already a declared dependency.tests/test_update_check.py25 new tests covering:
urlopensuccess / network error / malformed JSON / missing tagSPECIFY_SKIP_UPDATE_CHECK=1short-circuitsCHANGELOG.mdEntry under new
## [Unreleased]section.docs/installation.mdShort "Update Notifications" subsection listing the opt-out conditions.
Test plan
uv run pytest tests/test_update_check.py— 25 passeduv run pytest— full suite: 1299 passed, 20 skipped, 1 failed. The single failure (tests/integrations/test_cli.py::TestForceExistingDirectory::test_without_force_errors_on_existing_dir) also fails on unmodifiedmain— it's a pre-existing brittleness where Rich wrapsalready existsacross panel lines. Not caused by this PR.SPECIFY_SKIP_UPDATE_CHECK=1 uv run specify --help— banner renders, no warning, no crashManual warning output
Notes for reviewers
Opened as Draft per the CONTRIBUTING guidance on larger changes. The feature scope matches the maintainer-spec'd issue #1320 verbatim; happy to split, pare back, or rework before un-drafting if there's a preferred approach. Also related (but out of scope here): #2185 is a direct downstream symptom of the same class of problem this PR mitigates.