bug: prevent paccy empty selection crashes#86
Open
ALX99 wants to merge 4 commits into
Open
Conversation
5cf571d to
dd748db
Compare
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.
Bug
paccycrashes or performs the wrong package-manager action when an interactivefzfpicker is cancelled or returns no selected rows.The affected flows are:
.local/bin/paccy -rt/--remove-tracked.local/bin/paccy -ru/--remove-untracked.local/bin/paccy -it/--install-tracked.local/bin/paccy -sSeverity and impact
This is a reliability bug in an interactive package-management helper. Cancelling an
fzfselection is a normal user flow and should be a no-op. Instead, the remove/install-tracked paths can crash underset -u, while the search path can proceed toyay -Swith an empty package list. The practical impact is noisy failures and a confusing package-manager prompt/action after the user deliberately selected nothing.Evidence
Relevant code:
.local/bin/paccy,set -eou pipefailenables nounset behavior..local/bin/paccy, the-rt,-ru, and-itbranches append topkgsonly inside awhile readloop fed byfzf, then immediately evaluate${#pkgs[@]}..local/bin/paccy, the-sbranch appends topkgsfromyay -Slq | fzf, but previously had no empty-selection guard before callingyay -S "${pkgs[@]}".Observed incorrect behavior before the fix, as captured by the added regression test:
fzfconsumes stdin and exits130, matching the normal cancel path.-rt,-ru, and-it, nopkgsarray element is ever assigned; withset -u, checking${#pkgs[@]}is an unbound-variable crash.-s, the cancelled picker leaves no selected package, but the script still reaches the install command. The test's stubbedyaytreats any package-manager action other than the initialyay -Slqlisting as unexpected.This is a real defect rather than a hypothetical edge case because
fzfcancellation is a standard interactive outcome, and the script already tries to handle empty selections in several branches with[ ${#pkgs[@]} -eq 0 ] && exit; the implementation just forgot to ensurepkgsexists and forgot the equivalent guard in search mode.Reproduce
Setup:
Create a temporary test harness with stubbed commands:
fzfreads stdin and exits130.yay -Slqprints a couple of candidate packages; any otheryayinvocation is recorded as unexpected.pacmanprints deterministic installed/tracked package lists.sudorecords any invocation as unexpected.Exact command added in this PR:
Actual result before the fix:
paccy -rt,paccy -ru, orpaccy -itexits nonzero with an unboundpkgsarray error after the cancelled picker; orpaccy -sreachesyay -Seven though no package was selected.Expected result:
Root cause
pkgswas created implicitly only when thewhile readloop appended at least one package. If the picker returned no rows,pkgsremained unset andset -umade${#pkgs[@]}fatal. The search branch also lacked the same empty-selection guard used by the remove/install-tracked branches.Fix
Declare
pkgsas an empty local array at the start ofmain, making all${#pkgs[@]}checks safe when no rows are selected. Add the missing empty-selection guard in the-sbranch before callingyay -Sor logging installed packages.Validation
Commands run / checks performed:
compare master...audit-20260709via GitHub compare API — result: branch is 2 commits ahead, 0 behind; changed files are only.local/bin/paccyandtests/paccy-empty-selection.test.sh.repo:ALX99/dotfiles is:pr paccy fzf selection pkgs— no existing PR found for this issue.Regression test added:
tests/paccy-empty-selection.test.shcovers cancelled selections for-rt,-ru,-it, and-susing stubbedfzf,yay,pacman, andsudocommands.I could not execute the shell test locally in this connector runtime because direct container execution and repository cloning were unavailable. The test is self-contained and can be run by a reviewer with the command above.
Scope
This PR intentionally does not change package-list parsing, package-manager command behavior for real selections, the
-Sdirect install path, unrelatedpaccyoptions, or any other dotfile scripts.