From f6e92834b2d4c351195f44c23ab3b07d51b97fad Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Tue, 21 Apr 2026 15:09:02 +0200 Subject: [PATCH 1/4] feat(pins): cycle-aware status + pin_label helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pins for multi-version tools store three distinct things in the same slot: - ``"never"`` — never install / update - the cycle string (``"3.12"``) — hold this cycle, any patch - an explicit version (``"3.12.7"``)— hold exactly this patch The previous Python side treated everything except ``"never"`` as an exact-version pin, which mis-flagged valid cycle-holds and escalated harmless stale skip-markers (e.g. pin ``"8.5.3"`` on ``php@8.5`` when installed is ``"8.5.5"``) to a loud CONFLICT. Additions to ``cli_audit/pins.py``: - ``classify_pin(pin, cycle)`` — returns ``none|never|cycle|version``. - ``apply_pin_to_status`` grows an optional ``cycle`` arg. Cycle-holds are honored when installed is within the cycle; multi-version rows with a stale patch-pin pass their upstream status through unchanged rather than escalating. - ``pin_label(pin, cycle, installed)`` — single source of truth for the rendered suffix: ``PIN:never`` / ``CYCLE:3.12`` / ``PIN:x`` / ``PIN:x stale``. 17 new tests in ``tests/test_pins.py`` cover the full matrix. Signed-off-by: Sebastian Mendel --- cli_audit/__init__.py | 4 ++ cli_audit/pins.py | 104 +++++++++++++++++++++++++++++++++++++----- tests/test_pins.py | 68 +++++++++++++++++++++++++++ 3 files changed, 165 insertions(+), 11 deletions(-) diff --git a/cli_audit/__init__.py b/cli_audit/__init__.py index 6167e8d..779448f 100644 --- a/cli_audit/__init__.py +++ b/cli_audit/__init__.py @@ -60,7 +60,9 @@ is_pinned, is_never, should_skip, + classify_pin, apply_pin_to_status, + pin_label, ) from .install_plan import InstallPlan, InstallStep, generate_install_plan, dry_run_install # noqa: E402 @@ -184,7 +186,9 @@ "is_pinned", "is_never", "should_skip", + "classify_pin", "apply_pin_to_status", + "pin_label", "InstallPlan", "InstallStep", "generate_install_plan", diff --git a/cli_audit/pins.py b/cli_audit/pins.py index a8d7deb..4e0988d 100644 --- a/cli_audit/pins.py +++ b/cli_audit/pins.py @@ -150,7 +150,36 @@ def should_skip(tool_name: str, latest_version: str, pins: dict[str, Any] | None return pin == latest_version -def apply_pin_to_status(status: str, installed: str, pin: str) -> str: +def classify_pin(pin: str, cycle: str | None) -> str: + """Classify a pin value against the row's cycle (if any). + + The ``pins.json`` schema stores pins as strings but the same slot + can mean three different things: + + - ``"never"`` → deliberately-not-installed sentinel + - pin equal to cycle → "hold this cycle, accept any patch" + - anything else → specific version pin (patch-level intent) + + For single-version tools ``cycle`` is ``None`` and every non-never + pin is treated as a specific version. + + Returns one of: ``"none"``, ``"never"``, ``"cycle"``, ``"version"``. + """ + if not pin: + return "none" + if pin == "never": + return "never" + if cycle is not None and pin == cycle: + return "cycle" + return "version" + + +def apply_pin_to_status( + status: str, + installed: str, + pin: str, + cycle: str | None = None, +) -> str: """Adjust a snapshot status value using the user's pin as the target. The snapshot's ``status`` is computed against ``latest_upstream`` and @@ -159,24 +188,77 @@ def apply_pin_to_status(status: str, installed: str, pin: str) -> str: respect it — ``UP-TO-DATE`` must not be reported on a row whose installed version diverges from the pin. + Cycle-awareness (see :func:`classify_pin`): a pin value equal to the + row's ``cycle`` (e.g. ``"3.12"`` on ``python@3.12``) means "hold this + cycle, any patch", so an installed ``3.12.7`` is up-to-date with + respect to the pin. For multi-version tools, a stale patch-level pin + (installed differs from pin and pin is not the cycle) is left at its + upstream status rather than escalated to ``CONFLICT``: the schema + can't distinguish a deliberate patch-hold from a skip-marker that + guide.sh left behind after the world moved on. + Rules: - - ``pin`` empty → pass through (no pin applies). - - ``pin == "never"`` - - nothing installed → ``UP-TO-DATE`` (the pin is honored). + - pin kind ``none`` → pass through. + - pin kind ``never`` + - nothing installed → ``UP-TO-DATE`` (pin 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). + - pin kind ``cycle`` + - nothing installed → ``NOT INSTALLED``. + - installed in cycle → ``UP-TO-DATE``. + - installed elsewhere→ ``CONFLICT``. + - pin kind ``version`` + - nothing installed → ``NOT INSTALLED``. + - installed == pin → ``UP-TO-DATE``. + - installed != pin, multi-version row → pass through (stale). + - installed != pin, single-version row → ``CONFLICT``. """ - if not pin: + kind = classify_pin(pin, cycle) + if kind == "none": return status - if pin == "never": + if kind == "never": return "UP-TO-DATE" if not installed else "CONFLICT" - # Specific-version pin. + if kind == "cycle": + if not installed: + return "NOT INSTALLED" + # installed is within the pinned cycle if it equals the cycle + # (e.g. bare "3.12") or looks like "3.12.x". + assert cycle is not None # guaranteed by classify_pin + if installed == cycle or installed.startswith(cycle + "."): + return "UP-TO-DATE" + return "CONFLICT" + # kind == "version" — specific patch/exact pin. if not installed: return "NOT INSTALLED" if installed == pin: return "UP-TO-DATE" + if cycle is not None: + # Multi-version: the patch-pin is either stale or the world + # moved past it (see module docstring). Don't elevate. + return status return "CONFLICT" + + +def pin_label(pin: str, cycle: str | None, installed: str) -> str: + """Human-readable label for the pin suffix in the ``installed`` column. + + Returns an empty string when there's no pin. Otherwise one of: + + - ``PIN:never`` + - ``CYCLE:3.12`` (cycle-hold — pin matches the row's cycle) + - ``PIN:8.5.3`` (explicit patch-hold, currently honored) + - ``PIN:8.5.3 stale`` (patch-hold on a multi-version row where + installed no longer matches the pin — treated + as fossil data, kept visible for awareness) + """ + kind = classify_pin(pin, cycle) + if kind == "none": + return "" + if kind == "never": + return "PIN:never" + if kind == "cycle": + return f"CYCLE:{pin}" + # version + if installed and installed != pin and cycle is not None: + return f"PIN:{pin} stale" + return f"PIN:{pin}" diff --git a/tests/test_pins.py b/tests/test_pins.py index de3c1d9..25bd6f4 100644 --- a/tests/test_pins.py +++ b/tests/test_pins.py @@ -9,10 +9,12 @@ from cli_audit.pins import ( apply_pin_to_status, + classify_pin, is_never, is_pinned, load_pins, lookup_pin, + pin_label, reset_cache, should_skip, ) @@ -179,6 +181,72 @@ def test_specific_pin_with_nothing_installed_is_not_installed(self): assert apply_pin_to_status("NOT INSTALLED", "", "1.2.3") == "NOT INSTALLED" +class TestClassifyPin: + def test_empty(self): + assert classify_pin("", None) == "none" + + def test_never(self): + assert classify_pin("never", "3.12") == "never" + + def test_cycle_hold(self): + assert classify_pin("3.12", "3.12") == "cycle" + + def test_version_when_no_cycle(self): + assert classify_pin("14.1.0", None) == "version" + + def test_version_when_pin_differs_from_cycle(self): + assert classify_pin("3.12.7", "3.12") == "version" + + +class TestApplyPinToStatusCycleAware: + """Cycle-aware branch of apply_pin_to_status.""" + + def test_cycle_hold_with_matching_installed(self): + # installed within the pinned cycle → UP-TO-DATE, latest ignored. + assert apply_pin_to_status("OUTDATED", "3.12.7", "3.12", "3.12") == "UP-TO-DATE" + + def test_cycle_hold_bare_installed(self): + # installed that *equals* the cycle string is within-cycle too. + assert apply_pin_to_status("OUTDATED", "3.12", "3.12", "3.12") == "UP-TO-DATE" + + def test_cycle_hold_cross_cycle_is_conflict(self): + # Pin says hold 3.12, but installed is 3.13.1 — row shouldn't exist. + assert apply_pin_to_status("OUTDATED", "3.13.1", "3.12", "3.12") == "CONFLICT" + + def test_cycle_hold_with_nothing_installed(self): + assert apply_pin_to_status("NOT INSTALLED", "", "3.12", "3.12") == "NOT INSTALLED" + + def test_patch_pin_multi_version_mismatch_passes_through(self): + # Multi-version row, pin is patch-level, installed differs → stale. + # Original status (OUTDATED here) must stand — no CONFLICT escalation. + assert apply_pin_to_status("OUTDATED", "8.5.5", "8.5.3", "8.5") == "OUTDATED" + + def test_patch_pin_single_version_mismatch_conflicts(self): + # Single-version tool: no cycle, pin mismatch IS a real conflict. + assert apply_pin_to_status("UP-TO-DATE", "14.1.0", "14.0.0", None) == "CONFLICT" + + +class TestPinLabel: + def test_no_pin(self): + assert pin_label("", None, "") == "" + + def test_never(self): + assert pin_label("never", "3.12", "") == "PIN:never" + + def test_cycle(self): + assert pin_label("3.12", "3.12", "3.12.7") == "CYCLE:3.12" + + def test_patch_honored(self): + assert pin_label("8.5.3", "8.5", "8.5.3") == "PIN:8.5.3" + + def test_patch_stale_on_multi_version(self): + assert pin_label("8.5.3", "8.5", "8.5.5") == "PIN:8.5.3 stale" + + def test_single_version_mismatch_not_marked_stale(self): + # Without a cycle we don't know it's a multi-version stale case. + assert pin_label("14.0.0", None, "14.1.0") == "PIN:14.0.0" + + class TestDefaultPath: def test_env_override(self, monkeypatch: pytest.MonkeyPatch, tmp_path: Path): """``CLI_AUDIT_PINS_PATH`` should override the default location.""" From 73a9be5b3ec3f5963252cb9c6c6ee1c2abed3222 Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Tue, 21 Apr 2026 15:09:16 +0200 Subject: [PATCH 2/4] feat(render): relocate AUTO to installed column, drop notes auto-flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three rendering changes drive the install column from "version" to "version + declared intent": - The row's cycle is extracted (``_row_cycle``) and threaded through to the pin-aware status computation, so ``python@3.12`` pinned to ``"3.12"`` (cycle-hold) renders ``✅`` on any ``3.12.x`` installed. - Pin + AUTO markers are combined into a single bracketed suffix on the installed column (``3.14.4 [AUTO]``, ``3.12.7 [CYCLE:3.12 AUTO]``, ``8.5.5 [PIN:8.5.3 stale]``) via ``_installed_markers``. - ``AUTO`` is suppressed when the pin is ``never`` — those two states contradict each other and the user read the combination as a bug. - ``load_config`` promoted to a module-level import so tests can ``monkeypatch.setattr`` it, and an autouse fixture neutralizes the developer's real ``~/.config/cli-audit/config.yml`` so other tests don't leak stray ``[AUTO]`` markers into assertions. 6 new render tests: cycle-hold, stale-patch, violated-never, single-version conflict, AUTO visibility, AUTO base-tool inheritance. Signed-off-by: Sebastian Mendel --- cli_audit/render.py | 102 +++++++++++++++++-------- tests/test_render.py | 176 ++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 245 insertions(+), 33 deletions(-) diff --git a/cli_audit/render.py b/cli_audit/render.py index 48cda90..4d983fb 100644 --- a/cli_audit/render.py +++ b/cli_audit/render.py @@ -8,7 +8,8 @@ import sys from typing import Any -from .pins import apply_pin_to_status, load_pins, lookup_pin +from .config import load_config +from .pins import apply_pin_to_status, load_pins, lookup_pin, pin_label # Environment options @@ -122,8 +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 - # 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") @@ -160,33 +159,71 @@ def render_table(tools: list[dict[str, Any]]) -> None: _render_tool_row(tool, pins, config) -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 _row_cycle(tool: dict[str, Any]) -> str | None: + """Extract the cycle string for a multi-version row, else None. + + Prefer the snapshot field ``version_cycle``; fall back to splitting + the tool name on ``@`` so callers don't need to synthesize the field. + """ + if tool.get("is_multi_version"): + cycle = tool.get("version_cycle") + if cycle: + return str(cycle) + name = tool.get("tool", "") + if "@" in name: + return name.split("@", 1)[1] or None + return None -def _build_notes(tool: dict[str, Any], config: Any) -> str: - """Compose the ``notes`` cell: ``method · auto``. +def _auto_update_explicit(tool: dict[str, Any], config: Any) -> bool | None: + """Return the explicit auto_update state for a tool, or ``None``. - Pin info is rendered in the ``installed`` column, not here. + Only reports ``True`` / ``False`` when the user wrote an explicit + ``auto_update`` for this tool (cycle-qualified or base) in config.yml. + The global ``preferences.auto_upgrade`` default is intentionally + *not* reflected — it would put a marker on every row and drown the + signal. """ - parts: list[str] = [] - method = tool.get("installed_method") or "" - if method: - parts.append(method) + if config is None: + return 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 None: + return None + return tool_cfg.auto_update # may be None if the key isn't set + - 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") +def _installed_markers(pin: str, cycle: str | None, installed: str, auto: bool | None) -> str: + """Build the bracketed suffix that appends to the ``installed`` column. - return " · ".join(parts) + Collects, in order: the pin label (if any) and the AUTO marker (if + explicitly enabled). Markers are space-separated and wrapped in a + single pair of square brackets. Empty when no marker applies. + + ``AUTO`` is suppressed when the pin says ``never`` — the two states + contradict each other and showing both confuses the row. + """ + markers: list[str] = [] + label = pin_label(pin, cycle, installed) + if label: + markers.append(label) + if auto is True and pin != "never": + markers.append("AUTO") + if not markers: + return "" + return " [" + " ".join(markers) + "]" + + +def _build_notes(tool: dict[str, Any]) -> str: + """Compose the ``notes`` cell — now just the install method. + + Pin info and auto-update flag render in the ``installed`` column + (see :func:`_installed_markers`) so the ``notes`` column is + reserved for provenance (``apt`` / ``cargo`` / ``manual``). + """ + method = tool.get("installed_method") or "" + return method def _render_tool_row( @@ -205,8 +242,11 @@ def _render_tool_row( # 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. + # ``cycle`` is passed so cycle-holds (``pin == "3.12"`` on + # ``python@3.12``) are interpreted as "any patch of 3.12 is fine". pin_value = lookup_pin(name, pins) - status = apply_pin_to_status(raw_status, installed, pin_value) + cycle = _row_cycle(tool) + status = apply_pin_to_status(raw_status, installed, pin_value, cycle) # Icon icon = status_icon(status, installed) @@ -244,12 +284,13 @@ def _render_tool_row( if latest_url: latest_display = osc8(latest_url, latest_display) - # 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)}" + # Attach pin + AUTO markers to the version they describe (installed + # column). The suffix renders outside the hyperlink so it stays + # readable when nothing is installed. + auto = _auto_update_explicit(tool, config) + installed_display = f"{installed_display}{_installed_markers(pin_value, cycle, installed, auto)}" - notes = _build_notes(tool, config) + notes = _build_notes(tool) print("|".join((icon, name_display, installed_display, latest_display, notes))) @@ -271,6 +312,7 @@ def _effective(t: dict[str, Any]) -> str: t.get("status", "UNKNOWN"), t.get("installed", ""), lookup_pin(t.get("tool", ""), pins), + _row_cycle(t), ) effective = [_effective(t) for t in tools] diff --git a/tests/test_render.py b/tests/test_render.py index 8485a19..a68a783 100644 --- a/tests/test_render.py +++ b/tests/test_render.py @@ -34,6 +34,21 @@ def _no_color_no_links(monkeypatch: pytest.MonkeyPatch): monkeypatch.setattr(render_mod, "USE_EMOJI", False) +@pytest.fixture(autouse=True) +def _empty_user_config(monkeypatch: pytest.MonkeyPatch): + """Default every test to an empty user config. + + Without this the renderer picks up the developer's real + ``~/.config/cli-audit/config.yml`` and leaks ``[AUTO]`` markers into + unrelated assertions. Tests that exercise the AUTO marker override + this with ``monkeypatch.setattr(render_mod, 'load_config', ...)``. + """ + import cli_audit.render as render_mod + from cli_audit.config import Config + + monkeypatch.setattr(render_mod, "load_config", lambda: Config()) + + @pytest.fixture def empty_pins(monkeypatch: pytest.MonkeyPatch, tmp_path: Path): """Point the pins reader at an empty file so nothing is pinned.""" @@ -129,8 +144,11 @@ def test_specific_pin_appears_next_to_installed(self, pinned_world): ) 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. + def test_stale_patch_pin_is_informational_not_conflict(self, pinned_world): + """On multi-version rows, a patch-level pin that no longer matches + ``installed`` is marked ``stale`` but does not escalate the row to + CONFLICT — the schema can't distinguish a stale skip-marker from a + deliberate patch-hold the world moved past.""" rows = _render( [ { @@ -139,10 +157,53 @@ def test_violated_pin_becomes_conflict_icon(self, pinned_world): "latest_upstream": "8.5.5", "status": "UP-TO-DATE", "installed_method": "apt", + "is_multi_version": True, + "version_cycle": "8.5", + } + ] + ) + assert rows == ["✓|php@8.5|8.5.5 [PIN:8.5.3 stale]|8.5.5|apt"] + + def test_cycle_hold_any_patch_is_up_to_date(self, monkeypatch, tmp_path): + """``python@3.12`` pinned to the cycle string ``"3.12"`` means + "stay within 3.12.x". An installed ``3.12.7`` is UP-TO-DATE even + when ``latest_upstream`` is newer.""" + import json + + path = tmp_path / "pins.json" + path.write_text(json.dumps({"python": {"3.12": "3.12"}})) + pins_module.reset_cache() + monkeypatch.setattr(pins_module, "DEFAULT_PINS_PATH", str(path)) + rows = _render( + [ + { + "tool": "python@3.12", + "installed": "3.12.7", + "latest_upstream": "3.12.13", + "status": "OUTDATED", + "installed_method": "apt", + "is_multi_version": True, + "version_cycle": "3.12", + } + ] + ) + assert rows == ["✓|python@3.12|3.12.7 [CYCLE:3.12]|3.12.13|apt"] + + def test_single_version_violated_pin_still_conflicts(self, pinned_world): + """Outside the multi-version world there's no cycle notion — an + exact pin that doesn't match installed is a real conflict.""" + rows = _render( + [ + { + "tool": "ripgrep", + "installed": "15.0.0", + "latest_upstream": "15.0.0", + "status": "UP-TO-DATE", + "installed_method": "cargo", } ] ) - assert rows == ["⚠|php@8.5|8.5.5 [PIN:8.5.3]|8.5.5|apt"] + assert rows == ["⚠|ripgrep|15.0.0 [PIN:14.1.0]|15.0.0|cargo"] def test_never_plus_absent_renders_up_to_date(self, pinned_world): # php@8.2 pinned never, not installed → ✓ (intent honored). @@ -154,11 +215,36 @@ def test_never_plus_absent_renders_up_to_date(self, pinned_world): "latest_upstream": "8.2.30", "status": "NOT INSTALLED", "installed_method": "", + "is_multi_version": True, + "version_cycle": "8.2", } ] ) assert rows == ["✓|php@8.2| [PIN:never]|8.2.30|"] + def test_never_plus_installed_is_conflict(self, pinned_world, monkeypatch, tmp_path): + """PIN:never applied but the tool is present → ⚠️ conflict.""" + import json + + path = tmp_path / "pins.json" + path.write_text(json.dumps({"ruby": {"3.3": "never"}})) + pins_module.reset_cache() + monkeypatch.setattr(pins_module, "DEFAULT_PINS_PATH", str(path)) + rows = _render( + [ + { + "tool": "ruby@3.3", + "installed": "3.3.6", + "latest_upstream": "3.3.11", + "status": "OUTDATED", + "installed_method": "manual", + "is_multi_version": True, + "version_cycle": "3.3", + } + ] + ) + assert rows == ["⚠|ruby@3.3|3.3.6 [PIN:never]|3.3.11|manual"] + class TestConflictPrefixStripping: """Regression test for the ANSI-vs-raw comparison bug — the sentinel @@ -185,5 +271,89 @@ def test_conflict_prefix_stripped(self, empty_pins): assert "14.0.0 at" in installed_col +class TestAutoMarker: + """Tests for the ``[AUTO]`` marker driven by explicit config entries.""" + + def _config(self, tools: dict[str, bool | None]): + """Build a minimal ``Config`` with per-tool auto_update values.""" + from cli_audit.config import Config, ToolConfig + + return Config(tools={k: ToolConfig(auto_update=v) for k, v in tools.items()}) + + def test_auto_marker_shown_when_explicit_true( + self, empty_pins, monkeypatch: pytest.MonkeyPatch + ): + import cli_audit.render as render_mod + + cfg = self._config({"ripgrep": True}) + monkeypatch.setattr(render_mod, "load_config", lambda: cfg) + 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 [AUTO]|14.1.0|cargo"] + + def test_auto_marker_hidden_when_pin_is_never( + self, monkeypatch: pytest.MonkeyPatch, tmp_path: Path + ): + """AUTO and PIN:never contradict each other — don't show both.""" + import json + + import cli_audit.render as render_mod + + path = tmp_path / "pins.json" + path.write_text(json.dumps({"ruby": {"3.3": "never"}})) + pins_module.reset_cache() + monkeypatch.setattr(pins_module, "DEFAULT_PINS_PATH", str(path)) + cfg = self._config({"ruby": True}) # base auto_update=True + monkeypatch.setattr(render_mod, "load_config", lambda: cfg) + rows = _render( + [ + { + "tool": "ruby@3.3", + "installed": "3.3.6", + "latest_upstream": "3.3.11", + "status": "OUTDATED", + "installed_method": "manual", + "is_multi_version": True, + "version_cycle": "3.3", + } + ] + ) + # [AUTO] must NOT appear; the row is a ⚠️ conflict only. + assert rows == ["⚠|ruby@3.3|3.3.6 [PIN:never]|3.3.11|manual"] + + def test_auto_marker_inherits_from_base_tool( + self, empty_pins, monkeypatch: pytest.MonkeyPatch + ): + """``python: auto_update: true`` should surface as [AUTO] on + ``python@3.14`` rows (base-tool fallback).""" + import cli_audit.render as render_mod + + cfg = self._config({"python": True}) + monkeypatch.setattr(render_mod, "load_config", lambda: cfg) + rows = _render( + [ + { + "tool": "python@3.14", + "installed": "3.14.4", + "latest_upstream": "3.14.4", + "status": "UP-TO-DATE", + "installed_method": "manual", + "is_multi_version": True, + "version_cycle": "3.14", + } + ] + ) + assert rows == ["✓|python@3.14|3.14.4 [AUTO]|3.14.4|manual"] + + # (Env-var override of DEFAULT_PINS_PATH is covered by # ``tests/test_pins.py::TestDefaultPath::test_env_override``.) From 499701ce7e6cb11ef275023696be8b35ee9cbfca Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Tue, 21 Apr 2026 15:09:27 +0200 Subject: [PATCH 3/4] feat(scripts): reset_pins.sh --stale flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add ``--stale`` (and ``--dry-run``) to ``reset_pins.sh``. Stale = patch-level pin on a tool that is currently installed at a different version, AND the pin is not a cycle-string, AND the pin is not ``"never"``. These are almost always fossil records from the ``s`` (skip) or failed-upgrade prompt paths in ``guide.sh``, left behind after the system moved past them. Preserved by design: - ``PIN:never`` — deliberate "don't install". - ``CYCLE:`` — the user wants to hold a cycle. - Pins where ``installed == pin`` — the pin is actively honored. - Pins for tools not in the current snapshot — can't judge staleness. ``--dry-run`` prints the classification for every pin without touching the file; good for sanity-checking before committing to a cleanup. ``scripts/lib/pins.sh`` already exposes ``pins_remove`` and ``pins_remove_cycle``; no library changes needed. Signed-off-by: Sebastian Mendel --- scripts/reset_pins.sh | 164 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 149 insertions(+), 15 deletions(-) diff --git a/scripts/reset_pins.sh b/scripts/reset_pins.sh index 1fd6ea1..843f75c 100755 --- a/scripts/reset_pins.sh +++ b/scripts/reset_pins.sh @@ -1,30 +1,164 @@ #!/usr/bin/env bash -# reset_pins.sh - Remove all version pins from all tools +# reset_pins.sh - Remove version pins from ~/.config/cli-audit/pins.json +# +# Default: wipe every pin. +# --stale: only remove pins the user almost certainly doesn't want anymore: +# patch-level pins where installed != pin and pin is not a +# cycle-string (i.e. fossil skip-markers and overridden holds). +# "never" pins and cycle-holds (pin == cycle) are preserved. +# Tools not present in the snapshot are preserved. + set -euo pipefail DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +ROOT="$(cd "$DIR/.." && pwd)" # Load pin library . "$DIR/lib/pins.sh" -echo "Resetting all version pins..." +MODE="all" +DRY_RUN=0 +SNAP_FILE="${CLI_AUDIT_SNAPSHOT_FILE:-$ROOT/tools_snapshot.json}" + +usage() { + cat <&2; usage >&2; exit 2 ;; + esac + shift +done -# Show what's being removed -if [ -f "$PINS_FILE" ]; then - current="$(jq -r 'to_entries[] | if (.value | type) == "object" then "\(.key): \(.value | to_entries | map("\(.key)=\(.value)") | join(", "))" else "\(.key): \(.value)" end' "$PINS_FILE" 2>/dev/null)" - if [ -n "$current" ]; then - while IFS= read -r line; do - echo " Removing pin: $line" - done <<< "$current" +if [ ! -f "$PINS_FILE" ]; then + echo "No pins file ($PINS_FILE)." + exit 0 +fi + +if [ "$MODE" = "all" ]; then + current="$(jq -r 'to_entries[] | if (.value | type) == "object" then "\(.key): \(.value | to_entries | map("\(.key)=\(.value)") | join(", "))" else "\(.key): \(.value)" end' "$PINS_FILE" 2>/dev/null || true)" + if [ -z "$current" ]; then + echo "No pins found." + exit 0 + fi + echo "Resetting all version pins..." + while IFS= read -r line; do + echo " Removing pin: $line" + done <<< "$current" + if [ "$DRY_RUN" -eq 1 ]; then echo "" - pins_reset_all - echo "✓ All pins removed" + echo "(dry-run, no changes made)" + exit 0 + fi + echo "" + pins_reset_all + echo "✓ All pins removed" + echo "" + echo "All tools will now appear in upgrade prompts." + exit 0 +fi + +# --stale mode needs the snapshot. +if [ ! -f "$SNAP_FILE" ]; then + echo "Snapshot not found at $SNAP_FILE. Run 'make update' first." >&2 + exit 1 +fi + +# Walk the pins file and decide per-entry. jq does the shape work; the +# loop here formats the decision and emits the updated JSON. +mapfile -t actions < <( + jq -r --slurpfile snap "$SNAP_FILE" ' + # Build a map (tool_name -> installed) from the snapshot tools array. + ($snap[0].tools // []) as $tools + | ([ $tools[] | select(.tool) | {key: .tool, value: (.installed // "")} ] | from_entries) as $installed + | to_entries[] + | .key as $tool + | if (.value | type) == "object" then + .value | to_entries[] | .key as $cycle | .value as $pin + | "\($tool)@\($cycle)\t\($cycle)\t\($pin)\t\($installed["\($tool)@\($cycle)"] // "")" + else + .key as $name | .value as $pin + | "\($name)\t\t\($pin)\t\($installed[$name] // "")" + end + ' "$PINS_FILE" +) + +to_remove=() # "tool\tcycle" pairs (cycle blank for flat) +to_keep=() +for line in "${actions[@]}"; do + IFS=$'\t' read -r row_name cycle pin installed <<< "$line" + tool="${row_name%@*}" + # Classify. + if [ "$pin" = "never" ]; then + to_keep+=("$row_name: PIN:never (kept)") + continue + fi + if [ -n "$cycle" ] && [ "$pin" = "$cycle" ]; then + to_keep+=("$row_name: CYCLE:$pin (kept)") + continue + fi + if [ -z "$installed" ]; then + # Nothing installed — pin is intent-to-stay-at-absent; leave it. + to_keep+=("$row_name: PIN:$pin (kept — not installed)") + continue + fi + if [ "$installed" = "$pin" ]; then + to_keep+=("$row_name: PIN:$pin (kept — honored)") + continue + fi + # Stale patch-level pin. + to_remove+=("$row_name: PIN:$pin → installed $installed (removing)") + if [ -n "$cycle" ]; then + printf 'REMOVE\t%s\t%s\n' "$tool" "$cycle" >> /tmp/.cli-audit-stale-pins.$$ else - echo "No pins found." + printf 'REMOVE\t%s\t\n' "$tool" >> /tmp/.cli-audit-stale-pins.$$ fi -else - echo "No pins found." +done + +trap 'rm -f /tmp/.cli-audit-stale-pins.$$' EXIT + +if [ "${#to_keep[@]}" -gt 0 ]; then + echo "Preserved:" + printf ' %s\n' "${to_keep[@]}" + echo "" +fi + +if [ "${#to_remove[@]}" -eq 0 ]; then + echo "No stale pins found." + exit 0 fi +echo "Stale pins to remove:" +printf ' %s\n' "${to_remove[@]}" echo "" -echo "All tools will now appear in upgrade prompts." + +if [ "$DRY_RUN" -eq 1 ]; then + echo "(dry-run, no changes made)" + exit 0 +fi + +if [ ! -s /tmp/.cli-audit-stale-pins.$$ ]; then + exit 0 +fi + +while IFS=$'\t' read -r _ tool cycle; do + if [ -n "$cycle" ]; then + pins_remove_cycle "$tool" "$cycle" + else + pins_remove "$tool" + fi +done < /tmp/.cli-audit-stale-pins.$$ + +echo "✓ Removed ${#to_remove[@]} stale pin(s)." From f1dd64199ee58ae757d120aa76fd4f81ed2dce01 Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Tue, 21 Apr 2026 15:43:27 +0200 Subject: [PATCH 4/4] fix: address PR #79 review findings Three issues, five review comments on PR #79: 1. ``scripts/reset_pins.sh`` silently swallowed invalid JSON (both the pins file and the snapshot). Add a ``validate_json_object`` helper that emits "not valid JSON" or "valid JSON but not a top-level object" with the offending path before exiting non-zero. Applies to both ``--stale`` and the default all-pins mode. (reviewers: gemini, copilot) 2. ``scripts/reset_pins.sh`` wrote to a predictable ``/tmp`` path and only installed the cleanup trap *after* the loop. Replaced with ``mktemp -t cli-audit-stale-pins.XXXXXX``, trap on ``EXIT INT TERM`` installed immediately after creation, and no temp file written at all in ``--dry-run`` mode (the plan is printed from an in-memory array now). (reviewers: gemini, copilot) 3. ``cli_audit/render.py`` header comment still said the ``notes`` column carried the auto-update flag; updated to reflect the actual placement (pin + AUTO live next to ``installed``, ``notes`` is method-only). (reviewer: copilot) 608 tests still pass; manually verified the new JSON-validation paths reject a malformed pins file and a top-level list with clear errors, and no ``cli-audit-stale-pins.*`` temp files leak on normal or interrupted runs. Signed-off-by: Sebastian Mendel --- cli_audit/render.py | 4 +-- scripts/reset_pins.sh | 67 ++++++++++++++++++++++++++++++------------- 2 files changed, 49 insertions(+), 22 deletions(-) diff --git a/cli_audit/render.py b/cli_audit/render.py index 4d983fb..d5e015c 100644 --- a/cli_audit/render.py +++ b/cli_audit/render.py @@ -123,8 +123,8 @@ 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.""" - # Header — 5 columns. Pin info lives next to the ``installed`` value - # it constrains; ``notes`` carries install method and auto-update flag. + # Header — 5 columns. Auto-update and pin markers live next to the + # ``installed`` value they constrain; ``notes`` carries install method only. headers = ("state", "tool", "installed", "latest_upstream", "notes") print("|".join(headers)) diff --git a/scripts/reset_pins.sh b/scripts/reset_pins.sh index 843f75c..e89ce44 100755 --- a/scripts/reset_pins.sh +++ b/scripts/reset_pins.sh @@ -47,8 +47,26 @@ if [ ! -f "$PINS_FILE" ]; then exit 0 fi +# Validate a file is a JSON object. Explicit error messages for invalid / +# non-object payloads — silent "no pins found" on bad JSON was masking +# real problems (reviewer: gemini, copilot on PR #79). +validate_json_object() { + local path="$1" label="$2" + if ! jq -e 'type == "object"' "$path" >/dev/null 2>&1; then + if ! jq -e . "$path" >/dev/null 2>&1; then + echo "Error: $label ($path) is not valid JSON." >&2 + else + echo "Error: $label ($path) is valid JSON but not a top-level object." >&2 + fi + return 1 + fi + return 0 +} + +validate_json_object "$PINS_FILE" "pins file" || exit 1 + if [ "$MODE" = "all" ]; then - current="$(jq -r 'to_entries[] | if (.value | type) == "object" then "\(.key): \(.value | to_entries | map("\(.key)=\(.value)") | join(", "))" else "\(.key): \(.value)" end' "$PINS_FILE" 2>/dev/null || true)" + current="$(jq -r 'to_entries[] | if (.value | type) == "object" then "\(.key): \(.value | to_entries | map("\(.key)=\(.value)") | join(", "))" else "\(.key): \(.value)" end' "$PINS_FILE")" if [ -z "$current" ]; then echo "No pins found." exit 0 @@ -76,8 +94,10 @@ if [ ! -f "$SNAP_FILE" ]; then exit 1 fi +validate_json_object "$SNAP_FILE" "snapshot" || exit 1 + # Walk the pins file and decide per-entry. jq does the shape work; the -# loop here formats the decision and emits the updated JSON. +# loop here formats the decision and classifies each pin. mapfile -t actions < <( jq -r --slurpfile snap "$SNAP_FILE" ' # Build a map (tool_name -> installed) from the snapshot tools array. @@ -95,7 +115,11 @@ mapfile -t actions < <( ' "$PINS_FILE" ) -to_remove=() # "tool\tcycle" pairs (cycle blank for flat) +# Collect removals as tab-separated lines in an array; only materialise a +# temp file when we actually need to hand work to the pins_remove loop. +# (Reviewer: gemini, copilot — use mktemp, skip the file in dry-run, +# install the trap *before* writing.) +remove_lines=() to_keep=() for line in "${actions[@]}"; do IFS=$'\t' read -r row_name cycle pin installed <<< "$line" @@ -110,7 +134,6 @@ for line in "${actions[@]}"; do continue fi if [ -z "$installed" ]; then - # Nothing installed — pin is intent-to-stay-at-absent; leave it. to_keep+=("$row_name: PIN:$pin (kept — not installed)") continue fi @@ -119,29 +142,30 @@ for line in "${actions[@]}"; do continue fi # Stale patch-level pin. - to_remove+=("$row_name: PIN:$pin → installed $installed (removing)") - if [ -n "$cycle" ]; then - printf 'REMOVE\t%s\t%s\n' "$tool" "$cycle" >> /tmp/.cli-audit-stale-pins.$$ - else - printf 'REMOVE\t%s\t\n' "$tool" >> /tmp/.cli-audit-stale-pins.$$ - fi + remove_lines+=("$(printf '%s\t%s' "$tool" "$cycle")") done -trap 'rm -f /tmp/.cli-audit-stale-pins.$$' EXIT - +# Present the plan. if [ "${#to_keep[@]}" -gt 0 ]; then echo "Preserved:" printf ' %s\n' "${to_keep[@]}" echo "" fi -if [ "${#to_remove[@]}" -eq 0 ]; then +if [ "${#remove_lines[@]}" -eq 0 ]; then echo "No stale pins found." exit 0 fi echo "Stale pins to remove:" -printf ' %s\n' "${to_remove[@]}" +for rl in "${remove_lines[@]}"; do + IFS=$'\t' read -r tool cycle <<< "$rl" + if [ -n "$cycle" ]; then + echo " ${tool}@${cycle}" + else + echo " ${tool}" + fi +done echo "" if [ "$DRY_RUN" -eq 1 ]; then @@ -149,16 +173,19 @@ if [ "$DRY_RUN" -eq 1 ]; then exit 0 fi -if [ ! -s /tmp/.cli-audit-stale-pins.$$ ]; then - exit 0 -fi +# Create the temp file securely and set the trap immediately; even if +# the pins_remove loop dies, cleanup still fires. +STALE_LIST="$(mktemp -t cli-audit-stale-pins.XXXXXX)" +trap 'rm -f "$STALE_LIST"' EXIT INT TERM + +printf '%s\n' "${remove_lines[@]}" > "$STALE_LIST" -while IFS=$'\t' read -r _ tool cycle; do +while IFS=$'\t' read -r tool cycle; do if [ -n "$cycle" ]; then pins_remove_cycle "$tool" "$cycle" else pins_remove "$tool" fi -done < /tmp/.cli-audit-stale-pins.$$ +done < "$STALE_LIST" -echo "✓ Removed ${#to_remove[@]} stale pin(s)." +echo "✓ Removed ${#remove_lines[@]} stale pin(s)."