Repository navigation
Conversation
`specify extension update` read the catalog version with `ext_info["version"]`, so an entry with no `version` raised KeyError, and a null or list value raised TypeError or AttributeError. Any of these aborted the command before the remaining installed extensions were checked. Read the value the way `specify workflow update` does, so these entries take the existing "Invalid catalog version ... (skipping)" path. Fixes github#5082
mnriem
added
the
triage-must-have
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Numeric catalog versions now pass discovery but are rejected during download, offering updates that cannot complete.
1 open issue
What changed in this PR
Addresses #5082 by preventing missing or malformed catalog version fields from aborting Specify CLI extension update checks.
Changes:
- Reads catalog versions defensively and reuses the existing skip warning.
- Adds discovery and CLI regression coverage for malformed entries, continued checking, and unchanged installed state.
The PR’s reported before-and-after results were reviewed; project tests were not rerun here.
| File | Description |
|---|---|
tests/specify_cli/extensions/test_command_update.py |
Checks successful CLI completion, the skip warning, and unchanged registry state. |
tests/specify_cli/extensions/test_command_update_discovery.py |
Covers missing, null, and list versions while verifying discovery continues. |
src/specify_cli/extensions/_command_update_discovery.py |
Converts optional catalog version values to strings before parsing. |
Review effort: Max
Collaborator
|
Please address Copilot feedback |
…date A numeric catalog version such as `"version": 2` passed discovery once coerced with str(), but the download rejects non-string versions, so the update was offered and then always failed. Require a string and take the existing "Invalid catalog version ... (skipping)" path otherwise.
Comment on lines
+102
to
+103
| if not isinstance(raw_catalog_version, str): | ||
| raise pkg_version.InvalidVersion(raw_catalog_version) |
Collaborator
|
Please address Copilot feedback |
This branch has not been deployed
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.
Description
discover_updatesread the catalog version withext_info["version"], so a catalog entry withoutversionmadespecify extension updateexit 1 withKeyError: 'version', and the installed extensions after it were never checked. Anullor list value crashed the same way, withTypeErrorandAttributeError.This PR reads the value with
ext_info.get("version")and accepts it only when it is a string. Anything else (missing,null, a list, or a number such as2) now takes the existingInvalid catalog version ... (skipping)branch and the loop carries on. Entries with a valid version behave as before. A number is rejected rather than converted: the download step passes the catalog's raw value tobuild_safe_download_path, which accepts only a string (_download_security.py:442), so a numeric version would be offered as an update that then fails to download.The issue's repro script, on
main(67e0f72), last traceback line only:With this PR:
Fixes #5082
Testing
uv run specify --helpuv sync && uv run pytestNew tests:
test_discovery_skips_catalog_entry_without_valid_versionintests/specify_cli/extensions/test_command_update_discovery.py, for a missing,null, list and numericversion. The broken entry comes first, and the test checks it is reported as skipped and that the second extension's update is still found.test_update_skips_catalog_entry_without_versionintests/specify_cli/extensions/test_command_update.pyrunsspecify extension updatethrough the CLI and checks exit code 0, the skip warning, and an unchanged registry.With only
src/reverted tomain, all 5 new cases fail (KeyError,TypeError,AttributeError, exit code 1). With the fix, the 31 tests in those two files pass.Full suite on macOS, Python 3.14.6,
pytest -n 2as in CI: 12807 passed, 434 skipped, none failed.uvx ruff@0.15.0 check src testspasses, and so doesgit diff --check.AI Disclosure
AI disclosure: Claude Code (model: claude-opus-5-5, autonomous) reproduced the bug, wrote the fix and tests, ran the suite and lint, and drafted this description.