Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions audit.py
Original file line number Diff line number Diff line change
Expand Up @@ -1483,6 +1483,26 @@ def _plan(tool):
print(f" {r.tool}: skipped (protected system tool)", file=sys.stderr)
elif not r.success and r.error_message:
print(f" ✗ {r.tool}: {r.error_message}", file=sys.stderr)
# Aggregate confirmed-but-failed removals into one copy-paste
# command per remedy instead of dozens of per-tool sudo hints.
pm_tools: dict[str, set[str]] = {}
rm_paths: set[str] = set()
for r in conflicts:
if r.action_taken != "removed" or r.success:
continue
for inst in r.installations:
if inst == r.preferred or inst in r.removed_installations:
continue
if inst.method in ("apt", "dnf", "pacman"):
pm_tools.setdefault(inst.method, set()).add(inst.tool)
elif inst.method in ("manual", "unknown", "go"):
rm_paths.add(inst.path)
if pm_tools or rm_paths:
print("\nTo finish the confirmed removals manually, run:", file=sys.stderr)
for pm in sorted(pm_tools):
print(f" sudo {pm} remove {' '.join(sorted(pm_tools[pm]))}", file=sys.stderr)
if rm_paths:
print(f" sudo rm -f {' '.join(sorted(rm_paths))}", file=sys.stderr)
print(result.summary(), file=sys.stderr)
# Fail only on real removal errors — not on protected tools (blocked)
# or tools the user declined (aborted), which are expected outcomes.
Expand Down
46 changes: 41 additions & 5 deletions cli_audit/reconcile.py
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,10 @@ def _classify_via_path(path: str) -> str:
return "pyenv"
elif "/.rbenv/" in path:
return "rbenv"
elif "/go/bin" in path:
# GOPATH bin (`go install` target); must precede the generic '/bin'
# fallback or user-owned binaries get labeled 'system'.
return "go"

# System-level installations (check specific patterns before generic ones)
elif "/snap/bin" in path:
Expand Down Expand Up @@ -570,7 +574,7 @@ def get_preference_tier(installation: Installation) -> int:
return 1

# Tier 2: User-level generic
if method in ("cargo", "pip", "npm"):
if method in ("cargo", "pip", "npm", "go"):
return 2
if "/.cargo/" in installation.path or "/.local/" in installation.path:
return 2
Expand Down Expand Up @@ -943,6 +947,34 @@ def _confirm_removal(tool_name: str, to_remove: list[Installation]) -> bool:
return response in ("y", "yes")


def _cargo_package_for(binary: str, tool: str) -> str:
"""Map an installed binary to its owning cargo package.

`cargo uninstall` needs the crate name, which can differ from the binary
(git-delta installs `delta`, watchexec-cli installs `watchexec`). Parses
`cargo install --list`; falls back to the tool name.
"""
try:
result = subprocess.run(
["cargo", "install", "--list"],
capture_output=True,
text=True,
timeout=10,
check=False,
)
if result.returncode != 0:
return tool
package = None
for line in result.stdout.splitlines():
if line and not line[0].isspace():
package = line.split()[0]
elif package and line.strip() in (binary, tool):
return package
except Exception:
pass
return tool


def _uninstall_installation(installation: Installation, verbose: bool) -> tuple[bool, str | None]:
"""
Uninstall a single installation.
Expand All @@ -960,7 +992,7 @@ def _uninstall_installation(installation: Installation, verbose: bool) -> tuple[
if method == "cargo":
try:
result = subprocess.run(
["cargo", "uninstall", tool],
["cargo", "uninstall", _cargo_package_for(os.path.basename(path), tool)],
capture_output=True,
text=True,
timeout=30,
Expand Down Expand Up @@ -1025,11 +1057,15 @@ def _uninstall_installation(installation: Installation, verbose: bool) -> tuple[
return (False, str(e))

# System package managers (require sudo - don't auto-execute)
elif method in ("apt", "dnf", "pacman", "system"):
elif method in ("apt", "dnf", "pacman"):
return (False, f"System package removal requires manual sudo: sudo {method} remove {tool}")

# Manual removal (for GitHub releases, etc.)
elif method == "unknown" or method == "manual":
# Unmanaged system-location binary — no package manager to invoke
elif method == "system":
return (False, f"Unmanaged system binary — remove manually: sudo rm {path}")

# Manual removal (GitHub releases, `go install` binaries, etc.)
elif method in ("unknown", "manual", "go"):
try:
if os.path.exists(path):
os.remove(path)
Expand Down
83 changes: 81 additions & 2 deletions tests/test_reconcile.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,17 @@ def test_classify_via_path_opt_homebrew_without_brew_is_manual(self):
method = _classify_via_path(path)
assert method == "manual"

def test_classify_via_path_gopath_bin_is_go(self):
"""GOPATH bin binaries are go installs, not 'system'.

