fix(audit): aggregate manual removal commands and fix go/cargo uninstalls - #134
Merged
Merged
Conversation
…alls Follow-up to #133: errors are now visible, but a full `make reconcile-all` run still left the user with dozens of per-tool sudo hints and two new defect classes: - Aggregate confirmed-but-failed removals at the end of `--all --apply` into one copy-paste command per remedy: a single `sudo apt remove <pkg...>` per system package manager and a single `sudo rm -f <path...>` for unmanaged binaries. - Classify GOPATH bin (`~/go/bin`) as 'go' instead of falling through to the generic '/bin' → 'system' branch, which produced nonsense `sudo system remove <tool>` guidance. go-installed binaries are removed by deleting the file (user-owned, no sudo). Shell-side capability.sh already did this. - `cargo uninstall <binary>` fails when the crate name differs from the binary (git-delta installs `delta`, watchexec-cli installs `watchexec`). Map binary → owning crate via `cargo install --list`, falling back to the tool name. - 'system'-classified binaries now get `sudo rm <path>` guidance instead of the nonexistent `sudo system remove <tool>` command. Claude-Session: https://claude.ai/code/session_01MH3EaniXCnJdwqNvrMB4Ym Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
Contributor
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
|
CybotTM
added a commit
that referenced
this pull request
Jul 30, 2026
…ics, survivor validation (#135) ## Problems Three issues from real `make reconcile-all` runs after #134: 1. **Ctrl-C hung the run.** Prompts executed inside ThreadPool workers; Ctrl-C killed the main thread while workers kept prompting, then Python's atexit hung joining a worker blocked in `input()` — repeated Ctrl-C required. 2. **`make reconcile-all` ended in `Error 1` even when everything worked as designed** — removals that need sudo are refused on purpose and handed over as copy-paste commands, yet still counted as failures for the exit code. 3. **A kept "duplicate" can be silently broken by the removal.** Real incident: reconcile kept a stray copy of byobu's launcher script in `~/.local/bin` and removed the apt package that owned `/usr/lib/byobu` — the survivor sourced those files and broke. ## Fixes (one commit each) - **Main-thread prompting + a/q answers.** `bulk_reconcile` now detects in parallel (workers, no stdin) and prompts/removes sequentially on the main thread. Ctrl-C acts as quit: current prompt declined, remaining tools marked aborted, clean summary, no traceback. Prompt is now `[y/N/a=all/q=quit]` with sticky all/quit semantics. - **`manual_required` outcome.** Failures that only need a manual sudo command classify as `manual_required`: shown with ⚠, counted in a new "Manual action required" summary line, included in the aggregated command block, and excluded from the non-zero exit. Genuine errors still exit 1. - **Survivor validation.** After a successful removal, the kept installation is probed with its version command; if it no longer runs, the result fails with guidance to reinstall the removed package or delete the broken survivor. ## Test plan - 9 new tests, written first and watched fail: sticky a/q/Ctrl-C prompt semantics, prompts-run-in-MainThread assertion, manual_required classification (all-manual vs mixed), exit-0 CLI behavior with summary line, broken/working survivor probes. - One existing test updated for changed behavior (`test_reconcile_aggressive_mode` now patches the survivor probe). - Full suite: 789 passed, 1 skipped; smoke test OK; pre-commit (flake8, isort, black) green. https://claude.ai/code/session_01MH3EaniXCnJdwqNvrMB4Ym
CybotTM
added a commit
to netresearch/cli-tools-skill
that referenced
this pull request
Jul 30, 2026
…cile binary (#42) ## Problem Field failures from `coding_agent_cli_toolset` reconcile runs (see netresearch/coding_agent_cli_toolset#134 / #135) exist identically in this skill's `scripts/lib/reconcile.sh`: 1. `cargo uninstall "$tool"` uses the binary name, but cargo needs the **crate** name — `git-delta` installs `delta`, `watchexec-cli` installs `watchexec` — and the `|| true` swallowed the failure silently, leaving the old install behind. 2. The post-reconcile verify only checks `command -v`. Presence is not function: a surviving wrapper script can depend on files the removed package owned (real incident: byobu's launcher script sourcing `/usr/lib/byobu`, broken by `apt remove byobu`). ## Fix - Map binary → owning crate via `cargo install --list` (awk two-state parse; fallback to tool name), and warn on uninstall failure instead of hiding it. - After reconciliation, probe `--version`/`version`; warn when the binary is on PATH but no longer runs. Note: the other removal branches share the `|| true` silent-swallow pattern — candidate for a follow-up sweep. ## Test plan - `bash -n` + shellcheck clean (only pre-existing SC1091 info). - awk mapping verified against real `cargo install --list` format: `delta → git-delta`, `watchexec → watchexec-cli`, absent packages fall back to the tool name. ## Quality gate note The `Smoke Test` check fails with `✗ github_release_binary.sh fd install failed:` (empty error). This failure is pre-existing on `main` — identical signature in [run 30123292238](https://github.com/netresearch/cli-tools-skill/actions/runs/30123292238) (2026-07-24, before this branch existed) — and the check is non-required (`mergeStateStatus: UNSTABLE`). This PR touches only `scripts/lib/reconcile.sh`; the failing path is the `github_release_binary.sh` installer. Tracked separately.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Follow-up to #133. With failures now visible, a full
make reconcile-allrun surfaced three remaining issues:sudo apt remove X/sudo rm <path>commands one by one from the output.~/go/binbinaries (golangci-lint, shfmt) fell through the generic/binheuristic tosystem, producing nonsense guidance:sudo system remove golangci-lint. They are user-ownedgo installbinaries — removal is deleting the file, no sudo needed.cargo uninstall delta/watchexecfailed with "package ID specification did not match any packages": the owning crates aregit-deltaandwatchexec-cli.Fix
--reconcile --all --applynow ends with one copy-paste command per remedy: a singlesudo apt remove <pkg…>per system package manager and a singlesudo rm -f <path…>for unmanaged binaries (deduped, sorted; only for conflicts the user confirmed)./go/binasgo(before the generic/binfallback);gobinaries are removed like manual installs and rank tier 2 alongside cargo/pip/npm. Shell-sidecapability.shalready handled this.cargo install --list(fallback: tool name).system-classified binaries getsudo rm <path>guidance instead of the nonexistentsudo system remove <tool>.Test plan
--all --applyoutput.classify_install_method('/home/sme/go/bin/shfmt') == 'go';_cargo_package_for('delta') == 'git-delta';_cargo_package_for('watchexec') == 'watchexec-cli'.https://claude.ai/code/session_01MH3EaniXCnJdwqNvrMB4Ym