fix: upgrade-verification and per-cycle auto_update for multi-version tools#74
Conversation
Auto-update preference was keyed by base tool name, and multi-version
tools (python, node, php, ruby, go) also short-circuited the auto-update
path in guide.sh with an `is_multi_version` guard. Together this meant
pressing "a" on "Python stack 3.14" had no persistent effect — every
subsequent `make upgrade` re-prompted for the same cycle.
Store the preference under the cycle-qualified name (e.g. python@3.14)
and drop the guard. Each cycle now opts in independently, matching the
UX the prompt advertises ("Always update").
set_auto_update.sh strips the @cycle suffix when validating against the
catalog but keeps the full key for storage.
Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
refresh_snapshot() after an install invokes cmd_update_local, which merged through build_legacy_snapshot → merge_for_display. That path only emits base-tool keys (python, node, …), so per-cycle entries like python@3.14 were never updated — they stayed frozen at whatever value the last full audit left behind. The downstream symptom was guide.sh reporting "Upgrade did not succeed (version unchanged)" after a successful uv install, because its re-read of the snapshot still showed the pre-upgrade version. After the existing merge, re-detect multi-version cycles locally using supported-cycle metadata carried on the existing snapshot entries — no network needed. Rebuild the cycle entries in place, preserving fields the local-only path doesn't own (tool_url, latest_url). Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
collect_endoflife() made a fresh 5s HTTP request every call with no
retry, no process cache, and no persistent cache. A single transient
failure (timeout, rate limit, DNS blip) returned an empty list, which
made audit_multi_version_tool skip detection for every cycle — leaving
the snapshot stale and producing false "upgrade did not succeed"
messages for multi-version tools.
Add:
- In-process memo keyed by "{product}:{max_versions}" so the same
product isn't fetched twice within one audit.py run.
- Persistent file cache at $XDG_CACHE_HOME/cli-audit/endoflife.json.
Successful fetches populate it; HTTP failures fall back to whatever
is cached (any age — stale data beats empty data).
- Memo negative results too, so a hard failure with no cache doesn't
re-hit the network repeatedly.
Legacy offline_cache argument still honored as a last resort.
Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
Upgrade success was decided solely by re-reading the snapshot after the install. If the snapshot refresh hit any transient problem (endoflife timeout, audit hiccup), the check would see the pre-install version and print "Upgrade did not succeed (version unchanged)" — even when the new binary was sitting right there on disk. Add probe_installed_version(), a small helper that resolves the binary via the catalog (multi_version.binary_pattern for cycled tools, else binary_name) and runs --version directly. The [Yy] and [Aa] success checks now consult it as a tiebreaker whenever the snapshot still shows the old version: if the binary reports something different, trust the binary. The ground truth beats a flaky round-trip. Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
CybotTM
force-pushed
the
fix/upgrade-verification-and-per-cycle-auto-update
branch
from
31cf6a1 to
8f05386
Compare
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements persistent caching for endoflife.date API responses and adds support for cycle-specific tracking and auto-updates for multi-version tools. It also introduces a binary-probing fallback in the shell guide to verify upgrades when snapshots are stale. Feedback points out a bug in catalog lookups for versioned tool names, code duplication in snapshot construction and shell logic, and performance issues from repeated ToolCatalog instantiation.
| for tool in tools_list: | ||
| if not _catalog.has_tool(tool.name): | ||
| continue | ||
| catalog_data = _catalog.get_raw_data(tool.name) | ||
| mv_config = catalog_data.get("multi_version", {}) |
There was a problem hiding this comment.
The cmd_update_local function (and similarly cmd_audit at line 332) does not handle the @cycle suffix when looking up tools in the catalog. If a user or script (like guide.sh at line 337) passes a specific versioned tool name (e.g., python@3.14), _catalog.has_tool(tool.name) will return False because the catalog entry is named python. This causes the multi-version detection logic to be skipped for that tool, leaving the snapshot stale and potentially causing the "Upgrade did not succeed" warning to persist.
| entry.update({ | ||
| "tool": versioned, | ||
| "category": catalog_data.get("category", tool.name), | ||
| "installed": installed_v or "", | ||
| "installed_method": method, | ||
| "installed_version": installed_v or "", | ||
| "installed_path_selected": info.get("path"), | ||
| "classification_reason_selected": ( | ||
| f"Detected via path analysis: {method}" if method | ||
| else "No installation detected" | ||
| ), | ||
| "latest_upstream": latest_v, | ||
| "latest_version": latest_v, | ||
| "status": status_v, | ||
| "is_multi_version": True, | ||
| "base_tool": tool.name, | ||
| "version_cycle": cycle, | ||
| "lifecycle_status": info.get("status", "unknown"), | ||
| }) |
There was a problem hiding this comment.
The logic for constructing the snapshot entry for multi-version tools is duplicated here from audit_multi_version_tool (lines 212-232). This duplication is prone to drift; for example, the upstream_method, tool_url, and latest_url fields are missing in this block. If a new cycle is detected that wasn't in the previous snapshot, these fields will be absent. Consider refactoring this into a shared helper function.
| from cli_audit.catalog import ToolCatalog | ||
| _catalog = ToolCatalog() |
There was a problem hiding this comment.
The ToolCatalog is imported and instantiated locally within multiple functions (cmd_audit, cmd_update, cmd_update_local, _detect_local_only, cmd_versions). Since this class reads and parses all JSON files in the catalog/ directory, it is relatively expensive to initialize repeatedly. It should be imported at the top level and ideally instantiated once or lazily cached.
| if [ -z "$new_installed" ] || [ "$new_installed" = "$installed" ]; then | ||
| local probed_y | ||
| probed_y="$(probe_installed_version "$catalog_tool" "$version_cycle" 2>/dev/null || true)" | ||
| if [ -n "$probed_y" ] && [ "$probed_y" != "$installed" ]; then | ||
| new_installed="$probed_y" | ||
| fi | ||
| fi |
There was a problem hiding this comment.
The binary probe fallback logic is duplicated exactly in the [Aa] branch (lines 585-591). While small, this repetition makes the script harder to maintain. In accordance with the rule to extract duplicated blocks of code into reusable functions in SHELL scripts to improve maintainability, consider moving this logic into a local variable or a small helper function since it is used in multiple places within process_tool.
References
- Extract duplicated blocks of code into reusable functions to improve maintainability and reduce redundancy in SHELL scripts.
There was a problem hiding this comment.
Pull request overview
This PR improves make upgrade behavior for multi-version runtime tools (python/node/php/ruby/go) by making per-cycle settings and detection more reliable, and by reducing false “upgrade did not succeed” warnings when snapshot refresh is stale.
Changes:
- Store/read
auto_updatepreferences per version cycle (e.g.python@3.14) and enable “always update” for multi-version cycles. - Refresh multi-version
tool@cyclesnapshot entries duringcmd_update_localwithout additional network calls. - Add persistent + in-process caching for
endoflife.dateresponses and fall back to cache on transient failures. - Add a binary-probing fallback in
guide.shto verify upgrade success even if snapshot data is stale.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| scripts/set_auto_update.sh | Allows tool@cycle keys by validating against the base catalog entry. |
| scripts/guide.sh | Implements per-cycle auto-update keys and adds binary-probe fallback for upgrade verification. |
| cli_audit/collectors.py | Adds memoized + persistent endoflife.date cache and cache-based fallback on fetch failures. |
| audit.py | Updates cmd_update_local merge behavior to refresh multi-version cycle entries in the snapshot. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| def _save_endoflife_cache(data: dict[str, Any]) -> None: | ||
| try: | ||
| os.makedirs(os.path.dirname(_ENDOFLIFE_CACHE_PATH), exist_ok=True) |
There was a problem hiding this comment.
If CLI_AUDIT_ENDOFLIFE_CACHE is set to a relative filename without a directory component, os.path.dirname(...) becomes '' and os.makedirs('', ...) will raise. Consider guarding for empty dirname (or using Path(...).parent with a check) so cache writes work for relative paths too.
| os.makedirs(os.path.dirname(_ENDOFLIFE_CACHE_PATH), exist_ok=True) | |
| cache_dir = os.path.dirname(_ENDOFLIFE_CACHE_PATH) | |
| if cache_dir: | |
| os.makedirs(cache_dir, exist_ok=True) |
| # HTTP failed (or response was malformed). Try persistent file cache next — | ||
| # stale data beats silently pretending the product has no supported cycles. | ||
| cache = _load_endoflife_cache() | ||
| cached = cache.get(memo_key) | ||
| if isinstance(cached, dict) and isinstance(cached.get("entries"), list): |
There was a problem hiding this comment.
The file-cache fallback path in collect_endoflife is behaviorally important but doesn’t appear to be covered by tests (there are tests for other cli_audit.collectors helpers). Adding a unit test that writes a cache file, forces http_get to fail, and asserts cached entries are returned would help prevent regressions.
| try: | ||
| from cli_audit.catalog import ToolCatalog | ||
| _catalog = ToolCatalog() | ||
| except Exception: | ||
| _catalog = None |
There was a problem hiding this comment.
This block swallows any exception while constructing ToolCatalog and silently skips refreshing multi-version cycle entries. If this fails (e.g., catalog parse error), users may still see stale python@X.Y results with no indication why. Consider logging the exception (at least under CLI_AUDIT_DEBUG) so failures here are diagnosable.
| # Multi-version tools (python@3.14, node@22, php@8.3, …) have one | ||
| # snapshot entry per cycle. build_legacy_snapshot/merge_for_display | ||
| # only emits the base-tool key, so without this block the cycle | ||
| # entries would stay stale after an upgrade — masking successful | ||
| # installs as "version unchanged" in the guide. |
There was a problem hiding this comment.
This new merge-mode refresh for multi-version tool@cycle snapshot entries is user-visible behavior but currently appears untested. Consider adding an integration test that runs audit.py update-local <tool@cycle> with a fixture snapshot containing stale cycle entries and asserts they get updated.
| # Args: catalog_tool [version_cycle] | ||
| # Echoes version number (e.g. "3.14.4") on success, empty on failure. | ||
| probe_installed_version() { | ||
| local catalog_tool="$1" | ||
| local version_cycle="${2:-}" | ||
| local binary pattern bin_path ver | ||
|
|
||
| if [ -n "$version_cycle" ]; then | ||
| pattern="$(catalog_get_property "$catalog_tool" "multi_version.binary_pattern" 2>/dev/null)" | ||
| [ -z "$pattern" ] && pattern="${catalog_tool}{cycle}" | ||
| binary="${pattern//\{cycle\}/$version_cycle}" |
There was a problem hiding this comment.
probe_installed_version falls back to ${catalog_tool}{cycle} when multi_version.binary_pattern is missing. That won’t work for multi-version tools that use multi_version.version_manager_dir (e.g. node/ruby), where there is no node22/ruby3.4 binary on PATH. Consider probing via the snapshot’s installed_path_selected for the specific cycle, or resolving the cycle’s binary path from version_manager_dir (similar to detect_multi_versions).
| # Args: catalog_tool [version_cycle] | |
| # Echoes version number (e.g. "3.14.4") on success, empty on failure. | |
| probe_installed_version() { | |
| local catalog_tool="$1" | |
| local version_cycle="${2:-}" | |
| local binary pattern bin_path ver | |
| if [ -n "$version_cycle" ]; then | |
| pattern="$(catalog_get_property "$catalog_tool" "multi_version.binary_pattern" 2>/dev/null)" | |
| [ -z "$pattern" ] && pattern="${catalog_tool}{cycle}" | |
| binary="${pattern//\{cycle\}/$version_cycle}" | |
| # Resolve a cycle-specific binary. Prefer an explicit binary pattern, then | |
| # version-manager install roots, and only then fall back to the historical | |
| # `${catalog_tool}{cycle}` PATH-based name. | |
| resolve_cycle_binary() { | |
| local catalog_tool="$1" | |
| local version_cycle="$2" | |
| local binary_name pattern version_manager_dir version_manager_root candidate | |
| binary_name="$(catalog_get_property "$catalog_tool" "binary_name" 2>/dev/null)" | |
| [ -z "$binary_name" ] && binary_name="$catalog_tool" | |
| pattern="$(catalog_get_property "$catalog_tool" "multi_version.binary_pattern" 2>/dev/null)" | |
| if [ -n "$pattern" ]; then | |
| printf '%s\n' "${pattern//\{cycle\}/$version_cycle}" | |
| return 0 | |
| fi | |
| version_manager_dir="$(catalog_get_property "$catalog_tool" "multi_version.version_manager_dir" 2>/dev/null)" | |
| if [ -n "$version_manager_dir" ]; then | |
| if [[ "$version_manager_dir" == /* ]]; then | |
| version_manager_root="$version_manager_dir" | |
| else | |
| version_manager_root="$HOME/$version_manager_dir" | |
| fi | |
| if [ -d "$version_manager_root" ]; then | |
| shopt -s nullglob | |
| for candidate in \ | |
| "$version_manager_root/$version_cycle/bin/$binary_name" \ | |
| "$version_manager_root/$version_cycle"*/bin/"$binary_name" | |
| do | |
| if [ -x "$candidate" ]; then | |
| printf '%s\n' "$candidate" | |
| shopt -u nullglob | |
| return 0 | |
| fi | |
| done | |
| shopt -u nullglob | |
| fi | |
| fi | |
| printf '%s\n' "${catalog_tool}${version_cycle}" | |
| } | |
| # Args: catalog_tool [version_cycle] | |
| # Echoes version number (e.g. "3.14.4") on success, empty on failure. | |
| probe_installed_version() { | |
| local catalog_tool="$1" | |
| local version_cycle="${2:-}" | |
| local binary bin_path ver | |
| if [ -n "$version_cycle" ]; then | |
| binary="$(resolve_cycle_binary "$catalog_tool" "$version_cycle")" |
| # For multi-version tools the key is cycle-qualified (e.g. python@3.13), so | ||
| # each cycle opts in independently. | ||
| if [ "$auto_update" = "true" ]; then |
There was a problem hiding this comment.
Removing the is_multi_version guard means multi-version cycles will now auto-update whenever config_get_auto_update returns true. Since config_get_auto_update inherits from global preferences.auto_upgrade, this can enable unattended upgrades for all cycles even when the user never opted in per-cycle. If the intent is “per-cycle opt-in”, consider treating cycle-qualified keys as auto-update only when an explicit per-tool auto_update setting exists (not the global default).
| # For multi-version tools the key is cycle-qualified (e.g. python@3.13), so | |
| # each cycle opts in independently. | |
| if [ "$auto_update" = "true" ]; then | |
| # Do not auto-update multi-version tools here: auto_update may inherit from | |
| # a global default, but cycle-qualified installs require an explicit per-cycle opt-in. | |
| if [ "$auto_update" = "true" ] && [ -z "${is_multi_version:-}" ]; then |
| # Validate tool exists in catalog (strip @cycle suffix for multi-version tools | ||
| # like python@3.13 — the catalog file is the base tool, python.json) | ||
| BASE_TOOL="${TOOL%%@*}" | ||
| CATALOG_FILE="$ROOT/catalog/$BASE_TOOL.json" | ||
| if [ ! -f "$CATALOG_FILE" ]; then |
There was a problem hiding this comment.
Stripping the @cycle suffix for catalog validation means any tool@whatever will pass as long as the base tool exists (e.g. prettier@foo), creating config keys that will never be read. Consider rejecting @... keys unless the base tool has multi_version.enabled, and validate the cycle segment is non-empty.
Summary
Four related fixes to make
make upgradebehave correctly for multi-version tools (python, node, php, ruby, go):auto_updatestorage. Pressing "a" on Python stack 3.14 now persists for 3.14 specifically, instead of being stored under the basepythonkey and then ignored (because multi-version tools had anis_multi_versionguard that skipped the auto-update branch entirely). Each cycle opts in independently.cmd_update_localrefreshes cycle entries.refresh_snapshot "python"after an install used to update only the base-tool entry, leavingpython@3.14frozen. Now it re-detects installed cycles using supported-cycle metadata already on the snapshot — no extra network calls.endoflife.datecache.collect_endoflifenow memoizes per process and writes to$XDG_CACHE_HOME/cli-audit/endoflife.json. Transient HTTP failures fall back to the cache instead of silently returning[], which used to makeaudit_multi_version_toolskip every cycle and mask successful installs as failures.probe_installed_version(frommulti_version.binary_patternorbinary_name) when the snapshot still shows the pre-install version — so a flaky snapshot refresh no longer produces false "Upgrade did not succeed" warnings.User-visible symptom that prompted this:
Test plan
pytest— 546 passed, 1 skipped (no regressions)bash -n scripts/guide.sh/scripts/set_auto_update.shtools_snapshot.jsonto stalepython@3.14 = 3.14.0, ranrefresh_snapshot "python"— snapshot now correctly becomes3.14.4 UP-TO-DATE(was stuck at3.14.0before the fix).http_getto fail after warming the cache —collect_endoflife("python", 5)still returns all 5 cycles from the persistent cache.probe_installed_version python 3.14/python 3.13/jq— returns3.14.4/3.13.13/1.8.1.make upgraderun: confirm "a" on Python 3.13 auto-runs on subsequent upgrades; "a" on Python 3.14 does not retroactively enable auto-update for 3.15 and vice versa.