From 46a0c277d6066d39144f2393c301fc717315484e Mon Sep 17 00:00:00 2001 From: pli Date: Thu, 7 May 2026 12:02:22 +0900 Subject: [PATCH 01/25] feat(cli): implement specify self upgrade Replace the v0.7.5 reserved stub from #2316 with an actually working self-upgrade command. Detects the install method (uv tool / pipx / uvx-ephemeral / source-checkout / unsupported) via a 3-tier ladder (path-prefix match -> editable-install marker -> PATH+registry reconciliation), runs the appropriate installer subprocess, and verifies the result by spawning a child `specify --version`. - Bare `specify self upgrade` executes immediately, matching the `pip install -U` / `uv tool upgrade` / `npm update` convention. - `--dry-run` prints the preview (method, current, target, argv) and exits 0 without launching any subprocess. - `--tag vX.Y.Z[suffix]` pins a specific release tag (validated against `^v\d+\.\d+\.\d+(?:[a-z0-9.+\-]*)?$`; rejects bare `latest`, branch names, and hash refs). - Token scrubbing: GH_TOKEN / GITHUB_TOKEN are never forwarded to the installer subprocess environment or surfaced in argv. - Exit codes: 0 (success / no-op-success), 1 (resolution failure), 2 (verification mismatch), 3 (installer missing), 124 (subprocess timeout when SPECIFY_UPGRADE_TIMEOUT_SECS is set), other (installer exit propagated verbatim). - uvx-ephemeral runs and source checkouts emit path-specific guidance and exit 0 instead of running an installer. Documentation: README and docs/upgrade.md now lead with the self-management commands as the recommended path; manual `uv tool install --force` / `pipx install --force` remain as fallback for older installs and explicit-control users. docs/installation.md adds a "Stay current" pointer. Closes the unresolved portion of #2282. --- README.md | 20 +- docs/installation.md | 2 + docs/upgrade.md | 58 ++- src/specify_cli/__init__.py | 834 ++++++++++++++++++++++++++++++++++- tests/test_self_upgrade.py | 858 ++++++++++++++++++++++++++++++++++++ tests/test_upgrade.py | 33 -- 6 files changed, 1750 insertions(+), 55 deletions(-) create mode 100644 tests/test_self_upgrade.py diff --git a/README.md b/README.md index 79940074e4..8b8bae8d83 100644 --- a/README.md +++ b/README.md @@ -92,7 +92,25 @@ specify init --here --integration copilot specify check ``` -To upgrade Specify, see the [Upgrade Guide](./docs/upgrade.md) for detailed instructions. Quick upgrade: +To upgrade Specify, the CLI ships with two self-management commands that handle the common case for you. See the [Upgrade Guide](./docs/upgrade.md) for detailed scenarios and customization options. + +```bash +# Check whether a newer release is available (read-only — does not modify anything) +specify self check + +# Preview what would run, without actually upgrading +specify self upgrade --dry-run + +# Upgrade in place to the latest stable release (auto-detects uv tool vs pipx install) +specify self upgrade + +# Or pin a specific release tag +specify self upgrade --tag vX.Y.Z +``` + +Bare `specify self upgrade` executes immediately, matching the `pip install -U` / `uv tool upgrade` / `npm update` convention. `uvx`-ephemeral runs and source checkouts are detected and produce path-specific guidance instead of running an installer. Set `SPECIFY_UPGRADE_TIMEOUT_SECS` to cap how long the installer subprocess may run (default: no timeout — interrupt with `Ctrl+C` if needed). + +If you prefer to drive the installer yourself, the manual equivalents still work: ```bash uv tool install specify-cli --force --from git+https://github.com/github/spec-kit.git@vX.Y.Z diff --git a/docs/installation.md b/docs/installation.md index 86ad35559f..ebbc3dc93c 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -91,6 +91,8 @@ specify version This helps verify you are running the official Spec Kit build from GitHub, not an unrelated package with the same name. +**Stay current:** Run `specify self check` periodically to learn whether a newer release is available — it is read-only and never modifies your installation. When you are ready to upgrade, follow the [Upgrade Guide](./upgrade.md). + After initialization, you should see the following commands available in your coding agent: - `/speckit.specify` - Create specifications diff --git a/docs/upgrade.md b/docs/upgrade.md index ec87662cbc..6ceb75ef80 100644 --- a/docs/upgrade.md +++ b/docs/upgrade.md @@ -8,10 +8,12 @@ | What to Upgrade | Command | When to Use | |----------------|---------|-------------| -| **CLI Tool Only** | `uv tool install specify-cli --force --from git+https://github.com/github/spec-kit.git@vX.Y.Z` | Get latest CLI features without touching project files | -| **CLI Tool Only (pipx)** | `pipx install --force git+https://github.com/github/spec-kit.git@vX.Y.Z` | Reinstall/upgrade a pipx-installed CLI to a specific release | -| **Project Files** | `specify init --here --force --integration ` | Update slash commands, templates, and scripts in your project | -| **Both** | Run CLI upgrade, then project update | Recommended for major version updates | +| **CLI Tool (recommended)** | `specify self upgrade` | Latest stable release, in place. Auto-detects whether you installed via `uv tool` or `pipx`. | +| **CLI Tool — pin a version** | `specify self upgrade --tag vX.Y.Z` | Upgrade to a specific release tag instead of the latest stable. | +| **CLI Tool — manual fallback** | `uv tool install specify-cli --force --from git+https://github.com/github/spec-kit.git@vX.Y.Z` | When `specify self upgrade` isn't available (older installs) or when you want explicit control. | +| **CLI Tool — manual fallback (pipx)** | `pipx install --force git+https://github.com/github/spec-kit.git@vX.Y.Z` | Same as above, for pipx installs. | +| **Project Files** | `specify init --here --force --integration ` | Update slash commands, templates, and scripts in your project. | +| **Both** | Run CLI upgrade, then project update | Recommended for major version updates. | --- @@ -19,6 +21,30 @@ The CLI tool (`specify`) is separate from your project files. Upgrade it to get the latest features and bug fixes. +### Recommended: `specify self upgrade` + +The CLI ships with two self-management commands that handle the common case automatically: + +```bash +# Check whether a newer release is available (read-only — does not modify anything) +specify self check + +# Preview what would run, without actually upgrading +specify self upgrade --dry-run + +# Upgrade in place to the latest stable release (auto-detects uv tool vs pipx install) +specify self upgrade + +# Or pin a specific release tag +specify self upgrade --tag vX.Y.Z +``` + +Bare `specify self upgrade` executes immediately, matching the `pip install -U` / `uv tool upgrade` / `npm update` convention. The CLI classifies your runtime into one of: `uv tool`, `pipx`, `uvx (ephemeral)`, source checkout, or unsupported. Only `uv tool` and `pipx` are upgraded automatically; the other paths print path-specific guidance and exit 0 without touching anything. + +Set `SPECIFY_UPGRADE_TIMEOUT_SECS` to cap how long the installer subprocess may run (default: no timeout — interrupt with `Ctrl+C` if needed). + +If `specify self upgrade` isn't available on your install (it shipped in a recent release) or you want explicit control over the installer command, use the manual equivalents below. + ### If you installed with `uv tool install` Upgrade to a specific release (check [Releases](https://github.com/github/spec-kit/releases) for the latest tag): @@ -46,10 +72,14 @@ pipx install --force git+https://github.com/github/spec-kit.git@vX.Y.Z ### Verify the upgrade ```bash +# Confirms the CLI is working and shows installed tools specify check + +# Confirms the installed version against the latest GitHub release +specify self check ``` -This shows installed tools and confirms the CLI is working. +`specify check` shows the surrounding tool environment; `specify self check` is read-only and tells you whether you're now on the latest release (`Up to date: X.Y.Z`) or if a newer one became available between releases. --- @@ -178,8 +208,8 @@ Restart your IDE to refresh the command list. ### Scenario 1: "I just want new slash commands" ```bash -# Upgrade CLI (if using persistent install) -uv tool install specify-cli --force --from git+https://github.com/github/spec-kit.git +# Upgrade CLI (auto-detects uv tool vs pipx install) +specify self upgrade # Update project files to get new commands specify init --here --force --integration copilot @@ -196,7 +226,7 @@ cp .specify/memory/constitution.md /tmp/constitution-backup.md cp -r .specify/templates /tmp/templates-backup # 2. Upgrade CLI -uv tool install specify-cli --force --from git+https://github.com/github/spec-kit.git +specify self upgrade # 3. Update project specify init --here --force --integration copilot @@ -380,7 +410,17 @@ Only Spec Kit infrastructure files: ### "CLI upgrade doesn't seem to work" -Verify the installation: +First, ask the CLI itself: + +```bash +# Read-only — prints "Up to date: X.Y.Z" or "Update available: X.Y.Z → Y.Z.W" +specify self check + +# Preview the install method, current version, and target tag the upgrade would use +specify self upgrade --dry-run +``` + +If `self check` shows the wrong version, verify the installation: ```bash # Check installed tools diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 325692900e..43b31a824b 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -27,6 +27,7 @@ """ import os +import re import subprocess import sys import zipfile @@ -37,8 +38,11 @@ import stat import shlex import urllib.error +import urllib.parse import urllib.request import yaml +from dataclasses import dataclass +from enum import Enum from pathlib import Path from packaging.version import InvalidVersion, Version @@ -1786,6 +1790,690 @@ def _fetch_latest_release_tag() -> tuple[str | None, str | None]: return None, "offline or timeout" +_INSTALLER_PATH_PREFIXES: dict[str, list[str]] = { + "uv-tool": [ + "~/.local/share/uv/tools/specify-cli/", + "%LOCALAPPDATA%\\uv\\tools\\specify-cli\\", + ], + "pipx": [ + "~/.local/pipx/venvs/specify-cli/", + "%LOCALAPPDATA%\\pipx\\venvs\\specify-cli\\", + ], + "uvx-ephemeral": [ + "~/.cache/uv/archive-v0/", + "%LOCALAPPDATA%\\uv\\cache\\archive-v0\\", + ], +} + +_RESOLUTION_FAILURE_CATEGORIES: frozenset[str] = frozenset( + { + "offline or timeout", + "rate limited (try setting GH_TOKEN or GITHUB_TOKEN)", + } +) + +_EXECUTION_FAILURE_CATEGORIES: frozenset[str] = frozenset( + { + "installer-missing", + "installer-failed", + "verification-mismatch", + } +) + + +class _InstallMethod(str, Enum): + """Classify the runtime environment for `specify self upgrade`. + + String-valued so that f-string rendering uses the hyphenated lowercase + token directly (no .value ceremony). Exactly one member returned per + invocation. + """ + + UV_TOOL = "uv-tool" + PIPX = "pipx" + UVX_EPHEMERAL = "uvx-ephemeral" + SOURCE_CHECKOUT = "source-checkout" + UNSUPPORTED = "unsupported" + + +@dataclass(frozen=True) +class _UpgradePlan: + """Frozen snapshot of the upgrade decision. + + Produced once per invocation by _build_upgrade_plan; consumed by both the + preview (--dry-run) path and the bare-apply path. installer_argv is None + iff method is UVX_EPHEMERAL / SOURCE_CHECKOUT / UNSUPPORTED. + """ + + method: _InstallMethod + current_version: str + target_tag: str + installer_argv: list[str] | None + preview_summary: str + pre_upgrade_snapshot: str + + +@dataclass(frozen=True) +class _UpgradeOutcome: + """Frozen snapshot of the installer run. + + reconciliation_needed is True iff installer_exit_code == 0 AND + verified_version != plan.target_tag. failure_category is one of + strings (installer-missing / installer-failed / + verification-mismatch) or None on success. + """ + + installer_exit_code: int | None + verified_version: str | None + reconciliation_needed: bool + failure_category: str | None + + +@dataclass(frozen=True) +class _DetectionSignals: + """Test-only record describing which detection tier fired. + + Production call sites discard this (default return of + _detect_install_method is a bare _InstallMethod); tests pass + include_signals=True to retrieve the pair and assert on the chosen tier. + """ + + sys_argv0: str + matched_tier: int | None + matched_prefix: str | None + editable_marker_seen: bool + installer_registries_consulted: list[str] + resolved_method: _InstallMethod + + +def _scrubbed_env() -> dict[str, str]: + """Return a copy of os.environ with GitHub token env vars removed. + + Used for both installer invocation and the post-upgrade + verification child process so neither inherits tokens it does not need. + """ + + return { + k: v for k, v in os.environ.items() if k not in {"GH_TOKEN", "GITHUB_TOKEN"} + } + + +_TAG_REGEX = re.compile(r"^v\d+\.\d+\.\d+(?:[a-z0-9.+\-]*)?$") + + +def _validate_tag(tag: str) -> str: + """Validate a user-supplied --tag value. + + Accepts vX.Y.Z plus optional PEP-440-ish suffix (dev0, rc1, beta.1, + +build.42). Rejects everything else (including bare 'latest', hash refs, + branch names, or a numeric version without the 'v' prefix). + """ + if not _TAG_REGEX.match(tag): + raise typer.BadParameter("Invalid --tag: expected vMAJOR.MINOR.PATCH[suffix]") + + return tag + + +def _expand_prefix(prefix: str) -> Path: + """Expand ~ and %LOCALAPPDATA% style tokens in a prefix string to a Path. + + Safe to call on POSIX with Windows-style prefixes (the expansion is a + no-op and the resulting Path will simply not match anything on disk). + """ + + expanded = os.path.expandvars(os.path.expanduser(prefix)) + return Path(expanded).resolve() if Path(expanded).is_absolute() else Path(expanded) + + +def _git_ancestor(path: Path) -> Path | None: + """Return the closest ancestor that looks like a git worktree root.""" + for ancestor in [path, *path.parents]: + if (ancestor / ".git").exists(): + return ancestor + return None + + +def _editable_direct_url_path() -> Path | None: + """Return the editable checkout root recorded in direct_url.json, if any.""" + import importlib.metadata as _md + + try: + dist = _md.distribution("specify-cli") + except _md.PackageNotFoundError: + return None + + payload = dist.read_text("direct_url.json") + if not payload: + return None + + try: + data = json.loads(payload) + except (TypeError, ValueError): + return None + + if not data.get("dir_info", {}).get("editable"): + return None + + url = data.get("url") + if not isinstance(url, str): + return None + + parsed = urllib.parse.urlsplit(url) + if parsed.scheme != "file": + return None + + url_path = urllib.request.url2pathname(urllib.parse.unquote(parsed.path)) + if parsed.netloc and parsed.netloc not in {"", "localhost"}: + url_path = f"//{parsed.netloc}{url_path}" + + try: + return Path(url_path).resolve() + except OSError: + return None + + +def _editable_marker_seen() -> bool: + """Return True if specify-cli's installed files live under a git worktree. + + Detects `pip install -e .` style editable installs by walking the dist's + file list for any path whose ancestor contains a `.git` directory. + Returns False when metadata is unreadable or no git ancestor is present. + """ + import importlib.metadata as _md + + editable_root = _editable_direct_url_path() + if editable_root is not None and _git_ancestor(editable_root) is not None: + return True + + try: + dist = _md.distribution("specify-cli") + except _md.PackageNotFoundError: + return False + files = dist.files or [] + for f in files: + abs_path = Path(dist.locate_file(f)).resolve() + if _git_ancestor(abs_path) is not None: + return True + return False + + +def _detect_install_method( + argv0: str | None = None, + include_signals: bool = False, +) -> "_InstallMethod | tuple[_InstallMethod, _DetectionSignals]": + """Classify the current runtime into exactly one _InstallMethod. + + Tiers: + 1. sys.argv[0] path prefix match against _INSTALLER_PATH_PREFIXES. + 2. Editable-install marker (.git ancestor in dist files). + 3. PATH + installer registry reconciliation (uv tool list / pipx list). + Fallthrough -> UNSUPPORTED. + + When include_signals=True returns (_InstallMethod, _DetectionSignals) so + tests can assert on which tier fired. Detection is deterministic — no + wall-clock, no randomness. + """ + argv0_resolved = str(Path(argv0 or sys.argv[0]).resolve()) + + # --- Tier 1: path prefix match --- + for method_str, prefixes in _INSTALLER_PATH_PREFIXES.items(): + for prefix in prefixes: + expanded = _expand_prefix(prefix) + if argv0_resolved.startswith(str(expanded)): + method = _InstallMethod(method_str) + if include_signals: + return method, _DetectionSignals( + sys_argv0=argv0_resolved, + matched_tier=1, + matched_prefix=prefix, + editable_marker_seen=False, + installer_registries_consulted=[], + resolved_method=method, + ) + return method + + # --- Tier 2: editable install marker --- + if _editable_marker_seen(): + method = _InstallMethod.SOURCE_CHECKOUT + if include_signals: + return method, _DetectionSignals( + sys_argv0=argv0_resolved, + matched_tier=2, + matched_prefix=None, + editable_marker_seen=True, + installer_registries_consulted=[], + resolved_method=method, + ) + return method + + # --- Tier 3: PATH + registry reconciliation --- + consulted: list[str] = [] + uv_bin = shutil.which("uv") + if uv_bin is not None: + consulted.append("uv tool list") + try: + result = subprocess.run( + [uv_bin, "tool", "list"], + capture_output=True, + text=True, + timeout=5, + env=_scrubbed_env(), + check=False, + ) + if result.returncode == 0 and "specify-cli" in (result.stdout or ""): + method = _InstallMethod.UV_TOOL + if include_signals: + return method, _DetectionSignals( + sys_argv0=argv0_resolved, + matched_tier=3, + matched_prefix=None, + editable_marker_seen=False, + installer_registries_consulted=consulted, + resolved_method=method, + ) + return method + except (subprocess.TimeoutExpired, OSError): + pass + + pipx_bin = shutil.which("pipx") + if pipx_bin is not None: + consulted.append("pipx list --json") + try: + result = subprocess.run( + [pipx_bin, "list", "--json"], + capture_output=True, + text=True, + timeout=5, + env=_scrubbed_env(), + check=False, + ) + if result.returncode == 0 and "specify-cli" in (result.stdout or ""): + method = _InstallMethod.PIPX + if include_signals: + return method, _DetectionSignals( + sys_argv0=argv0_resolved, + matched_tier=3, + matched_prefix=None, + editable_marker_seen=False, + installer_registries_consulted=consulted, + resolved_method=method, + ) + return method + except (subprocess.TimeoutExpired, OSError): + pass + + # Fallthrough + method = _InstallMethod.UNSUPPORTED + if include_signals: + return method, _DetectionSignals( + sys_argv0=argv0_resolved, + matched_tier=None, + matched_prefix=None, + editable_marker_seen=False, + installer_registries_consulted=consulted, + resolved_method=method, + ) + return method + + +_GITHUB_SOURCE_URL = "git+https://github.com/github/spec-kit.git" + + +def _assemble_installer_argv( + method: _InstallMethod, target_tag: str +) -> list[str] | None: + """Build the installer subprocess argv for the given method and tag. + + Returns None for non-upgradable methods (UVX_EPHEMERAL, SOURCE_CHECKOUT, + UNSUPPORTED) so the caller routes to _emit_guidance instead. argv is + always a list[str] for shell=False invocation. + """ + source_spec = f"{_GITHUB_SOURCE_URL}@{target_tag}" + + if method == _InstallMethod.UV_TOOL: + uv_bin = shutil.which("uv") or "uv" # resolve at invocation time too + return [ + uv_bin, + "tool", + "install", + "--force", + "--from", + source_spec, + "specify-cli", + ] + + if method == _InstallMethod.PIPX: + # pipx 1.5+ removed `--spec`; PACKAGE_SPEC is now positional and the + # package name is auto-detected from the source's pyproject.toml. + pipx_bin = shutil.which("pipx") or "pipx" + return [ + pipx_bin, + "install", + "--force", + source_spec, + ] + + return None + + +def _method_label(method: _InstallMethod) -> str: + """Render a method enum as the human label used in notices (contracts §Output shapes).""" + return { + _InstallMethod.UV_TOOL: "uv tool", + _InstallMethod.PIPX: "pipx", + _InstallMethod.UVX_EPHEMERAL: "uvx (ephemeral)", + _InstallMethod.SOURCE_CHECKOUT: "source checkout", + _InstallMethod.UNSUPPORTED: "unsupported", + }[method] + + +def _build_upgrade_plan( + target_tag_override: str | None, +) -> tuple[_UpgradePlan | None, str | None]: + """Build a _UpgradePlan or return (None, failure_reason). + + failure_reason is one of the resolver categories + when _fetch_latest_release_tag cannot produce a tag AND no override was + supplied. When target_tag_override is a valid pinned tag, the network + resolver is NOT called — this keeps `--dry-run --tag X` fast and network-free. + + The orchestrator branches on the (plan, reason) tuple: + - (plan, None) -> proceed to preview / apply path + - (None, reason) -> emit resolution-failure output + exit non-zero + """ + if target_tag_override is not None: + target_tag = target_tag_override + else: + tag, failure_reason = _fetch_latest_release_tag() + if tag is None: + return None, failure_reason # surfaces as exit 1 in the orchestrator + target_tag = tag + + method = _detect_install_method() + current = _get_installed_version() + argv = _assemble_installer_argv(method, target_tag) + + preview = ( + f"Detected install method: {_method_label(method)}\n" + f"Current version: {current}\n" + f"Target version: {target_tag}\n" + f"Command that would be executed: " + f"{shlex.join(argv) if argv is not None else '(none — non-upgradable path)'}" + ) + + plan = _UpgradePlan( + method=method, + current_version=current, + target_tag=target_tag, + installer_argv=argv, + preview_summary=preview, + pre_upgrade_snapshot=current, + ) + return plan, None + + +def _run_installer(plan: _UpgradePlan) -> subprocess.CompletedProcess | None: + """Invoke the installer subprocess. + + Returns None if the installer binary is not found on PATH (caller routes + to installer-missing output with exit 3). Returns a CompletedProcess for + every other case including non-zero exit — caller inspects returncode. + + stdout/stderr are inherited (not captured) so the user sees installer + progress in real time. The child environment has GH_TOKEN / + GITHUB_TOKEN removed. + + Timeout: by default the subprocess runs with no timeout — installer + operations (dependency resolution, large wheel downloads) can legitimately + take many minutes. Set the env var SPECIFY_UPGRADE_TIMEOUT_SECS to an + integer/float to enforce a hard cap; on timeout we return a synthetic + CompletedProcess with returncode=124 so the orchestrator surfaces the + same installer-failed code path the user already understands. An + unparseable or non-positive value is silently ignored (no timeout). + """ + if plan.installer_argv is None: + # Internal routing error: the orchestrator must route non-upgradable + # methods to _emit_guidance and never reach this function. Use a real + # raise (not assert) so the guard survives `python -O`. + raise RuntimeError( + "internal routing error: _run_installer received a plan without an " + "installer_argv (non-upgradable methods must route to _emit_guidance)" + ) + + # Use the argv assembled at plan-build time verbatim. The pre-execution + # notice and the actual subprocess argv must be byte-for-byte identical; + # any re-resolution here would risk diverging from what the user just + # saw printed. A lightweight pre-flight via `shutil.which` short-circuits + # the obvious "binary disappeared" case before spawning, and the + # try/except below catches the residual race window. + installer_name = Path(plan.installer_argv[0]).name + if shutil.which(installer_name) is None: + return None # signals installer-missing to caller + + timeout_raw = os.environ.get("SPECIFY_UPGRADE_TIMEOUT_SECS") + timeout: float | None = None + if timeout_raw is not None: + try: + timeout = float(timeout_raw) + if timeout <= 0: + timeout = None + except ValueError: + timeout = None + + try: + return subprocess.run( + plan.installer_argv, + shell=False, + check=False, + env=_scrubbed_env(), + timeout=timeout, + ) + except subprocess.TimeoutExpired: + # Surface as installer-failed via a synthetic CompletedProcess with a + # non-zero exit code so the orchestrator's installer-failed path emits + # the manual-retry argv + rollback hint. + return subprocess.CompletedProcess( + args=plan.installer_argv, + returncode=124, # POSIX-ish convention for "timeout" + stdout=None, + stderr=None, + ) + except (FileNotFoundError, OSError): + return None # disappeared between pre-flight and exec + + +_VERIFY_VERSION_REGEX = re.compile(r"specify (\S+)") + + +def _verify_upgrade(plan: _UpgradePlan) -> str | None: + """Spawn a child `specify --version` and parse its output. + + Returns the version string on success, None on parse failure, timeout, + or missing binary. Caller compares the returned version to plan.target_tag + and raises verification-mismatch if they differ. + + Uses a child process (not in-process importlib.metadata) because Python + cannot hot-swap the running module after the installer has replaced it — + only a fresh process picks up the new binary. + """ + specify_bin = shutil.which("specify") + if specify_bin is None: + return None + try: + result = subprocess.run( + [specify_bin, "--version"], + shell=False, + check=False, + capture_output=True, + text=True, + timeout=10, + env=_scrubbed_env(), + ) + except (subprocess.TimeoutExpired, OSError): + return None + match = _VERIFY_VERSION_REGEX.search(result.stdout or "") + return match.group(1) if match else None + + +def _source_checkout_path() -> Path | None: + """Best-effort: return the working-tree root for editable installs.""" + import importlib.metadata as _md + + editable_root = _editable_direct_url_path() + if editable_root is not None: + git_root = _git_ancestor(editable_root) + if git_root is not None: + return git_root + + try: + dist = _md.distribution("specify-cli") + except _md.PackageNotFoundError: + return None + files = dist.files or [] + for f in files: + try: + abs_path = Path(dist.locate_file(f)).resolve() + except Exception: + continue + git_root = _git_ancestor(abs_path) + if git_root is not None: + return git_root + return None + + +def _emit_guidance(method: _InstallMethod, target_tag: str) -> None: + """Print path-specific guidance for non-upgradable install methods.""" + if method == _InstallMethod.UVX_EPHEMERAL: + console.print( + "Running via uvx (ephemeral); the next uvx invocation already " + "resolves to latest — no upgrade action needed.", + soft_wrap=True, + ) + return + + if method == _InstallMethod.SOURCE_CHECKOUT: + tree = _source_checkout_path() + tree_str = str(tree) if tree else "(path unavailable)" + console.print( + f"Running from a source checkout at {tree_str}; " + f"upgrade by running: git pull && pip install -e .", + soft_wrap=True, + ) + return + + if method == _InstallMethod.UNSUPPORTED: + console.print( + "Could not identify your install method automatically; " + "run one of the following manually:", + soft_wrap=True, + ) + console.print( + f" uv tool install --force --from " + f"git+https://github.com/github/spec-kit.git@{target_tag} specify-cli", + soft_wrap=True, + ) + console.print( + f" pipx install --force " + f"git+https://github.com/github/spec-kit.git@{target_tag}", + soft_wrap=True, + ) + return + + raise RuntimeError( + f"internal routing error: _emit_guidance called on upgradable method: {method}" + ) + + +def _rollback_hint(plan: _UpgradePlan) -> str: + """Construct the rollback-suggestion line. + + Never a destructive action — always a command the user can run by hand. + Uses the pre-upgrade version snapshot captured before the installer ran; + when that snapshot is 'unknown' the hint degrades to a releases-page + pointer rather than printing a malformed git ref. + """ + if plan.pre_upgrade_snapshot == "unknown": + return ( + "Could not determine the previous version; " + "reinstall manually from: https://github.com/github/spec-kit/releases" + ) + return ( + f"To pin back to the previous version: uv tool install --force --from " + f"git+https://github.com/github/spec-kit.git@v{plan.pre_upgrade_snapshot} " + f"specify-cli" + ) + + +def _emit_failure( + category: str, + plan: _UpgradePlan | None = None, + installer_exit: int | None = None, + installer_name: str | None = None, + verified_version: str | None = None, +) -> None: + """Render the user-visible failure output for any documented category. + + Resolver categories are forwarded as-is so the failure vocabulary stays + identical between `self check` and `self upgrade`. Execution categories + (installer-missing, installer-failed, verification-mismatch) format + their own multi-line block including a manual-retry argv and rollback + hint where applicable. + """ + if ( + category in _RESOLUTION_FAILURE_CATEGORIES + or category.startswith("HTTP ") + or category.startswith("http-") + ): + console.print(f"Upgrade aborted: {category}", soft_wrap=True) + return + + if category == "installer-missing": + name = installer_name or "(unknown)" + console.print( + f"Installer {name} not found on PATH; reinstall it and retry.", + soft_wrap=True, + ) + return + + if category == "installer-failed": + if plan is None or installer_exit is None: + raise RuntimeError( + "internal routing error: installer-failed requires both " + "plan and installer_exit to be set" + ) + argv_str = shlex.join(plan.installer_argv) if plan.installer_argv else "" + console.print( + f"Upgrade failed. Installer exit code: {installer_exit}.", + soft_wrap=True, + ) + console.print( + f"Try again or run the command manually: {argv_str}", + soft_wrap=True, + ) + console.print(_rollback_hint(plan), soft_wrap=True) + return + + if category == "verification-mismatch": + if plan is None: + raise RuntimeError( + "internal routing error: verification-mismatch requires plan to be set" + ) + verified_str = verified_version or "(unknown)" + console.print( + f"Verification failed: installer reported success but " + f"'specify --version' resolves to {verified_str} " + f"(expected {plan.target_tag}).", + soft_wrap=True, + ) + console.print( + "The new version may take effect on your next invocation.", + soft_wrap=True, + ) + return + + raise RuntimeError(f"Unknown failure category: {category!r}") + + # ===== Self Commands ===== self_app = typer.Typer( name="self", @@ -1844,20 +2532,142 @@ def self_check() -> None: @self_app.command("upgrade") -def self_upgrade() -> None: - """Reserved command surface for self-upgrade; not implemented in this release. +def self_upgrade( + dry_run: bool = typer.Option( + False, + "--dry-run", + help="Print the preview (method, current, target, installer argv) and " + "exit 0 without launching any subprocess.", + ), + tag: Optional[str] = typer.Option( + None, + "--tag", + help="Pin the target version (vX.Y.Z[suffix]). Without --tag, the " + "latest stable release is resolved via GitHub Releases.", + callback=lambda v: _validate_tag(v) if v is not None else None, + ), +) -> None: + """Upgrade specify-cli to the latest release (or a pinned --tag). + + Bare invocation executes immediately with no confirmation prompt, matching + pip install -U / uv tool upgrade / npm update conventions. Use --dry-run + to preview without mutating anything. See `specify self check` for the + non-destructive read-only counterpart. + + Detection classifies the runtime into uv-tool / pipx / uvx-ephemeral / + source-checkout / unsupported. Only uv-tool and pipx are upgraded + automatically; the other three paths print path-specific guidance and + exit 0. + + Exit codes: + 0 success or no-op-success (already on latest, --dry-run, or + non-upgradable path with guidance shown) + 1 target-tag resolution failure or --tag regex validation failure + 2 verification mismatch (installer exited 0 but `specify --version` + does not resolve to the target tag) + 3 installer binary not found on PATH + 124 installer subprocess timed out (only when SPECIFY_UPGRADE_TIMEOUT_SECS is set) + other installer exit code propagated verbatim + + Environment variables: + SPECIFY_UPGRADE_TIMEOUT_SECS Optional integer/float seconds. Caps how + long the installer subprocess may run. Unset (default) means no + timeout — interrupt with Ctrl+C if the installer hangs. + """ + plan, failure_reason = _build_upgrade_plan(target_tag_override=tag) + + # Resolver could not produce a tag → surface the categorized failure + # and exit non-zero so scripts notice (action-oriented unlike `self check`). + if plan is None: + if failure_reason is None: + # _build_upgrade_plan's contract: if plan is None, failure_reason + # is set. Defend explicitly so the guard survives `python -O`. + raise RuntimeError( + "internal contract violation: _build_upgrade_plan returned (None, None)" + ) + _emit_failure(failure_reason) + raise typer.Exit(1) + + # --dry-run preview path. Non-upgradable methods still emit guidance + # rather than a fake preview block — there is nothing to preview when + # there is nothing the CLI would launch. + if dry_run: + if plan.method in ( + _InstallMethod.UVX_EPHEMERAL, + _InstallMethod.SOURCE_CHECKOUT, + _InstallMethod.UNSUPPORTED, + ): + _emit_guidance(plan.method, plan.target_tag) + raise typer.Exit(0) + console.print("Dry run — no changes will be made.") + for line in plan.preview_summary.splitlines(): + console.print(line) + raise typer.Exit(0) + + # Non-upgradable runtime: never launch an installer regardless of flags. + if plan.method in ( + _InstallMethod.UVX_EPHEMERAL, + _InstallMethod.SOURCE_CHECKOUT, + _InstallMethod.UNSUPPORTED, + ): + _emit_guidance(plan.method, plan.target_tag) + raise typer.Exit(0) + + # No-op success when the user is already on the latest tag. + latest_normalized = _normalize_tag(plan.target_tag) + if plan.current_version != "unknown" and not _is_newer( + latest_normalized, plan.current_version + ): + console.print(f"Already on latest release: {plan.target_tag}") + raise typer.Exit(0) + + # One-line pre-execution notice so the user sees exactly what will run + # before the installer's own output starts streaming. + argv_str = shlex.join(plan.installer_argv) if plan.installer_argv else "" + console.print( + f"Upgrading specify-cli {plan.current_version} → {plan.target_tag} " + f"via {_method_label(plan.method)}: {argv_str}", + soft_wrap=True, + ) + + # Launch the installer. Stdout/stderr stream through (no capture) so the + # user sees real-time progress. We never pass shell=True. + completed = _run_installer(plan) + + if completed is None: + installer_name = ( + Path(plan.installer_argv[0]).name if plan.installer_argv else None + ) + _emit_failure("installer-missing", plan=plan, installer_name=installer_name) + raise typer.Exit(3) + + if completed.returncode != 0: + _emit_failure( + "installer-failed", + plan=plan, + installer_exit=completed.returncode, + ) + raise typer.Exit(completed.returncode) + + # Verify in a child process: this Python process is still running the + # pre-upgrade module, so importlib.metadata would lie. A fresh `specify + # --version` is the only signal that the new binary is actually live. + verified = _verify_upgrade(plan) + if verified is None or _normalize_tag(plan.target_tag) != verified: + _emit_failure( + "verification-mismatch", + plan=plan, + verified_version=verified, + ) + raise typer.Exit(2) + + console.print( + f"Upgraded specify-cli: {plan.pre_upgrade_snapshot} → {verified}", + soft_wrap=True, + ) + - 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_self_upgrade.py b/tests/test_self_upgrade.py new file mode 100644 index 0000000000..fc3226cd5b --- /dev/null +++ b/tests/test_self_upgrade.py @@ -0,0 +1,858 @@ +"""Tests for the `specify self upgrade` actual-upgrade implementation (Phase 2). + +Network + subprocess isolation contract: + - Every `subprocess.run` call MUST be patched via unittest.mock. + - Every `shutil.which` call MUST be patched. + - Every `urllib.request.urlopen` call MUST be patched (inherits from Phase 1). + - No real network traffic, no real subprocess execution, no filesystem + writes outside the test's own tmp_path fixture. + +Phase-1 byte-identity gate: + - `tests/test_upgrade.py` covers `specify self check` and must pass + unmodified on this branch — except for the removal of + `TestSelfUpgradeStub` (the Phase-1 stub is the thing being replaced + by this feature). + +This file is the TDD red phase. Tests reference Phase-2 symbols +(`_InstallMethod`, `_UpgradePlan`, `_detect_install_method`, etc.) +that are added during Phase 2 implementation. +Until those symbols exist the import block at the top of this file +will fail and pytest will mark every case as a collection error. +That failure mode IS the red phase — proceed to implementation. +""" + +import json +import subprocess +import urllib.error +from unittest.mock import MagicMock, patch + +import pytest +from typer.testing import CliRunner + +from specify_cli import ( # noqa: F401 — symbols added in Phase 2 + _InstallMethod, + _UpgradePlan, + _UpgradeOutcome, + _DetectionSignals, + _editable_marker_seen, + _source_checkout_path, + _detect_install_method, + _assemble_installer_argv, + _build_upgrade_plan, + _run_installer, + _verify_upgrade, + _emit_guidance, + _emit_failure, + _scrubbed_env, + _validate_tag, + app, +) + +from tests.conftest import strip_ansi + +runner = CliRunner() + +SENTINEL_GH_TOKEN = "SENTINEL-GH-TOKEN-VALUE" +SENTINEL_GITHUB_TOKEN = "SENTINEL-GITHUB-TOKEN-VALUE" + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _mock_urlopen_response(payload: dict) -> MagicMock: + """Build a urlopen() context-manager mock whose .read() returns the JSON payload.""" + 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 _completed_process( + returncode: int, stdout: str = "", stderr: str = "" +) -> subprocess.CompletedProcess: + """Build a subprocess.CompletedProcess for installer / verification calls.""" + return subprocess.CompletedProcess( + args=["mocked"], + returncode=returncode, + stdout=stdout, + stderr=stderr, + ) + + +# --------------------------------------------------------------------------- +# Fixtures +# --------------------------------------------------------------------------- + + +@pytest.fixture +def clean_environ(monkeypatch): + """Strip any real GH_TOKEN / GITHUB_TOKEN from the test environment.""" + monkeypatch.delenv("GH_TOKEN", raising=False) + monkeypatch.delenv("GITHUB_TOKEN", raising=False) + + +@pytest.fixture +def uv_tool_argv0(monkeypatch, tmp_path): + """Point sys.argv[0] at a simulated `uv tool` install path under tmp HOME. + + Sets HOME=tmp_path so _expand_prefix("~/.local/share/uv/tools/specify-cli/") + expands to a path that actually contains the fake binary. This avoids + needing a `_UV_TOOL_ROOT_OVERRIDE` knob in production code. + """ + monkeypatch.setenv("HOME", str(tmp_path)) + fake_dir = tmp_path / ".local" / "share" / "uv" / "tools" / "specify-cli" / "bin" + fake_dir.mkdir(parents=True) + fake_specify = fake_dir / "specify" + fake_specify.write_text("#!/usr/bin/env python\n") + monkeypatch.setattr("sys.argv", [str(fake_specify)]) + return fake_specify + + +@pytest.fixture +def pipx_argv0(monkeypatch, tmp_path): + """Point sys.argv[0] at a simulated pipx install path under tmp HOME.""" + monkeypatch.setenv("HOME", str(tmp_path)) + fake_dir = tmp_path / ".local" / "pipx" / "venvs" / "specify-cli" / "bin" + fake_dir.mkdir(parents=True) + fake_specify = fake_dir / "specify" + fake_specify.write_text("#!/usr/bin/env python\n") + monkeypatch.setattr("sys.argv", [str(fake_specify)]) + return fake_specify + + +@pytest.fixture +def uvx_ephemeral_argv0(monkeypatch, tmp_path): + """Point sys.argv[0] at a simulated uvx ephemeral-cache path under tmp HOME.""" + monkeypatch.setenv("HOME", str(tmp_path)) + fake_dir = tmp_path / ".cache" / "uv" / "archive-v0" / "abc123" / "bin" + fake_dir.mkdir(parents=True) + fake_specify = fake_dir / "specify" + fake_specify.write_text("#!/usr/bin/env python\n") + monkeypatch.setattr("sys.argv", [str(fake_specify)]) + return fake_specify + + +@pytest.fixture +def unsupported_argv0(monkeypatch, tmp_path): + """Point sys.argv[0] at a path that does not match any installer prefix.""" + monkeypatch.setenv("HOME", str(tmp_path)) + fake_dir = tmp_path / "random" / "location" / "bin" + fake_dir.mkdir(parents=True) + fake_specify = fake_dir / "specify" + fake_specify.write_text("#!/usr/bin/env python\n") + monkeypatch.setattr("sys.argv", [str(fake_specify)]) + return fake_specify + + +# =========================================================================== +# Phase 3 — User Story 1: `uv tool` immediate upgrade (P1, MVP) +# =========================================================================== + + +class TestDetection_UvTool: + """Tier-1 path-prefix detection for uv-tool installs.""" + + def test_posix_uv_tool_prefix_matches(self, uv_tool_argv0): + method, signals = _detect_install_method(include_signals=True) + assert method == _InstallMethod.UV_TOOL + assert signals.matched_tier == 1 + assert "uv/tools/specify-cli" in signals.matched_prefix.replace("\\", "/") + + def test_detection_is_deterministic(self, uv_tool_argv0): + a = _detect_install_method() + b = _detect_install_method() + assert a == b == _InstallMethod.UV_TOOL + + def test_no_argv_match_falls_through_to_unsupported(self, unsupported_argv0): + with patch("specify_cli.shutil.which", return_value=None), patch( + "specify_cli._editable_marker_seen", return_value=False + ): + method = _detect_install_method() + assert method == _InstallMethod.UNSUPPORTED + + def test_include_signals_false_returns_bare_enum(self, uv_tool_argv0): + result = _detect_install_method(include_signals=False) + assert isinstance(result, _InstallMethod) + + +class TestArgvAssembly_UvTool: + """uv-tool installer argv shape.""" + + def test_stable_tag_produces_expected_argv(self): + with patch("specify_cli.shutil.which", return_value="/usr/bin/uv"): + argv = _assemble_installer_argv(_InstallMethod.UV_TOOL, "v0.7.6") + assert argv == [ + "/usr/bin/uv", + "tool", + "install", + "--force", + "--from", + "git+https://github.com/github/spec-kit.git@v0.7.6", + "specify-cli", + ] + + def test_dev_suffix_tag_embedded_literally(self): + with patch("specify_cli.shutil.which", return_value="/usr/bin/uv"): + argv = _assemble_installer_argv(_InstallMethod.UV_TOOL, "v0.8.0.dev0") + assert "git+https://github.com/github/spec-kit.git@v0.8.0.dev0" in argv + assert ( + "upgrade" not in argv + ) # never `uv tool upgrade` — does not accept --tag pinning + + +class TestBareUpgrade_UvTool: + """uv-tool happy path, bare invocation.""" + + def test_happy_path_end_to_end(self, uv_tool_argv0, clean_environ): + with patch("specify_cli.urllib.request.urlopen") as mock_urlopen, patch( + "specify_cli.shutil.which", return_value="/usr/bin/uv" + ), patch("specify_cli.subprocess.run") as mock_run, patch( + "specify_cli._get_installed_version", return_value="0.7.5" + ): + mock_urlopen.return_value = _mock_urlopen_response({"tag_name": "v0.7.6"}) + mock_run.side_effect = [ + _completed_process(0), # installer + _completed_process(0, stdout="specify 0.7.6\n"), # verify + ] + result = runner.invoke(app, ["self", "upgrade"]) + + assert result.exit_code == 0 + out = strip_ansi(result.output) + assert "Upgrading specify-cli 0.7.5 → v0.7.6 via uv tool:" in out + assert "Upgraded specify-cli: 0.7.5 → 0.7.6" in out + assert mock_run.call_count == 2 + for call in mock_run.call_args_list: + assert call.kwargs.get("shell", False) is False + + def test_one_user_action_no_prompt(self, uv_tool_argv0, clean_environ): + # The single `invoke` represents the single user action — no prompt. + # If a prompt existed, runner.invoke would hang waiting for input. + with patch("specify_cli.urllib.request.urlopen") as mock_urlopen, patch( + "specify_cli.shutil.which", return_value="/usr/bin/uv" + ), patch("specify_cli.subprocess.run") as mock_run, patch( + "specify_cli._get_installed_version", return_value="0.7.5" + ): + mock_urlopen.return_value = _mock_urlopen_response({"tag_name": "v0.7.6"}) + mock_run.side_effect = [ + _completed_process(0), + _completed_process(0, stdout="specify 0.7.6\n"), + ] + result = runner.invoke(app, ["self", "upgrade"]) + assert result.exit_code == 0 + + +class TestAlreadyLatest_UvTool: + """already on latest, no installer launched.""" + + def test_already_latest_exits_zero_no_subprocess( + self, uv_tool_argv0, clean_environ + ): + with patch("specify_cli.urllib.request.urlopen") as mock_urlopen, patch( + "specify_cli.subprocess.run" + ) as mock_run, patch( + "specify_cli.shutil.which", return_value="/usr/bin/uv" + ), patch("specify_cli._get_installed_version", return_value="0.7.6"): + mock_urlopen.return_value = _mock_urlopen_response({"tag_name": "v0.7.6"}) + result = runner.invoke(app, ["self", "upgrade"]) + + assert result.exit_code == 0 + assert "Already on latest release: v0.7.6" in strip_ansi(result.output) + assert mock_run.call_count == 0 + + +class TestDryRun_UvTool: + """--dry-run preview path + --dry-run combined with --tag.""" + + def test_dry_run_without_tag_resolves_network_but_no_subprocess( + self, + uv_tool_argv0, + clean_environ, + ): + with patch("specify_cli.urllib.request.urlopen") as mock_urlopen, patch( + "specify_cli.subprocess.run" + ) as mock_run, patch( + "specify_cli.shutil.which", return_value="/usr/bin/uv" + ), patch("specify_cli._get_installed_version", return_value="0.7.5"): + mock_urlopen.return_value = _mock_urlopen_response({"tag_name": "v0.7.6"}) + result = runner.invoke(app, ["self", "upgrade", "--dry-run"]) + + assert result.exit_code == 0 + out = strip_ansi(result.output) + assert "Dry run — no changes will be made." in out + assert "Detected install method: uv tool" in out + assert "Current version: 0.7.5" in out + assert "Target version: v0.7.6" in out + assert "Command that would be executed:" in out + assert mock_run.call_count == 0 + + def test_dry_run_with_tag_skips_network(self, uv_tool_argv0, clean_environ): + # --dry-run with --tag must NOT hit the network. + with patch("specify_cli.urllib.request.urlopen") as mock_urlopen, patch( + "specify_cli.subprocess.run" + ), patch("specify_cli.shutil.which", return_value="/usr/bin/uv"), patch( + "specify_cli._get_installed_version", return_value="0.7.5" + ): + result = runner.invoke( + app, + ["self", "upgrade", "--dry-run", "--tag", "v0.8.0"], + ) + assert result.exit_code == 0 + assert "Target version: v0.8.0" in strip_ansi(result.output) + mock_urlopen.assert_not_called() + + +# =========================================================================== +# Phase 4 — User Story 2: `pipx` immediate upgrade (P2) +# =========================================================================== + + +class TestDetection_Pipx: + """Pipx detection — tier 1 (path) and tier 3 (registry).""" + + def test_posix_pipx_prefix_matches(self, pipx_argv0): + method, signals = _detect_install_method(include_signals=True) + assert method == _InstallMethod.PIPX + assert signals.matched_tier == 1 + + def test_tier3_pipx_when_no_prefix_match_but_registry_lists_it( + self, + unsupported_argv0, + ): + def fake_which(name): + return "/usr/bin/pipx" if name == "pipx" else None + + def fake_run(argv, *args, **kwargs): + if argv[0] == "/usr/bin/pipx" and argv[1] == "list": + return subprocess.CompletedProcess( + args=argv, + returncode=0, + stdout='{"venvs":{"specify-cli":{}}}', + stderr="", + ) + return subprocess.CompletedProcess( + args=argv, returncode=1, stdout="", stderr="" + ) + + with patch("specify_cli.shutil.which", side_effect=fake_which), patch( + "specify_cli.subprocess.run", side_effect=fake_run + ), patch("specify_cli._editable_marker_seen", return_value=False): + method, signals = _detect_install_method(include_signals=True) + assert method == _InstallMethod.PIPX + assert signals.matched_tier == 3 + assert "pipx list --json" in signals.installer_registries_consulted + + +class TestEditableInstallMetadata: + def test_direct_url_editable_install_marks_source_checkout(self, tmp_path): + project_root = tmp_path / "spec-kit" + project_root.mkdir() + (project_root / ".git").mkdir() + + class FakeDist: + files = [] + + def read_text(self, name): + if name == "direct_url.json": + return json.dumps( + { + "dir_info": {"editable": True}, + "url": project_root.as_uri(), + } + ) + return None + + def locate_file(self, file): + return file + + with patch("importlib.metadata.distribution", return_value=FakeDist()): + assert _editable_marker_seen() is True + assert _source_checkout_path() == project_root.resolve() + + +class TestArgvAssembly_Pipx: + """pipx installer argv shape — pipx 1.5+ uses positional PACKAGE_SPEC, never `--spec` or `upgrade`.""" + + def test_pipx_argv_uses_install_force_positional_not_upgrade(self): + with patch("specify_cli.shutil.which", return_value="/usr/bin/pipx"): + argv = _assemble_installer_argv(_InstallMethod.PIPX, "v0.7.6") + assert argv == [ + "/usr/bin/pipx", + "install", + "--force", + "git+https://github.com/github/spec-kit.git@v0.7.6", + ] + assert "upgrade" not in argv # pipx upgrade does not accept arbitrary refs + assert "--spec" not in argv # pipx 1.5+ dropped the --spec flag + + +class TestBareUpgrade_Pipx: + """pipx happy path.""" + + def test_happy_path(self, pipx_argv0, clean_environ): + with patch("specify_cli.urllib.request.urlopen") as mock_urlopen, patch( + "specify_cli.shutil.which", return_value="/usr/bin/pipx" + ), patch("specify_cli.subprocess.run") as mock_run, patch( + "specify_cli._get_installed_version", return_value="0.7.5" + ): + mock_urlopen.return_value = _mock_urlopen_response({"tag_name": "v0.7.6"}) + mock_run.side_effect = [ + _completed_process(0), + _completed_process(0, stdout="specify 0.7.6\n"), + ] + result = runner.invoke(app, ["self", "upgrade"]) + + assert result.exit_code == 0 + out = strip_ansi(result.output) + assert "via pipx:" in out + assert "Upgraded specify-cli: 0.7.5 → 0.7.6" in out + + +class TestTiebreak_UvVsPipx: + """argv0 prefix wins over registry tiebreak.""" + + def test_pipx_argv0_wins_over_uv_registry_when_both_listed( + self, + pipx_argv0, + clean_environ, + ): + # pipx_argv0 makes tier-1 fire for pipx; uv-registry tier-3 never + # runs because tier-1 already short-circuited. + def fake_run(argv, *args, **kwargs): + if argv[1:3] == ["tool", "list"]: + return subprocess.CompletedProcess( + args=argv, + returncode=0, + stdout="specify-cli 0.7.5", + stderr="", + ) + return subprocess.CompletedProcess( + args=argv, returncode=1, stdout="", stderr="" + ) + + with patch("specify_cli.shutil.which", return_value="/usr/bin/X"), patch( + "specify_cli.subprocess.run", side_effect=fake_run + ): + method = _detect_install_method() + assert method == _InstallMethod.PIPX + + +class TestDryRun_Pipx: + def test_dry_run_preview_names_pipx(self, pipx_argv0, clean_environ): + with patch("specify_cli.urllib.request.urlopen") as mock_urlopen, patch( + "specify_cli.shutil.which", return_value="/usr/bin/pipx" + ), patch("specify_cli.subprocess.run") as mock_run, patch( + "specify_cli._get_installed_version", return_value="0.7.5" + ): + mock_urlopen.return_value = _mock_urlopen_response({"tag_name": "v0.7.6"}) + result = runner.invoke(app, ["self", "upgrade", "--dry-run"]) + assert result.exit_code == 0 + assert "Detected install method: pipx" in strip_ansi(result.output) + assert mock_run.call_count == 0 + + +# =========================================================================== +# Phase 5 — User Story 3: non-upgradable path guidance (P3) +# =========================================================================== + + +class TestUvxEphemeral: + """uvx ephemeral path emits exact one-liner, no installer call.""" + + def test_uvx_argv0_prints_exact_one_liner_and_exits_zero( + self, + uvx_ephemeral_argv0, + clean_environ, + ): + with patch("specify_cli.urllib.request.urlopen") as mock_urlopen, patch( + "specify_cli.subprocess.run" + ) as mock_run: + mock_urlopen.return_value = _mock_urlopen_response({"tag_name": "v0.7.6"}) + result = runner.invoke(app, ["self", "upgrade"]) + assert result.exit_code == 0 + expected = ( + "Running via uvx (ephemeral); the next uvx invocation already " + "resolves to latest — no upgrade action needed." + ) + assert expected in strip_ansi(result.output) + assert mock_run.call_count == 0 + + +class TestSourceCheckout: + """Editable install path emits git pull guidance.""" + + def test_source_checkout_prints_git_pull_guidance( + self, + unsupported_argv0, + tmp_path, + clean_environ, + ): + fake_tree = tmp_path / "worktree" + fake_tree.mkdir() + (fake_tree / ".git").mkdir() + + with patch("specify_cli._editable_marker_seen", return_value=True), patch( + "specify_cli._source_checkout_path", return_value=fake_tree + ), patch("specify_cli.urllib.request.urlopen") as mock_urlopen, patch( + "specify_cli.subprocess.run" + ) as mock_run: + mock_urlopen.return_value = _mock_urlopen_response({"tag_name": "v0.7.6"}) + result = runner.invoke(app, ["self", "upgrade"]) + + assert result.exit_code == 0 + out = strip_ansi(result.output) + assert f"Running from a source checkout at {fake_tree}" in out + assert "git pull && pip install -e ." in out + assert mock_run.call_count == 0 + + +class TestUnsupported: + """Unsupported path enumerates manual reinstall commands.""" + + def test_unsupported_prints_both_reinstall_commands( + self, + unsupported_argv0, + clean_environ, + ): + with patch("specify_cli._editable_marker_seen", return_value=False), patch( + "specify_cli.shutil.which", return_value=None + ), patch("specify_cli.urllib.request.urlopen") as mock_urlopen, patch( + "specify_cli.subprocess.run" + ) as mock_run: + mock_urlopen.return_value = _mock_urlopen_response({"tag_name": "v0.7.6"}) + result = runner.invoke(app, ["self", "upgrade"]) + + assert result.exit_code == 0 + out = strip_ansi(result.output) + assert "Could not identify your install method automatically" in out + assert ( + "uv tool install --force --from " + "git+https://github.com/github/spec-kit.git@v0.7.6 specify-cli" + ) in out + assert ( + "pipx install --force git+https://github.com/github/spec-kit.git@v0.7.6" + ) in out + assert mock_run.call_count == 0 + + +class TestDryRun_NonUpgradablePaths: + """--dry-run on non-upgradable paths emits guidance, not preview.""" + + def test_dry_run_on_uvx_ephemeral_emits_guidance_not_preview( + self, + uvx_ephemeral_argv0, + clean_environ, + ): + with patch("specify_cli.urllib.request.urlopen") as mock_urlopen: + mock_urlopen.return_value = _mock_urlopen_response({"tag_name": "v0.7.6"}) + result = runner.invoke(app, ["self", "upgrade", "--dry-run"]) + assert result.exit_code == 0 + out = strip_ansi(result.output) + assert "Dry run — no changes will be made." not in out + assert "uvx (ephemeral)" in out + + def test_dry_run_on_unsupported_emits_manual_commands( + self, + unsupported_argv0, + clean_environ, + ): + with patch("specify_cli._editable_marker_seen", return_value=False), patch( + "specify_cli.shutil.which", return_value=None + ), patch("specify_cli.urllib.request.urlopen") as mock_urlopen: + mock_urlopen.return_value = _mock_urlopen_response({"tag_name": "v0.7.6"}) + result = runner.invoke(app, ["self", "upgrade", "--dry-run"]) + assert result.exit_code == 0 + assert "Could not identify your install method" in strip_ansi(result.output) + + +# =========================================================================== +# Phase 6 — User Story 4: failure recovery (P2) +# =========================================================================== + + +class TestInstallerMissing: + """Installer disappeared between detection and run → exit 3.""" + + def test_uv_missing_exits_3(self, uv_tool_argv0, clean_environ): + which_results = {"specify": "/usr/local/bin/specify"} + with patch("specify_cli.urllib.request.urlopen") as mock_urlopen, patch( + "specify_cli.shutil.which", side_effect=lambda n: which_results.get(n) + ), patch("specify_cli._get_installed_version", return_value="0.7.5"): + mock_urlopen.return_value = _mock_urlopen_response({"tag_name": "v0.7.6"}) + result = runner.invoke(app, ["self", "upgrade"]) + assert result.exit_code == 3 + assert "Installer uv not found on PATH; reinstall it and retry." in strip_ansi( + result.output + ) + + def test_pipx_missing_exits_3(self, pipx_argv0, clean_environ): + which_results: dict[str, str] = {} + with patch("specify_cli.urllib.request.urlopen") as mock_urlopen, patch( + "specify_cli.shutil.which", side_effect=lambda n: which_results.get(n) + ), patch("specify_cli._get_installed_version", return_value="0.7.5"): + mock_urlopen.return_value = _mock_urlopen_response({"tag_name": "v0.7.6"}) + result = runner.invoke(app, ["self", "upgrade"]) + assert result.exit_code == 3 + assert "Installer pipx not found on PATH" in strip_ansi(result.output) + + +class TestInstallerFailed: + """Installer non-zero exit → propagate code, print rollback hint.""" + + def test_installer_exit_2_propagates(self, uv_tool_argv0, clean_environ): + with patch("specify_cli.urllib.request.urlopen") as mock_urlopen, patch( + "specify_cli.shutil.which", return_value="/usr/bin/uv" + ), patch("specify_cli.subprocess.run") as mock_run, patch( + "specify_cli._get_installed_version", return_value="0.7.5" + ): + mock_urlopen.return_value = _mock_urlopen_response({"tag_name": "v0.7.6"}) + mock_run.side_effect = [_completed_process(2)] # installer fails + result = runner.invoke(app, ["self", "upgrade"]) + + assert result.exit_code == 2 + out = strip_ansi(result.output) + assert "Upgrade failed. Installer exit code: 2." in out + assert "Try again or run the command manually:" in out + assert "git+https://github.com/github/spec-kit.git@v0.7.6" in out + assert ( + "To pin back to the previous version: " + "uv tool install --force --from " + "git+https://github.com/github/spec-kit.git@v0.7.5 specify-cli" + ) in out + # No verification attempted after a failed installer run. + assert mock_run.call_count == 1 + + def test_installer_exit_127_propagates(self, uv_tool_argv0, clean_environ): + with patch("specify_cli.urllib.request.urlopen") as mock_urlopen, patch( + "specify_cli.shutil.which", return_value="/usr/bin/uv" + ), patch("specify_cli.subprocess.run") as mock_run, patch( + "specify_cli._get_installed_version", return_value="0.7.5" + ): + mock_urlopen.return_value = _mock_urlopen_response({"tag_name": "v0.7.6"}) + mock_run.side_effect = [_completed_process(127)] + result = runner.invoke(app, ["self", "upgrade"]) + assert result.exit_code == 127 + + +class TestVerificationMismatch: + """Installer says 0 but the binary is still the old version → exit 2.""" + + def test_installer_ok_but_verify_returns_old_version( + self, + uv_tool_argv0, + clean_environ, + ): + with patch("specify_cli.urllib.request.urlopen") as mock_urlopen, patch( + "specify_cli.shutil.which", return_value="/usr/bin/uv" + ), patch("specify_cli.subprocess.run") as mock_run, patch( + "specify_cli._get_installed_version", return_value="0.7.5" + ): + mock_urlopen.return_value = _mock_urlopen_response({"tag_name": "v0.7.6"}) + mock_run.side_effect = [ + _completed_process(0), # installer OK + _completed_process(0, stdout="specify 0.7.5\n"), # verify: OLD! + ] + result = runner.invoke(app, ["self", "upgrade"]) + + assert result.exit_code == 2 + out = strip_ansi(result.output) + assert "Verification failed" in out + assert "resolves to 0.7.5 (expected v0.7.6)" in out + assert "The new version may take effect on your next invocation." in out + + +class TestResolutionFailures: + """Pre-installer resolution failure → exit 1, reusing the resolver category strings.""" + + def test_offline_exits_1_with_phase1_string(self, uv_tool_argv0, clean_environ): + with patch( + "specify_cli.urllib.request.urlopen", + side_effect=urllib.error.URLError("nope"), + ): + result = runner.invoke(app, ["self", "upgrade"]) + assert result.exit_code == 1 + assert "Upgrade aborted: offline or timeout" in strip_ansi(result.output) + + def test_rate_limited_exits_1(self, uv_tool_argv0, clean_environ): + err = urllib.error.HTTPError( + url="https://api.github.com", + code=403, + msg="rate limited", + hdrs={}, # type: ignore[arg-type] + fp=None, + ) + with patch("specify_cli.urllib.request.urlopen", side_effect=err): + result = runner.invoke(app, ["self", "upgrade"]) + assert result.exit_code == 1 + assert ( + "Upgrade aborted: rate limited (try setting GH_TOKEN or GITHUB_TOKEN)" + in strip_ansi(result.output) + ) + + def test_http_500_exits_1(self, uv_tool_argv0, clean_environ): + err = urllib.error.HTTPError( + url="https://api.github.com", + code=500, + msg="srv err", + hdrs={}, # type: ignore[arg-type] + fp=None, + ) + with patch("specify_cli.urllib.request.urlopen", side_effect=err): + result = runner.invoke(app, ["self", "upgrade"]) + assert result.exit_code == 1 + assert "Upgrade aborted: HTTP 500" in strip_ansi(result.output) + + +class TestTagValidation: + """--tag regex enforcement.""" + + def test_valid_stable_tag(self, uv_tool_argv0, clean_environ): + with patch("specify_cli.shutil.which", return_value="/usr/bin/uv"), patch( + "specify_cli._get_installed_version", return_value="0.7.5" + ): + result = runner.invoke( + app, + ["self", "upgrade", "--dry-run", "--tag", "v0.7.6"], + ) + assert result.exit_code == 0 + + def test_valid_dev_suffix_tag(self, uv_tool_argv0, clean_environ): + with patch("specify_cli.shutil.which", return_value="/usr/bin/uv"), patch( + "specify_cli._get_installed_version", return_value="0.7.5" + ): + result = runner.invoke( + app, + ["self", "upgrade", "--dry-run", "--tag", "v0.8.0.dev0"], + ) + assert result.exit_code == 0 + assert "Target version: v0.8.0.dev0" in strip_ansi(result.output) + + def test_valid_rc_tag(self, uv_tool_argv0, clean_environ): + with patch("specify_cli.shutil.which", return_value="/usr/bin/uv"), patch( + "specify_cli._get_installed_version", return_value="0.7.5" + ): + result = runner.invoke( + app, + ["self", "upgrade", "--dry-run", "--tag", "v1.0.0-rc1"], + ) + assert result.exit_code == 0 + + def test_valid_build_metadata_tag(self, uv_tool_argv0, clean_environ): + with patch("specify_cli.shutil.which", return_value="/usr/bin/uv"), patch( + "specify_cli._get_installed_version", return_value="0.7.5" + ): + result = runner.invoke( + app, + ["self", "upgrade", "--dry-run", "--tag", "v0.8.0+build.42"], + ) + assert result.exit_code == 0 + assert "Target version: v0.8.0+build.42" in strip_ansi(result.output) + + @pytest.mark.parametrize("bad_tag", ["latest", "0.7.5", "main", "v7", ""]) + def test_invalid_tags_rejected(self, bad_tag, clean_environ): + result = runner.invoke(app, ["self", "upgrade", "--tag", bad_tag]) + # typer.BadParameter exits 1 or 2 depending on Typer version. + assert result.exit_code != 0 + combined = strip_ansi(result.output) + strip_ansi(result.stderr or "") + assert "Invalid --tag" in combined or "expected vMAJOR.MINOR.PATCH" in combined + + +class TestUnknownCurrent: + """'unknown' current version renders literally in notice and success message.""" + + def test_unknown_current_renders_literal_in_notice( + self, + uv_tool_argv0, + clean_environ, + ): + with patch("specify_cli.urllib.request.urlopen") as mock_urlopen, patch( + "specify_cli.shutil.which", return_value="/usr/bin/uv" + ), patch("specify_cli.subprocess.run") as mock_run, patch( + "specify_cli._get_installed_version", return_value="unknown" + ): + mock_urlopen.return_value = _mock_urlopen_response({"tag_name": "v0.7.6"}) + mock_run.side_effect = [ + _completed_process(0), + _completed_process(0, stdout="specify 0.7.6\n"), + ] + result = runner.invoke(app, ["self", "upgrade"]) + + assert result.exit_code == 0 + out = strip_ansi(result.output) + assert "Upgrading specify-cli unknown → v0.7.6 via uv tool:" in out + assert "Upgraded specify-cli: unknown → 0.7.6" in out + + def test_unknown_current_rollback_hint_degrades( + self, + uv_tool_argv0, + clean_environ, + ): + with patch("specify_cli.urllib.request.urlopen") as mock_urlopen, patch( + "specify_cli.shutil.which", return_value="/usr/bin/uv" + ), patch("specify_cli.subprocess.run") as mock_run, patch( + "specify_cli._get_installed_version", return_value="unknown" + ): + mock_urlopen.return_value = _mock_urlopen_response({"tag_name": "v0.7.6"}) + mock_run.side_effect = [_completed_process(2)] # installer fails + result = runner.invoke(app, ["self", "upgrade"]) + + assert result.exit_code == 2 + out = strip_ansi(result.output) + assert "Could not determine the previous version" in out + assert "https://github.com/github/spec-kit/releases" in out + + +class TestTokenScrubbing: + """GH_TOKEN / GITHUB_TOKEN are stripped from every child env.""" + + def test_env_passed_to_subprocess_has_no_github_tokens( + self, + uv_tool_argv0, + monkeypatch, + ): + monkeypatch.setenv("GH_TOKEN", SENTINEL_GH_TOKEN) + monkeypatch.setenv("GITHUB_TOKEN", SENTINEL_GITHUB_TOKEN) + + with patch("specify_cli.urllib.request.urlopen") as mock_urlopen, patch( + "specify_cli.shutil.which", return_value="/usr/bin/uv" + ), patch("specify_cli.subprocess.run") as mock_run, patch( + "specify_cli._get_installed_version", return_value="0.7.5" + ): + mock_urlopen.return_value = _mock_urlopen_response({"tag_name": "v0.7.6"}) + mock_run.side_effect = [ + _completed_process(0), + _completed_process(0, stdout="specify 0.7.6\n"), + ] + runner.invoke(app, ["self", "upgrade"]) + + assert mock_run.call_count >= 1 + for call in mock_run.call_args_list: + env_kwarg = call.kwargs.get("env") or {} + assert "GH_TOKEN" not in env_kwarg, f"env leaked GH_TOKEN: {env_kwarg!r}" + assert "GITHUB_TOKEN" not in env_kwarg + for v in env_kwarg.values(): + assert SENTINEL_GH_TOKEN not in v + assert SENTINEL_GITHUB_TOKEN not in v + + +class TestDryRunWithTag_SkipsNetwork: + """--dry-run with --tag never calls urlopen.""" + + def test_urlopen_not_called_when_tag_supplied_with_dry_run( + self, + uv_tool_argv0, + clean_environ, + ): + with patch("specify_cli.urllib.request.urlopen") as mock_urlopen, patch( + "specify_cli.shutil.which", return_value="/usr/bin/uv" + ), patch("specify_cli._get_installed_version", return_value="0.7.5"): + result = runner.invoke( + app, + ["self", "upgrade", "--dry-run", "--tag", "v0.8.0"], + ) + assert result.exit_code == 0 + mock_urlopen.assert_not_called() diff --git a/tests/test_upgrade.py b/tests/test_upgrade.py index 7169c44df0..22dcce3ad2 100644 --- a/tests/test_upgrade.py +++ b/tests/test_upgrade.py @@ -55,39 +55,6 @@ def _http_error(code: int, message: str = "error") -> urllib.error.HTTPError: ) -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): - # The stub must not hit the network via either urllib path: - # unauthenticated requests use urlopen() directly; authenticated ones - # go through build_opener(...).open(). Both are patched so that any - # accidental network call raises immediately. - network_error = AssertionError("stub must not hit the network") - with ( - patch( - "specify_cli.authentication.http.urllib.request.urlopen", - side_effect=network_error, - ), - patch( - "specify_cli.authentication.http.urllib.request.build_opener", - side_effect=network_error, - ), - ): - 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 From 6fc0cd2d1fda7e565e1bd92229ed4cec212e8133 Mon Sep 17 00:00:00 2001 From: pli Date: Thu, 7 May 2026 12:36:23 +0900 Subject: [PATCH 02/25] fix(cli): update self group help and self_check docstring to reflect Phase 2 The `specify self` Typer help and `self_check()` docstring were still describing the v0.7.5 stub state ("reserved upgrade command", "future release will use for actual self-upgrade", "behavior is not implemented in this release"). Now that `specify self upgrade` is a real, working command, these strings contradicted the implementation when surfaced through `specify self --help` and `specify self check --help`. - self_app help: "Manage the specify CLI itself: check for newer releases (read-only) and upgrade in place." - self_check docstring: points readers at `specify self upgrade` and `specify self upgrade --dry-run` as the natural follow-ups, instead of claiming the upgrade command is unimplemented. No behavior change. --- src/specify_cli/__init__.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 43b31a824b..3a1050068a 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -2477,7 +2477,7 @@ def _emit_failure( # ===== Self Commands ===== self_app = typer.Typer( name="self", - help="Manage the specify CLI itself (read-only check and reserved upgrade command).", + help="Manage the specify CLI itself: check for newer releases (read-only) and upgrade in place.", add_completion=False, ) app.add_typer(self_app, name="self") @@ -2487,10 +2487,9 @@ 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. + Use `specify self upgrade` to actually perform the upgrade once you've seen + the result here, or `specify self upgrade --dry-run` to preview the + installer command without running it. """ installed = _get_installed_version() From e481250c0d8d1d0b8c1655a745b2db9f67c837b5 Mon Sep 17 00:00:00 2001 From: pli Date: Thu, 7 May 2026 15:09:05 +0900 Subject: [PATCH 03/25] chore: trigger PR ref refresh From e364e30a4f5fc08c1a1580384ac3ace10103a6f4 Mon Sep 17 00:00:00 2001 From: pli Date: Thu, 7 May 2026 15:19:42 +0900 Subject: [PATCH 04/25] docs(cli): trim self-upgrade comments --- src/specify_cli/__init__.py | 105 ++++++++---------------------------- tests/test_self_upgrade.py | 42 ++------------- 2 files changed, 26 insertions(+), 121 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 3a1050068a..f8cff8461f 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1822,12 +1822,7 @@ def _fetch_latest_release_tag() -> tuple[str | None, str | None]: class _InstallMethod(str, Enum): - """Classify the runtime environment for `specify self upgrade`. - - String-valued so that f-string rendering uses the hyphenated lowercase - token directly (no .value ceremony). Exactly one member returned per - invocation. - """ + """Install-method classification for `specify self upgrade`.""" UV_TOOL = "uv-tool" PIPX = "pipx" @@ -1838,12 +1833,7 @@ class _InstallMethod(str, Enum): @dataclass(frozen=True) class _UpgradePlan: - """Frozen snapshot of the upgrade decision. - - Produced once per invocation by _build_upgrade_plan; consumed by both the - preview (--dry-run) path and the bare-apply path. installer_argv is None - iff method is UVX_EPHEMERAL / SOURCE_CHECKOUT / UNSUPPORTED. - """ + """Resolved upgrade decision shared by preview and apply paths.""" method: _InstallMethod current_version: str @@ -1855,13 +1845,7 @@ class _UpgradePlan: @dataclass(frozen=True) class _UpgradeOutcome: - """Frozen snapshot of the installer run. - - reconciliation_needed is True iff installer_exit_code == 0 AND - verified_version != plan.target_tag. failure_category is one of - strings (installer-missing / installer-failed / - verification-mismatch) or None on success. - """ + """Summary of installer execution and post-run verification.""" installer_exit_code: int | None verified_version: str | None @@ -1871,12 +1855,7 @@ class _UpgradeOutcome: @dataclass(frozen=True) class _DetectionSignals: - """Test-only record describing which detection tier fired. - - Production call sites discard this (default return of - _detect_install_method is a bare _InstallMethod); tests pass - include_signals=True to retrieve the pair and assert on the chosen tier. - """ + """Test-only record of which detection tier fired.""" sys_argv0: str matched_tier: int | None @@ -1887,11 +1866,7 @@ class _DetectionSignals: def _scrubbed_env() -> dict[str, str]: - """Return a copy of os.environ with GitHub token env vars removed. - - Used for both installer invocation and the post-upgrade - verification child process so neither inherits tokens it does not need. - """ + """Return a copy of `os.environ` without GitHub token variables.""" return { k: v for k, v in os.environ.items() if k not in {"GH_TOKEN", "GITHUB_TOKEN"} @@ -1915,11 +1890,7 @@ def _validate_tag(tag: str) -> str: def _expand_prefix(prefix: str) -> Path: - """Expand ~ and %LOCALAPPDATA% style tokens in a prefix string to a Path. - - Safe to call on POSIX with Windows-style prefixes (the expansion is a - no-op and the resulting Path will simply not match anything on disk). - """ + """Expand `~` or `%LOCALAPPDATA%`-style tokens in a path prefix.""" expanded = os.path.expandvars(os.path.expanduser(prefix)) return Path(expanded).resolve() if Path(expanded).is_absolute() else Path(expanded) @@ -1973,12 +1944,7 @@ def _editable_direct_url_path() -> Path | None: def _editable_marker_seen() -> bool: - """Return True if specify-cli's installed files live under a git worktree. - - Detects `pip install -e .` style editable installs by walking the dist's - file list for any path whose ancestor contains a `.git` directory. - Returns False when metadata is unreadable or no git ancestor is present. - """ + """Return whether the installed distribution appears to come from a git checkout.""" import importlib.metadata as _md editable_root = _editable_direct_url_path() @@ -2003,15 +1969,12 @@ def _detect_install_method( ) -> "_InstallMethod | tuple[_InstallMethod, _DetectionSignals]": """Classify the current runtime into exactly one _InstallMethod. - Tiers: - 1. sys.argv[0] path prefix match against _INSTALLER_PATH_PREFIXES. - 2. Editable-install marker (.git ancestor in dist files). - 3. PATH + installer registry reconciliation (uv tool list / pipx list). - Fallthrough -> UNSUPPORTED. + Detection order: + 1. `sys.argv[0]` path prefix match against `_INSTALLER_PATH_PREFIXES` + 2. editable-install marker + 3. installer registry reconciliation (`uv tool list` / `pipx list`) - When include_signals=True returns (_InstallMethod, _DetectionSignals) so - tests can assert on which tier fired. Detection is deterministic — no - wall-clock, no randomness. + When `include_signals=True`, also return `_DetectionSignals`. """ argv0_resolved = str(Path(argv0 or sys.argv[0]).resolve()) @@ -2122,12 +2085,7 @@ def _detect_install_method( def _assemble_installer_argv( method: _InstallMethod, target_tag: str ) -> list[str] | None: - """Build the installer subprocess argv for the given method and tag. - - Returns None for non-upgradable methods (UVX_EPHEMERAL, SOURCE_CHECKOUT, - UNSUPPORTED) so the caller routes to _emit_guidance instead. argv is - always a list[str] for shell=False invocation. - """ + """Build the installer argv for an upgradable install method.""" source_spec = f"{_GITHUB_SOURCE_URL}@{target_tag}" if method == _InstallMethod.UV_TOOL: @@ -2157,7 +2115,7 @@ def _assemble_installer_argv( def _method_label(method: _InstallMethod) -> str: - """Render a method enum as the human label used in notices (contracts §Output shapes).""" + """Render the user-facing label for an install method.""" return { _InstallMethod.UV_TOOL: "uv tool", _InstallMethod.PIPX: "pipx", @@ -2170,16 +2128,9 @@ def _method_label(method: _InstallMethod) -> str: def _build_upgrade_plan( target_tag_override: str | None, ) -> tuple[_UpgradePlan | None, str | None]: - """Build a _UpgradePlan or return (None, failure_reason). + """Return a resolved upgrade plan or `(None, failure_reason)`. - failure_reason is one of the resolver categories - when _fetch_latest_release_tag cannot produce a tag AND no override was - supplied. When target_tag_override is a valid pinned tag, the network - resolver is NOT called — this keeps `--dry-run --tag X` fast and network-free. - - The orchestrator branches on the (plan, reason) tuple: - - (plan, None) -> proceed to preview / apply path - - (None, reason) -> emit resolution-failure output + exit non-zero + A valid `target_tag_override` skips network resolution entirely. """ if target_tag_override is not None: target_tag = target_tag_override @@ -2227,9 +2178,8 @@ def _run_installer(plan: _UpgradePlan) -> subprocess.CompletedProcess | None: operations (dependency resolution, large wheel downloads) can legitimately take many minutes. Set the env var SPECIFY_UPGRADE_TIMEOUT_SECS to an integer/float to enforce a hard cap; on timeout we return a synthetic - CompletedProcess with returncode=124 so the orchestrator surfaces the - same installer-failed code path the user already understands. An - unparseable or non-positive value is silently ignored (no timeout). + CompletedProcess with returncode=124. An unparseable or non-positive + value is silently ignored (no timeout). """ if plan.installer_argv is None: # Internal routing error: the orchestrator must route non-upgradable @@ -2316,7 +2266,7 @@ def _verify_upgrade(plan: _UpgradePlan) -> str | None: def _source_checkout_path() -> Path | None: - """Best-effort: return the working-tree root for editable installs.""" + """Return the working-tree root for an editable install when discoverable.""" import importlib.metadata as _md editable_root = _editable_direct_url_path() @@ -2385,13 +2335,7 @@ def _emit_guidance(method: _InstallMethod, target_tag: str) -> None: def _rollback_hint(plan: _UpgradePlan) -> str: - """Construct the rollback-suggestion line. - - Never a destructive action — always a command the user can run by hand. - Uses the pre-upgrade version snapshot captured before the installer ran; - when that snapshot is 'unknown' the hint degrades to a releases-page - pointer rather than printing a malformed git ref. - """ + """Build a manual rollback suggestion from the pre-upgrade version.""" if plan.pre_upgrade_snapshot == "unknown": return ( "Could not determine the previous version; " @@ -2411,14 +2355,7 @@ def _emit_failure( installer_name: str | None = None, verified_version: str | None = None, ) -> None: - """Render the user-visible failure output for any documented category. - - Resolver categories are forwarded as-is so the failure vocabulary stays - identical between `self check` and `self upgrade`. Execution categories - (installer-missing, installer-failed, verification-mismatch) format - their own multi-line block including a manual-retry argv and rollback - hint where applicable. - """ + """Render user-facing output for resolver, installer, or verification failures.""" if ( category in _RESOLUTION_FAILURE_CATEGORIES or category.startswith("HTTP ") diff --git a/tests/test_self_upgrade.py b/tests/test_self_upgrade.py index fc3226cd5b..9edd618c6a 100644 --- a/tests/test_self_upgrade.py +++ b/tests/test_self_upgrade.py @@ -1,24 +1,7 @@ -"""Tests for the `specify self upgrade` actual-upgrade implementation (Phase 2). - -Network + subprocess isolation contract: - - Every `subprocess.run` call MUST be patched via unittest.mock. - - Every `shutil.which` call MUST be patched. - - Every `urllib.request.urlopen` call MUST be patched (inherits from Phase 1). - - No real network traffic, no real subprocess execution, no filesystem - writes outside the test's own tmp_path fixture. - -Phase-1 byte-identity gate: - - `tests/test_upgrade.py` covers `specify self check` and must pass - unmodified on this branch — except for the removal of - `TestSelfUpgradeStub` (the Phase-1 stub is the thing being replaced - by this feature). - -This file is the TDD red phase. Tests reference Phase-2 symbols -(`_InstallMethod`, `_UpgradePlan`, `_detect_install_method`, etc.) -that are added during Phase 2 implementation. -Until those symbols exist the import block at the top of this file -will fail and pytest will mark every case as a collection error. -That failure mode IS the red phase — proceed to implementation. +"""Tests for `specify self upgrade`. + +These cases patch subprocess, PATH lookup, and release-tag resolution so the +suite stays isolated from the real environment. """ import json @@ -29,7 +12,7 @@ import pytest from typer.testing import CliRunner -from specify_cli import ( # noqa: F401 — symbols added in Phase 2 +from specify_cli import ( _InstallMethod, _UpgradePlan, _UpgradeOutcome, @@ -56,11 +39,6 @@ SENTINEL_GITHUB_TOKEN = "SENTINEL-GITHUB-TOKEN-VALUE" -# --------------------------------------------------------------------------- -# Helpers -# --------------------------------------------------------------------------- - - def _mock_urlopen_response(payload: dict) -> MagicMock: """Build a urlopen() context-manager mock whose .read() returns the JSON payload.""" body = json.dumps(payload).encode("utf-8") @@ -84,11 +62,6 @@ def _completed_process( ) -# --------------------------------------------------------------------------- -# Fixtures -# --------------------------------------------------------------------------- - - @pytest.fixture def clean_environ(monkeypatch): """Strip any real GH_TOKEN / GITHUB_TOKEN from the test environment.""" @@ -149,11 +122,6 @@ def unsupported_argv0(monkeypatch, tmp_path): return fake_specify -# =========================================================================== -# Phase 3 — User Story 1: `uv tool` immediate upgrade (P1, MVP) -# =========================================================================== - - class TestDetection_UvTool: """Tier-1 path-prefix detection for uv-tool installs.""" From 5cb2c701d7c037592e1a5b31a919a52addd9ad66 Mon Sep 17 00:00:00 2001 From: pli Date: Thu, 7 May 2026 15:33:04 +0900 Subject: [PATCH 05/25] fix(cli): address self-upgrade review feedback --- src/specify_cli/__init__.py | 51 ++++++++++++++++++++------ tests/test_self_upgrade.py | 71 +++++++++++++++++++++++++++++++++---- 2 files changed, 104 insertions(+), 18 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index f8cff8461f..05e8bfa1d8 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1896,6 +1896,19 @@ def _expand_prefix(prefix: str) -> Path: return Path(expanded).resolve() if Path(expanded).is_absolute() else Path(expanded) +def _path_is_within_prefix(path: Path, prefix: Path) -> bool: + """Return whether `path` is under `prefix` using path-aware matching.""" + if not prefix.is_absolute(): + return False + try: + common = os.path.commonpath( + [os.path.normcase(str(path)), os.path.normcase(str(prefix))] + ) + except ValueError: + return False + return common == os.path.normcase(str(prefix)) + + def _git_ancestor(path: Path) -> Path | None: """Return the closest ancestor that looks like a git worktree root.""" for ancestor in [path, *path.parents]: @@ -1976,13 +1989,14 @@ def _detect_install_method( When `include_signals=True`, also return `_DetectionSignals`. """ - argv0_resolved = str(Path(argv0 or sys.argv[0]).resolve()) + argv0_path = Path(argv0 or sys.argv[0]).resolve() + argv0_resolved = str(argv0_path) # --- Tier 1: path prefix match --- for method_str, prefixes in _INSTALLER_PATH_PREFIXES.items(): for prefix in prefixes: expanded = _expand_prefix(prefix) - if argv0_resolved.startswith(str(expanded)): + if _path_is_within_prefix(argv0_path, expanded): method = _InstallMethod(method_str) if include_signals: return method, _DetectionSignals( @@ -2094,10 +2108,10 @@ def _assemble_installer_argv( uv_bin, "tool", "install", + "specify-cli", "--force", "--from", source_spec, - "specify-cli", ] if method == _InstallMethod.PIPX: @@ -2196,8 +2210,13 @@ def _run_installer(plan: _UpgradePlan) -> subprocess.CompletedProcess | None: # saw printed. A lightweight pre-flight via `shutil.which` short-circuits # the obvious "binary disappeared" case before spawning, and the # try/except below catches the residual race window. - installer_name = Path(plan.installer_argv[0]).name - if shutil.which(installer_name) is None: + installer_cmd = Path(plan.installer_argv[0]) + if installer_cmd.is_absolute(): + if installer_cmd.exists() and ( + not installer_cmd.is_file() or not os.access(installer_cmd, os.X_OK) + ): + return None + elif shutil.which(plan.installer_argv[0]) is None: return None # signals installer-missing to caller timeout_raw = os.environ.get("SPECIFY_UPGRADE_TIMEOUT_SECS") @@ -2318,8 +2337,8 @@ def _emit_guidance(method: _InstallMethod, target_tag: str) -> None: soft_wrap=True, ) console.print( - f" uv tool install --force --from " - f"git+https://github.com/github/spec-kit.git@{target_tag} specify-cli", + f" uv tool install specify-cli --force --from " + f"git+https://github.com/github/spec-kit.git@{target_tag}", soft_wrap=True, ) console.print( @@ -2341,10 +2360,14 @@ def _rollback_hint(plan: _UpgradePlan) -> str: "Could not determine the previous version; " "reinstall manually from: https://github.com/github/spec-kit/releases" ) + if plan.method == _InstallMethod.PIPX: + return ( + f"To pin back to the previous version: pipx install --force " + f"git+https://github.com/github/spec-kit.git@v{plan.pre_upgrade_snapshot}" + ) return ( - f"To pin back to the previous version: uv tool install --force --from " - f"git+https://github.com/github/spec-kit.git@v{plan.pre_upgrade_snapshot} " - f"specify-cli" + f"To pin back to the previous version: uv tool install specify-cli --force " + f"--from git+https://github.com/github/spec-kit.git@v{plan.pre_upgrade_snapshot}" ) @@ -2480,7 +2503,6 @@ def self_upgrade( "--tag", help="Pin the target version (vX.Y.Z[suffix]). Without --tag, the " "latest stable release is resolved via GitHub Releases.", - callback=lambda v: _validate_tag(v) if v is not None else None, ), ) -> None: """Upgrade specify-cli to the latest release (or a pinned --tag). @@ -2510,6 +2532,13 @@ def self_upgrade( long the installer subprocess may run. Unset (default) means no timeout — interrupt with Ctrl+C if the installer hangs. """ + if tag is not None: + try: + tag = _validate_tag(tag) + except typer.BadParameter as exc: + console.print(str(exc), soft_wrap=True) + raise typer.Exit(1) from exc + plan, failure_reason = _build_upgrade_plan(target_tag_override=tag) # Resolver could not produce a tag → surface the categorized failure diff --git a/tests/test_self_upgrade.py b/tests/test_self_upgrade.py index 9edd618c6a..1067f5be70 100644 --- a/tests/test_self_upgrade.py +++ b/tests/test_self_upgrade.py @@ -147,6 +147,19 @@ def test_include_signals_false_returns_bare_enum(self, uv_tool_argv0): result = _detect_install_method(include_signals=False) assert isinstance(result, _InstallMethod) + def test_prefix_match_does_not_accept_sibling_directory(self, monkeypatch, tmp_path): + monkeypatch.setenv("HOME", str(tmp_path)) + fake_dir = tmp_path / ".local" / "share" / "uv" / "tools" / "specify-cli2" / "bin" + fake_dir.mkdir(parents=True) + fake_specify = fake_dir / "specify" + fake_specify.write_text("#!/usr/bin/env python\n") + monkeypatch.setattr("sys.argv", [str(fake_specify)]) + with patch("specify_cli.shutil.which", return_value=None), patch( + "specify_cli._editable_marker_seen", return_value=False + ): + method = _detect_install_method() + assert method == _InstallMethod.UNSUPPORTED + class TestArgvAssembly_UvTool: """uv-tool installer argv shape.""" @@ -158,10 +171,10 @@ def test_stable_tag_produces_expected_argv(self): "/usr/bin/uv", "tool", "install", + "specify-cli", "--force", "--from", "git+https://github.com/github/spec-kit.git@v0.7.6", - "specify-cli", ] def test_dev_suffix_tag_embedded_literally(self): @@ -498,8 +511,8 @@ def test_unsupported_prints_both_reinstall_commands( out = strip_ansi(result.output) assert "Could not identify your install method automatically" in out assert ( - "uv tool install --force --from " - "git+https://github.com/github/spec-kit.git@v0.7.6 specify-cli" + "uv tool install specify-cli --force --from " + "git+https://github.com/github/spec-kit.git@v0.7.6" ) in out assert ( "pipx install --force git+https://github.com/github/spec-kit.git@v0.7.6" @@ -567,6 +580,35 @@ def test_pipx_missing_exits_3(self, pipx_argv0, clean_environ): assert result.exit_code == 3 assert "Installer pipx not found on PATH" in strip_ansi(result.output) + def test_absolute_installer_path_does_not_require_path_lookup( + self, uv_tool_argv0, clean_environ, tmp_path + ): + fake_uv = tmp_path / "uv" + fake_uv.write_text("#!/bin/sh\n") + fake_uv.chmod(0o755) + with patch("specify_cli.urllib.request.urlopen") as mock_urlopen, patch( + "specify_cli.shutil.which", side_effect=lambda name: None + ), patch("specify_cli.subprocess.run") as mock_run, patch( + "specify_cli._get_installed_version", return_value="0.7.5" + ), patch( + "specify_cli._verify_upgrade", return_value="0.7.6" + ), patch( + "specify_cli._assemble_installer_argv", + return_value=[ + str(fake_uv), + "tool", + "install", + "specify-cli", + "--force", + "--from", + "git+https://github.com/github/spec-kit.git@v0.7.6", + ], + ): + mock_urlopen.return_value = _mock_urlopen_response({"tag_name": "v0.7.6"}) + mock_run.side_effect = [_completed_process(0)] + result = runner.invoke(app, ["self", "upgrade"]) + assert result.exit_code == 0 + class TestInstallerFailed: """Installer non-zero exit → propagate code, print rollback hint.""" @@ -588,8 +630,8 @@ def test_installer_exit_2_propagates(self, uv_tool_argv0, clean_environ): assert "git+https://github.com/github/spec-kit.git@v0.7.6" in out assert ( "To pin back to the previous version: " - "uv tool install --force --from " - "git+https://github.com/github/spec-kit.git@v0.7.5 specify-cli" + "uv tool install specify-cli --force --from " + "git+https://github.com/github/spec-kit.git@v0.7.5" ) in out # No verification attempted after a failed installer run. assert mock_run.call_count == 1 @@ -605,6 +647,22 @@ def test_installer_exit_127_propagates(self, uv_tool_argv0, clean_environ): result = runner.invoke(app, ["self", "upgrade"]) assert result.exit_code == 127 + def test_pipx_failure_prints_pipx_rollback_hint(self, pipx_argv0, clean_environ): + with patch("specify_cli.urllib.request.urlopen") as mock_urlopen, patch( + "specify_cli.shutil.which", return_value="/usr/bin/pipx" + ), patch("specify_cli.subprocess.run") as mock_run, patch( + "specify_cli._get_installed_version", return_value="0.7.5" + ): + mock_urlopen.return_value = _mock_urlopen_response({"tag_name": "v0.7.6"}) + mock_run.side_effect = [_completed_process(2)] + result = runner.invoke(app, ["self", "upgrade"]) + assert result.exit_code == 2 + out = strip_ansi(result.output) + assert ( + "To pin back to the previous version: pipx install --force " + "git+https://github.com/github/spec-kit.git@v0.7.5" + ) in out + class TestVerificationMismatch: """Installer says 0 but the binary is still the old version → exit 2.""" @@ -723,8 +781,7 @@ def test_valid_build_metadata_tag(self, uv_tool_argv0, clean_environ): @pytest.mark.parametrize("bad_tag", ["latest", "0.7.5", "main", "v7", ""]) def test_invalid_tags_rejected(self, bad_tag, clean_environ): result = runner.invoke(app, ["self", "upgrade", "--tag", bad_tag]) - # typer.BadParameter exits 1 or 2 depending on Typer version. - assert result.exit_code != 0 + assert result.exit_code == 1 combined = strip_ansi(result.output) + strip_ansi(result.stderr or "") assert "Invalid --tag" in combined or "expected vMAJOR.MINOR.PATCH" in combined From 42bee3f32657674db92a86131aafcfd3e18c182b Mon Sep 17 00:00:00 2001 From: pli Date: Thu, 7 May 2026 15:55:53 +0900 Subject: [PATCH 06/25] fix(cli): tighten self-upgrade edge cases --- src/specify_cli/__init__.py | 60 +++++++++++++---------- tests/test_self_upgrade.py | 95 ++++++++++++++++++++++++++++++++----- 2 files changed, 119 insertions(+), 36 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 05e8bfa1d8..0e00a0ff16 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1837,7 +1837,7 @@ class _UpgradePlan: method: _InstallMethod current_version: str - target_tag: str + target_tag: str | None installer_argv: list[str] | None preview_summary: str pre_upgrade_snapshot: str @@ -2049,7 +2049,7 @@ def _detect_install_method( resolved_method=method, ) return method - except (subprocess.TimeoutExpired, OSError): + except (subprocess.TimeoutExpired, OSError, ValueError): pass pipx_bin = shutil.which("pipx") @@ -2064,19 +2064,22 @@ def _detect_install_method( env=_scrubbed_env(), check=False, ) - if result.returncode == 0 and "specify-cli" in (result.stdout or ""): - method = _InstallMethod.PIPX - if include_signals: - return method, _DetectionSignals( - sys_argv0=argv0_resolved, - matched_tier=3, - matched_prefix=None, - editable_marker_seen=False, - installer_registries_consulted=consulted, - resolved_method=method, - ) - return method - except (subprocess.TimeoutExpired, OSError): + if result.returncode == 0: + payload = json.loads(result.stdout or "") + venvs = payload.get("venvs") if isinstance(payload, dict) else None + if isinstance(venvs, dict) and "specify-cli" in venvs: + method = _InstallMethod.PIPX + if include_signals: + return method, _DetectionSignals( + sys_argv0=argv0_resolved, + matched_tier=3, + matched_prefix=None, + editable_marker_seen=False, + installer_registries_consulted=consulted, + resolved_method=method, + ) + return method + except (subprocess.TimeoutExpired, OSError, ValueError): pass # Fallthrough @@ -2096,11 +2099,16 @@ def _detect_install_method( _GITHUB_SOURCE_URL = "git+https://github.com/github/spec-kit.git" +def _source_spec(target_tag: str | None) -> str: + """Build a git source spec, optionally pinned to a release tag.""" + return f"{_GITHUB_SOURCE_URL}@{target_tag}" if target_tag else _GITHUB_SOURCE_URL + + def _assemble_installer_argv( - method: _InstallMethod, target_tag: str + method: _InstallMethod, target_tag: str | None ) -> list[str] | None: """Build the installer argv for an upgradable install method.""" - source_spec = f"{_GITHUB_SOURCE_URL}@{target_tag}" + source_spec = _source_spec(target_tag) if method == _InstallMethod.UV_TOOL: uv_bin = shutil.which("uv") or "uv" # resolve at invocation time too @@ -2146,22 +2154,25 @@ def _build_upgrade_plan( A valid `target_tag_override` skips network resolution entirely. """ + method = _detect_install_method() + if target_tag_override is not None: target_tag = target_tag_override - else: + elif method in (_InstallMethod.UV_TOOL, _InstallMethod.PIPX): tag, failure_reason = _fetch_latest_release_tag() if tag is None: return None, failure_reason # surfaces as exit 1 in the orchestrator target_tag = tag + else: + target_tag = None - method = _detect_install_method() current = _get_installed_version() argv = _assemble_installer_argv(method, target_tag) preview = ( f"Detected install method: {_method_label(method)}\n" f"Current version: {current}\n" - f"Target version: {target_tag}\n" + f"Target version: {target_tag or '(not resolved for this install method)'}\n" f"Command that would be executed: " f"{shlex.join(argv) if argv is not None else '(none — non-upgradable path)'}" ) @@ -2280,6 +2291,8 @@ def _verify_upgrade(plan: _UpgradePlan) -> str | None: ) except (subprocess.TimeoutExpired, OSError): return None + if result.returncode != 0: + return None match = _VERIFY_VERSION_REGEX.search(result.stdout or "") return match.group(1) if match else None @@ -2310,7 +2323,7 @@ def _source_checkout_path() -> Path | None: return None -def _emit_guidance(method: _InstallMethod, target_tag: str) -> None: +def _emit_guidance(method: _InstallMethod, target_tag: str | None) -> None: """Print path-specific guidance for non-upgradable install methods.""" if method == _InstallMethod.UVX_EPHEMERAL: console.print( @@ -2338,12 +2351,11 @@ def _emit_guidance(method: _InstallMethod, target_tag: str) -> None: ) console.print( f" uv tool install specify-cli --force --from " - f"git+https://github.com/github/spec-kit.git@{target_tag}", + f"{_source_spec(target_tag)}", soft_wrap=True, ) console.print( - f" pipx install --force " - f"git+https://github.com/github/spec-kit.git@{target_tag}", + f" pipx install --force {_source_spec(target_tag)}", soft_wrap=True, ) return diff --git a/tests/test_self_upgrade.py b/tests/test_self_upgrade.py index 1067f5be70..8674eae9b3 100644 --- a/tests/test_self_upgrade.py +++ b/tests/test_self_upgrade.py @@ -14,19 +14,10 @@ from specify_cli import ( _InstallMethod, - _UpgradePlan, - _UpgradeOutcome, - _DetectionSignals, _editable_marker_seen, _source_checkout_path, _detect_install_method, _assemble_installer_argv, - _build_upgrade_plan, - _run_installer, - _verify_upgrade, - _emit_guidance, - _emit_failure, - _scrubbed_env, _validate_tag, app, ) @@ -327,6 +318,31 @@ def fake_run(argv, *args, **kwargs): assert signals.matched_tier == 3 assert "pipx list --json" in signals.installer_registries_consulted + def test_tier3_pipx_ignores_malformed_json_output( + self, + unsupported_argv0, + ): + def fake_which(name): + return "/usr/bin/pipx" if name == "pipx" else None + + def fake_run(argv, *args, **kwargs): + if argv[0] == "/usr/bin/pipx" and argv[1] == "list": + return subprocess.CompletedProcess( + args=argv, + returncode=0, + stdout="not json but mentions specify-cli", + stderr="", + ) + return subprocess.CompletedProcess( + args=argv, returncode=1, stdout="", stderr="" + ) + + with patch("specify_cli.shutil.which", side_effect=fake_which), patch( + "specify_cli.subprocess.run", side_effect=fake_run + ), patch("specify_cli._editable_marker_seen", return_value=False): + method = _detect_install_method() + assert method == _InstallMethod.UNSUPPORTED + class TestEditableInstallMetadata: def test_direct_url_editable_install_marks_source_checkout(self, tmp_path): @@ -462,6 +478,19 @@ def test_uvx_argv0_prints_exact_one_liner_and_exits_zero( assert expected in strip_ansi(result.output) assert mock_run.call_count == 0 + def test_offline_still_exits_zero_without_tag_resolution( + self, + uvx_ephemeral_argv0, + clean_environ, + ): + with patch( + "specify_cli.urllib.request.urlopen", + side_effect=AssertionError("non-upgradable uvx path must not hit network"), + ): + result = runner.invoke(app, ["self", "upgrade"]) + assert result.exit_code == 0 + assert "uvx (ephemeral)" in strip_ansi(result.output) + class TestSourceCheckout: """Editable install path emits git pull guidance.""" @@ -512,12 +541,32 @@ def test_unsupported_prints_both_reinstall_commands( assert "Could not identify your install method automatically" in out assert ( "uv tool install specify-cli --force --from " - "git+https://github.com/github/spec-kit.git@v0.7.6" + "git+https://github.com/github/spec-kit.git" ) in out + assert "pipx install --force git+https://github.com/github/spec-kit.git" in out + assert mock_run.call_count == 0 + + def test_unsupported_offline_degrades_to_unpinned_manual_commands( + self, + unsupported_argv0, + clean_environ, + ): + with patch("specify_cli._editable_marker_seen", return_value=False), patch( + "specify_cli.shutil.which", return_value=None + ), patch( + "specify_cli.urllib.request.urlopen", + side_effect=AssertionError("unsupported guidance should not require network"), + ): + result = runner.invoke(app, ["self", "upgrade"]) + + assert result.exit_code == 0 + out = strip_ansi(result.output) + assert "Could not identify your install method automatically" in out assert ( - "pipx install --force git+https://github.com/github/spec-kit.git@v0.7.6" + "uv tool install specify-cli --force --from " + "git+https://github.com/github/spec-kit.git" ) in out - assert mock_run.call_count == 0 + assert "pipx install --force git+https://github.com/github/spec-kit.git" in out class TestDryRun_NonUpgradablePaths: @@ -690,6 +739,28 @@ def test_installer_ok_but_verify_returns_old_version( assert "resolves to 0.7.5 (expected v0.7.6)" in out assert "The new version may take effect on your next invocation." in out + def test_verify_nonzero_exit_is_not_treated_as_success( + self, + uv_tool_argv0, + clean_environ, + ): + with patch("specify_cli.urllib.request.urlopen") as mock_urlopen, patch( + "specify_cli.shutil.which", return_value="/usr/bin/uv" + ), patch("specify_cli.subprocess.run") as mock_run, patch( + "specify_cli._get_installed_version", return_value="0.7.5" + ): + mock_urlopen.return_value = _mock_urlopen_response({"tag_name": "v0.7.6"}) + mock_run.side_effect = [ + _completed_process(0), + _completed_process(1, stdout="specify 0.7.6\n"), + ] + result = runner.invoke(app, ["self", "upgrade"]) + + assert result.exit_code == 2 + out = strip_ansi(result.output) + assert "Verification failed" in out + assert "(unknown) (expected v0.7.6)" in out + class TestResolutionFailures: """Pre-installer resolution failure → exit 1, reusing the resolver category strings.""" From ce0b3c02bd1ed71b50dd6cd062edc08f6dfaae68 Mon Sep 17 00:00:00 2001 From: pli Date: Thu, 7 May 2026 16:07:46 +0900 Subject: [PATCH 07/25] fix(cli): verify against current entrypoint --- src/specify_cli/__init__.py | 25 ++++++------------------- tests/test_self_upgrade.py | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 19 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 0e00a0ff16..ec9ac71d70 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1812,14 +1812,6 @@ def _fetch_latest_release_tag() -> tuple[str | None, str | None]: } ) -_EXECUTION_FAILURE_CATEGORIES: frozenset[str] = frozenset( - { - "installer-missing", - "installer-failed", - "verification-mismatch", - } -) - class _InstallMethod(str, Enum): """Install-method classification for `specify self upgrade`.""" @@ -1843,16 +1835,6 @@ class _UpgradePlan: pre_upgrade_snapshot: str -@dataclass(frozen=True) -class _UpgradeOutcome: - """Summary of installer execution and post-run verification.""" - - installer_exit_code: int | None - verified_version: str | None - reconciliation_needed: bool - failure_category: str | None - - @dataclass(frozen=True) class _DetectionSignals: """Test-only record of which detection tier fired.""" @@ -2276,7 +2258,12 @@ def _verify_upgrade(plan: _UpgradePlan) -> str | None: cannot hot-swap the running module after the installer has replaced it — only a fresh process picks up the new binary. """ - specify_bin = shutil.which("specify") + argv0 = Path(sys.argv[0]).resolve() + specify_bin = ( + str(argv0) + if argv0.exists() and argv0.is_file() and os.access(argv0, os.X_OK) + else shutil.which("specify") + ) if specify_bin is None: return None try: diff --git a/tests/test_self_upgrade.py b/tests/test_self_upgrade.py index 8674eae9b3..3054f17854 100644 --- a/tests/test_self_upgrade.py +++ b/tests/test_self_upgrade.py @@ -761,6 +761,40 @@ def test_verify_nonzero_exit_is_not_treated_as_success( assert "Verification failed" in out assert "(unknown) (expected v0.7.6)" in out + def test_verify_uses_current_entrypoint_when_not_on_path( + self, + uv_tool_argv0, + clean_environ, + ): + from specify_cli import _UpgradePlan, _verify_upgrade + + def fake_which(name): + if name == "specify": + return None + return None + + plan = _UpgradePlan( + method=_InstallMethod.UV_TOOL, + current_version="0.7.5", + target_tag="v0.7.6", + installer_argv=["/usr/bin/uv", "tool", "install", "specify-cli"], + preview_summary="", + pre_upgrade_snapshot="0.7.5", + ) + + with patch( + "specify_cli.shutil.which", side_effect=fake_which + ), patch("specify_cli.subprocess.run") as mock_run, patch( + "specify_cli.sys.argv", [str(uv_tool_argv0)] + ), patch( + "specify_cli.os.access", return_value=True + ): + mock_run.return_value = _completed_process(0, stdout="specify 0.7.6\n") + verified = _verify_upgrade(plan) + + assert verified == "0.7.6" + assert mock_run.call_args.args[0][0] == str(uv_tool_argv0) + class TestResolutionFailures: """Pre-installer resolution failure → exit 1, reusing the resolver category strings.""" From 4a95701837d76d138401b2cf0df8d709d0176dac Mon Sep 17 00:00:00 2001 From: pli Date: Thu, 7 May 2026 20:38:42 +0900 Subject: [PATCH 08/25] fix(cli): improve self-upgrade entrypoint detection --- src/specify_cli/__init__.py | 31 +++++++++++++++++++++++++++++-- tests/test_self_upgrade.py | 28 ++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 2 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index ec9ac71d70..54eebea103 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1891,6 +1891,17 @@ def _path_is_within_prefix(path: Path, prefix: Path) -> bool: return common == os.path.normcase(str(prefix)) +def _resolved_argv0_path(argv0: str | None = None) -> Path: + """Resolve the running entrypoint path, consulting PATH for bare commands.""" + raw = argv0 or sys.argv[0] + candidate = Path(raw) + if not candidate.is_absolute() and len(candidate.parts) == 1: + resolved = shutil.which(raw) or shutil.which("specify") + if resolved: + return Path(resolved).resolve() + return candidate.resolve() + + def _git_ancestor(path: Path) -> Path | None: """Return the closest ancestor that looks like a git worktree root.""" for ancestor in [path, *path.parents]: @@ -1971,7 +1982,7 @@ def _detect_install_method( When `include_signals=True`, also return `_DetectionSignals`. """ - argv0_path = Path(argv0 or sys.argv[0]).resolve() + argv0_path = _resolved_argv0_path(argv0) argv0_resolved = str(argv0_path) # --- Tier 1: path prefix match --- @@ -2258,7 +2269,7 @@ def _verify_upgrade(plan: _UpgradePlan) -> str | None: cannot hot-swap the running module after the installer has replaced it — only a fresh process picks up the new binary. """ - argv0 = Path(sys.argv[0]).resolve() + argv0 = _resolved_argv0_path() specify_bin = ( str(argv0) if argv0.exists() and argv0.is_file() and os.access(argv0, os.X_OK) @@ -2401,6 +2412,22 @@ def _emit_failure( "plan and installer_exit to be set" ) argv_str = shlex.join(plan.installer_argv) if plan.installer_argv else "" + if installer_exit == 124: + timeout_value = os.environ.get("SPECIFY_UPGRADE_TIMEOUT_SECS", "(unknown)") + console.print( + "Upgrade timed out while waiting for the installer subprocess.", + soft_wrap=True, + ) + console.print( + f"Configured timeout: SPECIFY_UPGRADE_TIMEOUT_SECS={timeout_value}", + soft_wrap=True, + ) + console.print( + f"Try again or run the command manually: {argv_str}", + soft_wrap=True, + ) + console.print(_rollback_hint(plan), soft_wrap=True) + return console.print( f"Upgrade failed. Installer exit code: {installer_exit}.", soft_wrap=True, diff --git a/tests/test_self_upgrade.py b/tests/test_self_upgrade.py index 3054f17854..dfbb6b5166 100644 --- a/tests/test_self_upgrade.py +++ b/tests/test_self_upgrade.py @@ -138,6 +138,17 @@ def test_include_signals_false_returns_bare_enum(self, uv_tool_argv0): result = _detect_install_method(include_signals=False) assert isinstance(result, _InstallMethod) + def test_bare_argv0_is_resolved_via_path_lookup(self, monkeypatch, tmp_path): + monkeypatch.setenv("HOME", str(tmp_path)) + fake_dir = tmp_path / ".local" / "share" / "uv" / "tools" / "specify-cli" / "bin" + fake_dir.mkdir(parents=True) + fake_specify = fake_dir / "specify" + fake_specify.write_text("#!/usr/bin/env python\n") + monkeypatch.setattr("sys.argv", ["specify"]) + with patch("specify_cli.shutil.which", side_effect=lambda name: str(fake_specify) if name == "specify" else None): + method = _detect_install_method() + assert method == _InstallMethod.UV_TOOL + def test_prefix_match_does_not_accept_sibling_directory(self, monkeypatch, tmp_path): monkeypatch.setenv("HOME", str(tmp_path)) fake_dir = tmp_path / ".local" / "share" / "uv" / "tools" / "specify-cli2" / "bin" @@ -696,6 +707,23 @@ def test_installer_exit_127_propagates(self, uv_tool_argv0, clean_environ): result = runner.invoke(app, ["self", "upgrade"]) assert result.exit_code == 127 + def test_installer_timeout_prints_timeout_specific_message( + self, uv_tool_argv0, clean_environ, monkeypatch + ): + monkeypatch.setenv("SPECIFY_UPGRADE_TIMEOUT_SECS", "12") + with patch("specify_cli.urllib.request.urlopen") as mock_urlopen, patch( + "specify_cli.shutil.which", return_value="/usr/bin/uv" + ), patch("specify_cli.subprocess.run") as mock_run, patch( + "specify_cli._get_installed_version", return_value="0.7.5" + ): + mock_urlopen.return_value = _mock_urlopen_response({"tag_name": "v0.7.6"}) + mock_run.side_effect = [_completed_process(124)] + result = runner.invoke(app, ["self", "upgrade"]) + assert result.exit_code == 124 + out = strip_ansi(result.output) + assert "Upgrade timed out while waiting for the installer subprocess." in out + assert "SPECIFY_UPGRADE_TIMEOUT_SECS=12" in out + def test_pipx_failure_prints_pipx_rollback_hint(self, pipx_argv0, clean_environ): with patch("specify_cli.urllib.request.urlopen") as mock_urlopen, patch( "specify_cli.shutil.which", return_value="/usr/bin/pipx" From 1587611e62c35584a3f02fdb1c025d0b4f3439fb Mon Sep 17 00:00:00 2001 From: pli Date: Thu, 7 May 2026 20:49:14 +0900 Subject: [PATCH 09/25] fix(cli): narrow self-upgrade verify entrypoint --- src/specify_cli/__init__.py | 12 +++++++++++- tests/test_self_upgrade.py | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 54eebea103..42bfab294c 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1902,6 +1902,11 @@ def _resolved_argv0_path(argv0: str | None = None) -> Path: return candidate.resolve() +def _looks_like_specify_entrypoint(path: Path) -> bool: + """Return whether a path looks like the `specify` CLI entrypoint.""" + return path.name.lower() in {"specify", "specify.exe"} + + def _git_ancestor(path: Path) -> Path | None: """Return the closest ancestor that looks like a git worktree root.""" for ancestor in [path, *path.parents]: @@ -2272,7 +2277,12 @@ def _verify_upgrade(plan: _UpgradePlan) -> str | None: argv0 = _resolved_argv0_path() specify_bin = ( str(argv0) - if argv0.exists() and argv0.is_file() and os.access(argv0, os.X_OK) + if ( + argv0.exists() + and argv0.is_file() + and os.access(argv0, os.X_OK) + and _looks_like_specify_entrypoint(argv0) + ) else shutil.which("specify") ) if specify_bin is None: diff --git a/tests/test_self_upgrade.py b/tests/test_self_upgrade.py index dfbb6b5166..fca381cdf8 100644 --- a/tests/test_self_upgrade.py +++ b/tests/test_self_upgrade.py @@ -823,6 +823,39 @@ def fake_which(name): assert verified == "0.7.6" assert mock_run.call_args.args[0][0] == str(uv_tool_argv0) + def test_verify_ignores_python_entrypoint_and_falls_back_to_specify( + self, + clean_environ, + tmp_path, + ): + from specify_cli import _UpgradePlan, _verify_upgrade + + fake_python = tmp_path / "python3" + fake_python.write_text("#!/bin/sh\n") + fake_python.chmod(0o755) + + plan = _UpgradePlan( + method=_InstallMethod.UV_TOOL, + current_version="0.7.5", + target_tag="v0.7.6", + installer_argv=["/usr/bin/uv", "tool", "install", "specify-cli"], + preview_summary="", + pre_upgrade_snapshot="0.7.5", + ) + + with patch( + "specify_cli.shutil.which", side_effect=lambda name: "/usr/local/bin/specify" if name == "specify" else None + ), patch("specify_cli.subprocess.run") as mock_run, patch( + "specify_cli.sys.argv", [str(fake_python)] + ), patch( + "specify_cli.os.access", return_value=True + ): + mock_run.return_value = _completed_process(0, stdout="specify 0.7.6\n") + verified = _verify_upgrade(plan) + + assert verified == "0.7.6" + assert mock_run.call_args.args[0][0] == "/usr/local/bin/specify" + class TestResolutionFailures: """Pre-installer resolution failure → exit 1, reusing the resolver category strings.""" From 4d6e1fe19d5a6ba74986c7f750fdf1324e855021 Mon Sep 17 00:00:00 2001 From: pli Date: Thu, 7 May 2026 21:04:08 +0900 Subject: [PATCH 10/25] fix(cli): respect pinned self-upgrade targets --- src/specify_cli/__init__.py | 42 +++++++++++++++++++++++++------- tests/test_self_upgrade.py | 48 +++++++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 9 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 42bfab294c..425d71f4c1 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -2189,9 +2189,11 @@ def _build_upgrade_plan( def _run_installer(plan: _UpgradePlan) -> subprocess.CompletedProcess | None: """Invoke the installer subprocess. - Returns None if the installer binary is not found on PATH (caller routes - to installer-missing output with exit 3). Returns a CompletedProcess for - every other case including non-zero exit — caller inspects returncode. + Returns None if the installer binary is not found on PATH. Returns a + synthetic CompletedProcess with returncode=126 when an absolute installer + path exists but is not executable or not a regular file. Returns a + CompletedProcess for every other case including non-zero exit — caller + inspects returncode. stdout/stderr are inherited (not captured) so the user sees installer progress in real time. The child environment has GH_TOKEN / @@ -2224,7 +2226,12 @@ def _run_installer(plan: _UpgradePlan) -> subprocess.CompletedProcess | None: if installer_cmd.exists() and ( not installer_cmd.is_file() or not os.access(installer_cmd, os.X_OK) ): - return None + return subprocess.CompletedProcess( + args=plan.installer_argv, + returncode=126, + stdout=None, + stderr=None, + ) elif shutil.which(plan.installer_argv[0]) is None: return None # signals installer-missing to caller @@ -2415,6 +2422,14 @@ def _emit_failure( ) return + if category == "installer-invalid": + name = installer_name or "(unknown)" + console.print( + f"Installer {name} is not an executable file; fix the path or reinstall it and retry.", + soft_wrap=True, + ) + return + if category == "installer-failed": if plan is None or installer_exit is None: raise RuntimeError( @@ -2616,11 +2631,13 @@ def self_upgrade( # No-op success when the user is already on the latest tag. latest_normalized = _normalize_tag(plan.target_tag) - if plan.current_version != "unknown" and not _is_newer( - latest_normalized, plan.current_version - ): - console.print(f"Already on latest release: {plan.target_tag}") - raise typer.Exit(0) + if plan.current_version != "unknown": + if tag is None and not _is_newer(latest_normalized, plan.current_version): + console.print(f"Already on latest release: {plan.target_tag}") + raise typer.Exit(0) + if tag is not None and plan.current_version == latest_normalized: + console.print(f"Already on requested release: {plan.target_tag}") + raise typer.Exit(0) # One-line pre-execution notice so the user sees exactly what will run # before the installer's own output starts streaming. @@ -2642,6 +2659,13 @@ def self_upgrade( _emit_failure("installer-missing", plan=plan, installer_name=installer_name) raise typer.Exit(3) + if completed.returncode == 126: + installer_name = ( + Path(plan.installer_argv[0]).name if plan.installer_argv else None + ) + _emit_failure("installer-invalid", plan=plan, installer_name=installer_name) + raise typer.Exit(3) + if completed.returncode != 0: _emit_failure( "installer-failed", diff --git a/tests/test_self_upgrade.py b/tests/test_self_upgrade.py index fca381cdf8..2e22a904cf 100644 --- a/tests/test_self_upgrade.py +++ b/tests/test_self_upgrade.py @@ -247,6 +247,26 @@ def test_already_latest_exits_zero_no_subprocess( assert "Already on latest release: v0.7.6" in strip_ansi(result.output) assert mock_run.call_count == 0 + def test_pinned_older_tag_still_runs_installer( + self, uv_tool_argv0, clean_environ + ): + with patch("specify_cli.shutil.which", return_value="/usr/bin/uv"), patch( + "specify_cli.subprocess.run" + ) as mock_run, patch( + "specify_cli._get_installed_version", return_value="0.7.6" + ): + mock_run.side_effect = [ + _completed_process(0), + _completed_process(0, stdout="specify 0.7.5\n"), + ] + result = runner.invoke(app, ["self", "upgrade", "--tag", "v0.7.5"]) + + assert result.exit_code == 0 + out = strip_ansi(result.output) + assert "Already on latest release" not in out + assert "Upgrading specify-cli 0.7.6 → v0.7.5 via uv tool:" in out + assert mock_run.call_count == 2 + class TestDryRun_UvTool: """--dry-run preview path + --dry-run combined with --tag.""" @@ -669,6 +689,34 @@ def test_absolute_installer_path_does_not_require_path_lookup( result = runner.invoke(app, ["self", "upgrade"]) assert result.exit_code == 0 + def test_absolute_installer_path_not_executable_gets_specific_message( + self, uv_tool_argv0, clean_environ, tmp_path + ): + fake_uv = tmp_path / "uv" + fake_uv.write_text("#!/bin/sh\n") + fake_uv.chmod(0o644) + with patch("specify_cli.urllib.request.urlopen") as mock_urlopen, patch( + "specify_cli.shutil.which", side_effect=lambda name: None + ), patch("specify_cli._get_installed_version", return_value="0.7.5"), patch( + "specify_cli._assemble_installer_argv", + return_value=[ + str(fake_uv), + "tool", + "install", + "specify-cli", + "--force", + "--from", + "git+https://github.com/github/spec-kit.git@v0.7.6", + ], + ): + mock_urlopen.return_value = _mock_urlopen_response({"tag_name": "v0.7.6"}) + result = runner.invoke(app, ["self", "upgrade"]) + assert result.exit_code == 3 + assert ( + "Installer uv is not an executable file; fix the path or reinstall it and retry." + in strip_ansi(result.output) + ) + class TestInstallerFailed: """Installer non-zero exit → propagate code, print rollback hint.""" From e50abf6bc0ab3f8e3b4769973692d3885dea9758 Mon Sep 17 00:00:00 2001 From: pli Date: Thu, 7 May 2026 21:18:45 +0900 Subject: [PATCH 11/25] fix(cli): harden self-upgrade metadata detection --- src/specify_cli/__init__.py | 9 ++++++--- tests/test_self_upgrade.py | 13 +++++++++++++ 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 425d71f4c1..85c6deffd3 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1968,7 +1968,10 @@ def _editable_marker_seen() -> bool: return False files = dist.files or [] for f in files: - abs_path = Path(dist.locate_file(f)).resolve() + try: + abs_path = Path(dist.locate_file(f)).resolve() + except Exception: + continue if _git_ancestor(abs_path) is not None: return True return False @@ -2547,7 +2550,7 @@ def self_upgrade( False, "--dry-run", help="Print the preview (method, current, target, installer argv) and " - "exit 0 without launching any subprocess.", + "exit 0 without launching the installer subprocess.", ), tag: Optional[str] = typer.Option( None, @@ -2574,7 +2577,7 @@ def self_upgrade( 1 target-tag resolution failure or --tag regex validation failure 2 verification mismatch (installer exited 0 but `specify --version` does not resolve to the target tag) - 3 installer binary not found on PATH + 3 installer binary not found on PATH, or resolved to a non-executable file 124 installer subprocess timed out (only when SPECIFY_UPGRADE_TIMEOUT_SECS is set) other installer exit code propagated verbatim diff --git a/tests/test_self_upgrade.py b/tests/test_self_upgrade.py index 2e22a904cf..2d651bc4d3 100644 --- a/tests/test_self_upgrade.py +++ b/tests/test_self_upgrade.py @@ -401,6 +401,19 @@ def locate_file(self, file): assert _editable_marker_seen() is True assert _source_checkout_path() == project_root.resolve() + def test_editable_marker_ignores_locate_file_errors(self): + class FakeDist: + files = ["bad"] + + def read_text(self, name): + return None + + def locate_file(self, file): + raise OSError("boom") + + with patch("importlib.metadata.distribution", return_value=FakeDist()): + assert _editable_marker_seen() is False + class TestArgvAssembly_Pipx: """pipx installer argv shape — pipx 1.5+ uses positional PACKAGE_SPEC, never `--spec` or `upgrade`.""" From c75d13943cee22e93481a9d3cfa2a71378513891 Mon Sep 17 00:00:00 2001 From: pli Date: Thu, 7 May 2026 21:34:02 +0900 Subject: [PATCH 12/25] docs(cli): polish self-upgrade messaging --- src/specify_cli/__init__.py | 18 ++++++++++++++---- tests/test_self_upgrade.py | 1 - 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 85c6deffd3..b3a034973d 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1743,6 +1743,12 @@ def _normalize_tag(tag: str) -> str: """ return tag[1:] if tag.startswith("v") else tag + +def _render_argv(argv: list[str]) -> str: + """Render argv for copy/paste on the current platform.""" + return subprocess.list2cmdline(argv) if os.name == "nt" else shlex.join(argv) + + def _is_newer(latest: str, current: str) -> bool: """Return True iff `latest` is strictly greater than `current` under PEP 440. @@ -2175,7 +2181,7 @@ def _build_upgrade_plan( f"Current version: {current}\n" f"Target version: {target_tag or '(not resolved for this install method)'}\n" f"Command that would be executed: " - f"{shlex.join(argv) if argv is not None else '(none — non-upgradable path)'}" + f"{_render_argv(argv) if argv is not None else '(none — non-upgradable path)'}" ) plan = _UpgradePlan( @@ -2439,7 +2445,7 @@ def _emit_failure( "internal routing error: installer-failed requires both " "plan and installer_exit to be set" ) - argv_str = shlex.join(plan.installer_argv) if plan.installer_argv else "" + argv_str = _render_argv(plan.installer_argv) if plan.installer_argv else "" if installer_exit == 124: timeout_value = os.environ.get("SPECIFY_UPGRADE_TIMEOUT_SECS", "(unknown)") console.print( @@ -2525,7 +2531,9 @@ def self_check() -> None: # 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("\nTo upgrade:") + console.print(" specify self upgrade") + console.print("\nManual fallback:") console.print(" uv tool install specify-cli --force \\") console.print(f" --from git+https://github.com/github/spec-kit.git@{tag}") return @@ -2533,6 +2541,8 @@ def self_check() -> None: if _is_newer(latest_normalized, installed): console.print(f"[green]Update available:[/green] {installed} → {latest_normalized}") console.print("\nTo upgrade:") + console.print(" specify self upgrade") + console.print("\nManual fallback:") console.print(" uv tool install specify-cli --force \\") console.print(f" --from git+https://github.com/github/spec-kit.git@{tag}") return @@ -2644,7 +2654,7 @@ def self_upgrade( # One-line pre-execution notice so the user sees exactly what will run # before the installer's own output starts streaming. - argv_str = shlex.join(plan.installer_argv) if plan.installer_argv else "" + argv_str = _render_argv(plan.installer_argv) if plan.installer_argv else "" console.print( f"Upgrading specify-cli {plan.current_version} → {plan.target_tag} " f"via {_method_label(plan.method)}: {argv_str}", diff --git a/tests/test_self_upgrade.py b/tests/test_self_upgrade.py index 2d651bc4d3..5e2e46f8cb 100644 --- a/tests/test_self_upgrade.py +++ b/tests/test_self_upgrade.py @@ -18,7 +18,6 @@ _source_checkout_path, _detect_install_method, _assemble_installer_argv, - _validate_tag, app, ) From 3cc721ec3a23d62bfb0ddeab8971000b14a4909b Mon Sep 17 00:00:00 2001 From: pli Date: Thu, 7 May 2026 21:55:41 +0900 Subject: [PATCH 13/25] docs(cli): clarify self-upgrade fallback messaging --- src/specify_cli/__init__.py | 8 ++++++-- tests/test_self_upgrade.py | 15 +++++++++++++++ tests/test_upgrade.py | 2 ++ 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index b3a034973d..b5ac670f28 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -2418,7 +2418,6 @@ def _emit_failure( if ( category in _RESOLUTION_FAILURE_CATEGORIES or category.startswith("HTTP ") - or category.startswith("http-") ): console.print(f"Upgrade aborted: {category}", soft_wrap=True) return @@ -2536,6 +2535,7 @@ def self_check() -> None: console.print("\nManual fallback:") console.print(" uv tool install specify-cli --force \\") console.print(f" --from git+https://github.com/github/spec-kit.git@{tag}") + console.print(f" pipx install --force git+https://github.com/github/spec-kit.git@{tag}") return if _is_newer(latest_normalized, installed): @@ -2545,6 +2545,7 @@ def self_check() -> None: console.print("\nManual fallback:") console.print(" uv tool install specify-cli --force \\") console.print(f" --from git+https://github.com/github/spec-kit.git@{tag}") + console.print(f" pipx install --force git+https://github.com/github/spec-kit.git@{tag}") return # Installed is parseable AND is >= latest → "up to date" (FR-006). @@ -2646,7 +2647,10 @@ def self_upgrade( latest_normalized = _normalize_tag(plan.target_tag) if plan.current_version != "unknown": if tag is None and not _is_newer(latest_normalized, plan.current_version): - console.print(f"Already on latest release: {plan.target_tag}") + if plan.current_version == latest_normalized: + console.print(f"Already on latest release: {plan.target_tag}") + else: + console.print(f"Already on latest release or newer: {plan.current_version}") raise typer.Exit(0) if tag is not None and plan.current_version == latest_normalized: console.print(f"Already on requested release: {plan.target_tag}") diff --git a/tests/test_self_upgrade.py b/tests/test_self_upgrade.py index 5e2e46f8cb..f13dc3426a 100644 --- a/tests/test_self_upgrade.py +++ b/tests/test_self_upgrade.py @@ -246,6 +246,21 @@ def test_already_latest_exits_zero_no_subprocess( assert "Already on latest release: v0.7.6" in strip_ansi(result.output) assert mock_run.call_count == 0 + def test_dev_build_ahead_of_release_reports_newer_noop( + self, uv_tool_argv0, clean_environ + ): + with patch("specify_cli.urllib.request.urlopen") as mock_urlopen, patch( + "specify_cli.subprocess.run" + ) as mock_run, patch( + "specify_cli.shutil.which", return_value="/usr/bin/uv" + ), patch("specify_cli._get_installed_version", return_value="0.7.7.dev0"): + mock_urlopen.return_value = _mock_urlopen_response({"tag_name": "v0.7.6"}) + result = runner.invoke(app, ["self", "upgrade"]) + + assert result.exit_code == 0 + assert "Already on latest release or newer: 0.7.7.dev0" in strip_ansi(result.output) + assert mock_run.call_count == 0 + def test_pinned_older_tag_still_runs_installer( self, uv_tool_argv0, clean_environ ): diff --git a/tests/test_upgrade.py b/tests/test_upgrade.py index 22dcce3ad2..25589f19cd 100644 --- a/tests/test_upgrade.py +++ b/tests/test_upgrade.py @@ -162,6 +162,8 @@ def test_unknown_installed_still_prints_latest_and_reinstall(self): 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 + assert "specify self upgrade" in output + assert "pipx install --force 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( From 875bd5bf6eb35cf2a8f4f475b84ed142887c96ac Mon Sep 17 00:00:00 2001 From: pli Date: Thu, 7 May 2026 22:08:46 +0900 Subject: [PATCH 14/25] fix(cli): tighten uv tool detection --- src/specify_cli/__init__.py | 16 +++++++++++- tests/test_self_upgrade.py | 52 +++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 1 deletion(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index b5ac670f28..83bb623ad8 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1913,6 +1913,18 @@ def _looks_like_specify_entrypoint(path: Path) -> bool: return path.name.lower() in {"specify", "specify.exe"} +def _uv_tool_list_contains_specify_cli(stdout: str) -> bool: + """Return whether `uv tool list` output includes an exact `specify-cli` entry.""" + for raw_line in stdout.splitlines(): + line = raw_line.strip() + if not line: + continue + first_token = line.split(None, 1)[0] + if first_token == "specify-cli": + return True + return False + + def _git_ancestor(path: Path) -> Path | None: """Return the closest ancestor that looks like a git worktree root.""" for ancestor in [path, *path.parents]: @@ -2044,7 +2056,9 @@ def _detect_install_method( env=_scrubbed_env(), check=False, ) - if result.returncode == 0 and "specify-cli" in (result.stdout or ""): + if result.returncode == 0 and _uv_tool_list_contains_specify_cli( + result.stdout or "" + ): method = _InstallMethod.UV_TOOL if include_signals: return method, _DetectionSignals( diff --git a/tests/test_self_upgrade.py b/tests/test_self_upgrade.py index f13dc3426a..3053301989 100644 --- a/tests/test_self_upgrade.py +++ b/tests/test_self_upgrade.py @@ -161,6 +161,58 @@ def test_prefix_match_does_not_accept_sibling_directory(self, monkeypatch, tmp_p method = _detect_install_method() assert method == _InstallMethod.UNSUPPORTED + def test_tier3_uv_tool_when_registry_lists_exact_name( + self, + unsupported_argv0, + ): + def fake_which(name): + return "/usr/bin/uv" if name == "uv" else None + + def fake_run(argv, *args, **kwargs): + if argv[:3] == ["/usr/bin/uv", "tool", "list"]: + return subprocess.CompletedProcess( + args=argv, + returncode=0, + stdout="specify-cli v0.7.6\nother-tool v1.2.3\n", + stderr="", + ) + return subprocess.CompletedProcess( + args=argv, returncode=1, stdout="", stderr="" + ) + + with patch("specify_cli.shutil.which", side_effect=fake_which), patch( + "specify_cli.subprocess.run", side_effect=fake_run + ), patch("specify_cli._editable_marker_seen", return_value=False): + method, signals = _detect_install_method(include_signals=True) + assert method == _InstallMethod.UV_TOOL + assert signals.matched_tier == 3 + assert "uv tool list" in signals.installer_registries_consulted + + def test_tier3_uv_tool_ignores_substring_false_positive( + self, + unsupported_argv0, + ): + def fake_which(name): + return "/usr/bin/uv" if name == "uv" else None + + def fake_run(argv, *args, **kwargs): + if argv[:3] == ["/usr/bin/uv", "tool", "list"]: + return subprocess.CompletedProcess( + args=argv, + returncode=0, + stdout="my-specify-cli-helper v0.1.0\n", + stderr="", + ) + return subprocess.CompletedProcess( + args=argv, returncode=1, stdout="", stderr="" + ) + + with patch("specify_cli.shutil.which", side_effect=fake_which), patch( + "specify_cli.subprocess.run", side_effect=fake_run + ), patch("specify_cli._editable_marker_seen", return_value=False): + method = _detect_install_method() + assert method == _InstallMethod.UNSUPPORTED + class TestArgvAssembly_UvTool: """uv-tool installer argv shape.""" From 5031ca8f94790421ce5965137f6ab8199c42859a Mon Sep 17 00:00:00 2001 From: pli Date: Thu, 7 May 2026 22:22:30 +0900 Subject: [PATCH 15/25] fix(cli): constrain self-upgrade tier-3 detection --- src/specify_cli/__init__.py | 127 ++++++++++++++++++++---------------- tests/test_self_upgrade.py | 84 +++++++++++++++++++++++- 2 files changed, 153 insertions(+), 58 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 83bb623ad8..b83439b8a8 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1913,6 +1913,16 @@ def _looks_like_specify_entrypoint(path: Path) -> bool: return path.name.lower() in {"specify", "specify.exe"} +def _tier3_registry_lookup_allowed(argv0: str | None, argv0_path: Path) -> bool: + """Return whether tier-3 registry reconciliation is safe for this entrypoint.""" + raw = argv0 or sys.argv[0] + candidate = Path(raw) + return ( + (not candidate.is_absolute() and len(candidate.parts) == 1) + or not argv0_path.exists() + ) + + def _uv_tool_list_contains_specify_cli(stdout: str) -> bool: """Return whether `uv tool list` output includes an exact `specify-cli` entry.""" for raw_line in stdout.splitlines(): @@ -2044,52 +2054,23 @@ def _detect_install_method( # --- Tier 3: PATH + registry reconciliation --- consulted: list[str] = [] - uv_bin = shutil.which("uv") - if uv_bin is not None: - consulted.append("uv tool list") - try: - result = subprocess.run( - [uv_bin, "tool", "list"], - capture_output=True, - text=True, - timeout=5, - env=_scrubbed_env(), - check=False, - ) - if result.returncode == 0 and _uv_tool_list_contains_specify_cli( - result.stdout or "" - ): - method = _InstallMethod.UV_TOOL - if include_signals: - return method, _DetectionSignals( - sys_argv0=argv0_resolved, - matched_tier=3, - matched_prefix=None, - editable_marker_seen=False, - installer_registries_consulted=consulted, - resolved_method=method, - ) - return method - except (subprocess.TimeoutExpired, OSError, ValueError): - pass - - pipx_bin = shutil.which("pipx") - if pipx_bin is not None: - consulted.append("pipx list --json") - try: - result = subprocess.run( - [pipx_bin, "list", "--json"], - capture_output=True, - text=True, - timeout=5, - env=_scrubbed_env(), - check=False, - ) - if result.returncode == 0: - payload = json.loads(result.stdout or "") - venvs = payload.get("venvs") if isinstance(payload, dict) else None - if isinstance(venvs, dict) and "specify-cli" in venvs: - method = _InstallMethod.PIPX + if _tier3_registry_lookup_allowed(argv0, argv0_path): + uv_bin = shutil.which("uv") + if uv_bin is not None: + consulted.append("uv tool list") + try: + result = subprocess.run( + [uv_bin, "tool", "list"], + capture_output=True, + text=True, + timeout=5, + env=_scrubbed_env(), + check=False, + ) + if result.returncode == 0 and _uv_tool_list_contains_specify_cli( + result.stdout or "" + ): + method = _InstallMethod.UV_TOOL if include_signals: return method, _DetectionSignals( sys_argv0=argv0_resolved, @@ -2100,8 +2081,38 @@ def _detect_install_method( resolved_method=method, ) return method - except (subprocess.TimeoutExpired, OSError, ValueError): - pass + except (subprocess.TimeoutExpired, OSError, ValueError): + pass + + pipx_bin = shutil.which("pipx") + if pipx_bin is not None: + consulted.append("pipx list --json") + try: + result = subprocess.run( + [pipx_bin, "list", "--json"], + capture_output=True, + text=True, + timeout=5, + env=_scrubbed_env(), + check=False, + ) + if result.returncode == 0: + payload = json.loads(result.stdout or "") + venvs = payload.get("venvs") if isinstance(payload, dict) else None + if isinstance(venvs, dict) and "specify-cli" in venvs: + method = _InstallMethod.PIPX + if include_signals: + return method, _DetectionSignals( + sys_argv0=argv0_resolved, + matched_tier=3, + matched_prefix=None, + editable_marker_seen=False, + installer_registries_consulted=consulted, + resolved_method=method, + ) + return method + except (subprocess.TimeoutExpired, OSError, ValueError): + pass # Fallthrough method = _InstallMethod.UNSUPPORTED @@ -2437,11 +2448,17 @@ def _emit_failure( return if category == "installer-missing": - name = installer_name or "(unknown)" - console.print( - f"Installer {name} not found on PATH; reinstall it and retry.", - soft_wrap=True, - ) + if installer_name and os.path.isabs(installer_name): + console.print( + f"Installer path {installer_name} no longer exists; reinstall it and retry.", + soft_wrap=True, + ) + else: + name = installer_name or "(unknown)" + console.print( + f"Installer {name} not found on PATH; reinstall it and retry.", + soft_wrap=True, + ) return if category == "installer-invalid": @@ -2684,9 +2701,7 @@ def self_upgrade( completed = _run_installer(plan) if completed is None: - installer_name = ( - Path(plan.installer_argv[0]).name if plan.installer_argv else None - ) + installer_name = plan.installer_argv[0] if plan.installer_argv else None _emit_failure("installer-missing", plan=plan, installer_name=installer_name) raise typer.Exit(3) diff --git a/tests/test_self_upgrade.py b/tests/test_self_upgrade.py index 3053301989..1686808aba 100644 --- a/tests/test_self_upgrade.py +++ b/tests/test_self_upgrade.py @@ -163,8 +163,10 @@ def test_prefix_match_does_not_accept_sibling_directory(self, monkeypatch, tmp_p def test_tier3_uv_tool_when_registry_lists_exact_name( self, - unsupported_argv0, + monkeypatch, ): + monkeypatch.setattr("sys.argv", ["specify"]) + def fake_which(name): return "/usr/bin/uv" if name == "uv" else None @@ -213,6 +215,31 @@ def fake_run(argv, *args, **kwargs): method = _detect_install_method() assert method == _InstallMethod.UNSUPPORTED + def test_tier3_uv_tool_does_not_override_absolute_unsupported_entrypoint( + self, + unsupported_argv0, + ): + def fake_which(name): + return "/usr/bin/uv" if name == "uv" else None + + def fake_run(argv, *args, **kwargs): + if argv[:3] == ["/usr/bin/uv", "tool", "list"]: + return subprocess.CompletedProcess( + args=argv, + returncode=0, + stdout="specify-cli v0.7.6\n", + stderr="", + ) + return subprocess.CompletedProcess( + args=argv, returncode=1, stdout="", stderr="" + ) + + with patch("specify_cli.shutil.which", side_effect=fake_which), patch( + "specify_cli.subprocess.run", side_effect=fake_run + ), patch("specify_cli._editable_marker_seen", return_value=False): + method = _detect_install_method() + assert method == _InstallMethod.UNSUPPORTED + class TestArgvAssembly_UvTool: """uv-tool installer argv shape.""" @@ -390,8 +417,10 @@ def test_posix_pipx_prefix_matches(self, pipx_argv0): def test_tier3_pipx_when_no_prefix_match_but_registry_lists_it( self, - unsupported_argv0, + monkeypatch, ): + monkeypatch.setattr("sys.argv", ["specify"]) + def fake_which(name): return "/usr/bin/pipx" if name == "pipx" else None @@ -415,6 +444,31 @@ def fake_run(argv, *args, **kwargs): assert signals.matched_tier == 3 assert "pipx list --json" in signals.installer_registries_consulted + def test_tier3_pipx_does_not_override_absolute_unsupported_entrypoint( + self, + unsupported_argv0, + ): + def fake_which(name): + return "/usr/bin/pipx" if name == "pipx" else None + + def fake_run(argv, *args, **kwargs): + if argv[0] == "/usr/bin/pipx" and argv[1] == "list": + return subprocess.CompletedProcess( + args=argv, + returncode=0, + stdout='{"venvs":{"specify-cli":{}}}', + stderr="", + ) + return subprocess.CompletedProcess( + args=argv, returncode=1, stdout="", stderr="" + ) + + with patch("specify_cli.shutil.which", side_effect=fake_which), patch( + "specify_cli.subprocess.run", side_effect=fake_run + ), patch("specify_cli._editable_marker_seen", return_value=False): + method = _detect_install_method() + assert method == _InstallMethod.UNSUPPORTED + def test_tier3_pipx_ignores_malformed_json_output( self, unsupported_argv0, @@ -796,6 +850,32 @@ def test_absolute_installer_path_not_executable_gets_specific_message( in strip_ansi(result.output) ) + def test_absolute_installer_path_missing_gets_path_specific_message( + self, uv_tool_argv0, clean_environ, tmp_path + ): + fake_uv = tmp_path / "uv" + with patch("specify_cli.urllib.request.urlopen") as mock_urlopen, patch( + "specify_cli.shutil.which", side_effect=lambda name: None + ), patch("specify_cli._get_installed_version", return_value="0.7.5"), patch( + "specify_cli._assemble_installer_argv", + return_value=[ + str(fake_uv), + "tool", + "install", + "specify-cli", + "--force", + "--from", + "git+https://github.com/github/spec-kit.git@v0.7.6", + ], + ): + mock_urlopen.return_value = _mock_urlopen_response({"tag_name": "v0.7.6"}) + result = runner.invoke(app, ["self", "upgrade"]) + assert result.exit_code == 3 + assert ( + f"Installer path {fake_uv} no longer exists; reinstall it and retry." + in strip_ansi(result.output) + ) + class TestInstallerFailed: """Installer non-zero exit → propagate code, print rollback hint.""" From cf9bbfbb7d93ba4c18df1a4ae97c18933bcb303b Mon Sep 17 00:00:00 2001 From: pli Date: Thu, 7 May 2026 22:29:31 +0900 Subject: [PATCH 16/25] fix(cli): clarify invalid installer path errors --- src/specify_cli/__init__.py | 20 ++++++++++++-------- tests/test_self_upgrade.py | 2 +- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index b83439b8a8..344c007a7f 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -2462,11 +2462,17 @@ def _emit_failure( return if category == "installer-invalid": - name = installer_name or "(unknown)" - console.print( - f"Installer {name} is not an executable file; fix the path or reinstall it and retry.", - soft_wrap=True, - ) + if installer_name and os.path.isabs(installer_name): + console.print( + f"Installer path {installer_name} is not an executable file; fix the path or reinstall it and retry.", + soft_wrap=True, + ) + else: + name = installer_name or "(unknown)" + console.print( + f"Installer {name} is not an executable file; fix the path or reinstall it and retry.", + soft_wrap=True, + ) return if category == "installer-failed": @@ -2706,9 +2712,7 @@ def self_upgrade( raise typer.Exit(3) if completed.returncode == 126: - installer_name = ( - Path(plan.installer_argv[0]).name if plan.installer_argv else None - ) + installer_name = plan.installer_argv[0] if plan.installer_argv else None _emit_failure("installer-invalid", plan=plan, installer_name=installer_name) raise typer.Exit(3) diff --git a/tests/test_self_upgrade.py b/tests/test_self_upgrade.py index 1686808aba..8353ec80fe 100644 --- a/tests/test_self_upgrade.py +++ b/tests/test_self_upgrade.py @@ -846,7 +846,7 @@ def test_absolute_installer_path_not_executable_gets_specific_message( result = runner.invoke(app, ["self", "upgrade"]) assert result.exit_code == 3 assert ( - "Installer uv is not an executable file; fix the path or reinstall it and retry." + f"Installer path {fake_uv} is not an executable file; fix the path or reinstall it and retry." in strip_ansi(result.output) ) From 649f54937f2790febec4a5e2f4dc3831b771b79d Mon Sep 17 00:00:00 2001 From: pli Date: Fri, 8 May 2026 07:44:05 +0900 Subject: [PATCH 17/25] fix(cli): tighten editable self-upgrade detection --- src/specify_cli/__init__.py | 22 +++++----------------- tests/test_self_upgrade.py | 33 +++++++++++++++++++++++++++++---- 2 files changed, 34 insertions(+), 21 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 344c007a7f..23b0ad1400 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1814,7 +1814,7 @@ def _fetch_latest_release_tag() -> tuple[str | None, str | None]: _RESOLUTION_FAILURE_CATEGORIES: frozenset[str] = frozenset( { "offline or timeout", - "rate limited (try setting GH_TOKEN or GITHUB_TOKEN)", + "rate limited (configure ~/.specify/auth.json with a GitHub token)", } ) @@ -1871,6 +1871,9 @@ def _validate_tag(tag: str) -> str: +build.42). Rejects everything else (including bare 'latest', hash refs, branch names, or a numeric version without the 'v' prefix). """ + tag = tag.strip() + if not tag: + raise typer.BadParameter("Invalid --tag: expected vMAJOR.MINOR.PATCH[suffix]") if not _TAG_REGEX.match(tag): raise typer.BadParameter("Invalid --tag: expected vMAJOR.MINOR.PATCH[suffix]") @@ -1983,25 +1986,10 @@ def _editable_direct_url_path() -> Path | None: def _editable_marker_seen() -> bool: - """Return whether the installed distribution appears to come from a git checkout.""" - import importlib.metadata as _md - + """Return whether the installed distribution is explicitly marked editable.""" editable_root = _editable_direct_url_path() if editable_root is not None and _git_ancestor(editable_root) is not None: return True - - try: - dist = _md.distribution("specify-cli") - except _md.PackageNotFoundError: - return False - files = dist.files or [] - for f in files: - try: - abs_path = Path(dist.locate_file(f)).resolve() - except Exception: - continue - if _git_ancestor(abs_path) is not None: - return True return False diff --git a/tests/test_self_upgrade.py b/tests/test_self_upgrade.py index 8353ec80fe..342048d866 100644 --- a/tests/test_self_upgrade.py +++ b/tests/test_self_upgrade.py @@ -521,20 +521,45 @@ def locate_file(self, file): assert _editable_marker_seen() is True assert _source_checkout_path() == project_root.resolve() - def test_editable_marker_ignores_locate_file_errors(self): + def test_editable_marker_false_without_explicit_editable_metadata(self, tmp_path): + repo_root = tmp_path / "repo" + repo_root.mkdir() + (repo_root / ".git").mkdir() + venv_file = repo_root / ".venv" / "lib" / "python3.13" / "site-packages" / "specify_cli.py" + venv_file.parent.mkdir(parents=True) + venv_file.write_text("# installed module\n") + class FakeDist: - files = ["bad"] + files = ["specify_cli.py"] def read_text(self, name): return None def locate_file(self, file): - raise OSError("boom") + return venv_file with patch("importlib.metadata.distribution", return_value=FakeDist()): assert _editable_marker_seen() is False +class TestTagValidation: + def test_tag_whitespace_is_trimmed_before_validation(self, uv_tool_argv0, clean_environ): + with patch("specify_cli.urllib.request.urlopen") as mock_urlopen, patch( + "specify_cli.shutil.which", return_value="/usr/bin/uv" + ), patch("specify_cli.subprocess.run") as mock_run, patch( + "specify_cli._get_installed_version", return_value="0.7.5" + ): + mock_urlopen.return_value = _mock_urlopen_response({"tag_name": "v9.9.9"}) + mock_run.side_effect = [ + _completed_process(0), + _completed_process(0, stdout="specify 0.8.0\n"), + ] + result = runner.invoke(app, ["self", "upgrade", "--tag", " v0.8.0 "]) + + assert result.exit_code == 0 + assert "v0.8.0" in strip_ansi(result.output) + + class TestArgvAssembly_Pipx: """pipx installer argv shape — pipx 1.5+ uses positional PACKAGE_SPEC, never `--spec` or `upgrade`.""" @@ -1088,7 +1113,7 @@ def test_rate_limited_exits_1(self, uv_tool_argv0, clean_environ): result = runner.invoke(app, ["self", "upgrade"]) assert result.exit_code == 1 assert ( - "Upgrade aborted: rate limited (try setting GH_TOKEN or GITHUB_TOKEN)" + "Upgrade aborted: rate limited (configure ~/.specify/auth.json with a GitHub token)" in strip_ansi(result.output) ) From 7c34b781429950165fef0a8e511f79937deee0fd Mon Sep 17 00:00:00 2001 From: pli Date: Fri, 8 May 2026 07:54:44 +0900 Subject: [PATCH 18/25] test(cli): collect all self-upgrade tag cases --- tests/test_self_upgrade.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_self_upgrade.py b/tests/test_self_upgrade.py index 342048d866..3e80829a5a 100644 --- a/tests/test_self_upgrade.py +++ b/tests/test_self_upgrade.py @@ -542,7 +542,7 @@ def locate_file(self, file): assert _editable_marker_seen() is False -class TestTagValidation: +class TestTagValidationWhitespace: def test_tag_whitespace_is_trimmed_before_validation(self, uv_tool_argv0, clean_environ): with patch("specify_cli.urllib.request.urlopen") as mock_urlopen, patch( "specify_cli.shutil.which", return_value="/usr/bin/uv" From 4d81b745c18a91d2fdc774ef6d2a461e150fd09a Mon Sep 17 00:00:00 2001 From: pli Date: Fri, 8 May 2026 08:09:04 +0900 Subject: [PATCH 19/25] test(cli): harden self-upgrade fixture coverage --- src/specify_cli/__init__.py | 4 +++- tests/test_self_upgrade.py | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 23b0ad1400..689aa4badf 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1857,7 +1857,9 @@ def _scrubbed_env() -> dict[str, str]: """Return a copy of `os.environ` without GitHub token variables.""" return { - k: v for k, v in os.environ.items() if k not in {"GH_TOKEN", "GITHUB_TOKEN"} + k: v + for k, v in os.environ.items() + if k.upper() not in {"GH_TOKEN", "GITHUB_TOKEN"} } diff --git a/tests/test_self_upgrade.py b/tests/test_self_upgrade.py index 3e80829a5a..c7a537ad27 100644 --- a/tests/test_self_upgrade.py +++ b/tests/test_self_upgrade.py @@ -72,6 +72,7 @@ def uv_tool_argv0(monkeypatch, tmp_path): fake_dir.mkdir(parents=True) fake_specify = fake_dir / "specify" fake_specify.write_text("#!/usr/bin/env python\n") + fake_specify.chmod(0o755) monkeypatch.setattr("sys.argv", [str(fake_specify)]) return fake_specify @@ -84,6 +85,7 @@ def pipx_argv0(monkeypatch, tmp_path): fake_dir.mkdir(parents=True) fake_specify = fake_dir / "specify" fake_specify.write_text("#!/usr/bin/env python\n") + fake_specify.chmod(0o755) monkeypatch.setattr("sys.argv", [str(fake_specify)]) return fake_specify @@ -96,6 +98,7 @@ def uvx_ephemeral_argv0(monkeypatch, tmp_path): fake_dir.mkdir(parents=True) fake_specify = fake_dir / "specify" fake_specify.write_text("#!/usr/bin/env python\n") + fake_specify.chmod(0o755) monkeypatch.setattr("sys.argv", [str(fake_specify)]) return fake_specify @@ -108,6 +111,7 @@ def unsupported_argv0(monkeypatch, tmp_path): fake_dir.mkdir(parents=True) fake_specify = fake_dir / "specify" fake_specify.write_text("#!/usr/bin/env python\n") + fake_specify.chmod(0o755) monkeypatch.setattr("sys.argv", [str(fake_specify)]) return fake_specify @@ -1261,6 +1265,35 @@ def test_env_passed_to_subprocess_has_no_github_tokens( assert SENTINEL_GH_TOKEN not in v assert SENTINEL_GITHUB_TOKEN not in v + def test_env_scrubbing_is_case_insensitive( + self, + uv_tool_argv0, + monkeypatch, + ): + monkeypatch.setenv("gh_token", SENTINEL_GH_TOKEN) + monkeypatch.setenv("GitHub_Token", SENTINEL_GITHUB_TOKEN) + + with patch("specify_cli.urllib.request.urlopen") as mock_urlopen, patch( + "specify_cli.shutil.which", return_value="/usr/bin/uv" + ), patch("specify_cli.subprocess.run") as mock_run, patch( + "specify_cli._get_installed_version", return_value="0.7.5" + ): + mock_urlopen.return_value = _mock_urlopen_response({"tag_name": "v0.7.6"}) + mock_run.side_effect = [ + _completed_process(0), + _completed_process(0, stdout="specify 0.7.6\n"), + ] + runner.invoke(app, ["self", "upgrade"]) + + assert mock_run.call_count >= 1 + for call in mock_run.call_args_list: + env_kwarg = call.kwargs.get("env") or {} + assert "gh_token" not in env_kwarg + assert "GitHub_Token" not in env_kwarg + for v in env_kwarg.values(): + assert SENTINEL_GH_TOKEN not in v + assert SENTINEL_GITHUB_TOKEN not in v + class TestDryRunWithTag_SkipsNetwork: """--dry-run with --tag never calls urlopen.""" From 19ade110c54185a5c6aa587529216d5750f7f840 Mon Sep 17 00:00:00 2001 From: pli Date: Fri, 8 May 2026 08:23:36 +0900 Subject: [PATCH 20/25] fix(cli): disambiguate self-upgrade timeout exits --- src/specify_cli/__init__.py | 12 +++++++++--- tests/test_self_upgrade.py | 19 ++++++++++++++++++- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 689aa4badf..d29013a65b 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1818,6 +1818,8 @@ def _fetch_latest_release_tag() -> tuple[str | None, str | None]: } ) +_INSTALLER_TIMEOUT_SENTINEL = 900124 + class _InstallMethod(str, Enum): """Install-method classification for `specify self upgrade`.""" @@ -2283,7 +2285,7 @@ def _run_installer(plan: _UpgradePlan) -> subprocess.CompletedProcess | None: # the manual-retry argv + rollback hint. return subprocess.CompletedProcess( args=plan.installer_argv, - returncode=124, # POSIX-ish convention for "timeout" + returncode=_INSTALLER_TIMEOUT_SENTINEL, stdout=None, stderr=None, ) @@ -2472,7 +2474,7 @@ def _emit_failure( "plan and installer_exit to be set" ) argv_str = _render_argv(plan.installer_argv) if plan.installer_argv else "" - if installer_exit == 124: + if installer_exit == _INSTALLER_TIMEOUT_SENTINEL: timeout_value = os.environ.get("SPECIFY_UPGRADE_TIMEOUT_SECS", "(unknown)") console.print( "Upgrade timed out while waiting for the installer subprocess.", @@ -2712,7 +2714,11 @@ def self_upgrade( plan=plan, installer_exit=completed.returncode, ) - raise typer.Exit(completed.returncode) + raise typer.Exit( + 124 + if completed.returncode == _INSTALLER_TIMEOUT_SENTINEL + else completed.returncode + ) # Verify in a child process: this Python process is still running the # pre-upgrade module, so importlib.metadata would lie. A fresh `specify diff --git a/tests/test_self_upgrade.py b/tests/test_self_upgrade.py index c7a537ad27..1aec71867e 100644 --- a/tests/test_self_upgrade.py +++ b/tests/test_self_upgrade.py @@ -14,6 +14,7 @@ from specify_cli import ( _InstallMethod, + _INSTALLER_TIMEOUT_SENTINEL, _editable_marker_seen, _source_checkout_path, _detect_install_method, @@ -953,13 +954,29 @@ def test_installer_timeout_prints_timeout_specific_message( "specify_cli._get_installed_version", return_value="0.7.5" ): mock_urlopen.return_value = _mock_urlopen_response({"tag_name": "v0.7.6"}) - mock_run.side_effect = [_completed_process(124)] + mock_run.side_effect = [_completed_process(_INSTALLER_TIMEOUT_SENTINEL)] result = runner.invoke(app, ["self", "upgrade"]) assert result.exit_code == 124 out = strip_ansi(result.output) assert "Upgrade timed out while waiting for the installer subprocess." in out assert "SPECIFY_UPGRADE_TIMEOUT_SECS=12" in out + def test_real_installer_exit_124_is_not_treated_as_timeout( + self, uv_tool_argv0, clean_environ + ): + with patch("specify_cli.urllib.request.urlopen") as mock_urlopen, patch( + "specify_cli.shutil.which", return_value="/usr/bin/uv" + ), patch("specify_cli.subprocess.run") as mock_run, patch( + "specify_cli._get_installed_version", return_value="0.7.5" + ): + mock_urlopen.return_value = _mock_urlopen_response({"tag_name": "v0.7.6"}) + mock_run.side_effect = [_completed_process(124)] + result = runner.invoke(app, ["self", "upgrade"]) + assert result.exit_code == 124 + out = strip_ansi(result.output) + assert "Upgrade failed. Installer exit code: 124." in out + assert "Upgrade timed out while waiting for the installer subprocess." not in out + def test_pipx_failure_prints_pipx_rollback_hint(self, pipx_argv0, clean_environ): with patch("specify_cli.urllib.request.urlopen") as mock_urlopen, patch( "specify_cli.shutil.which", return_value="/usr/bin/pipx" From af664788e013ffb8620249f0f98195fc60939d1b Mon Sep 17 00:00:00 2001 From: pli Date: Fri, 8 May 2026 08:46:03 +0900 Subject: [PATCH 21/25] fix(cli): preserve installer exit 126 semantics --- src/specify_cli/__init__.py | 5 +++-- tests/test_self_upgrade.py | 17 +++++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index d29013a65b..a561f0e20a 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1818,6 +1818,7 @@ def _fetch_latest_release_tag() -> tuple[str | None, str | None]: } ) +_INSTALLER_INVALID_SENTINEL = 900126 _INSTALLER_TIMEOUT_SENTINEL = 900124 @@ -2254,7 +2255,7 @@ def _run_installer(plan: _UpgradePlan) -> subprocess.CompletedProcess | None: ): return subprocess.CompletedProcess( args=plan.installer_argv, - returncode=126, + returncode=_INSTALLER_INVALID_SENTINEL, stdout=None, stderr=None, ) @@ -2703,7 +2704,7 @@ def self_upgrade( _emit_failure("installer-missing", plan=plan, installer_name=installer_name) raise typer.Exit(3) - if completed.returncode == 126: + if completed.returncode == _INSTALLER_INVALID_SENTINEL: installer_name = plan.installer_argv[0] if plan.installer_argv else None _emit_failure("installer-invalid", plan=plan, installer_name=installer_name) raise typer.Exit(3) diff --git a/tests/test_self_upgrade.py b/tests/test_self_upgrade.py index 1aec71867e..d78ba5ebfe 100644 --- a/tests/test_self_upgrade.py +++ b/tests/test_self_upgrade.py @@ -14,6 +14,7 @@ from specify_cli import ( _InstallMethod, + _INSTALLER_INVALID_SENTINEL, _INSTALLER_TIMEOUT_SENTINEL, _editable_marker_seen, _source_checkout_path, @@ -880,6 +881,22 @@ def test_absolute_installer_path_not_executable_gets_specific_message( in strip_ansi(result.output) ) + def test_real_installer_exit_126_is_not_treated_as_invalid_path( + self, uv_tool_argv0, clean_environ + ): + with patch("specify_cli.urllib.request.urlopen") as mock_urlopen, patch( + "specify_cli.shutil.which", return_value="/usr/bin/uv" + ), patch("specify_cli.subprocess.run") as mock_run, patch( + "specify_cli._get_installed_version", return_value="0.7.5" + ): + mock_urlopen.return_value = _mock_urlopen_response({"tag_name": "v0.7.6"}) + mock_run.side_effect = [_completed_process(126)] + result = runner.invoke(app, ["self", "upgrade"]) + assert result.exit_code == 126 + out = strip_ansi(result.output) + assert "Upgrade failed. Installer exit code: 126." in out + assert "not an executable file" not in out + def test_absolute_installer_path_missing_gets_path_specific_message( self, uv_tool_argv0, clean_environ, tmp_path ): From 76b0f1ec5d32effe1a9eaaf140ae1e5604adfc9b Mon Sep 17 00:00:00 2001 From: pli Date: Fri, 8 May 2026 08:54:47 +0900 Subject: [PATCH 22/25] test(cli): harden self-upgrade release mocks --- src/specify_cli/__init__.py | 16 +++++++++------- tests/test_self_upgrade.py | 14 ++++++++++++++ 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index a561f0e20a..b7970691ab 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -2217,10 +2217,10 @@ def _run_installer(plan: _UpgradePlan) -> subprocess.CompletedProcess | None: """Invoke the installer subprocess. Returns None if the installer binary is not found on PATH. Returns a - synthetic CompletedProcess with returncode=126 when an absolute installer - path exists but is not executable or not a regular file. Returns a - CompletedProcess for every other case including non-zero exit — caller - inspects returncode. + synthetic CompletedProcess with returncode=`_INSTALLER_INVALID_SENTINEL` + when an absolute installer path exists but is not executable or not a + regular file. Returns a CompletedProcess for every other case including + non-zero exit — caller inspects returncode. stdout/stderr are inherited (not captured) so the user sees installer progress in real time. The child environment has GH_TOKEN / @@ -2230,8 +2230,9 @@ def _run_installer(plan: _UpgradePlan) -> subprocess.CompletedProcess | None: operations (dependency resolution, large wheel downloads) can legitimately take many minutes. Set the env var SPECIFY_UPGRADE_TIMEOUT_SECS to an integer/float to enforce a hard cap; on timeout we return a synthetic - CompletedProcess with returncode=124. An unparseable or non-positive - value is silently ignored (no timeout). + CompletedProcess with returncode=`_INSTALLER_TIMEOUT_SENTINEL`, which the + orchestrator maps to user-facing exit code `124`. An unparseable or + non-positive value is silently ignored (no timeout). """ if plan.installer_argv is None: # Internal routing error: the orchestrator must route non-upgradable @@ -2619,7 +2620,8 @@ def self_upgrade( 2 verification mismatch (installer exited 0 but `specify --version` does not resolve to the target tag) 3 installer binary not found on PATH, or resolved to a non-executable file - 124 installer subprocess timed out (only when SPECIFY_UPGRADE_TIMEOUT_SECS is set) + 124 internal installer timeout when SPECIFY_UPGRADE_TIMEOUT_SECS is set, + or a real installer exit code 124 propagated verbatim other installer exit code propagated verbatim Environment variables: diff --git a/tests/test_self_upgrade.py b/tests/test_self_upgrade.py index d78ba5ebfe..f3fce7a6d0 100644 --- a/tests/test_self_upgrade.py +++ b/tests/test_self_upgrade.py @@ -5,6 +5,7 @@ """ import json +import specify_cli import subprocess import urllib.error from unittest.mock import MagicMock, patch @@ -61,6 +62,19 @@ def clean_environ(monkeypatch): monkeypatch.delenv("GITHUB_TOKEN", raising=False) +@pytest.fixture(autouse=True) +def route_open_url_through_urlopen(monkeypatch): + """Keep release-tag tests hermetic even when ~/.specify/auth.json exists.""" + + def _open_url(url, timeout=10, extra_headers=None): + req = specify_cli.urllib.request.Request(url) + for key, value in (extra_headers or {}).items(): + req.add_header(key, value) + return specify_cli.urllib.request.urlopen(req, timeout=timeout) + + monkeypatch.setattr("specify_cli.authentication.http.open_url", _open_url) + + @pytest.fixture def uv_tool_argv0(monkeypatch, tmp_path): """Point sys.argv[0] at a simulated `uv tool` install path under tmp HOME. From 34030ae7cf6c166222f62d5684249265e73f96f8 Mon Sep 17 00:00:00 2001 From: pli Date: Fri, 8 May 2026 09:10:48 +0900 Subject: [PATCH 23/25] fix(cli): avoid ambiguous tier-3 self-upgrades --- src/specify_cli/__init__.py | 39 +++++++++++++++-------------------- tests/test_self_upgrade.py | 41 +++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 22 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index b7970691ab..41cdaf1107 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -2048,6 +2048,7 @@ def _detect_install_method( # --- Tier 3: PATH + registry reconciliation --- consulted: list[str] = [] if _tier3_registry_lookup_allowed(argv0, argv0_path): + uv_tool_match = False uv_bin = shutil.which("uv") if uv_bin is not None: consulted.append("uv tool list") @@ -2063,20 +2064,11 @@ def _detect_install_method( if result.returncode == 0 and _uv_tool_list_contains_specify_cli( result.stdout or "" ): - method = _InstallMethod.UV_TOOL - if include_signals: - return method, _DetectionSignals( - sys_argv0=argv0_resolved, - matched_tier=3, - matched_prefix=None, - editable_marker_seen=False, - installer_registries_consulted=consulted, - resolved_method=method, - ) - return method + uv_tool_match = True except (subprocess.TimeoutExpired, OSError, ValueError): pass + pipx_match = False pipx_bin = shutil.which("pipx") if pipx_bin is not None: consulted.append("pipx list --json") @@ -2093,20 +2085,23 @@ def _detect_install_method( payload = json.loads(result.stdout or "") venvs = payload.get("venvs") if isinstance(payload, dict) else None if isinstance(venvs, dict) and "specify-cli" in venvs: - method = _InstallMethod.PIPX - if include_signals: - return method, _DetectionSignals( - sys_argv0=argv0_resolved, - matched_tier=3, - matched_prefix=None, - editable_marker_seen=False, - installer_registries_consulted=consulted, - resolved_method=method, - ) - return method + pipx_match = True except (subprocess.TimeoutExpired, OSError, ValueError): pass + if uv_tool_match ^ pipx_match: + method = _InstallMethod.UV_TOOL if uv_tool_match else _InstallMethod.PIPX + if include_signals: + return method, _DetectionSignals( + sys_argv0=argv0_resolved, + matched_tier=3, + matched_prefix=None, + editable_marker_seen=False, + installer_registries_consulted=consulted, + resolved_method=method, + ) + return method + # Fallthrough method = _InstallMethod.UNSUPPORTED if include_signals: diff --git a/tests/test_self_upgrade.py b/tests/test_self_upgrade.py index f3fce7a6d0..33f79dee5b 100644 --- a/tests/test_self_upgrade.py +++ b/tests/test_self_upgrade.py @@ -514,6 +514,47 @@ def fake_run(argv, *args, **kwargs): method = _detect_install_method() assert method == _InstallMethod.UNSUPPORTED + def test_tier3_both_uv_tool_and_pipx_match_is_treated_as_unsupported( + self, + monkeypatch, + ): + monkeypatch.setattr("sys.argv", ["specify"]) + + def fake_which(name): + if name == "uv": + return "/usr/bin/uv" + if name == "pipx": + return "/usr/bin/pipx" + return None + + def fake_run(argv, *args, **kwargs): + if argv[:3] == ["/usr/bin/uv", "tool", "list"]: + return subprocess.CompletedProcess( + args=argv, + returncode=0, + stdout="specify-cli v0.7.6\n", + stderr="", + ) + if argv[:3] == ["/usr/bin/pipx", "list", "--json"]: + return subprocess.CompletedProcess( + args=argv, + returncode=0, + stdout='{"venvs":{"specify-cli":{}}}', + stderr="", + ) + return subprocess.CompletedProcess( + args=argv, returncode=1, stdout="", stderr="" + ) + + with patch("specify_cli.shutil.which", side_effect=fake_which), patch( + "specify_cli.subprocess.run", side_effect=fake_run + ), patch("specify_cli._editable_marker_seen", return_value=False): + method, signals = _detect_install_method(include_signals=True) + assert method == _InstallMethod.UNSUPPORTED + assert signals.matched_tier is None + assert "uv tool list" in signals.installer_registries_consulted + assert "pipx list --json" in signals.installer_registries_consulted + class TestEditableInstallMetadata: def test_direct_url_editable_install_marks_source_checkout(self, tmp_path): From fda45bb51bb07a60418e89343f708590ff48023b Mon Sep 17 00:00:00 2001 From: pli Date: Fri, 8 May 2026 09:21:20 +0900 Subject: [PATCH 24/25] docs(cli): pin unsupported self-upgrade guidance --- src/specify_cli/__init__.py | 10 ++++++++-- tests/test_self_upgrade.py | 16 +++++++++++----- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 41cdaf1107..1b1eca8a47 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -2117,6 +2117,7 @@ def _detect_install_method( _GITHUB_SOURCE_URL = "git+https://github.com/github/spec-kit.git" +_MANUAL_TAG_PLACEHOLDER = "vX.Y.Z" def _source_spec(target_tag: str | None) -> str: @@ -2124,6 +2125,11 @@ def _source_spec(target_tag: str | None) -> str: return f"{_GITHUB_SOURCE_URL}@{target_tag}" if target_tag else _GITHUB_SOURCE_URL +def _manual_source_spec(target_tag: str | None) -> str: + """Build a stable-release-oriented source spec for manual guidance.""" + return f"{_GITHUB_SOURCE_URL}@{target_tag or _MANUAL_TAG_PLACEHOLDER}" + + def _assemble_installer_argv( method: _InstallMethod, target_tag: str | None ) -> list[str] | None: @@ -2389,11 +2395,11 @@ def _emit_guidance(method: _InstallMethod, target_tag: str | None) -> None: ) console.print( f" uv tool install specify-cli --force --from " - f"{_source_spec(target_tag)}", + f"{_manual_source_spec(target_tag)}", soft_wrap=True, ) console.print( - f" pipx install --force {_source_spec(target_tag)}", + f" pipx install --force {_manual_source_spec(target_tag)}", soft_wrap=True, ) return diff --git a/tests/test_self_upgrade.py b/tests/test_self_upgrade.py index 33f79dee5b..90938a2b8a 100644 --- a/tests/test_self_upgrade.py +++ b/tests/test_self_upgrade.py @@ -791,12 +791,15 @@ def test_unsupported_prints_both_reinstall_commands( assert "Could not identify your install method automatically" in out assert ( "uv tool install specify-cli --force --from " - "git+https://github.com/github/spec-kit.git" + "git+https://github.com/github/spec-kit.git@vX.Y.Z" ) in out - assert "pipx install --force git+https://github.com/github/spec-kit.git" in out + assert ( + "pipx install --force git+https://github.com/github/spec-kit.git@vX.Y.Z" + in out + ) assert mock_run.call_count == 0 - def test_unsupported_offline_degrades_to_unpinned_manual_commands( + def test_unsupported_offline_degrades_to_placeholder_manual_commands( self, unsupported_argv0, clean_environ, @@ -814,9 +817,12 @@ def test_unsupported_offline_degrades_to_unpinned_manual_commands( assert "Could not identify your install method automatically" in out assert ( "uv tool install specify-cli --force --from " - "git+https://github.com/github/spec-kit.git" + "git+https://github.com/github/spec-kit.git@vX.Y.Z" ) in out - assert "pipx install --force git+https://github.com/github/spec-kit.git" in out + assert ( + "pipx install --force git+https://github.com/github/spec-kit.git@vX.Y.Z" + in out + ) class TestDryRun_NonUpgradablePaths: From b0ac2f2aae4a6ea81536255ca4f58c2c6aac244b Mon Sep 17 00:00:00 2001 From: pli Date: Fri, 8 May 2026 09:31:24 +0900 Subject: [PATCH 25/25] fix(cli): canonicalize self-upgrade version checks --- src/specify_cli/__init__.py | 28 +++++++++++++++++++++++----- tests/test_self_upgrade.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 5 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 1b1eca8a47..c6218ddab7 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1744,6 +1744,15 @@ def _normalize_tag(tag: str) -> str: return tag[1:] if tag.startswith("v") else tag +def _canonicalize_version_text(value: str) -> str: + """Normalize version-like text for equality checks when parseable.""" + normalized = _normalize_tag(value) + try: + return str(Version(normalized)) + except InvalidVersion: + return normalized + + def _render_argv(argv: list[str]) -> str: """Render argv for copy/paste on the current platform.""" return subprocess.list2cmdline(argv) if os.name == "nt" else shlex.join(argv) @@ -2677,15 +2686,20 @@ def self_upgrade( raise typer.Exit(0) # No-op success when the user is already on the latest tag. - latest_normalized = _normalize_tag(plan.target_tag) + target_canonical = _canonicalize_version_text(plan.target_tag) + current_canonical = ( + _canonicalize_version_text(plan.current_version) + if plan.current_version != "unknown" + else "unknown" + ) if plan.current_version != "unknown": - if tag is None and not _is_newer(latest_normalized, plan.current_version): - if plan.current_version == latest_normalized: + if tag is None and not _is_newer(target_canonical, plan.current_version): + if current_canonical == target_canonical: console.print(f"Already on latest release: {plan.target_tag}") else: console.print(f"Already on latest release or newer: {plan.current_version}") raise typer.Exit(0) - if tag is not None and plan.current_version == latest_normalized: + if tag is not None and current_canonical == target_canonical: console.print(f"Already on requested release: {plan.target_tag}") raise typer.Exit(0) @@ -2728,7 +2742,11 @@ def self_upgrade( # pre-upgrade module, so importlib.metadata would lie. A fresh `specify # --version` is the only signal that the new binary is actually live. verified = _verify_upgrade(plan) - if verified is None or _normalize_tag(plan.target_tag) != verified: + if ( + verified is None + or _canonicalize_version_text(plan.target_tag) + != _canonicalize_version_text(verified) + ): _emit_failure( "verification-mismatch", plan=plan, diff --git a/tests/test_self_upgrade.py b/tests/test_self_upgrade.py index 90938a2b8a..2df88e6e46 100644 --- a/tests/test_self_upgrade.py +++ b/tests/test_self_upgrade.py @@ -380,6 +380,17 @@ def test_pinned_older_tag_still_runs_installer( assert "Upgrading specify-cli 0.7.6 → v0.7.5 via uv tool:" in out assert mock_run.call_count == 2 + def test_pinned_rc_tag_uses_canonical_version_equality_for_noop( + self, uv_tool_argv0, clean_environ + ): + with patch("specify_cli.shutil.which", return_value="/usr/bin/uv"), patch( + "specify_cli._get_installed_version", return_value="1.0.0rc1" + ): + result = runner.invoke(app, ["self", "upgrade", "--tag", "v1.0.0-rc1"]) + + assert result.exit_code == 0 + assert "Already on requested release: v1.0.0-rc1" in strip_ansi(result.output) + class TestDryRun_UvTool: """--dry-run preview path + --dry-run combined with --tag.""" @@ -1120,6 +1131,26 @@ def test_verify_nonzero_exit_is_not_treated_as_success( assert "Verification failed" in out assert "(unknown) (expected v0.7.6)" in out + def test_verify_accepts_pep440_equivalent_rc_version( + self, + uv_tool_argv0, + clean_environ, + ): + with patch("specify_cli.urllib.request.urlopen") as mock_urlopen, patch( + "specify_cli.shutil.which", return_value="/usr/bin/uv" + ), patch("specify_cli.subprocess.run") as mock_run, patch( + "specify_cli._get_installed_version", return_value="0.9.0" + ): + mock_urlopen.return_value = _mock_urlopen_response({"tag_name": "v9.9.9"}) + mock_run.side_effect = [ + _completed_process(0), + _completed_process(0, stdout="specify 1.0.0rc1\n"), + ] + result = runner.invoke(app, ["self", "upgrade", "--tag", "v1.0.0-rc1"]) + + assert result.exit_code == 0 + assert "Upgraded specify-cli: 0.9.0 → 1.0.0rc1" in strip_ansi(result.output) + def test_verify_uses_current_entrypoint_when_not_on_path( self, uv_tool_argv0,