From 43f7118747ca141f58d1e732e40cfb4c1bc4d35b Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Tue, 21 Apr 2026 14:09:10 +0200 Subject: [PATCH 1/8] refactor(catalog): remove unused pinned_version field and methods MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ``pinned_version`` field on ``ToolCatalogEntry`` and the ``is_pinned`` / ``get_pinned_version`` / ``should_skip`` methods on ``ToolCatalog`` were never populated — zero ``catalog/*.json`` files set them, so the ``[PINNED]`` marker in the audit renderer could never fire from real data. Pins live per-user in ``~/.config/cli-audit/pins.json`` (managed by the shell helpers under ``scripts/``); the catalog-level mechanism was a dead parallel path. Documentation rewritten to point at the real pin store and the ``scripts/pin_version.sh`` / ``unpin_version.sh`` helpers. Signed-off-by: Sebastian Mendel --- cli_audit/catalog.py | 53 ------------------------------------------- docs/API_REFERENCE.md | 1 - docs/CATALOG_GUIDE.md | 53 +++++++++++++++++++++++++------------------ 3 files changed, 31 insertions(+), 76 deletions(-) diff --git a/cli_audit/catalog.py b/cli_audit/catalog.py index 0f7eb30..5e3e789 100644 --- a/cli_audit/catalog.py +++ b/cli_audit/catalog.py @@ -32,7 +32,6 @@ class ToolCatalogEntry: install_method: str = "" package_name: str = "" script: str = "" - pinned_version: str = "" notes: str = "" candidates: list[str] | None = None # NEW: Binary names to search for (defaults to [binary_name]) category: str = "" # NEW: Tool category (runtimes, search, editors, etc.) @@ -52,7 +51,6 @@ def from_dict(cls, data: dict[str, Any]) -> "ToolCatalogEntry": install_method=data.get("install_method", ""), package_name=data.get("package_name", ""), script=data.get("script", ""), - pinned_version=data.get("pinned_version", ""), notes=data.get("notes", ""), candidates=data.get("candidates"), # NEW category=data.get("category", ""), # NEW @@ -221,57 +219,6 @@ def get_raw_data(self, tool_name: str) -> dict[str, Any]: """ return self._raw_data.get(tool_name, {}) - def is_pinned(self, tool_name: str) -> bool: - """Check if a tool has a pinned version. - - Args: - tool_name: Tool name - - Returns: - True if tool has pinned version (not empty and not "never") - """ - entry = self.get(tool_name) - if not entry: - return False - - pinned = entry.pinned_version - return bool(pinned and pinned != "never") - - def get_pinned_version(self, tool_name: str) -> str: - """Get pinned version for a tool. - - Args: - tool_name: Tool name - - Returns: - Pinned version string or empty string if not pinned - """ - entry = self.get(tool_name) - if not entry: - return "" - - pinned = entry.pinned_version - if pinned and pinned != "never": - return pinned - return "" - - def should_skip(self, tool_name: str, latest_version: str) -> bool: - """Check if tool should be skipped (pinned and already at pinned version). - - Args: - tool_name: Tool name - latest_version: Latest available version - - Returns: - True if tool should be skipped - """ - pinned = self.get_pinned_version(tool_name) - if not pinned: - return False - - # Simple version comparison - if pinned matches latest, skip - return pinned == latest_version - def all_tools(self) -> list[str]: """Get list of all tool names in catalog. diff --git a/docs/API_REFERENCE.md b/docs/API_REFERENCE.md index 8b73150..fe26e41 100644 --- a/docs/API_REFERENCE.md +++ b/docs/API_REFERENCE.md @@ -86,7 +86,6 @@ class ToolCatalogEntry: install_method: str = "" package_name: str = "" script: str = "" - pinned_version: str = "" notes: str = "" candidates: list[str] | None = None category: str = "" diff --git a/docs/CATALOG_GUIDE.md b/docs/CATALOG_GUIDE.md index 28c9743..8005fc3 100644 --- a/docs/CATALOG_GUIDE.md +++ b/docs/CATALOG_GUIDE.md @@ -47,10 +47,11 @@ print(f"GitHub: {entry.github_repo}") if catalog.has("fzf"): fzf = catalog.get("fzf") -# Get pinned version -ctags = catalog.get("ctags") -if ctags.pinned_version: - print(f"ctags pinned to {ctags.pinned_version}") +# Check pin (pins live in ~/.config/cli-audit/pins.json, not the catalog) +from cli_audit.pins import lookup_pin +pin = lookup_pin("ctags") +if pin: + print(f"ctags pinned to {pin}") # Iterate all entries for name, entry in catalog.items(): @@ -85,9 +86,12 @@ for name, entry in catalog.items(): ], "requires": ["string (dependency names)"], "tags": ["string (categorization)"], - "pinned_version": "string (optional, pins to specific version)", "notes": "string (optional, additional context)" } + +// Note: version pins are stored per user in +// ~/.config/cli-audit/pins.json (managed by scripts/pin_version.sh), +// not in the catalog. ``` ### Field Descriptions @@ -166,11 +170,6 @@ for name, entry in catalog.items(): - Common tags: `"core"`, `"optional"`, `"dev"` - Used for filtering and organization -**`pinned_version`** (string) -- Pin tool to specific version -- Prevents upgrade suggestions -- Example: `"1.2.3"` or `"v1.2.3"` - **`notes`** (string) - Additional context or caveats - Displayed in audit output @@ -252,8 +251,7 @@ for name, entry in catalog.items(): "homepage": "https://github.com/universal-ctags/ctags", "github_repo": "universal-ctags/ctags", "binary_name": "ctags", - "pinned_version": "5.9.0", - "notes": "Pinned to 5.9.0 for compatibility" + "notes": "Users typically pin this via `make pin-ctags 5.9.0` for compatibility" } ``` @@ -421,28 +419,40 @@ tool -V # Capital variant ### Pinning Versions +**Where pins live:** `~/.config/cli-audit/pins.json` — per-user, not in the +catalog. Pins are a user preference, not a property of the tool definition. + **When to Pin:** - Breaking changes in new versions - Compatibility requirements - Stability for production use - Testing specific version behavior -**How to Pin:** -```json -{ - "pinned_version": "1.2.3", // Without 'v' - "notes": "Pinned for compatibility with project X" -} +**How to Pin:** use the shell helpers that read/write the pins file: + +```bash +# Single-version tool +scripts/pin_version.sh ripgrep 14.1.0 + +# Multi-version runtime (cycle-aware) +scripts/pin_version.sh python@3.12 3.12.7 +scripts/pin_version.sh node@22 never # hard skip +scripts/unpin_version.sh python@3.12 +scripts/reset_pins.sh # wipe all pins ``` -**Unpinning:** +The resulting `pins.json` looks like: ```json { - "pinned_version": "", // Empty string - // OR remove field entirely + "ripgrep": "14.1.0", + "python": {"3.12": "3.12.7"}, + "node": {"22": "never"} } ``` +Rendered in `make audit` as `PIN:14.1.0`, `PIN:3.12.7`, and `PIN:never` +in the `notes` column. + ## Fallback to Python TOOLS If a tool is not in the catalog, the system falls back to the Python `TOOLS` tuple in `cli_audit/tools.py`: @@ -579,7 +589,6 @@ entry.binary_name: str entry.install_method: str entry.package_name: str entry.script: str -entry.pinned_version: str entry.notes: str ``` From e1a0a333180fb68138b83d5881810d694570cf7e Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Tue, 21 Apr 2026 14:09:29 +0200 Subject: [PATCH 2/8] feat(pins): add reader for user-local pins.json MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add ``cli_audit/pins.py`` that reads ``~/.config/cli-audit/pins.json`` — the file the shell helpers (``scripts/pin_version.sh``, ``unpin_version.sh``, ``reset_pins.sh``) already populate. The Python side now understands: - flat ``{"tool": "version"}`` pins - nested ``{"tool": {"cycle": "version"}}`` for multi-version runtimes - the ``"never"`` sentinel meaning "do not install or update" - ``tool@cycle`` lookup for names like ``python@3.13`` ``audit.py`` (update path) switches from the now-removed catalog methods to the new API. Signed-off-by: Sebastian Mendel --- audit.py | 10 ++-- cli_audit/__init__.py | 13 +++++ cli_audit/pins.py | 123 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 142 insertions(+), 4 deletions(-) create mode 100644 cli_audit/pins.py diff --git a/audit.py b/audit.py index ed5ad4b..f6d8bdc 100755 --- a/audit.py +++ b/audit.py @@ -616,11 +616,13 @@ def cmd_update(args: argparse.Namespace) -> int: inst_display = _sanitize(inst) if inst else "n/a" latest_display = _sanitize(latest) if latest else "n/a" - # Add pinned/skip markers (reuse catalog from outer scope) + # Add pinned/skip markers from user pins file + from cli_audit.pins import lookup_pin, should_skip as _pin_should_skip + pin_val = lookup_pin(tool.name) markers = [] - if catalog.is_pinned(tool.name): - markers.append("PINNED") - if catalog.should_skip(tool.name, latest): + if pin_val: + markers.append("NEVER" if pin_val == "never" else f"PIN:{pin_val}") + if _pin_should_skip(tool.name, latest): markers.append("SKIP") marker_str = f" [{' '.join(markers)}]" if markers else "" diff --git a/cli_audit/__init__.py b/cli_audit/__init__.py index f88eac5..50aa1ae 100644 --- a/cli_audit/__init__.py +++ b/cli_audit/__init__.py @@ -54,6 +54,13 @@ validate_config, ) from .package_managers import PackageManager, select_package_manager, get_available_package_managers # noqa: E402 +from .pins import ( # noqa: E402 + load_pins, + lookup_pin, + is_pinned, + is_never, + should_skip, +) from .install_plan import InstallPlan, InstallStep, generate_install_plan, dry_run_install # noqa: E402 # Installation @@ -170,6 +177,12 @@ "PackageManager", "select_package_manager", "get_available_package_managers", + # Pins + "load_pins", + "lookup_pin", + "is_pinned", + "is_never", + "should_skip", "InstallPlan", "InstallStep", "generate_install_plan", diff --git a/cli_audit/pins.py b/cli_audit/pins.py new file mode 100644 index 0000000..cb283bd --- /dev/null +++ b/cli_audit/pins.py @@ -0,0 +1,123 @@ +""" +Version-pin reader for ``~/.config/cli-audit/pins.json``. + +Pins are managed by the shell scripts (``scripts/pin_version.sh``, +``scripts/unpin_version.sh``, ``scripts/reset_pins.sh``) and stored in a +user-local JSON file. The Python code only reads them. + +File format:: + + { + "ripgrep": "14.1.0", # single-version tool + "php": {"8.5": "8.5.3", # multi-version tool + "8.4": "8.4.18", + "8.2": "never"}, + "node": {"24": "never"} + } + +A value of ``"never"`` means "do not install or update" — effectively a +hard skip. Any other string is the pinned version. An empty/missing value +means not pinned. + +Tool names may arrive as ``"python@3.13"`` (multi-version runtime) or a +plain single-version name. :func:`lookup_pin` handles both. +""" + +from __future__ import annotations + +import json +import os +from functools import lru_cache +from typing import Any + + +DEFAULT_PINS_PATH = os.path.expanduser( + os.environ.get( + "CLI_AUDIT_PINS_PATH", + os.path.join( + os.environ.get("XDG_CONFIG_HOME", os.path.expanduser("~/.config")), + "cli-audit", + "pins.json", + ), + ) +) + + +@lru_cache(maxsize=1) +def _load_pins_cached(path: str) -> dict[str, Any]: + if not os.path.isfile(path): + return {} + try: + with open(path, "r", encoding="utf-8") as f: + data = json.load(f) + except (OSError, json.JSONDecodeError): + return {} + return data if isinstance(data, dict) else {} + + +def load_pins(path: str | None = None) -> dict[str, Any]: + """Load and return the pins mapping. + + Args: + path: Optional override path. Defaults to + ``~/.config/cli-audit/pins.json``. + """ + return _load_pins_cached(path or DEFAULT_PINS_PATH) + + +def reset_cache() -> None: + """Clear the in-process cache. Useful for tests.""" + _load_pins_cached.cache_clear() + + +def _split_tool(tool_name: str) -> tuple[str, str | None]: + """Split ``"python@3.13"`` into ``("python", "3.13")``. + + Returns ``(base, None)`` for single-version tools. + """ + if "@" in tool_name: + base, cycle = tool_name.split("@", 1) + return base, cycle + return tool_name, None + + +def lookup_pin(tool_name: str, pins: dict[str, Any] | None = None) -> str: + """Return the pinned value for a tool, or empty string if not pinned. + + ``"never"`` is a valid return value and means "never update/install". + """ + pins = pins if pins is not None else load_pins() + base, cycle = _split_tool(tool_name) + entry = pins.get(base) + if entry is None: + return "" + if isinstance(entry, dict): + if cycle is None: + return "" + value = entry.get(cycle, "") + return value if isinstance(value, str) else "" + return entry if isinstance(entry, str) else "" + + +def is_pinned(tool_name: str, pins: dict[str, Any] | None = None) -> bool: + """True if the tool has any pin (including ``"never"``).""" + return bool(lookup_pin(tool_name, pins)) + + +def is_never(tool_name: str, pins: dict[str, Any] | None = None) -> bool: + """True if the tool is pinned to the sentinel ``"never"``.""" + return lookup_pin(tool_name, pins) == "never" + + +def should_skip(tool_name: str, latest_version: str, pins: dict[str, Any] | None = None) -> bool: + """True if updates for this tool should be skipped. + + Skip when the tool is pinned to ``"never"`` or when the pinned version + already matches the latest upstream version. + """ + pin = lookup_pin(tool_name, pins) + if not pin: + return False + if pin == "never": + return True + return pin == latest_version From 9539807a5f339cd63cb6483feb63d2f6e4b7c695 Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Tue, 21 Apr 2026 14:09:49 +0200 Subject: [PATCH 3/8] refactor(audit): delete canned hint plumbing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ``make audit`` was emitting hardcoded hints like ``Install python 3.13: check your package manager or version manager`` on every not-installed and outdated multi-version row. They added no information — the tool name and state already tell the user what action is possible — so rip them out everywhere: - ``audit.py``: stop generating the canned strings in ``audit_multi_version_tool`` and in the local-only update path; delete the ``CLI_AUDIT_HINTS`` env var and ``SHOW_HINTS`` plumbing. Catalog-provided ``hint`` values are still honored. - ``Makefile.d/user.mk``: drop ``CLI_AUDIT_HINTS=1`` from every audit target (it was overriding the env default anyway). - ``docs/``: remove ``CLI_AUDIT_HINTS`` from CLI / quick / deployment references. Signed-off-by: Sebastian Mendel --- Makefile.d/user.mk | 10 +++++----- audit.py | 25 ++++++++++--------------- docs/CLI_REFERENCE.md | 1 - docs/DEPLOYMENT.md | 2 -- docs/QUICK_REFERENCE.md | 3 --- 5 files changed, 15 insertions(+), 26 deletions(-) diff --git a/Makefile.d/user.mk b/Makefile.d/user.mk index f85d4d5..a6cb922 100644 --- a/Makefile.d/user.mk +++ b/Makefile.d/user.mk @@ -47,15 +47,15 @@ audit: ## Render audit from snapshot (no network, <100ms) echo " Consider running '\''make update'\'' for fresh version data." >&2; \ fi; \ fi; \ - set -o pipefail; CLI_AUDIT_RENDER=1 CLI_AUDIT_GROUP=0 CLI_AUDIT_HINTS=1 CLI_AUDIT_LINKS=1 CLI_AUDIT_EMOJI=1 CLI_AUDIT_COLOR=1 $(PYTHON) audit.py | \ + set -o pipefail; CLI_AUDIT_RENDER=1 CLI_AUDIT_GROUP=0 CLI_AUDIT_LINKS=1 CLI_AUDIT_EMOJI=1 CLI_AUDIT_COLOR=1 $(PYTHON) audit.py | \ $(PYTHON) smart_column.py -s "|" -t --right 3,5 --header' || true audit-offline: ## Offline audit with hints (fast local scan) - @bash -c 'set -o pipefail; CLI_AUDIT_OFFLINE=1 CLI_AUDIT_RENDER=1 CLI_AUDIT_GROUP=0 CLI_AUDIT_HINTS=1 CLI_AUDIT_LINKS=1 CLI_AUDIT_EMOJI=1 CLI_AUDIT_COLOR=1 $(PYTHON) audit.py | \ + @bash -c 'set -o pipefail; CLI_AUDIT_OFFLINE=1 CLI_AUDIT_RENDER=1 CLI_AUDIT_GROUP=0 CLI_AUDIT_LINKS=1 CLI_AUDIT_EMOJI=1 CLI_AUDIT_COLOR=1 $(PYTHON) audit.py | \ $(PYTHON) smart_column.py -s "|" -t --right 3,5 --header' || true outdated: ## Show only missing and outdated tools - @bash -c 'set -o pipefail; CLI_AUDIT_RENDER=1 CLI_AUDIT_GROUP=0 CLI_AUDIT_HINTS=1 CLI_AUDIT_LINKS=1 CLI_AUDIT_EMOJI=1 CLI_AUDIT_COLOR=1 CLI_AUDIT_FILTER_STATUS="NOT INSTALLED,OUTDATED" $(PYTHON) audit.py | \ + @bash -c 'set -o pipefail; CLI_AUDIT_RENDER=1 CLI_AUDIT_GROUP=0 CLI_AUDIT_LINKS=1 CLI_AUDIT_EMOJI=1 CLI_AUDIT_COLOR=1 CLI_AUDIT_FILTER_STATUS="NOT INSTALLED,OUTDATED" $(PYTHON) audit.py | \ $(PYTHON) smart_column.py -s "|" -t --right 3,5 --header' || true audit-%: scripts-perms ## Audit single tool (e.g., make audit-ripgrep) @@ -63,7 +63,7 @@ audit-%: scripts-perms ## Audit single tool (e.g., make audit-ripgrep) $(PYTHON) smart_column.py -s "|" -t --right 3,5 --header' || true audit-offline-%: scripts-perms ## Offline audit subset (e.g., make audit-offline-python-core) - @bash -c 'set -o pipefail; CLI_AUDIT_OFFLINE=1 CLI_AUDIT_RENDER=1 CLI_AUDIT_GROUP=0 CLI_AUDIT_HINTS=1 CLI_AUDIT_LINKS=1 CLI_AUDIT_EMOJI=1 CLI_AUDIT_COLOR=1 $(PYTHON) audit.py $* | \ + @bash -c 'set -o pipefail; CLI_AUDIT_OFFLINE=1 CLI_AUDIT_RENDER=1 CLI_AUDIT_GROUP=0 CLI_AUDIT_LINKS=1 CLI_AUDIT_EMOJI=1 CLI_AUDIT_COLOR=1 $(PYTHON) audit.py $* | \ $(PYTHON) smart_column.py -s "|" -t --right 3,5 --header' || true SNAP_FILE?=$(shell python3 -c "import os;print(os.environ.get('CLI_AUDIT_SNAPSHOT_FILE','tools_snapshot.json'))") @@ -73,7 +73,7 @@ audit-auto: ## Update snapshot if missing, then render echo "# snapshot missing: $(SNAP_FILE); running update..."; \ CLI_AUDIT_COLLECT=1 CLI_AUDIT_DEBUG=1 CLI_AUDIT_PROGRESS=1 $(PYTHON) audit.py --update || true; \ fi; \ - CLI_AUDIT_RENDER=1 CLI_AUDIT_GROUP=0 CLI_AUDIT_HINTS=1 CLI_AUDIT_LINKS=1 CLI_AUDIT_EMOJI=1 $(PYTHON) audit.py | \ + CLI_AUDIT_RENDER=1 CLI_AUDIT_GROUP=0 CLI_AUDIT_LINKS=1 CLI_AUDIT_EMOJI=1 $(PYTHON) audit.py | \ $(PYTHON) smart_column.py -s "|" -t --right 3,5 --header || true # ---------------------------------------------------------------------------- diff --git a/audit.py b/audit.py index f6d8bdc..875be14 100755 --- a/audit.py +++ b/audit.py @@ -58,7 +58,8 @@ def _sanitize(s: str) -> str: # Configuration from environment OFFLINE_MODE = os.environ.get("CLI_AUDIT_OFFLINE", "0") == "1" MAX_WORKERS = int(os.environ.get("CLI_AUDIT_MAX_WORKERS", "16")) -SHOW_HINTS = os.environ.get("CLI_AUDIT_HINTS", "1") == "1" +# (CLI_AUDIT_HINTS is gone; canned hints added no information — the row +# state and tool name already tell the user what action is available.) COLLECT_MODE = os.environ.get("CLI_AUDIT_COLLECT", "0") == "1" RENDER_MODE = os.environ.get("CLI_AUDIT_RENDER", "0") == "1" JSON_MODE = os.environ.get("CLI_AUDIT_JSON", "0") == "1" @@ -201,13 +202,10 @@ def audit_multi_version_tool( else: classification_reason = "No installation detected" - # Hint for not installed or outdated - if status == "NOT INSTALLED": - hint = f"Install {tool_name} {cycle}: check your package manager or version manager" - elif status == "OUTDATED": - hint = f"Upgrade {tool_name} {cycle}: {installed} → {latest}" - else: - hint = "" + # Hint comes from catalog when provided (empty for generic runtimes — + # the tool name + state already tell the user what to do, and a + # canned "check your package manager" line adds no value). + hint = catalog_data.get("hint", "") results.append({ "tool": versioned_name, @@ -453,7 +451,7 @@ def cmd_audit(args: argparse.Namespace) -> int: print("", file=sys.stderr) # Render table - render_table(tools, show_hints=SHOW_HINTS) + render_table(tools) # Print summary print_summary(snapshot, tools) @@ -931,12 +929,9 @@ def cmd_update_local(args: argparse.Namespace) -> int: "version_cycle": cycle, "lifecycle_status": info.get("status", "unknown"), }) - if status_v == "OUTDATED": - entry["hint"] = f"Upgrade {tool.name} {cycle}: {installed_v} \u2192 {latest_v}" - elif status_v == "NOT INSTALLED": - entry["hint"] = f"Install {tool.name} {cycle}: check your package manager or version manager" - else: - entry["hint"] = "" + # Hint stays empty for generic multi-version runtimes; + # the tool name + state already tell the user what to do. + entry["hint"] = "" tools_by_name[versioned] = entry # Write merged snapshot diff --git a/docs/CLI_REFERENCE.md b/docs/CLI_REFERENCE.md index 0f22218..64d1e8a 100644 --- a/docs/CLI_REFERENCE.md +++ b/docs/CLI_REFERENCE.md @@ -183,7 +183,6 @@ CLI_AUDIT_OFFLINE=1 CLI_AUDIT_RENDER=1 python3 cli_audit.py | `CLI_AUDIT_TIMINGS` | bool | `1` | Show timing information | | `CLI_AUDIT_SORT` | string | `order` | Sort mode: `order` or `alpha` | | `CLI_AUDIT_GROUP` | bool | `1` | Group output by category | -| `CLI_AUDIT_HINTS` | bool | `1` | Show remediation hints | ### Snapshot Configuration diff --git a/docs/DEPLOYMENT.md b/docs/DEPLOYMENT.md index 113eaf9..94f391a 100644 --- a/docs/DEPLOYMENT.md +++ b/docs/DEPLOYMENT.md @@ -30,7 +30,6 @@ Render audit table from snapshot (render-only mode, no network). **Environment:** - `CLI_AUDIT_RENDER=1` - Render-only mode - `CLI_AUDIT_GROUP=0` - No category grouping -- `CLI_AUDIT_HINTS=1` - Show remediation hints - `CLI_AUDIT_LINKS=1` - Enable hyperlinks - `CLI_AUDIT_EMOJI=1` - Use emoji status icons @@ -574,7 +573,6 @@ CLI_AUDIT_LINKS=1 CLI_AUDIT_EMOJI=1 CLI_AUDIT_TIMINGS=1 CLI_AUDIT_GROUP=1 -CLI_AUDIT_HINTS=1 # Performance CLI_AUDIT_MAX_WORKERS=16 diff --git a/docs/QUICK_REFERENCE.md b/docs/QUICK_REFERENCE.md index b90e21b..3a16e98 100644 --- a/docs/QUICK_REFERENCE.md +++ b/docs/QUICK_REFERENCE.md @@ -131,9 +131,6 @@ CLI_AUDIT_LINKS=0 python3 cli_audit.py # Hide timing info CLI_AUDIT_TIMINGS=0 python3 cli_audit.py - -# Disable hints -CLI_AUDIT_HINTS=0 python3 cli_audit.py ``` ## Common Workflows From 48265afb94a3af16ec80a1e85f7f80be53973a9d Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Tue, 21 Apr 2026 14:10:08 +0200 Subject: [PATCH 4/8] feat(audit): pin-aware rendering with notes column MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three rendering gaps surfaced in ``make audit``: 1. Install method (apt / cargo / uv / nvm / …) was collected but never shown. 2. Version pins from ``pins.json`` were invisible. 3. Auto-update flags from ``config.yml`` were invisible. Fixes: - Header grows from 4 to 5 columns, adding ``notes`` = ``method · auto``. Smoke test updated to match (it was asserting 6 columns against a 4-column reality — silent pass via ``|| true``). - PIN marker renders next to the version it constrains (installed column), e.g. ``3.12.7 [PIN:3.12.7]`` or ``[PIN:never]`` on not-installed rows. - Status icon computation now respects pins: a tool pinned to X with installed == X is ``✅`` regardless of upstream latest; installed != X is ``⚠️ CONFLICT`` (pin being violated); ``PIN:never`` + not installed is ``✅`` (intent honored). Previously ``✅`` could render on rows whose installed version diverged from the pin. - Summary tallies use the same pin-aware status so "Readiness: N outdated, N missing" matches the visible icons. - ``status_icon`` consults ``status`` before the empty-installed short-circuit so ``PIN:never`` rows don't show red-X for absence that is the intended state. Signed-off-by: Sebastian Mendel --- cli_audit/render.py | 163 +++++++++++++++++++++++++++++++----------- scripts/test_smoke.sh | 7 +- 2 files changed, 127 insertions(+), 43 deletions(-) diff --git a/cli_audit/render.py b/cli_audit/render.py index ea6344f..719b8de 100644 --- a/cli_audit/render.py +++ b/cli_audit/render.py @@ -33,26 +33,29 @@ def status_icon(status: str, installed: str) -> str: Returns: Status icon string """ + # Status takes precedence: a PIN:never row that is correctly absent + # has ``UP-TO-DATE`` status with empty ``installed`` and should render + # green, not red-X. if not USE_EMOJI: - if installed == "X" or installed == "" or status == "NOT INSTALLED": - return "x" if status == "UP-TO-DATE": return "✓" if status == "OUTDATED": return "↑" if status == "CONFLICT": return "⚠" + if installed == "X" or installed == "" or status == "NOT INSTALLED": + return "x" return "?" # Emoji icons (using consistent single-width Unicode) - if installed == "X" or installed == "" or status == "NOT INSTALLED": - return "❌" if status == "UP-TO-DATE": return "✅" if status == "OUTDATED": return "⬆" # Single-width arrow without variation selector if status == "CONFLICT": return "⚠️" + if installed == "X" or installed == "" or status == "NOT INSTALLED": + return "❌" return "❓" @@ -115,21 +118,22 @@ def osc8(url: str, text: str) -> str: } -def render_table(tools: list[dict[str, Any]], show_hints: bool = False) -> None: - """Render tools as pipe-delimited table, optionally grouped by category. - - Args: - tools: List of tool dictionaries - show_hints: Whether to show installation hints - """ - from .catalog import ToolCatalog +def render_table(tools: list[dict[str, Any]]) -> None: + """Render tools as pipe-delimited table, optionally grouped by category.""" + from .config import load_config + from .pins import load_pins - # Header - headers = ("state", "tool", "installed", "latest_upstream") + # Header — 5 columns. Pin info lives next to the ``installed`` value + # it constrains; ``notes`` carries install method and auto-update flag. + headers = ("state", "tool", "installed", "latest_upstream", "notes") print("|".join(headers)) - # Load catalog for pinned versions - catalog = ToolCatalog() + # Load once so each row render is cheap. + pins = load_pins() + try: + config = load_config() + except Exception: + config = None # Group tools by category if enabled if GROUP_BY_CATEGORY: @@ -149,21 +153,95 @@ def render_table(tools: list[dict[str, Any]], show_hints: bool = False) -> None: desc = CATEGORY_DESC.get(cat, cat) print(f"# {icon} {desc} ({len(cat_tools)} tools)", file=sys.stderr) for tool in cat_tools: - _render_tool_row(tool, catalog, show_hints) + _render_tool_row(tool, pins, config) else: for tool in tools: - _render_tool_row(tool, catalog, show_hints) + _render_tool_row(tool, pins, config) -def _render_tool_row(tool: dict[str, Any], catalog: Any, show_hints: bool) -> None: +def _pin_suffix(pin: str) -> str: + """Format a pin value as an appendable suffix (empty if no pin).""" + if not pin: + return "" + if pin == "never": + return " [PIN:never]" + return f" [PIN:{pin}]" + + +def _apply_pin_to_status(status: str, installed: str, latest: str, pin: str) -> str: + """Adjust the snapshot status using the user's pin as the target. + + The snapshot's ``status`` was computed against ``latest_upstream`` with + no knowledge of pins. A pin is the user's stated target — rendering + must respect it so ``✅`` never appears on a row whose installed + version diverges from the pin. + + Rules (``pin == "never"`` is effectively ``installed must stay empty``): + + - ``pin`` empty → pass through, pin doesn't apply + - ``pin == "never"`` + - nothing installed → ``UP-TO-DATE`` (the pin is honored) + - something installed→ ``CONFLICT`` (user said never, but it's here) + - specific version pin + - nothing installed → ``NOT INSTALLED`` (unchanged) + - installed == pin → ``UP-TO-DATE`` (regardless of latest) + - installed != pin → ``CONFLICT`` (pin is being violated) + """ + if not pin: + return status + if pin == "never": + if not installed: + return "UP-TO-DATE" + return "CONFLICT" + # Specific-version pin. + if not installed: + return "NOT INSTALLED" + if installed == pin: + return "UP-TO-DATE" + return "CONFLICT" + + +def _build_notes(tool: dict[str, Any], config: Any) -> str: + """Compose the ``notes`` cell: ``method · auto``. + + Pin info is rendered in the ``installed`` column, not here. + """ + parts: list[str] = [] + method = tool.get("installed_method") or "" + if method: + parts.append(method) + + if config is not None: + name = tool.get("tool", "") + base = name.split("@", 1)[0] if "@" in name else name + tool_cfg = config.tools.get(name) or config.tools.get(base) + if tool_cfg is not None and tool_cfg.auto_update is True: + parts.append("auto") + + return " · ".join(parts) + + +def _render_tool_row( + tool: dict[str, Any], + pins: dict[str, Any], + config: Any, +) -> None: """Render a single tool row.""" + from .pins import lookup_pin + name = tool.get("tool", "") installed = tool.get("installed", "") latest = tool.get("latest_upstream", "") - status = tool.get("status", "UNKNOWN") + raw_status = tool.get("status", "UNKNOWN") tool_url = tool.get("tool_url", "") latest_url = tool.get("latest_url", "") + # A pin overrides the "upgrade target" for display purposes. The + # snapshot's ``status`` is computed against latest_upstream and does + # not know about pins, so fix it up here before choosing icon/colors. + pin_value = lookup_pin(name, pins) + status = _apply_pin_to_status(raw_status, installed, latest, pin_value) + # Icon icon = status_icon(status, installed) @@ -184,7 +262,7 @@ def _render_tool_row(tool: dict[str, Any], catalog: Any, show_hints: bool) -> No # Hyperlinks name_display = osc8(tool_url, name) if tool_url else name - # Apply colors to installed and latest (before adding markers/hints) + # Apply colors to installed and latest installed_display = colorize(installed, inst_color) latest_display = colorize(latest, latest_color) @@ -192,27 +270,18 @@ def _render_tool_row(tool: dict[str, Any], catalog: Any, show_hints: bool) -> No if latest_url: latest_display = osc8(latest_url, latest_display) - # Add pinned/skip markers - markers = [] - if catalog.is_pinned(name): - markers.append("PINNED") - if catalog.should_skip(name, latest): - markers.append("SKIP") + # Attach pin marker to the version it constrains (installed column). + # The suffix renders outside the hyperlink so it stays readable when + # nothing is installed. + installed_display = f"{installed_display}{_pin_suffix(pin_value)}" - if markers: - latest_display = f"{latest_display} [{' '.join(markers)}]" - - # Hint - if show_hints and status in ("NOT INSTALLED", "OUTDATED", "CONFLICT"): - hint = tool.get("hint", "") - if hint: - latest_display = f"{latest_display} [{hint}]" + notes = _build_notes(tool, config) # Add CONFLICT message to installed display if status == "CONFLICT" and installed_display.startswith("CONFLICT:"): installed_display = installed_display.replace("CONFLICT: ", "") # Show clean message - print("|".join((icon, name_display, installed_display, latest_display))) + print("|".join((icon, name_display, installed_display, latest_display, notes))) def print_summary(snapshot: dict[str, Any], tools: list[dict[str, Any]]) -> None: @@ -222,12 +291,26 @@ def print_summary(snapshot: dict[str, Any], tools: list[dict[str, Any]]) -> None snapshot: Snapshot metadata tools: List of tool dictionaries """ + from .pins import load_pins, lookup_pin + meta = snapshot.get("__meta__", {}) total = meta.get("count", len(tools)) - missing = sum(1 for t in tools if t.get("status") == "NOT INSTALLED") - outdated = sum(1 for t in tools if t.get("status") == "OUTDATED") - conflicts = sum(1 for t in tools if t.get("status") == "CONFLICT") - unknown = sum(1 for t in tools if t.get("status") == "UNKNOWN") + + pins = load_pins() + + def _effective(t: dict[str, Any]) -> str: + return _apply_pin_to_status( + t.get("status", "UNKNOWN"), + t.get("installed", ""), + t.get("latest_upstream", ""), + lookup_pin(t.get("tool", ""), pins), + ) + + effective = [_effective(t) for t in tools] + missing = sum(1 for s in effective if s == "NOT INSTALLED") + outdated = sum(1 for s in effective if s == "OUTDATED") + conflicts = sum(1 for s in effective if s == "CONFLICT") + unknown = sum(1 for s in effective if s == "UNKNOWN") offline_tag = " (offline)" if meta.get("offline") else "" # Build summary message diff --git a/scripts/test_smoke.sh b/scripts/test_smoke.sh index b45cb4a..ee852e3 100755 --- a/scripts/test_smoke.sh +++ b/scripts/test_smoke.sh @@ -4,10 +4,11 @@ set -euo pipefail ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" PY="${PYTHON:-python3}" -echo "[smoke] Checking 6-column header in table output" +echo "[smoke] Checking 5-column header in table output" HDR="$($PY "$ROOT_DIR/audit.py" | head -n1 || true)" -IFS='|' read -r c1 c2 c3 c4 c5 c6 <<<"${HDR:-}" -test -n "$c1" && test -n "$c2" && test -n "$c3" && test -n "$c4" && test -n "$c5" && test -n "$c6" +IFS='|' read -r c1 c2 c3 c4 c5 <<<"${HDR:-}" +test -n "$c1" && test -n "$c2" && test -n "$c3" && test -n "$c4" && test -n "$c5" +test "$c5" = "notes" echo "[smoke] Checking JSON fields presence" JSON="$(CLI_AUDIT_JSON=1 "$PY" "$ROOT_DIR/audit.py" || true)" From 0b5e5ece9a81fb84a446ea3ede36c2e262a2eb83 Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Tue, 21 Apr 2026 14:10:18 +0200 Subject: [PATCH 5/8] =?UTF-8?q?docs(adr):=20ADR-009=20=E2=80=94=20first-cl?= =?UTF-8?q?ass=20multi-install=20support?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Propose treating each discovered installation of a tool as individually addressable. Current asymmetry: detection finds every binary on ``PATH``, ``choose_highest()`` discards everything but one before snapshot write, and only ``reconcile.py`` re-detects the rest — purely to delete them. ADR-009 spells out the data model (``Installation`` dataclass, stable ``tool#method`` keys), snapshot schema bump, per-install pins (``"ripgrep#apt": "14.0.0"``), per-install upgrade, the narrowed role for ``reconcile``, open questions, and a phased rollout. Status: Proposed — companion to the rendering work landed in the earlier commits of this branch; implementation is follow-up. Signed-off-by: Sebastian Mendel --- docs/adr/ADR-009-first-class-multi-install.md | 258 ++++++++++++++++++ 1 file changed, 258 insertions(+) create mode 100644 docs/adr/ADR-009-first-class-multi-install.md diff --git a/docs/adr/ADR-009-first-class-multi-install.md b/docs/adr/ADR-009-first-class-multi-install.md new file mode 100644 index 0000000..ca23113 --- /dev/null +++ b/docs/adr/ADR-009-first-class-multi-install.md @@ -0,0 +1,258 @@ +# ADR-009: First-class support for multiple installations of the same tool + +**Status:** Proposed +**Date:** 2026-04-21 +**Deciders:** AI CLI Preparation Team +**Tags:** detection, audit, upgrade, pinning, reconciliation +**Supersedes-in-part:** ADR-003 (parallel installation) for the *data model* +only — the user-facing policy from ADR-003 (keep both, prefer user-level via +PATH) is preserved and extended. + +## Context + +The catalog can have several binaries of the same tool on a machine — e.g. +`/usr/bin/ripgrep` (apt, 14.0.0) next to `~/.cargo/bin/rg` (cargo, 14.1.1). +ADR-003 already says we keep both. The implementation does not. + +Current reality: + +- `audit_tool_installation()` (`cli_audit/detection.py:299`) discovers all + candidate paths in `PATH` when invoked with `deep=True`, but then calls + `choose_highest()` and returns **a single 4-tuple** + `(version, line, path, method)`. +- The snapshot writer (`audit.py`) records the chosen path as + `installed_path_selected` / `installed_method`. The other installs are + dropped before the snapshot is persisted. +- `make audit`, `make upgrade`, and the pin system all operate on a single + `tool` key — they cannot address "the apt ripgrep" vs "the cargo + ripgrep" separately. +- The only place the rest of the installations exist is + `cli_audit/reconcile.py`, which re-scans from scratch and exists solely + to *delete* duplicates. + +Effect on the user: + +- `make audit` reports one row per tool. If the PATH-preferred install is + up-to-date, the row is green; a second, stale install behind it is + invisible. +- `make upgrade ` only upgrades the PATH-preferred install. If an + apt copy is also on disk, it stays at whatever apt has. +- `scripts/pin_version.sh ` pins the name, not the install + — a user cannot say "pin the apt one, keep cargo rolling". +- The `notes` column added in the companion change can say `apt · PIN:1.0` + but only for whichever install won `choose_highest()`. + +User position (requirements for this ADR): + +1. Multi-install should be discovered and **listed**, not hidden. +2. Each install should be eligible for **update** independently. +3. Each install should be **pinnable** independently. +4. The duplicate itself is a signal worth warning about, but keeping + duplicates is a legitimate choice; once kept, they must be first-class. + +## Decision + +Treat each discovered installation of a tool as an individually +addressable record. The tool catalog entry remains the definition of +*what the tool is*; each installation is a concrete *instance* of that +tool on this machine. + +### 1. Data model + +New dataclass (in `cli_audit/detection.py`): + +```python +@dataclass(frozen=True) +class Installation: + tool: str # catalog name, e.g. "ripgrep" + cycle: str | None # for multi-version runtimes, e.g. "3.12" + path: str # absolute realpath of the binary + version: str # extracted version number + version_line: str # raw --version output (diagnostics) + method: str # apt|cargo|uv|pipx|brew|nvm|manual|system|… + is_primary: bool # first on PATH → the one currently invoked +``` + +`audit_tool_installation()` is renamed/split: + +- `detect_installations(tool, candidates, …) -> list[Installation]` + returns every hit (no `choose_highest`). +- A thin back-compat wrapper keeps the old 4-tuple for callers that still + want "the chosen one", implemented as "first `is_primary=True` entry". + +### 2. Addressing an installation + +Every install has a stable key: + +``` +[@]#[:] +``` + +- `ripgrep#apt` — no ambiguity: one apt binary +- `ripgrep#cargo` — the cargo one +- `python@3.12#apt` — apt's python3.12 +- `python@3.12#pyenv:abc123` — if two pyenv installs exist, disambiguate + with a short hash of the realpath + +This key appears in the `notes` column, in pin/unpin commands, and in +`make upgrade-`. + +### 3. Snapshot schema + +`tools_snapshot.json` grows a new field per tool entry: + +```json +{ + "tool": "ripgrep", + "installations": [ + {"path": "/home/u/.cargo/bin/rg", "version": "14.1.1", "method": "cargo", "is_primary": true}, + {"path": "/usr/bin/rg", "version": "14.0.0", "method": "apt", "is_primary": false} + ], + "installed": "14.1.1", // primary, kept for back-compat + "installed_method": "cargo", // '' + "installed_path_selected": "/home/u/.cargo/bin/rg" // '', to be deprecated +} +``` + +Legacy `installed_*` fields are kept for one release and marked deprecated +in the schema, then removed. + +### 4. Rendering + +`make audit` default stays one row per tool, showing the primary. When a +tool has >1 installation the `notes` column appends `+N more` (e.g. +`cargo · +1 more`). A new `--wide` / `CLI_AUDIT_WIDE=1` mode emits one row +per installation with a sub-indented second column +(` ↳ ripgrep#apt`), so the default view stays narrow but the detail is +reachable without a separate command. + +Status semantics per row: + +- Primary install: existing `UP-TO-DATE / OUTDATED / NOT INSTALLED`. +- Secondary install: same vocabulary, scoped to that install. The + tool-level `status` becomes the *worst* of its installs, so a tool with + one up-to-date and one outdated install is surfaced as `OUTDATED` in + the summary even if the primary is green. + +### 5. Pins + +`~/.config/cli-audit/pins.json` extends from + +```json +{"ripgrep": "14.1.0"} +``` + +to optionally accept the `tool#method` key: + +```json +{ + "ripgrep": "14.1.0", // applies to every ripgrep install + "ripgrep#apt": "14.0.0", // apt-only override + "ripgrep#cargo": "never" // stop touching the cargo one +} +``` + +Resolution order in `cli_audit/pins.py`: + +1. Exact `tool#method[:hash]` match — per-installation pin wins. +2. `tool@cycle` match — existing multi-version behavior. +3. Plain `tool` match — tool-wide default. + +Shell helpers (`scripts/pin_version.sh`, `unpin_version.sh`) accept the +`tool#method` form; `reset_pins.sh` is unchanged. + +### 6. Upgrade + +`upgrade_tool(name)` iterates over every `Installation` whose effective +pin is not `"never"` and whose method has an upgrader. Each install is +upgraded by its own method (apt upgrades apt, cargo upgrades cargo, +etc.). Results are a list: + +``` +BulkUpgradeResult + ripgrep#cargo → 14.1.1 → 14.1.2 OK + ripgrep#apt → 14.0.0 → 14.0.0 skipped (apt has no newer) +``` + +`make upgrade-` stays a convenience that upgrades all installs; +`make upgrade-#` targets one. + +### 7. Reconcile + +`reconcile` is no longer the *only* place multi-install exists; it +becomes what its name suggests — a cleanup tool. Its scope narrows to: + +- Detect tools with >1 install where the user has *not* pinned the + duplicate as intentional (`PIN:never` on the non-primary signals "keep + quiet"). +- Offer removal of the non-primary copy. +- Default is dry-run, as today. + +### 8. Warnings + +On `make audit`, tools with >1 installation get a `⚠ dup` suffix in +notes *unless* every non-primary install is explicitly pinned. The +warning is advisory, not a failure — multi-install is supported, the +warning just makes the situation visible. + +## Consequences + +**Positive:** + +- Hidden stale installs become visible. +- Per-install pinning matches how users actually run into this (apt ships + old, cargo ships new, both kept deliberately). +- `upgrade_all` stops leaving apt copies behind. +- `reconcile` stops being a parallel detection universe. + +**Negative:** + +- Snapshot schema bump; one release of legacy fields for transition. +- `audit`, `upgrade`, `pins`, `render`, `reconcile`, `snapshot`, + `detection`, and the shell helpers all change. Touch-point count is + the main risk, not individual difficulty. +- `make upgrade-` semantics change (now multi-target). PR notes + and CHANGELOG must flag this. + +**Neutral:** + +- `ADR-003`'s *policy* (keep both, prefer user-level on PATH) is + unchanged; ADR-009 only upgrades the *implementation* so we can act on + that policy. + +## Open questions + +1. **Cycle granularity for multi-version runtimes.** Current pins allow + `python@3.12` but not `python@3.12#apt`. Do we want three-level keys + (`python@3.12#pyenv:abc123`)? Likely yes for rigor, but the common + case is "one runtime per cycle" and the extra depth may be noise. +2. **Version-manager installs (nvm, pyenv, rbenv) as a special case.** + A pyenv install *is* a family of installs at `~/.pyenv/versions/*`. + Treat each as its own `Installation`, or treat pyenv as one + "method" with its own internal cycle list? Leaning toward the former + (pyenv installs are physical binaries just like apt binaries). +3. **Stable install IDs across reboots.** Realpaths change if a tool is + symlinked or reinstalled. Use `(method, realpath-basename, + short-hash)` so the ID is resilient to version bumps but changes + when the user switches install methods. +4. **Backwards compatibility window.** One release keeping legacy + `installed_*` fields, or two? + +## Rollout + +Phased, behind a feature flag to allow incremental landing: + +1. Add `Installation` dataclass + `detect_installations()`. Keep + existing `audit_tool_installation()` as a wrapper returning + `first_primary`. No behavior change. +2. Extend snapshot with `installations[]`; write both old and new + fields. `make audit` still reads old fields. +3. Render `+N more` in `notes`; add `--wide` view. +4. Extend pins.json parsing; shell helpers accept `tool#method`. +5. Rewrite `upgrade_tool` to iterate `installations[]`; keep + single-install path when `len(installations) == 1`. +6. Narrow `reconcile` scope; remove duplicate detection code paths. +7. Drop legacy `installed_*` fields from snapshot schema. + +Each step is independently shippable; the flag gate stays until step 5 +is proven. From 9bf57f084658147698e5564f93357cc87e6cbc53 Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Tue, 21 Apr 2026 14:15:53 +0200 Subject: [PATCH 6/8] =?UTF-8?q?refactor(pins):=20self-review=20=E2=80=94?= =?UTF-8?q?=20move=20status=20helper=20into=20pins,=20add=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Self-review of the PR surfaced three things worth addressing before a reviewer sees them: - ``_apply_pin_to_status`` lived in ``render.py`` but it's pin logic, not display logic. Moved into ``pins.py`` as public ``apply_pin_to_status`` (no ``latest`` arg — it was unused) and exported from the package API. - Inline ``from cli_audit.pins import …`` statements inside the ``cmd_update`` loop and ``_render_tool_row`` hot path promoted to module-level imports. - ``tests/test_pins.py``: 30 tests covering ``load_pins``, ``lookup_pin``, ``is_pinned``/``is_never``, ``should_skip``, and the state matrix for ``apply_pin_to_status``. One test caught a real ambiguity — looking up ``tool@cycle`` against a flat ``{tool: "x.y"}`` pin returned the flat value; shape-mismatched lookups now return empty instead, and the docstring says so. Signed-off-by: Sebastian Mendel --- audit.py | 2 +- cli_audit/__init__.py | 2 + cli_audit/pins.py | 40 +++++++++ cli_audit/render.py | 45 +--------- tests/test_pins.py | 185 ++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 232 insertions(+), 42 deletions(-) create mode 100644 tests/test_pins.py diff --git a/audit.py b/audit.py index 875be14..d05a1fe 100755 --- a/audit.py +++ b/audit.py @@ -30,6 +30,7 @@ from cli_audit.detection import audit_tool_installation, detect_multi_versions # noqa: E402 from cli_audit.snapshot import load_snapshot, write_snapshot, render_from_snapshot, get_snapshot_path # noqa: E402 from cli_audit.render import render_table, print_summary, status_icon # noqa: E402 +from cli_audit.pins import lookup_pin, should_skip as _pin_should_skip # noqa: E402 from cli_audit.collectors import get_github_rate_limit, get_github_rate_limit_help, get_gitlab_rate_limit, is_wsl, collect_endoflife # noqa: E402 from cli_audit import collectors # noqa: E402 from cli_audit.logging_config import setup_logging # noqa: E402 @@ -615,7 +616,6 @@ def cmd_update(args: argparse.Namespace) -> int: latest_display = _sanitize(latest) if latest else "n/a" # Add pinned/skip markers from user pins file - from cli_audit.pins import lookup_pin, should_skip as _pin_should_skip pin_val = lookup_pin(tool.name) markers = [] if pin_val: diff --git a/cli_audit/__init__.py b/cli_audit/__init__.py index 50aa1ae..6167e8d 100644 --- a/cli_audit/__init__.py +++ b/cli_audit/__init__.py @@ -60,6 +60,7 @@ is_pinned, is_never, should_skip, + apply_pin_to_status, ) from .install_plan import InstallPlan, InstallStep, generate_install_plan, dry_run_install # noqa: E402 @@ -183,6 +184,7 @@ "is_pinned", "is_never", "should_skip", + "apply_pin_to_status", "InstallPlan", "InstallStep", "generate_install_plan", diff --git a/cli_audit/pins.py b/cli_audit/pins.py index cb283bd..4d779d0 100644 --- a/cli_audit/pins.py +++ b/cli_audit/pins.py @@ -85,6 +85,10 @@ def lookup_pin(tool_name: str, pins: dict[str, Any] | None = None) -> str: """Return the pinned value for a tool, or empty string if not pinned. ``"never"`` is a valid return value and means "never update/install". + + Shape-mismatched lookups (flat pin queried with ``tool@cycle``, or a + nested pin queried bare) return empty — callers are expected to match + the pin file's structure, and silent fallbacks would mask bugs. """ pins = pins if pins is not None else load_pins() base, cycle = _split_tool(tool_name) @@ -92,10 +96,14 @@ def lookup_pin(tool_name: str, pins: dict[str, Any] | None = None) -> str: if entry is None: return "" if isinstance(entry, dict): + # Nested (multi-version) pin: caller must supply a cycle. if cycle is None: return "" value = entry.get(cycle, "") return value if isinstance(value, str) else "" + # Flat (single-version) pin: caller must not supply a cycle. + if cycle is not None: + return "" return entry if isinstance(entry, str) else "" @@ -121,3 +129,35 @@ def should_skip(tool_name: str, latest_version: str, pins: dict[str, Any] | None if pin == "never": return True return pin == latest_version + + +def apply_pin_to_status(status: str, installed: str, pin: str) -> str: + """Adjust a snapshot status value using the user's pin as the target. + + The snapshot's ``status`` is computed against ``latest_upstream`` and + has no knowledge of pins. The pin is the user's stated target, so any + downstream consumer (rendering, summary counts, bulk decisions) must + respect it — ``UP-TO-DATE`` must not be reported on a row whose + installed version diverges from the pin. + + Rules: + + - ``pin`` empty → pass through (no pin applies). + - ``pin == "never"`` + - nothing installed → ``UP-TO-DATE`` (the pin is honored). + - something installed→ ``CONFLICT`` (user said never). + - specific version pin + - nothing installed → ``NOT INSTALLED`` (unchanged). + - installed == pin → ``UP-TO-DATE`` (regardless of latest). + - installed != pin → ``CONFLICT`` (pin is being violated). + """ + if not pin: + return status + if pin == "never": + return "UP-TO-DATE" if not installed else "CONFLICT" + # Specific-version pin. + if not installed: + return "NOT INSTALLED" + if installed == pin: + return "UP-TO-DATE" + return "CONFLICT" diff --git a/cli_audit/render.py b/cli_audit/render.py index 719b8de..531f019 100644 --- a/cli_audit/render.py +++ b/cli_audit/render.py @@ -8,6 +8,8 @@ import sys from typing import Any +from .pins import apply_pin_to_status, load_pins, lookup_pin + # Environment options USE_EMOJI = os.environ.get("CLI_AUDIT_EMOJI", "1") == "1" @@ -121,7 +123,6 @@ def osc8(url: str, text: str) -> str: def render_table(tools: list[dict[str, Any]]) -> None: """Render tools as pipe-delimited table, optionally grouped by category.""" from .config import load_config - from .pins import load_pins # Header — 5 columns. Pin info lives next to the ``installed`` value # it constrains; ``notes`` carries install method and auto-update flag. @@ -168,39 +169,6 @@ def _pin_suffix(pin: str) -> str: return f" [PIN:{pin}]" -def _apply_pin_to_status(status: str, installed: str, latest: str, pin: str) -> str: - """Adjust the snapshot status using the user's pin as the target. - - The snapshot's ``status`` was computed against ``latest_upstream`` with - no knowledge of pins. A pin is the user's stated target — rendering - must respect it so ``✅`` never appears on a row whose installed - version diverges from the pin. - - Rules (``pin == "never"`` is effectively ``installed must stay empty``): - - - ``pin`` empty → pass through, pin doesn't apply - - ``pin == "never"`` - - nothing installed → ``UP-TO-DATE`` (the pin is honored) - - something installed→ ``CONFLICT`` (user said never, but it's here) - - specific version pin - - nothing installed → ``NOT INSTALLED`` (unchanged) - - installed == pin → ``UP-TO-DATE`` (regardless of latest) - - installed != pin → ``CONFLICT`` (pin is being violated) - """ - if not pin: - return status - if pin == "never": - if not installed: - return "UP-TO-DATE" - return "CONFLICT" - # Specific-version pin. - if not installed: - return "NOT INSTALLED" - if installed == pin: - return "UP-TO-DATE" - return "CONFLICT" - - def _build_notes(tool: dict[str, Any], config: Any) -> str: """Compose the ``notes`` cell: ``method · auto``. @@ -227,8 +195,6 @@ def _render_tool_row( config: Any, ) -> None: """Render a single tool row.""" - from .pins import lookup_pin - name = tool.get("tool", "") installed = tool.get("installed", "") latest = tool.get("latest_upstream", "") @@ -240,7 +206,7 @@ def _render_tool_row( # snapshot's ``status`` is computed against latest_upstream and does # not know about pins, so fix it up here before choosing icon/colors. pin_value = lookup_pin(name, pins) - status = _apply_pin_to_status(raw_status, installed, latest, pin_value) + status = apply_pin_to_status(raw_status, installed, pin_value) # Icon icon = status_icon(status, installed) @@ -291,18 +257,15 @@ def print_summary(snapshot: dict[str, Any], tools: list[dict[str, Any]]) -> None snapshot: Snapshot metadata tools: List of tool dictionaries """ - from .pins import load_pins, lookup_pin - meta = snapshot.get("__meta__", {}) total = meta.get("count", len(tools)) pins = load_pins() def _effective(t: dict[str, Any]) -> str: - return _apply_pin_to_status( + return apply_pin_to_status( t.get("status", "UNKNOWN"), t.get("installed", ""), - t.get("latest_upstream", ""), lookup_pin(t.get("tool", ""), pins), ) diff --git a/tests/test_pins.py b/tests/test_pins.py new file mode 100644 index 0000000..1ec06c9 --- /dev/null +++ b/tests/test_pins.py @@ -0,0 +1,185 @@ +"""Tests for the version-pin reader (``cli_audit.pins``).""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest + +from cli_audit.pins import ( + apply_pin_to_status, + is_never, + is_pinned, + load_pins, + lookup_pin, + reset_cache, + should_skip, +) + + +@pytest.fixture +def pins_file(tmp_path: Path) -> Path: + """Write a realistic pins.json and return its path.""" + path = tmp_path / "pins.json" + path.write_text( + json.dumps( + { + "ripgrep": "14.1.0", + "php": { + "8.5": "8.5.3", + "8.2": "never", + }, + "node": {"22": "never"}, + # Intentionally non-string values to exercise defensive parsing + "bogus_int": 42, + "bogus_null": None, + } + ) + ) + return path + + +@pytest.fixture(autouse=True) +def _clear_cache(): + """Ensure load_pins's LRU cache doesn't leak state between tests.""" + reset_cache() + yield + reset_cache() + + +class TestLoadPins: + def test_loads_valid_json(self, pins_file: Path): + data = load_pins(str(pins_file)) + assert data["ripgrep"] == "14.1.0" + assert data["php"]["8.5"] == "8.5.3" + + def test_missing_file_returns_empty(self, tmp_path: Path): + assert load_pins(str(tmp_path / "does-not-exist.json")) == {} + + def test_invalid_json_returns_empty(self, tmp_path: Path): + path = tmp_path / "pins.json" + path.write_text("{ not valid json") + assert load_pins(str(path)) == {} + + def test_top_level_list_rejected(self, tmp_path: Path): + path = tmp_path / "pins.json" + path.write_text("[1, 2, 3]") + assert load_pins(str(path)) == {} + + def test_cache_returns_same_object(self, pins_file: Path): + first = load_pins(str(pins_file)) + second = load_pins(str(pins_file)) + assert first is second + + +class TestLookupPin: + def test_single_version_tool(self, pins_file: Path): + pins = load_pins(str(pins_file)) + assert lookup_pin("ripgrep", pins) == "14.1.0" + + def test_multi_version_tool_with_cycle(self, pins_file: Path): + pins = load_pins(str(pins_file)) + assert lookup_pin("php@8.5", pins) == "8.5.3" + assert lookup_pin("php@8.2", pins) == "never" + + def test_multi_version_tool_unknown_cycle(self, pins_file: Path): + pins = load_pins(str(pins_file)) + assert lookup_pin("php@9.0", pins) == "" + + def test_multi_version_tool_without_cycle_returns_empty(self, pins_file: Path): + """Bare 'php' lookup on a nested entry returns empty; the caller must + supply a cycle to resolve nested pins.""" + pins = load_pins(str(pins_file)) + assert lookup_pin("php", pins) == "" + + def test_tool_not_in_pins(self, pins_file: Path): + pins = load_pins(str(pins_file)) + assert lookup_pin("git-branchless", pins) == "" + + def test_non_string_values_are_ignored(self, pins_file: Path): + pins = load_pins(str(pins_file)) + assert lookup_pin("bogus_int", pins) == "" + assert lookup_pin("bogus_null", pins) == "" + + +class TestIsPinnedAndIsNever: + def test_is_pinned_true_for_version(self, pins_file: Path): + pins = load_pins(str(pins_file)) + assert is_pinned("ripgrep", pins) + + def test_is_pinned_true_for_never(self, pins_file: Path): + pins = load_pins(str(pins_file)) + assert is_pinned("php@8.2", pins) + + def test_is_pinned_false_for_unknown(self, pins_file: Path): + pins = load_pins(str(pins_file)) + assert not is_pinned("ripgrep@1.0", pins) + + def test_is_never_only_true_for_never_sentinel(self, pins_file: Path): + pins = load_pins(str(pins_file)) + assert is_never("php@8.2", pins) + assert not is_never("ripgrep", pins) + assert not is_never("git-branchless", pins) + + +class TestShouldSkip: + def test_never_always_skips(self, pins_file: Path): + pins = load_pins(str(pins_file)) + assert should_skip("php@8.2", "9.9.9", pins) + + def test_skip_when_pin_matches_latest(self, pins_file: Path): + pins = load_pins(str(pins_file)) + assert should_skip("ripgrep", "14.1.0", pins) + + def test_no_skip_when_pin_differs_from_latest(self, pins_file: Path): + pins = load_pins(str(pins_file)) + assert not should_skip("ripgrep", "15.0.0", pins) + + def test_no_skip_when_not_pinned(self, pins_file: Path): + pins = load_pins(str(pins_file)) + assert not should_skip("git-branchless", "0.10.0", pins) + + +class TestApplyPinToStatus: + @pytest.mark.parametrize( + "status", ["UP-TO-DATE", "OUTDATED", "NOT INSTALLED", "CONFLICT", "UNKNOWN"] + ) + def test_no_pin_passes_through(self, status: str): + assert apply_pin_to_status(status, "1.0", pin="") == status + + def test_never_plus_not_installed_is_up_to_date(self): + assert apply_pin_to_status("NOT INSTALLED", "", "never") == "UP-TO-DATE" + + def test_never_plus_installed_is_conflict(self): + assert apply_pin_to_status("UP-TO-DATE", "1.0", "never") == "CONFLICT" + + def test_specific_pin_matches_installed_is_up_to_date(self): + # Pin honored even when snapshot thought it was outdated. + assert apply_pin_to_status("OUTDATED", "1.2.3", "1.2.3") == "UP-TO-DATE" + + def test_specific_pin_does_not_match_installed_is_conflict(self): + assert apply_pin_to_status("UP-TO-DATE", "1.2.4", "1.2.3") == "CONFLICT" + + def test_specific_pin_with_nothing_installed_is_not_installed(self): + assert apply_pin_to_status("NOT INSTALLED", "", "1.2.3") == "NOT INSTALLED" + + +class TestDefaultPath: + def test_env_override(self, monkeypatch: pytest.MonkeyPatch, tmp_path: Path): + """``CLI_AUDIT_PINS_PATH`` should override the default location.""" + path = tmp_path / "alt-pins.json" + path.write_text('{"ripgrep": "9.9.9"}') + monkeypatch.setenv("CLI_AUDIT_PINS_PATH", str(path)) + # Re-import to pick up the new env var; the module computes the path + # at import time. + import importlib + + import cli_audit.pins as pins_mod + + importlib.reload(pins_mod) + try: + assert pins_mod.load_pins()["ripgrep"] == "9.9.9" + finally: + # Restore clean module state for other tests. + importlib.reload(pins_mod) From 06a3824194551dcd6448e37609e16739da3814b0 Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Tue, 21 Apr 2026 14:19:07 +0200 Subject: [PATCH 7/8] fix(pins,render): address PR #78 review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three issues raised on the initial review: 1. ``pins.py`` silently swallowed ``json.JSONDecodeError`` and top-level non-dict payloads. A malformed ``pins.json`` would render as if no pins existed with zero diagnostic feedback. Now each failure mode logs a ``WARNING`` naming the file and the failure reason. 2. ``render.py`` had an unused ``latest`` parameter on the now-relocated ``apply_pin_to_status``. Dropped (was already gone in ``pins.apply_pin_to_status``; this just removes the stale reviewer hit on the earlier function-local copy). 3. ``render.py``: the ``CONFLICT:`` sentinel strip ran against the already-colorized ``installed_display`` (``\\033[33mCONFLICT:…``), so the ``startswith`` check never matched when color was enabled — i.e. always under ``make audit``. Strip the sentinel from the raw ``installed`` string before colorization. Tests: - ``tests/test_pins.py`` gains warning-log assertions for malformed JSON and non-dict payloads. - ``tests/test_render.py`` added, including a regression test for the CONFLICT-prefix / ANSI interaction. 585 tests pass; new files lint clean. Signed-off-by: Sebastian Mendel --- cli_audit/pins.py | 23 +++++- cli_audit/render.py | 14 ++-- tests/test_pins.py | 14 ++++ tests/test_render.py | 189 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 233 insertions(+), 7 deletions(-) create mode 100644 tests/test_render.py diff --git a/cli_audit/pins.py b/cli_audit/pins.py index 4d779d0..a8d7deb 100644 --- a/cli_audit/pins.py +++ b/cli_audit/pins.py @@ -26,11 +26,15 @@ from __future__ import annotations import json +import logging import os from functools import lru_cache from typing import Any +logger = logging.getLogger(__name__) + + DEFAULT_PINS_PATH = os.path.expanduser( os.environ.get( "CLI_AUDIT_PINS_PATH", @@ -50,9 +54,24 @@ def _load_pins_cached(path: str) -> dict[str, Any]: try: with open(path, "r", encoding="utf-8") as f: data = json.load(f) - except (OSError, json.JSONDecodeError): + except json.JSONDecodeError as e: + logger.warning( + "pins file %s is not valid JSON (%s); treating as empty. " + "Run `scripts/reset_pins.sh` to rewrite, or edit the file by hand.", + path, + e, + ) + return {} + except OSError as e: + logger.warning("cannot read pins file %s: %s; treating as empty.", path, e) + return {} + if not isinstance(data, dict): + logger.warning( + "pins file %s is valid JSON but not a top-level object; treating as empty.", + path, + ) return {} - return data if isinstance(data, dict) else {} + return data def load_pins(path: str | None = None) -> dict[str, Any]: diff --git a/cli_audit/render.py b/cli_audit/render.py index 531f019..48cda90 100644 --- a/cli_audit/render.py +++ b/cli_audit/render.py @@ -228,8 +228,16 @@ def _render_tool_row( # Hyperlinks name_display = osc8(tool_url, name) if tool_url else name + # Strip the ``CONFLICT:`` sentinel before colorizing so the prefix + # check can't be fooled by ANSI escape codes. The raw string is what + # originates the sentinel (see detection of multiple install + # conflicts), so check it first. + installed_clean = installed + if installed_clean.startswith("CONFLICT: "): + installed_clean = installed_clean[len("CONFLICT: "):] + # Apply colors to installed and latest - installed_display = colorize(installed, inst_color) + installed_display = colorize(installed_clean, inst_color) latest_display = colorize(latest, latest_color) # Apply hyperlinks (after colorization, hyperlinks wrap the colored text) @@ -243,10 +251,6 @@ def _render_tool_row( notes = _build_notes(tool, config) - # Add CONFLICT message to installed display - if status == "CONFLICT" and installed_display.startswith("CONFLICT:"): - installed_display = installed_display.replace("CONFLICT: ", "") # Show clean message - print("|".join((icon, name_display, installed_display, latest_display, notes))) diff --git a/tests/test_pins.py b/tests/test_pins.py index 1ec06c9..de3c1d9 100644 --- a/tests/test_pins.py +++ b/tests/test_pins.py @@ -62,11 +62,25 @@ def test_invalid_json_returns_empty(self, tmp_path: Path): path.write_text("{ not valid json") assert load_pins(str(path)) == {} + def test_invalid_json_logs_warning(self, tmp_path: Path, caplog: pytest.LogCaptureFixture): + path = tmp_path / "pins.json" + path.write_text("{ not valid json") + with caplog.at_level("WARNING", logger="cli_audit.pins"): + load_pins(str(path)) + assert any("not valid JSON" in r.message for r in caplog.records), caplog.records + def test_top_level_list_rejected(self, tmp_path: Path): path = tmp_path / "pins.json" path.write_text("[1, 2, 3]") assert load_pins(str(path)) == {} + def test_top_level_list_logs_warning(self, tmp_path: Path, caplog: pytest.LogCaptureFixture): + path = tmp_path / "pins.json" + path.write_text("[1, 2, 3]") + with caplog.at_level("WARNING", logger="cli_audit.pins"): + load_pins(str(path)) + assert any("top-level object" in r.message for r in caplog.records) + def test_cache_returns_same_object(self, pins_file: Path): first = load_pins(str(pins_file)) second = load_pins(str(pins_file)) diff --git a/tests/test_render.py b/tests/test_render.py new file mode 100644 index 0000000..8485a19 --- /dev/null +++ b/tests/test_render.py @@ -0,0 +1,189 @@ +"""Tests for ``cli_audit.render`` — pipe-delimited audit table rendering.""" + +from __future__ import annotations + +import io +import os +from contextlib import redirect_stdout +from pathlib import Path +from typing import Any + +import pytest + +from cli_audit import pins as pins_module +from cli_audit.render import render_table + + +@pytest.fixture(autouse=True) +def _no_grouping(monkeypatch: pytest.MonkeyPatch): + """Disable category grouping so every test emits a flat table.""" + monkeypatch.setenv("CLI_AUDIT_GROUP", "0") + # The render module reads this flag at import time; patch the binding. + import cli_audit.render as render_mod + + monkeypatch.setattr(render_mod, "GROUP_BY_CATEGORY", False) + + +@pytest.fixture(autouse=True) +def _no_color_no_links(monkeypatch: pytest.MonkeyPatch): + """Keep the output plain so assertions don't fight ANSI / OSC8.""" + import cli_audit.render as render_mod + + monkeypatch.setattr(render_mod, "USE_COLOR", False) + monkeypatch.setattr(render_mod, "ENABLE_LINKS", False) + monkeypatch.setattr(render_mod, "USE_EMOJI", False) + + +@pytest.fixture +def empty_pins(monkeypatch: pytest.MonkeyPatch, tmp_path: Path): + """Point the pins reader at an empty file so nothing is pinned.""" + path = tmp_path / "pins.json" + path.write_text("{}") + pins_module.reset_cache() + monkeypatch.setattr(pins_module, "DEFAULT_PINS_PATH", str(path)) + yield + pins_module.reset_cache() + + +@pytest.fixture +def pinned_world(monkeypatch: pytest.MonkeyPatch, tmp_path: Path): + """A pins.json covering the shapes the renderer has to handle.""" + import json + + path = tmp_path / "pins.json" + path.write_text( + json.dumps( + { + "ripgrep": "14.1.0", + "php": {"8.5": "8.5.3", "8.2": "never"}, + } + ) + ) + pins_module.reset_cache() + monkeypatch.setattr(pins_module, "DEFAULT_PINS_PATH", str(path)) + yield + pins_module.reset_cache() + + +def _render(tools: list[dict[str, Any]]) -> list[str]: + """Capture the rendered table lines (minus header).""" + buf = io.StringIO() + with redirect_stdout(buf): + render_table(tools) + lines = buf.getvalue().splitlines() + # First line is always the header + assert lines[0] == "state|tool|installed|latest_upstream|notes" + return lines[1:] + + +class TestHeader: + def test_header_has_five_columns(self, empty_pins): + buf = io.StringIO() + with redirect_stdout(buf): + render_table([]) + assert buf.getvalue().strip() == "state|tool|installed|latest_upstream|notes" + + +class TestNotesColumn: + def test_plain_installed_shows_method(self, empty_pins): + rows = _render( + [ + { + "tool": "ripgrep", + "installed": "14.1.0", + "latest_upstream": "14.1.0", + "status": "UP-TO-DATE", + "installed_method": "cargo", + } + ] + ) + assert rows == ["✓|ripgrep|14.1.0|14.1.0|cargo"] + + def test_missing_method_yields_empty_notes(self, empty_pins): + rows = _render( + [ + { + "tool": "mystery", + "installed": "1.0", + "latest_upstream": "1.0", + "status": "UP-TO-DATE", + } + ] + ) + # Trailing separator, empty notes cell. + assert rows == ["✓|mystery|1.0|1.0|"] + + +class TestPinRendering: + def test_specific_pin_appears_next_to_installed(self, pinned_world): + rows = _render( + [ + { + "tool": "ripgrep", + "installed": "14.1.0", + "latest_upstream": "14.1.0", + "status": "UP-TO-DATE", + "installed_method": "cargo", + } + ] + ) + assert rows == ["✓|ripgrep|14.1.0 [PIN:14.1.0]|14.1.0|cargo"] + + def test_violated_pin_becomes_conflict_icon(self, pinned_world): + # php@8.5 pinned to 8.5.3, installed 8.5.5 → pin violation. + rows = _render( + [ + { + "tool": "php@8.5", + "installed": "8.5.5", + "latest_upstream": "8.5.5", + "status": "UP-TO-DATE", + "installed_method": "apt", + } + ] + ) + assert rows == ["⚠|php@8.5|8.5.5 [PIN:8.5.3]|8.5.5|apt"] + + def test_never_plus_absent_renders_up_to_date(self, pinned_world): + # php@8.2 pinned never, not installed → ✓ (intent honored). + rows = _render( + [ + { + "tool": "php@8.2", + "installed": "", + "latest_upstream": "8.2.30", + "status": "NOT INSTALLED", + "installed_method": "", + } + ] + ) + assert rows == ["✓|php@8.2| [PIN:never]|8.2.30|"] + + +class TestConflictPrefixStripping: + """Regression test for the ANSI-vs-raw comparison bug — the sentinel + ``CONFLICT: …`` must be stripped before coloring, not after.""" + + def test_conflict_prefix_stripped(self, empty_pins): + rows = _render( + [ + { + "tool": "double-rg", + "installed": "CONFLICT: 14.0.0 at /usr/bin vs 14.1.0 at ~/.cargo/bin", + "latest_upstream": "14.1.0", + "status": "CONFLICT", + "installed_method": "multiple", + } + ] + ) + # Row starts with the conflict icon; installed column must NOT still + # carry the "CONFLICT: " prefix. + assert len(rows) == 1 + assert rows[0].startswith("⚠|double-rg|") + _, _, installed_col, _, _ = rows[0].split("|") + assert not installed_col.startswith("CONFLICT:"), installed_col + assert "14.0.0 at" in installed_col + + +# (Env-var override of DEFAULT_PINS_PATH is covered by +# ``tests/test_pins.py::TestDefaultPath::test_env_override``.) From d864d8156c211e4cc2dc8884c6cf87326e46dc26 Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Tue, 21 Apr 2026 14:21:39 +0200 Subject: [PATCH 8/8] fix(audit): address PR #78 review findings (round 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot's review raised three additional issues: 1. ``Makefile.d/user.mk``: ``smart_column.py --right 3,5`` was right-aligning the new ``notes`` text column and leaving the ``latest_upstream`` version column left-aligned. Swap to ``3,4`` so both version columns (installed, latest) right-align and the notes column stays left-aligned. Applied to every audit target. 2. ``Makefile.d/user.mk``: the ``audit-offline`` help string still said "with hints" after canned hints were removed. Updated to "fast local scan, no network". 3. ``docs/CATALOG_GUIDE.md``: the "Pinning Versions" section said pins render in the ``notes`` column, but the final layout puts ``[PIN:…]`` in the ``installed`` column (next to the version the pin constrains). Docs corrected to match. Signed-off-by: Sebastian Mendel --- Makefile.d/user.mk | 14 +++++++------- docs/CATALOG_GUIDE.md | 6 ++++-- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/Makefile.d/user.mk b/Makefile.d/user.mk index a6cb922..d4d07b0 100644 --- a/Makefile.d/user.mk +++ b/Makefile.d/user.mk @@ -48,23 +48,23 @@ audit: ## Render audit from snapshot (no network, <100ms) fi; \ fi; \ set -o pipefail; CLI_AUDIT_RENDER=1 CLI_AUDIT_GROUP=0 CLI_AUDIT_LINKS=1 CLI_AUDIT_EMOJI=1 CLI_AUDIT_COLOR=1 $(PYTHON) audit.py | \ - $(PYTHON) smart_column.py -s "|" -t --right 3,5 --header' || true + $(PYTHON) smart_column.py -s "|" -t --right 3,4 --header' || true -audit-offline: ## Offline audit with hints (fast local scan) +audit-offline: ## Offline audit (fast local scan, no network) @bash -c 'set -o pipefail; CLI_AUDIT_OFFLINE=1 CLI_AUDIT_RENDER=1 CLI_AUDIT_GROUP=0 CLI_AUDIT_LINKS=1 CLI_AUDIT_EMOJI=1 CLI_AUDIT_COLOR=1 $(PYTHON) audit.py | \ - $(PYTHON) smart_column.py -s "|" -t --right 3,5 --header' || true + $(PYTHON) smart_column.py -s "|" -t --right 3,4 --header' || true outdated: ## Show only missing and outdated tools @bash -c 'set -o pipefail; CLI_AUDIT_RENDER=1 CLI_AUDIT_GROUP=0 CLI_AUDIT_LINKS=1 CLI_AUDIT_EMOJI=1 CLI_AUDIT_COLOR=1 CLI_AUDIT_FILTER_STATUS="NOT INSTALLED,OUTDATED" $(PYTHON) audit.py | \ - $(PYTHON) smart_column.py -s "|" -t --right 3,5 --header' || true + $(PYTHON) smart_column.py -s "|" -t --right 3,4 --header' || true audit-%: scripts-perms ## Audit single tool (e.g., make audit-ripgrep) @bash -c 'set -o pipefail; CLI_AUDIT_RENDER=1 CLI_AUDIT_LINKS=1 CLI_AUDIT_EMOJI=1 CLI_AUDIT_COLOR=1 $(PYTHON) audit.py $* | \ - $(PYTHON) smart_column.py -s "|" -t --right 3,5 --header' || true + $(PYTHON) smart_column.py -s "|" -t --right 3,4 --header' || true audit-offline-%: scripts-perms ## Offline audit subset (e.g., make audit-offline-python-core) @bash -c 'set -o pipefail; CLI_AUDIT_OFFLINE=1 CLI_AUDIT_RENDER=1 CLI_AUDIT_GROUP=0 CLI_AUDIT_LINKS=1 CLI_AUDIT_EMOJI=1 CLI_AUDIT_COLOR=1 $(PYTHON) audit.py $* | \ - $(PYTHON) smart_column.py -s "|" -t --right 3,5 --header' || true + $(PYTHON) smart_column.py -s "|" -t --right 3,4 --header' || true SNAP_FILE?=$(shell python3 -c "import os;print(os.environ.get('CLI_AUDIT_SNAPSHOT_FILE','tools_snapshot.json'))") @@ -74,7 +74,7 @@ audit-auto: ## Update snapshot if missing, then render CLI_AUDIT_COLLECT=1 CLI_AUDIT_DEBUG=1 CLI_AUDIT_PROGRESS=1 $(PYTHON) audit.py --update || true; \ fi; \ CLI_AUDIT_RENDER=1 CLI_AUDIT_GROUP=0 CLI_AUDIT_LINKS=1 CLI_AUDIT_EMOJI=1 $(PYTHON) audit.py | \ - $(PYTHON) smart_column.py -s "|" -t --right 3,5 --header || true + $(PYTHON) smart_column.py -s "|" -t --right 3,4 --header || true # ---------------------------------------------------------------------------- # UPGRADE VARIANTS diff --git a/docs/CATALOG_GUIDE.md b/docs/CATALOG_GUIDE.md index 8005fc3..9732538 100644 --- a/docs/CATALOG_GUIDE.md +++ b/docs/CATALOG_GUIDE.md @@ -450,8 +450,10 @@ The resulting `pins.json` looks like: } ``` -Rendered in `make audit` as `PIN:14.1.0`, `PIN:3.12.7`, and `PIN:never` -in the `notes` column. +Rendered in `make audit` as `[PIN:14.1.0]`, `[PIN:3.12.7]`, and +`[PIN:never]` appended to the `installed` column, next to the version +the pin constrains. The separate `notes` column carries install +method and auto-update flags (e.g. `apt · auto`). ## Fallback to Python TOOLS