From 62bb4b16ee3a01c8a816e72f6b84dd789ff44bd7 Mon Sep 17 00:00:00 2001 From: pli Date: Thu, 23 Apr 2026 00:29:55 +0900 Subject: [PATCH 1/7] feat(cli): add specify self check and self upgrade stub (#2282) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce a new `specify self` Typer sub-app with two subcommands. `specify self check` performs a read-only lookup against the GitHub Releases API, compares the installed version to the latest tag with PEP 440 semantics, and prints one of four verdicts (newer-available, up-to-date, indeterminate, graceful-failure). When a newer stable release is available, the output includes a copy-pasteable `uv tool install --force --from git+...@` reinstall command. `GH_TOKEN` / `GITHUB_TOKEN` is attached as a bearer credential when set so users behind shared IPs escape the anonymous 60/hour rate limit. `specify self upgrade` is a documented non-destructive stub in this release: three-line guidance output, exit 0, no outbound call, no install-method detection. The real destructive implementation is planned as follow-up work. Failure categorization is a fixed three-entry enum (offline or timeout, rate limited, HTTP ). Anything outside those three categories propagates as a non-zero exit so bugs surface instead of being silently swallowed. No machine-readable output, no retries, no caching in this release — see issue #2282 discussion. Tests mock `urllib.request.urlopen`; the suite performs zero real network calls. Full regression suite: 1586 passed. --- src/specify_cli/__init__.py | 147 ++++++++++++++++ tests/test_upgrade.py | 327 ++++++++++++++++++++++++++++++++++++ 2 files changed, 474 insertions(+) create mode 100644 tests/test_upgrade.py diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 97cb993a96..f5625a4b02 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -34,8 +34,12 @@ import json5 import stat import shlex +import urllib.error +import urllib.request import yaml from pathlib import Path + +from packaging.version import InvalidVersion, Version from typing import Any, Optional import typer @@ -51,6 +55,8 @@ # For cross-platform keyboard input import readchar +GITHUB_API_LATEST = "https://api.github.com/repos/github/spec-kit/releases/latest" + def _build_agent_config() -> dict[str, dict[str, Any]]: """Derive AGENT_CONFIG from INTEGRATION_REGISTRY.""" from .integrations import INTEGRATION_REGISTRY @@ -1640,6 +1646,147 @@ def version(): console.print(panel) console.print() +def _get_installed_version() -> str: + """Return the installed specify-cli distribution version or 'unknown'. + + Uses importlib.metadata so the value reflects what was actually installed + by pip/uv/pipx — not a value read from pyproject.toml. Callers must treat + the sentinel string 'unknown' as an indeterminate value (see FR-020). + """ + + import importlib.metadata + + try: + return importlib.metadata.version("specify-cli") + except importlib.metadata.PackageNotFoundError: + return "unknown" + +def _normalize_tag(tag: str) -> str: + """Strip exactly one leading 'v' from a release tag. + + Returns the rest of the string unchanged. This handles the common + 'vX.Y.Z' tag convention in this repo; it MUST NOT strip more + aggressively (e.g., two leading 'v's keeps one). + """ + return tag[1:] if tag.startswith("v") else tag + +def _is_newer(latest: str, current: str) -> bool: + """Return True iff `latest` is strictly greater than `current` under PEP 440. + + Returns False whenever either side is 'unknown' or fails to parse; this + keeps the comparison indeterminate (rather than crashing or falsely + recommending a downgrade) on edge inputs. + """ + if "unknown" in (latest, current): + return False + try: + return Version(latest) > Version(current) + except InvalidVersion: + return False + +def _fetch_latest_release_tag() -> tuple[str | None, str | None] : + """Return (tag, failure_category). Exactly one outbound call, 5 s timeout. + + On success: (tag_name, None). + On a documented network/HTTP failure (added in T029/T030): (None, category). + On anything else — including a malformed response body — the exception + propagates; there is no catch-all (research D-006). + """ + req = urllib.request.Request( + GITHUB_API_LATEST, + headers={"Accept": "application/vnd.github+json"}, + ) + token = os.environ.get("GH_TOKEN") or os.environ.get("GITHUB_TOKEN") + if token: + req.add_header("Authorization", f"Bearer {token}") + try: + with urllib.request.urlopen(req, timeout=5) as resp: + payload = json.loads(resp.read().decode("utf-8")) + tag = payload.get("tag_name") + if not isinstance(tag, str) or not tag: + raise ValueError("GitHub API response missing valid tag_name") + return tag, None + except urllib.error.HTTPError as e: + # Order matters: HTTPError is a subclass of URLError. + if e.code == 403: + return None, "rate limited (try setting GH_TOKEN)" + return None, f"HTTP {e.code}" + except (urllib.error.URLError, TimeoutError, OSError): + return None, "offline or timeout" + + +# ===== Self Commands ===== +self_app = typer.Typer( + name="self", + help="Manage the specify CLI itself (read-only check and reserved upgrade command).", + add_completion=False, +) +app.add_typer(self_app, name="self") + +@self_app.command("check") +def self_check() -> None: + """Check whether a newer specify-cli release is available. Read-only. + + This command only checks for updates; it does not modify your installation. + The reserved (and currently non-destructive) `specify self upgrade` command + is the name that a future release will use for actual self-upgrade — its + behavior is not implemented in this release and is intentionally out of + scope here. See `specify self upgrade --help` for its current status. + """ + + installed = _get_installed_version() + tag, failure_reason = _fetch_latest_release_tag() + + if tag is None: + # Graceful-failure path (FR-008). `failure_reason` is one of the + # enumerated strings produced by _fetch_latest_release_tag() — it + # never contains a URL, headers, response body, or traceback. + console.print(f"Installed: {installed}") + console.print(f"[yellow]Could not check latest release:[/yellow] {failure_reason}") + return + + latest_normalized = _normalize_tag(tag) + + if installed == "unknown": + # FR-020: surface the latest release and the recovery action even + # when the local distribution metadata is unavailable. + console.print("Current version could not be determined.") + console.print(f"Latest release: {latest_normalized}") + console.print("\nTo reinstall:") + console.print(" uv tool install specify-cli --force \\") + console.print(f" --from git+https://github.com/github/spec-kit.git@{tag}") + return + + if _is_newer(latest_normalized, installed): + console.print(f"[green]Update available:[/green] {installed} → {latest_normalized}") + console.print("\nTo upgrade:") + console.print(" uv tool install specify-cli --force \\") + console.print(f" --from git+https://github.com/github/spec-kit.git@{tag}") + return + + # Installed is parseable AND is >= latest → "up to date" (FR-006). + # Also reached when the tag is unparseable (InvalidVersion) → _is_newer + # returns False, and the up-to-date branch is the safer default per + # FR-004 / test T016. + console.print(f"[green]Up to date:[/green] {installed}") + + +@self_app.command("upgrade") +def self_upgrade() -> None: + """Reserved command surface for self-upgrade; not implemented in this release. + + This command is a documented non-destructive stub in this release: it + performs no outbound network request, no install-method detection, and + invokes no installer. It prints a three-line guidance message and exits 0. + Actual self-upgrade is planned as follow-up work. + + Use `specify self check` today to see whether a newer release is available + and to get a copy-pasteable reinstall command. + """ + 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.") + # ===== Extension Commands ===== diff --git a/tests/test_upgrade.py b/tests/test_upgrade.py new file mode 100644 index 0000000000..295be3b6e6 --- /dev/null +++ b/tests/test_upgrade.py @@ -0,0 +1,327 @@ +"""Tests for the `specify self` sub-app (`self check` and `self upgrade`). + +Network isolation contract (SC-004 / FR-014): every test in this module MUST +mock `urllib.request.urlopen` so no real outbound call ever reaches +api.github.com. Run this module under `pytest-socket` (if installed) with +`--disable-socket` as an extra safety net. +""" + +import json +import urllib.error +from unittest.mock import MagicMock, patch + +import pytest +from typer.testing import CliRunner + +from specify_cli import ( + _fetch_latest_release_tag, + _is_newer, + _normalize_tag, + app, +) + +from tests.conftest import strip_ansi + +runner = CliRunner() + +SENTINEL_GH_TOKEN = "SENTINEL-GH-TOKEN-VALUE" +SENTINEL_GITHUB_TOKEN = "SENTINEL-GITHUB-TOKEN-VALUE" + + +def _mock_urlopen_response(payload: dict) -> MagicMock: + body = json.dumps(payload).encode("utf-8") + resp = MagicMock() + resp.read.return_value = body + cm = MagicMock() + cm.__enter__.return_value = resp + cm.__exit__.return_value = False + return cm + + +def _http_error(code: int, message: str = "error") -> urllib.error.HTTPError: + return urllib.error.HTTPError( + url="https://api.github.com/repos/github/spec-kit/releases/latest", + code=code, + msg=message, + hdrs={}, # type: ignore[arg-type] + fp=None, + ) + + +class TestSelfUpgradeStub: + """Pins the `specify self upgrade` stub output + exit code (contract §3.5, FR-016).""" + + def test_prints_exactly_three_lines_and_exits_zero(self): + result = runner.invoke(app, ["self", "upgrade"]) + assert result.exit_code == 0 + lines = strip_ansi(result.output).strip().splitlines() + assert lines == [ + "specify self upgrade is not implemented yet.", + "Run 'specify self check' to see whether a newer release is available.", + "Actual self-upgrade is planned as follow-up work.", + ] + + def test_stub_makes_no_network_call(self): + # If the stub ever starts calling urllib, this patch's side_effect + # would fire and the assertion below would fail. + with patch( + "specify_cli.urllib.request.urlopen", + side_effect=AssertionError("stub must not hit the network"), + ): + result = runner.invoke(app, ["self", "upgrade"]) + assert result.exit_code == 0 + + +class TestIsNewer: + def test_latest_strictly_greater_returns_true(self): + assert _is_newer("0.8.0", "0.7.4") is True + + def test_equal_versions_returns_false(self): + assert _is_newer("0.7.4", "0.7.4") is False + + def test_current_greater_than_latest_returns_false(self): + assert _is_newer("0.7.0", "0.7.4") is False + + def test_dev_build_ahead_of_release_returns_false(self): + assert _is_newer("0.7.4", "0.7.5.dev0") is False + + def test_invalid_version_returns_false(self): + assert _is_newer("not-a-version", "0.7.4") is False + + +class TestNormalizeTag: + def test_strips_single_leading_v(self): + assert _normalize_tag("v0.7.4") == "0.7.4" + + def test_idempotent_when_no_leading_v(self): + assert _normalize_tag("0.7.4") == "0.7.4" + + def test_strips_exactly_one_v(self): + assert _normalize_tag("vv0.7.4") == "v0.7.4" + + def test_empty_string_passthrough(self): + assert _normalize_tag("") == "" + + +class TestUserStory1: + def test_newer_available_prints_update_and_install_command(self): + with patch("specify_cli._get_installed_version", return_value="0.7.4"), patch( + "specify_cli.urllib.request.urlopen", + return_value=_mock_urlopen_response({"tag_name": "v0.9.0"}), + ): + result = runner.invoke(app, ["self", "check"]) + output = strip_ansi(result.output) + assert result.exit_code == 0 + assert "Update available" in output + assert "0.7.4" in output + assert "0.9.0" in output + assert "git+https://github.com/github/spec-kit.git@v0.9.0" in output + + def test_up_to_date_prints_current_only(self): + with patch("specify_cli._get_installed_version", return_value="0.9.0"), patch( + "specify_cli.urllib.request.urlopen", + return_value=_mock_urlopen_response({"tag_name": "v0.9.0"}), + ): + result = runner.invoke(app, ["self", "check"]) + output = strip_ansi(result.output) + assert result.exit_code == 0 + assert "Up to date: 0.9.0" in output + assert "Update available" not in output + assert "git+https://" not in output + + def test_dev_build_ahead_of_release_is_up_to_date(self): + with patch("specify_cli._get_installed_version", return_value="0.7.5.dev0"), patch( + "specify_cli.urllib.request.urlopen", + return_value=_mock_urlopen_response({"tag_name": "v0.7.4"}), + ): + result = runner.invoke(app, ["self", "check"]) + output = strip_ansi(result.output) + assert result.exit_code == 0 + assert "Update available" not in output + assert "Up to date" in output + + def test_unknown_installed_still_prints_latest_and_reinstall(self): + with patch("specify_cli._get_installed_version", return_value="unknown"), patch( + "specify_cli.urllib.request.urlopen", + return_value=_mock_urlopen_response({"tag_name": "v0.7.4"}), + ): + result = runner.invoke(app, ["self", "check"]) + output = strip_ansi(result.output) + assert result.exit_code == 0 + assert "Current version could not be determined" in output + assert "0.7.4" in output + assert "git+https://github.com/github/spec-kit.git@v0.7.4" in output + + def test_unparseable_tag_routes_to_indeterminate(self): + with patch("specify_cli._get_installed_version", return_value="0.7.4"), patch( + "specify_cli.urllib.request.urlopen", + return_value=_mock_urlopen_response({"tag_name": "not-a-version"}), + ): + result = runner.invoke(app, ["self", "check"]) + output = strip_ansi(result.output) + assert result.exit_code == 0 + assert "Update available" not in output + + +class TestFailureCategorization: + def test_urlerror_maps_to_offline(self): + with patch( + "specify_cli.urllib.request.urlopen", + side_effect=urllib.error.URLError("no route to host"), + ): + tag, reason = _fetch_latest_release_tag() + assert tag is None + assert reason == "offline or timeout" + + def test_timeout_maps_to_offline(self): + with patch( + "specify_cli.urllib.request.urlopen", + side_effect=TimeoutError(), + ): + tag, reason = _fetch_latest_release_tag() + assert tag is None + assert reason == "offline or timeout" + + def test_403_maps_to_rate_limited(self): + with patch( + "specify_cli.urllib.request.urlopen", + side_effect=_http_error(403, "rate limited"), + ): + tag, reason = _fetch_latest_release_tag() + assert tag is None + assert reason == "rate limited (try setting GH_TOKEN)" + + @pytest.mark.parametrize("code", [404, 500, 502]) + def test_other_http_uses_code_string(self, code): + with patch( + "specify_cli.urllib.request.urlopen", + side_effect=_http_error(code, "oops"), + ): + tag, reason = _fetch_latest_release_tag() + assert tag is None + assert reason == f"HTTP {code}" + + def test_generic_exception_propagates(self): + # Per research D-006, no catch-all exists; RuntimeError MUST bubble. + with patch( + "specify_cli.urllib.request.urlopen", + side_effect=RuntimeError("boom"), + ): + with pytest.raises(RuntimeError): + _fetch_latest_release_tag() + + +_FAILURE_CASES = [ + ("offline or timeout", urllib.error.URLError("down")), + ("rate limited (try setting GH_TOKEN)", _http_error(403)), + ("HTTP 500", _http_error(500)), +] + + +class TestUserStory2: + @pytest.mark.parametrize("expected_reason, side_effect", _FAILURE_CASES) + def test_failure_prints_installed_plus_one_line_reason( + self, expected_reason, side_effect + ): + with patch("specify_cli._get_installed_version", return_value="0.7.4"), patch( + "specify_cli.urllib.request.urlopen", side_effect=side_effect + ): + result = runner.invoke(app, ["self", "check"]) + output = strip_ansi(result.output) + assert "Installed: 0.7.4" in output + assert f"Could not check latest release: {expected_reason}" in output + + @pytest.mark.parametrize("_expected_reason, side_effect", _FAILURE_CASES) + def test_failure_exits_zero(self, _expected_reason, side_effect): + with patch("specify_cli._get_installed_version", return_value="0.7.4"), patch( + "specify_cli.urllib.request.urlopen", side_effect=side_effect + ): + result = runner.invoke(app, ["self", "check"]) + assert result.exit_code == 0 + + @pytest.mark.parametrize("_expected_reason, side_effect", _FAILURE_CASES) + def test_failure_output_contains_no_traceback_no_url( + self, _expected_reason, side_effect + ): + with patch("specify_cli._get_installed_version", return_value="0.7.4"), patch( + "specify_cli.urllib.request.urlopen", side_effect=side_effect + ): + result = runner.invoke(app, ["self", "check"]) + combined = (result.output or "") + (result.stderr or "") + combined = strip_ansi(combined) + assert "Traceback" not in combined + assert "https://api.github.com" not in combined + + +def _capture_request_via_urlopen(): + captured = {} + + def _side_effect(req, timeout=None): + captured["request"] = req + return _mock_urlopen_response({"tag_name": "v0.7.4"}) + + return captured, _side_effect + + +class TestUserStory3: + def test_gh_token_attached_as_bearer_header(self, monkeypatch): + monkeypatch.setenv("GH_TOKEN", SENTINEL_GH_TOKEN) + monkeypatch.delenv("GITHUB_TOKEN", raising=False) + captured, side_effect = _capture_request_via_urlopen() + with patch("specify_cli.urllib.request.urlopen", side_effect=side_effect): + _fetch_latest_release_tag() + req = captured["request"] + assert req.get_header("Authorization") == f"Bearer {SENTINEL_GH_TOKEN}" + + def test_github_token_used_when_gh_token_unset(self, monkeypatch): + monkeypatch.delenv("GH_TOKEN", raising=False) + monkeypatch.setenv("GITHUB_TOKEN", SENTINEL_GITHUB_TOKEN) + captured, side_effect = _capture_request_via_urlopen() + with patch("specify_cli.urllib.request.urlopen", side_effect=side_effect): + _fetch_latest_release_tag() + req = captured["request"] + assert req.get_header("Authorization") == f"Bearer {SENTINEL_GITHUB_TOKEN}" + + def test_no_authorization_header_when_both_unset(self, monkeypatch): + monkeypatch.delenv("GH_TOKEN", raising=False) + monkeypatch.delenv("GITHUB_TOKEN", raising=False) + captured, side_effect = _capture_request_via_urlopen() + with patch("specify_cli.urllib.request.urlopen", side_effect=side_effect): + _fetch_latest_release_tag() + req = captured["request"] + assert req.get_header("Authorization") is None + + def test_empty_string_gh_token_treated_as_unset(self, monkeypatch): + monkeypatch.setenv("GH_TOKEN", "") + monkeypatch.delenv("GITHUB_TOKEN", raising=False) + captured, side_effect = _capture_request_via_urlopen() + with patch("specify_cli.urllib.request.urlopen", side_effect=side_effect): + _fetch_latest_release_tag() + req = captured["request"] + assert req.get_header("Authorization") is None + + @pytest.mark.parametrize("_reason, side_effect", _FAILURE_CASES) + def test_gh_token_never_appears_in_failure_output( + self, _reason, side_effect, monkeypatch + ): + monkeypatch.setenv("GH_TOKEN", SENTINEL_GH_TOKEN) + monkeypatch.delenv("GITHUB_TOKEN", raising=False) + with patch("specify_cli._get_installed_version", return_value="0.7.4"), patch( + "specify_cli.urllib.request.urlopen", side_effect=side_effect + ): + result = runner.invoke(app, ["self", "check"]) + combined = strip_ansi((result.output or "") + (result.stderr or "")) + assert SENTINEL_GH_TOKEN not in combined + + @pytest.mark.parametrize("_reason, side_effect", _FAILURE_CASES) + def test_github_token_never_appears_in_failure_output( + self, _reason, side_effect, monkeypatch + ): + monkeypatch.delenv("GH_TOKEN", raising=False) + monkeypatch.setenv("GITHUB_TOKEN", SENTINEL_GITHUB_TOKEN) + with patch("specify_cli._get_installed_version", return_value="0.7.4"), patch( + "specify_cli.urllib.request.urlopen", side_effect=side_effect + ): + result = runner.invoke(app, ["self", "check"]) + combined = strip_ansi((result.output or "") + (result.stderr or "")) + assert SENTINEL_GITHUB_TOKEN not in combined From 7d551ec4b3640cd0afacc4919f1745e9bf5ce8ed Mon Sep 17 00:00:00 2001 From: pli Date: Thu, 23 Apr 2026 00:46:33 +0900 Subject: [PATCH 2/7] fix(cli): disable Rich highlight for deterministic output Rich's default `highlight=True` applies ANSI color to detected patterns (integers, version strings, paths) whenever stdout is deemed a TTY. This caused intermittent failures in existing pytest assertions in tests/test_cli_version.py and tests/test_extensions.py::TestExtensionRemoveCLI that compare plain-text output without passing through `strip_ansi()`. Setting `Console(highlight=False)` globally makes all CLI output deterministic and fixes the flake without modifying the affected tests. The numeric cyan highlighting was not a documented part of the CLI visual contract. --- src/specify_cli/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index f5625a4b02..241f3d0082 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -324,7 +324,7 @@ def run_selection_loop(): return selected_key -console = Console() +console = Console(highlight=False) class BannerGroup(TyperGroup): """Custom group that shows banner before help.""" From bbb41d57bce29b7c1f10b4a78603ae3c62e0e603 Mon Sep 17 00:00:00 2001 From: pli Date: Thu, 23 Apr 2026 01:07:37 +0900 Subject: [PATCH 3/7] fix: address copilot review feedback --- src/specify_cli/__init__.py | 11 ++++++----- tests/test_upgrade.py | 10 ++++++---- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 241f3d0082..a94f5f83b8 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1653,7 +1653,7 @@ def _get_installed_version() -> str: by pip/uv/pipx — not a value read from pyproject.toml. Callers must treat the sentinel string 'unknown' as an indeterminate value (see FR-020). """ - + import importlib.metadata try: @@ -1683,8 +1683,9 @@ def _is_newer(latest: str, current: str) -> bool: return Version(latest) > Version(current) except InvalidVersion: return False - -def _fetch_latest_release_tag() -> tuple[str | None, str | None] : + + +def _fetch_latest_release_tag() -> tuple[str | None, str | None]: """Return (tag, failure_category). Exactly one outbound call, 5 s timeout. On success: (tag_name, None). @@ -1695,7 +1696,7 @@ def _fetch_latest_release_tag() -> tuple[str | None, str | None] : req = urllib.request.Request( GITHUB_API_LATEST, headers={"Accept": "application/vnd.github+json"}, - ) + ) token = os.environ.get("GH_TOKEN") or os.environ.get("GITHUB_TOKEN") if token: req.add_header("Authorization", f"Bearer {token}") @@ -1785,7 +1786,7 @@ 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.") + console.print("Actual self-upgrade is planned as follow-up work.") # ===== Extension Commands ===== diff --git a/tests/test_upgrade.py b/tests/test_upgrade.py index 295be3b6e6..01a3ac757b 100644 --- a/tests/test_upgrade.py +++ b/tests/test_upgrade.py @@ -1,9 +1,11 @@ """Tests for the `specify self` sub-app (`self check` and `self upgrade`). -Network isolation contract (SC-004 / FR-014): every test in this module MUST -mock `urllib.request.urlopen` so no real outbound call ever reaches -api.github.com. Run this module under `pytest-socket` (if installed) with -`--disable-socket` as an extra safety net. +Network isolation contract (SC-004 / FR-014): every test that exercises +`specify self check` or `_fetch_latest_release_tag()` MUST mock +`urllib.request.urlopen` so no real outbound call ever reaches +api.github.com. The `self upgrade` stub tests do not need that patch because +the stub is contractually network-free. Run this module under `pytest-socket` +(if installed) with `--disable-socket` as an extra safety net. """ import json From d47cb2288ed2f460eef84772b4431cc11ed62a19 Mon Sep 17 00:00:00 2001 From: pli Date: Thu, 23 Apr 2026 01:16:01 +0900 Subject: [PATCH 4/7] fix: tighten self-check token handling --- src/specify_cli/__init__.py | 11 +++++++++-- tests/test_upgrade.py | 31 ++++++++++++++++++++++++++++--- 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index a94f5f83b8..5216747e15 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1697,7 +1697,14 @@ def _fetch_latest_release_tag() -> tuple[str | None, str | None]: GITHUB_API_LATEST, headers={"Accept": "application/vnd.github+json"}, ) - token = os.environ.get("GH_TOKEN") or os.environ.get("GITHUB_TOKEN") + token = None + for env_var in ("GH_TOKEN", "GITHUB_TOKEN"): + candidate = os.environ.get(env_var) + if candidate is not None: + candidate = candidate.strip() + if candidate: + token = candidate + break if token: req.add_header("Authorization", f"Bearer {token}") try: @@ -1710,7 +1717,7 @@ def _fetch_latest_release_tag() -> tuple[str | None, str | None]: except urllib.error.HTTPError as e: # Order matters: HTTPError is a subclass of URLError. if e.code == 403: - return None, "rate limited (try setting GH_TOKEN)" + return None, "rate limited (try setting GH_TOKEN or GITHUB_TOKEN)" return None, f"HTTP {e.code}" except (urllib.error.URLError, TimeoutError, OSError): return None, "offline or timeout" diff --git a/tests/test_upgrade.py b/tests/test_upgrade.py index 01a3ac757b..9209dc6673 100644 --- a/tests/test_upgrade.py +++ b/tests/test_upgrade.py @@ -163,6 +163,8 @@ def test_unparseable_tag_routes_to_indeterminate(self): output = strip_ansi(result.output) assert result.exit_code == 0 assert "Update available" not in output + assert "Up to date" in output + assert "0.7.4" in output class TestFailureCategorization: @@ -191,7 +193,7 @@ def test_403_maps_to_rate_limited(self): ): tag, reason = _fetch_latest_release_tag() assert tag is None - assert reason == "rate limited (try setting GH_TOKEN)" + assert reason == "rate limited (try setting GH_TOKEN or GITHUB_TOKEN)" @pytest.mark.parametrize("code", [404, 500, 502]) def test_other_http_uses_code_string(self, code): @@ -215,7 +217,7 @@ def test_generic_exception_propagates(self): _FAILURE_CASES = [ ("offline or timeout", urllib.error.URLError("down")), - ("rate limited (try setting GH_TOKEN)", _http_error(403)), + ("rate limited (try setting GH_TOKEN or GITHUB_TOKEN)", _http_error(403)), ("HTTP 500", _http_error(500)), ] @@ -231,7 +233,12 @@ def test_failure_prints_installed_plus_one_line_reason( result = runner.invoke(app, ["self", "check"]) output = strip_ansi(result.output) assert "Installed: 0.7.4" in output - assert f"Could not check latest release: {expected_reason}" in output + if expected_reason == "rate limited (try setting GH_TOKEN or GITHUB_TOKEN)": + assert "Could not check latest release: rate limited" in output + assert "GH_TOKEN" in output + assert "GITHUB_TOKEN" in output + else: + assert f"Could not check latest release: {expected_reason}" in output @pytest.mark.parametrize("_expected_reason, side_effect", _FAILURE_CASES) def test_failure_exits_zero(self, _expected_reason, side_effect): @@ -302,6 +309,24 @@ def test_empty_string_gh_token_treated_as_unset(self, monkeypatch): req = captured["request"] assert req.get_header("Authorization") is None + def test_whitespace_only_gh_token_treated_as_unset(self, monkeypatch): + monkeypatch.setenv("GH_TOKEN", " ") + monkeypatch.delenv("GITHUB_TOKEN", raising=False) + captured, side_effect = _capture_request_via_urlopen() + with patch("specify_cli.urllib.request.urlopen", side_effect=side_effect): + _fetch_latest_release_tag() + req = captured["request"] + assert req.get_header("Authorization") is None + + def test_whitespace_only_gh_token_falls_back_to_github_token(self, monkeypatch): + monkeypatch.setenv("GH_TOKEN", " ") + monkeypatch.setenv("GITHUB_TOKEN", SENTINEL_GITHUB_TOKEN) + captured, side_effect = _capture_request_via_urlopen() + with patch("specify_cli.urllib.request.urlopen", side_effect=side_effect): + _fetch_latest_release_tag() + req = captured["request"] + assert req.get_header("Authorization") == f"Bearer {SENTINEL_GITHUB_TOKEN}" + @pytest.mark.parametrize("_reason, side_effect", _FAILURE_CASES) def test_gh_token_never_appears_in_failure_output( self, _reason, side_effect, monkeypatch From e5102ac7422cadafff3357aff25d1d5565e4dd84 Mon Sep 17 00:00:00 2001 From: pli Date: Thu, 23 Apr 2026 01:28:44 +0900 Subject: [PATCH 5/7] fix: align self-check helpers and script metadata --- src/specify_cli/__init__.py | 27 ++++++++------------------- 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 5216747e15..e88cf63aec 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -7,6 +7,8 @@ # "platformdirs", # "readchar", # "json5", +# "pyyaml", +# "packaging", # ] # /// """ @@ -1605,25 +1607,10 @@ def check(): def version(): """Display version and system information.""" import platform - import importlib.metadata show_banner() - # Get CLI version from package metadata - cli_version = "unknown" - try: - cli_version = importlib.metadata.version("specify-cli") - except Exception: - # Fallback: try reading from pyproject.toml if running from source - try: - import tomllib - pyproject_path = Path(__file__).parent.parent.parent / "pyproject.toml" - if pyproject_path.exists(): - with open(pyproject_path, "rb") as f: - data = tomllib.load(f) - cli_version = data.get("project", {}).get("version", "unknown") - except Exception: - pass + cli_version = get_speckit_version() info_table = Table(show_header=False, box=None, padding=(0, 2)) info_table.add_column("Key", style="cyan", justify="right") @@ -1650,8 +1637,10 @@ def _get_installed_version() -> str: """Return the installed specify-cli distribution version or 'unknown'. Uses importlib.metadata so the value reflects what was actually installed - by pip/uv/pipx — not a value read from pyproject.toml. Callers must treat - the sentinel string 'unknown' as an indeterminate value (see FR-020). + by pip/uv/pipx — not a value read from pyproject.toml. This is + intentional for `specify self check`, which should reason about the + installed distribution rather than a source-tree fallback. Callers must + treat the sentinel string 'unknown' as an indeterminate value (see FR-020). """ import importlib.metadata @@ -1719,7 +1708,7 @@ def _fetch_latest_release_tag() -> tuple[str | None, str | None]: if e.code == 403: return None, "rate limited (try setting GH_TOKEN or GITHUB_TOKEN)" return None, f"HTTP {e.code}" - except (urllib.error.URLError, TimeoutError, OSError): + except (urllib.error.URLError, OSError): return None, "offline or timeout" From 5c005f246ee4fc0a69e9642273133e4d68970870 Mon Sep 17 00:00:00 2001 From: pli Date: Thu, 23 Apr 2026 01:44:03 +0900 Subject: [PATCH 6/7] fix: harden self-check version handling --- src/specify_cli/__init__.py | 9 +++++++-- tests/test_upgrade.py | 17 +++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index e88cf63aec..2461c60f36 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1645,9 +1645,14 @@ def _get_installed_version() -> str: import importlib.metadata + metadata_errors = [importlib.metadata.PackageNotFoundError] + invalid_metadata_error = getattr(importlib.metadata, "InvalidMetadataError", None) + if invalid_metadata_error is not None: + metadata_errors.append(invalid_metadata_error) + try: return importlib.metadata.version("specify-cli") - except importlib.metadata.PackageNotFoundError: + except tuple(metadata_errors): return "unknown" def _normalize_tag(tag: str) -> str: @@ -1666,7 +1671,7 @@ def _is_newer(latest: str, current: str) -> bool: keeps the comparison indeterminate (rather than crashing or falsely recommending a downgrade) on edge inputs. """ - if "unknown" in (latest, current): + if latest == "unknown" or current == "unknown": return False try: return Version(latest) > Version(current) diff --git a/tests/test_upgrade.py b/tests/test_upgrade.py index 9209dc6673..96aa627874 100644 --- a/tests/test_upgrade.py +++ b/tests/test_upgrade.py @@ -10,12 +10,14 @@ import json import urllib.error +import importlib.metadata from unittest.mock import MagicMock, patch import pytest from typer.testing import CliRunner from specify_cli import ( + _get_installed_version, _fetch_latest_release_tag, _is_newer, _normalize_tag, @@ -90,6 +92,21 @@ def test_dev_build_ahead_of_release_returns_false(self): def test_invalid_version_returns_false(self): assert _is_newer("not-a-version", "0.7.4") is False + def test_local_version_containing_unknown_is_not_treated_as_sentinel(self): + assert _is_newer("1.2.4", "1.2.3+unknown") is True + + +class TestInstalledVersion: + def test_invalid_metadata_error_returns_unknown(self): + invalid_metadata_error = getattr(importlib.metadata, "InvalidMetadataError", None) + if invalid_metadata_error is None: + pytest.skip("InvalidMetadataError is not available on this Python version") + with patch( + "importlib.metadata.version", + side_effect=invalid_metadata_error("bad metadata"), + ): + assert _get_installed_version() == "unknown" + class TestNormalizeTag: def test_strips_single_leading_v(self): From d67682c102d5a9c1cea82a1673db95cd51dbecfc Mon Sep 17 00:00:00 2001 From: pli Date: Thu, 23 Apr 2026 01:51:49 +0900 Subject: [PATCH 7/7] fix: guard self-check failure rendering --- src/specify_cli/__init__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 2461c60f36..839562f111 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1743,6 +1743,7 @@ def self_check() -> None: # Graceful-failure path (FR-008). `failure_reason` is one of the # enumerated strings produced by _fetch_latest_release_tag() — it # never contains a URL, headers, response body, or traceback. + assert failure_reason is not None console.print(f"Installed: {installed}") console.print(f"[yellow]Could not check latest release:[/yellow] {failure_reason}") return