feat: catalog-driven bash-completion installation#121
Merged
Conversation
Tools may declare an optional `bash_completion` object in their catalog entry with exactly one of `command` (generate to stdout) or `source_path` (copy from clone_path). Completions install to the XDG user completions dir, named after binary_name so the bash-completion framework lazy-loads them. - lib/completion.sh: install/remove/post-install hooks, framework bootstrap, and validation that rejects non-completion output (a bare "complete" in help text is not enough — some tools treat an unknown `completion` argument as a scan target and their output would otherwise pass). - lib/bashrc.sh: reusable idempotent managed-block insert/remove, hardened against unbalanced markers. - install_tool.sh: installs completion after a successful install/update/ reconcile, removes it on uninstall. Best-effort — never fails the tool op. - install_completion.sh: CLI wrapper, plus `--all` backfill. - make completions / make completion-<tool>. - catalog/gh.json: first declaration, verified end-to-end. - tests: both shapes, validation rejection, removal, --all, and catalog invariants (one shape per entry; no two entries share a binary_name). Claude-Session: https://claude.ai/code/session_01E3fMneH9KcaT7UYaHpfsHs Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
The completion lib is POSIX shell; invoking it via `bash` fails on windows-latest. Use the repo's existing skip_on_windows idiom for the shell-invoking classes; the catalog-schema invariants stay platform-independent. Claude-Session: https://claude.ai/code/session_01E3fMneH9KcaT7UYaHpfsHs Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
Audited all 112 catalog entries for a bash-completion generator. Each candidate generator was executed and its output validated (complete -…/compgen/COMPREPLY), then checked to confirm it registers the entry's own binary_name. 40 entries now declare bash_completion (39 command, 1 source_path for rbenv). Six tools have a generator that must NOT be used, because the completion file is named after binary_name and would shadow another tool: - gh-aw emits gh's completion (registers gh, never gh-aw) - rga forwards to ripgrep and returns ripgrep's script verbatim (registers rg) - compose has binary_name docker; docker already covers it - fzf --bash is shell integration (key bindings, global `complete -D`), not a completion script — belongs in .bashrc via eval - rustup completions registers rustup/cargo, never rustc - gcloud has no clone_path to resolve source_path against, and its file registers three commands in one lazily-loaded file codex, pip and pipx are not installed on the audit machine; their generators come from official docs. Runtime validation makes that safe — an invalid generator writes nothing. catalog/COVERAGE.md records the full classification. Claude-Session: https://claude.ai/code/session_01E3fMneH9KcaT7UYaHpfsHs Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
Contributor
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
1 similar comment
Contributor
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Found by adversarial review of #121; each was reproduced before fixing and verified after. - _completion_catalog_file returned 1 for an unknown tool. Callers capture it in an assignment, so a `set -e` caller aborted before its own no-op guard: `install_completion.sh <unknown>` exited 1 with no output, and `make completion-<typo>` failed with a bare "Error 1". Always return 0. - mkdir/mv were unchecked and the trailing echo forced a 0 return, so an unwritable target reported "installed" while writing nothing and leaking the temp file. Check both, clean up, return 1. In --all this had inflated the installed count. - The generator ran with stdin inherited from make/the terminal and no timeout, so a generator that reads stdin would hang the install forever. No shipped entry triggers it today (all 38 local generators were re-run to confirm), but the blast radius warrants `timeout 30 … </dev/null`. - The ~/.bashrc block lacked an interactive guard. The distro bash_completion has none of its own and enables extglob/progcomp at top level, so a non-interactive shell sourcing ~/.bashrc (`ssh host somescript`) silently got extended globbing, changing how !(…)/@(…) parse. Guard on `$- == *i*` and skip POSIX mode, as distro rc files do. Also make the post_install_completion call in install_tool.sh `|| true`, so a successful install can never exit non-zero, matching the documented contract and the sibling remove_completion call. Adds regressions for all four, plus direct unit tests for lib/bashrc.sh. Claude-Session: https://claude.ai/code/session_01E3fMneH9KcaT7UYaHpfsHs Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
The previous commit hard-coded `timeout 30` around the completion generator. macOS ships no GNU `timeout`, so on macos-latest the command failed, the generator produced no output, and every completion was rejected as invalid — three tests failed in CI. Route the generator through _completion_run, which uses `timeout` (GNU) or `gtimeout` (Homebrew coreutils) when present and otherwise runs the command directly. Verified both branches: with timeout absent (PATH built from the real bin dirs minus timeout/gtimeout) the completion installs correctly, and a generator that reads stdin still returns immediately — `</dev/null`, not the timeout, is what prevents the hang. The timeout remains as insurance for a generator that blocks on something else. Claude-Session: https://claude.ai/code/session_01E3fMneH9KcaT7UYaHpfsHs Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
|
CybotTM
added a commit
that referenced
this pull request
Jul 22, 2026
…ts (#122) CI runs the suite on `ubuntu-latest`, `macos-latest` **and `windows-latest`**, while everything under `scripts/` is POSIX Bash. Any test that invokes `bash`, sources a `scripts/lib/*.sh` helper, or executes an installer fails on the Windows leg. Thirteen test files already use the `skip_on_windows` marker, but the convention was undocumented — so it gets re-derived the hard way. It is invisible locally: the suite is green on Linux and only the Windows CI leg turns red (this happened on #120 and again on #121 during this work). Documents the marker, and that it applies to the shell-invoking class only — pure Python assertions (catalog JSON structure, schema invariants) must keep running on every platform. Docs only; no code or test changes. Came from /retro: yes
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.
Summary
Adds catalog-driven bash-completion installation. A tool declares how to get its
completion; the toolset installs it as part of the tool's normal lifecycle and can
backfill everything already installed.
Independent of #120.
Schema
A catalog entry may declare an optional
bash_completionobject with exactly one of:Files land in
${XDG_DATA_HOME:-~/.local/share}/bash-completion/completions/, named afterbinary_nameso the framework lazy-loads them for that command.Changes
lib/completion.sh— install/remove/post-install, framework bootstrap, output validationlib/bashrc.sh— reusable idempotent managed-block insert/removeinstall_tool.sh— installs completion after a successful install/update/reconcile,removes on uninstall; best-effort, never fails the surrounding tool operation
install_completion.sh— CLI wrapper +--allbackfillmake completions,make completion-<tool>bash_completion(39command, 1source_path)catalog/COVERAGE.mdrecords the full classificationVerification
All 112 catalog entries were audited: each candidate generator was executed, its output
validated, and checked to confirm it registers the entry's own
binary_name.End-to-end:
install_completion.sh --allinstalls 36 completions; every file registersits own command.
pytest→ 728 passed, 1 skipped.Findings that shaped the implementation
accepted
bandit complete bash, which actually ran a security scan. The predicate nowrequires
complete -…/compgen/COMPREPLY.binary_name, so a script registering a different command silently shadows another tool:gh-awemits gh's completion,rgareturns ripgrep's verbatim,composeisbinary_name: docker,fzf --bashis shell integration (key bindings + globalcomplete -D),rustupnever registersrustc,gcloudhas noclone_path.A test enforces that no two declaring entries share a
binary_name.Robustness fixes (post-review, each reproduced then verified)
_completion_catalog_filereturned 1 for an unknown tool, abortingset -ecallers beforetheir own no-op guard (
make completion-<typo>failed with a bareError 1).mkdir/mvwere unchecked and the trailingechoforced a 0 return — an unwritable targetreported "installed", wrote nothing, and leaked a temp file.
forever. Now
</dev/null(+ a timeout where the platform has one).~/.bashrcblock lacked an interactive guard; the distrobash_completionhas none ofits own and enables
extglob/progcompat top level, so a non-interactive shell sourcing~/.bashrcsilently got extended globbing.codex,pip,pipxare not installed on the audit machine, so their generators come fromofficial docs — runtime validation makes that safe (an invalid generator writes nothing).
Note on size
Exceeds the ~300 net-LOC guideline. The framework is small; the bulk is 40 mechanical
one-line catalog declarations plus the coverage table, done in one sweep.