feat(cli): implement specify self upgrade#2475
Open
chordpli wants to merge 32 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Implements a functional specify self upgrade command to replace the previous reserved stub, enabling in-place upgrades (uv tool / pipx) with install-method detection, dry-run previews, tag pinning, token scrubbing, and post-upgrade verification via a child specify --version.
Changes:
- Adds install-method detection + upgrade orchestration helpers and replaces the
self upgradestub with a working implementation. - Introduces a comprehensive new test suite for
specify self upgradeand removes the old stub-pin tests. - Updates README and docs to recommend
specify self upgradeand document the new flows.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
src/specify_cli/__init__.py |
Adds self-upgrade planning/execution/verification logic and updates self command help text. |
tests/test_self_upgrade.py |
New test module covering detection, argv assembly, dry-run behavior, tag validation, token scrubbing, and verification. |
tests/test_upgrade.py |
Removes tests that pinned the previous non-functional stub output. |
README.md |
Documents specify self check / specify self upgrade as the preferred upgrade path. |
docs/upgrade.md |
Refreshes upgrade guide to include self-upgrade commands, verification steps, and troubleshooting. |
docs/installation.md |
Adds “Stay current” pointer to specify self check and upgrade guide. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Collaborator
|
Please address Copilot feedback |
Contributor
Author
|
Hi @mnriem, I’ve addressed the latest review feedback and pushed the updates. |
Collaborator
|
Please address Copilot feedback. If not applicable, please explain why |
- README: promote "Optional Commands" to ### so it is a sibling of "Core Commands" under "Available Slash Commands" (consistent heading levels; avoids the h2->h4 jump a revert would create). - _version: allow --tag prerelease/dev and build-metadata suffixes to compose (e.g. v1.0.0-rc1+build.42), matching PEP 440 / semver; the Version() check still enforces canonical validity. - tests: compare resolved argv0 as Path objects instead of POSIX strings so the assertion holds on Windows; skip the relative-installer-path executable-bit tests on Windows via a new requires_posix marker (they rely on chmod/X_OK semantics and chdir-into-tmp teardown that do not hold there). Add a combined prerelease+build-metadata tag test.
- self_check: clarify that the "up to date" branch is reached only for parseable latest tags (the unparseable case returns earlier), so the InvalidVersion fallback assumption is not reintroduced. - self_upgrade: compare target/current as Version instances directly instead of re-parsing the canonical strings through _is_newer; the empty-current case stays explicit via the not-None guard. - tests: document the intentional broad GH_/GITHUB_ env scrub with a test asserting non-credential context vars (GH_HOST, GITHUB_REPOSITORY, …) are stripped from the installer subprocess env — a deliberate fail-safe that also catches credential-adjacent names without a recognized suffix.
- self_upgrade: unify the no-op short-circuits on packaging Version
equality instead of canonical-string equality. Version("1.0") equals
Version("1.0.0") but their str() forms differ, so the old check could
misreport an equal install as "already on latest release or newer".
Both the unpinned and pinned branches now use Version comparison.
- self_upgrade: compare the verified version as a parsed Version against
the target so a non-version verifier result is a mismatch (exit 2)
rather than a coincidental canonical-string match.
- resolver: map HTTP 429 (Too Many Requests / secondary rate limit) to
the rate-limited category so users get the same actionable token hint
as 403.
- _is_github_credential_env_key: document the precise (intentionally
broad) scrub matching contract in the docstring.
- tests: add a trailing-zero Version-equality regression test and a
parametrized HTTP-status categorization test (429 -> rate limited;
404/502 -> verbatim).
- self_upgrade: label a pinned target older than the installed version as "Downgrading" rather than "Upgrading" so `--tag <older>` is not mistaken for a forward upgrade. - resolver: drop the unused `typing.Optional` import and annotate the `--tag` option as `str | None`, consistent with the rest of the module (verified Typer resolves it on the supported Python versions). - _is_github_credential_env_key: add `_PASSWORD` and `_CREDENTIALS` to the recognized credential suffixes and document that only these shapes are scrubbed (not blanket coverage). - tests: assert the precise exit code (1) for the re-raised transient OSError path; skip the InvalidMetadataError test on Pythons where the real exception is absent instead of fabricating it; update the pinned downgrade test to expect the "Downgrading" label.
Fold a leading uppercase `V` (a common paste) to the canonical lowercase `v` before validating `--tag`. The remainder of the tag stays case-sensitive on purpose: the validated value is used verbatim as a git ref, which is case-sensitive on GitHub, so rewriting label/build-metadata casing could point at a tag that does not exist. Adds a normalization test.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replace the v0.7.5 reserved stub from #2316 with an actually working
specify self upgradecommand. Closes the unresolved portion of #2282.The CLI classifies the runtime via a 3-tier detection ladder, runs the appropriate installer subprocess, and verifies the result by spawning a fresh
specify --versionin a child process (the in-process module is still the pre-upgrade build).Behavior
specify self upgrade— executes immediately. No confirmation prompt. Matches thepip install -U/uv tool upgrade/npm updateconvention.--dry-run— on upgradable paths (uv tool,pipx) prints the preview block (method / current / target / installer argv) and exits 0 without launching any subprocess. On non-upgradable paths (uvx (ephemeral)/ source checkout / unsupported) emits the same path-specific guidance as a non-dry-run invocation and exits 0.--tag vX.Y.Z[suffix]— pins a specific release tag. Validated asvMAJOR.MINOR.PATCHplus optional dev, alpha/beta/rc, or build metadata suffixes; rejects barelatest, branch names, hash refs, and bare versions without thevprefix.uv tool/pipx(auto-upgrade),uvx (ephemeral)/ source-checkout / unsupported (path-specific guidance + exit 0, no installer launched).GH_TOKEN/GITHUB_TOKENscrubbed from child-process environments; never appears in installer argv.Exit codes
0--dry-run, or non-upgradable path with guidance)1--tagregex validation failure2specify --versiondoes not resolve to target; if the installer itself exits 2, that code is propagated verbatim3124SPECIFY_UPGRADE_TIMEOUT_SECSis set, or a real installer exit code 124 propagated verbatimFiles changed
src/specify_cli/_version.py— Phase 2 helpers (_InstallMethod,_UpgradePlan,_detect_install_method,_assemble_installer_argv,_build_upgrade_plan,_run_installer,_verify_upgrade,_emit_guidance,_emit_failure,_validate_tag,_scrubbed_env, …) + replacedself_upgrade()orchestrator body. Also refreshes the now-staleselfgroup help andself_check()docstring sospecify self --help/specify self check --helpreflect the current behavior. (Thespecify selfTyper command group is registered in__init__.py, which imports these helpers from_version.py.)tests/test_self_upgrade_detection.py,tests/test_self_upgrade_execution.py,tests/test_self_upgrade_guidance.py,tests/test_self_upgrade_verification.py(new) — focused test modules covering detection, argv assembly, dry-run, tag validation, token scrubbing, and verification flows.tests/self_upgrade_helpers.py,tests/http_helpers.py,tests/conftest.py— shared fixtures/helpers for the self-upgrade suite (installer / PATH / release-tag patching, plus therequires_posixskip marker for POSIX-only executable-bit tests).tests/test_upgrade.py— removedTestSelfUpgradeStub(the stub it pinned no longer exists).README.md—Get Started → Install Specify CLI → Option 1now leads withspecify self upgradeas the recommended path; manual--forceretained as fallback.docs/upgrade.md— Quick Reference table, Part 1, Verify, Common Scenarios, and Troubleshooting all updated.docs/installation.md— Verification section gets a "Stay current" pointer.Test plan
uvx ruff check src/— cleanuv sync --extra test+uv run pytest— 2828 passed, 34 skipped on Python 3.11 / 3.12 / 3.13tests/test_self_upgrade_*.py+tests/test_upgrade.py— full self-upgrade suite passingspecify self upgrade --dry-run→ preview, target=v0.8.6 (auto-resolved)specify self upgrade --dry-run --tag v0.8.0+build.42→ build-metadata tag acceptedspecify self upgrade --dry-run --tag latest→BadParameter(regex rejection)Notes for reviewers
pip install -U/uv tool upgrade/npm update)._fetch_latest_release_tag()(added in feat(cli): add specify self check and self upgrade stub #2316) is reused unchanged. No new GitHub HTTP code.re,dataclasses,enum,urllib.parse); PEP 723 deps unchanged._-prefixed; no new public API surface.Design and ripple review context
This change was developed through the full speckit workflow:
specify -> plan -> tasks -> blueprint -> implement -> ripple-scan -> ripple-resolve.Design artifacts (spec, plan, blueprint, ripple-report) were kept in
specs/002-self-upgrade-apply/during development and are not part of this PR.Ripple review surfaced 9 findings. Each was triaged as either resolved in-branch or accepted risk with explicit rationale.