refactor: hoist shared install helpers into prelude (DOT-46)#73
Merged
Conversation
The install libraries each carried a private copy of the same two
helpers: a check for whether a target array holds any non-empty entry,
and a warn-plus-list summary of failed items. install-prelude.sh already
states its job is to hold the shared work so each library keeps only its
unique logic, so both helpers move there.
have_any takes the array by value (${arr[@]-}) rather than a nameref so
it stays valid under the bash 3.2 that ships with macOS. report_failures
takes a tag and a noun phrase so each library keeps its own wording.
ai-mcp, ai-plugins, and ai-skills gain the prelude in their self-exec
guard, and their bats harnesses source it alongside log.sh, matching the
pattern graphify and vscode already used. Behavior is unchanged; the
warning text and exit codes are identical.
Fixes DOT-46
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
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
1 participant
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
Shell cleanup for the install libraries. The QA/DevOps audit's shell findings were mostly checked and rejected (see below); the one real item was duplication. Each install lib carried a private
_X_have_anyand_X_report_failures. Both are non-unique work, so they move intoinstall-prelude.sh, whose docstring already declares that as its purpose. Behavior is unchanged; net -55 lines.Changes
install-prelude.shgainshave_any(array holds a non-empty entry) andreport_failures(warn + list, with caller-supplied tag and noun phrase to preserve per-lib wording).ai-mcp,ai-plugins,ai-skills,graphify-skillsdrop their private copies and call the shared helpers.uv-tools,vscodedrop their inline failure-report blocks forreport_failures.ai-mcp,ai-plugins,ai-skillsself-exec guards now source the prelude; their bats harnesses source it too, matching the patterngraphify/vscodealready used.Design notes
have_anytakes the array by value (${arr[@]-}), not alocal -nnameref, so it stays valid under the bash 3.2 that ships with macOS before Homebrew installs a newer bash.Audit items deliberately NOT done (verified false or low-value)
install-prelude.shmissingset -euo pipefail" — it is a sourced prelude, not an entry script; each lib sets its own options. Adding it would change sourcing-shell state for no benefit.command -v claudetorequire_command" — would rewrite tested, user-facing error messages for a cosmetic gain.cdin statusline (L163, L177)" — both are already guarded (cd && gitinside$(...), andcd ... || exit 0in a subshell). Not bugs.failedarray in ai-plugins" — it is populated and reported. Not unused.Verification
mise test— 122/122 pass.mise lint— clean (treefmt + shellcheck).Fixes DOT-46