Regression: /home/<user>/go/bin matched the generic '/bin' fallback and
was labeled 'system', producing nonsense `sudo system remove <tool>`
guidance for user-owned `go install` binaries.
"""
path = "/home/user/go/bin/golangci-lint"
method = _classify_via_path(path)
assert method == "go"

def test_classify_via_path_apt(self):
"""Test apt classification via path."""
path = "/usr/bin/ripgrep"
Expand Down Expand Up @@ -766,7 +777,14 @@ class TestUninstallInstallation:
@patch("cli_audit.reconcile.subprocess.run")
def test_uninstall_cargo(self, mock_run):
"""Test cargo uninstall."""
mock_run.return_value = MagicMock(returncode=0)
listing = "ripgrep v14.1.0:\n rg\n"

def side_effect(cmd, **kwargs):
if cmd == ["cargo", "install", "--list"]:
return MagicMock(returncode=0, stdout=listing)
return MagicMock(returncode=0)

mock_run.side_effect = side_effect

inst = Installation(
tool="ripgrep",
Expand All @@ -780,9 +798,37 @@ def test_uninstall_cargo(self, mock_run):

assert success is True
assert error is None
mock_run.assert_called_once()
assert mock_run.call_args[0][0] == ["cargo", "uninstall", "ripgrep"]

@patch("cli_audit.reconcile.subprocess.run")
def test_uninstall_cargo_maps_binary_to_package(self, mock_run):
"""Cargo uninstall uses the owning package, not the binary name.

Regression: `cargo uninstall delta` fails with "package ID specification
`delta` did not match any packages" — the crate is `git-delta`.
"""
listing = "git-delta v0.18.2:\n delta\nwatchexec-cli v2.3.2:\n watchexec\n"

def side_effect(cmd, **kwargs):
if cmd == ["cargo", "install", "--list"]:
return MagicMock(returncode=0, stdout=listing)
return MagicMock(returncode=0)

mock_run.side_effect = side_effect

inst = Installation(
tool="delta",
version="0.18.2",
method="cargo",
path="/home/user/.cargo/bin/delta",
active=False,
)

success, error = _uninstall_installation(inst, False)

assert success is True
assert mock_run.call_args[0][0] == ["cargo", "uninstall", "git-delta"]

@patch("cli_audit.reconcile.subprocess.run")
def test_uninstall_pipx(self, mock_run):
"""Test pipx uninstall."""
Expand Down Expand Up @@ -853,6 +899,39 @@ def test_uninstall_manual(self, mock_exists, mock_remove):
assert success is True
mock_remove.assert_called_once_with("/home/user/bin/tool")

def test_uninstall_system_binary_suggests_rm(self):
"""'system' has no package manager to invoke — suggest sudo rm, not `sudo system remove`."""
inst = Installation(
tool="mytool",
version="1.0.0",
method="system",
path="/bin/mytool",
active=False,
)

success, error = _uninstall_installation(inst, False)

assert success is False
assert "sudo rm /bin/mytool" in error
assert "sudo system remove" not in error

@patch("os.remove")
@patch("os.path.exists", return_value=True)
def test_uninstall_go_removes_binary(self, mock_exists, mock_remove):
"""go-installed binaries are removed by deleting the file (no sudo)."""
inst = Installation(
tool="shfmt",
version="3.12.0",
method="go",
path="/home/user/go/bin/shfmt",
active=False,
)

success, error = _uninstall_installation(inst, False)

assert success is True
mock_remove.assert_called_once_with("/home/user/go/bin/shfmt")

@patch("os.remove", side_effect=PermissionError(13, "Permission denied"))
@patch("os.path.exists", return_value=True)
def test_uninstall_manual_permission_denied_suggests_sudo(self, mock_exists, mock_remove):
Expand Down
47 changes: 47 additions & 0 deletions tests/test_reconcile_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,53 @@ def test_all_apply_prints_failures_and_declines_to_stderr(self):
assert "npm" in output
assert "declined" in output

def test_all_apply_aggregates_manual_commands(self):
"""Failed removals end with ONE apt command and ONE rm command to copy-paste.

A run over dozens of tools otherwise prints dozens of individual sudo
hints; the summary must aggregate them per remedy.
"""

def _failed(tool, *installs):
kept = Installation(tool, "1.0.0", "pipx", f"/home/u/.local/bin/{tool}", True)
return ReconciliationResult(
tool=tool,
installations=(kept, *installs),
preferred=kept,
active=kept,
path_issues=(),
action_taken="removed",
success=False,
error_message="removal failed",
)

bulk = BulkReconciliationResult(
tools_checked=3,
conflicts_found=3,
conflicts_resolved=0,
results=(
_failed("ripgrep", Installation("ripgrep", "13.0.0", "apt", "/usr/bin/rg", False)),
_failed("fx", Installation("fx", "24.0.0", "manual", "/usr/local/bin/fx", False)),
_failed(
"jq",
Installation("jq", "1.6", "apt", "/usr/bin/jq", False),
Installation("jq", "1.8.1", "manual", "/usr/local/bin/jq", False),
),
),
duration_seconds=0.1,
)
with patch("cli_audit.reconcile.bulk_reconcile", return_value=bulk):
err = io.StringIO()
with redirect_stderr(err):
rc = audit.cmd_reconcile(_ns(all=True, apply=True, yes=True))
assert rc == 1
output = err.getvalue()
assert "sudo apt remove jq ripgrep" in output
assert "sudo rm -f /usr/local/bin/fx /usr/local/bin/jq" in output
# aggregated once, not per tool
assert output.count("sudo apt remove") == 1
assert output.count("sudo rm -f") == 1

def test_apply_protected_returns_nonzero(self):
kept = _inst("/usr/bin/demo", active=True)
result = ReconciliationResult(
Expand Down
Loading