chore: initial release setup#1
Conversation
Summary of ChangesHello @jbdevprimary, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request lays the groundwork for a portable and auditable Bash setup, 'get-bashed'. It introduces a robust installer that supports interactive and non-interactive configurations, profiles, and feature-based tool installations. The changes also include comprehensive CI/CD automation, pre-commit hooks for maintaining code quality and security, and an automated documentation system to keep the project well-documented and easy to understand for contributors and users alike. Highlights
Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Summary
This PR introduces the initial get-bashed OSS setup with critical security vulnerabilities and logic errors that block merge.
Critical Issues Found (Must Fix Before Merge)
Security Vulnerabilities:
- Shell injection in bin/ram_usage: Using
shell=Truewith subprocess enables command injection attacks - Unverified code execution in installers/brew.sh: Downloads and executes Homebrew installer without integrity verification
- Unverified code execution in README.md: Quick Start recommends piping curl to bash without verification
Logic Errors:
- Missing argument validation in install.sh: Arguments like
--prefixcrash when missing values instead of showing usage - Unquoted variable expansion in install.sh: Multiple instances where
$INSTALLERSis unquoted, causing word splitting failures with special characters
Required Actions
All identified issues must be fixed before this PR can be merged. The security vulnerabilities expose users to arbitrary code execution, and the logic errors cause crashes with common usage patterns.
Please address the specific code suggestions provided in the inline comments.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughAdds repository infrastructure and a full Bash installer/runtime: Dependabot and CI workflows, pre-commit and release automation, a shdoc docs pipeline, modular shell runtime and modules, installer helpers and tool registry, numerous scripts and CLI tools, profiles, tests, and documentation. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Installer as install.bash
participant Registry as installers/tools.sh
participant Helpers as installers/_helpers.sh
participant PackageMgr as System/PackageManagers
rect rgba(200,220,255,0.5)
User->>Installer: invoke with profile/features/options
Installer->>Registry: query tool metadata & dependencies
Registry-->>Installer: return tool list and install methods
Installer->>Helpers: resolve platform & select install method
Helpers->>PackageMgr: call apt/dnf/pacman/brew/asdf/git/curl/pipx
PackageMgr-->>Helpers: installation result/status
Helpers-->>Installer: report success/failure
Installer->>User: write get-bashedrc.sh, create symlinks/backups, summary
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive setup for the get-bashed project, including the installer, modular configuration files, documentation, and CI/CD pipelines. A critical security audit revealed several high-severity vulnerabilities, including command injection in install.sh and installers/shdoc.sh due to eval and bash -c with unsanitized input, and a path traversal vulnerability in install.sh via the --profiles argument, which could lead to arbitrary code execution. Additionally, a critical issue in the pre-commit configuration would break the build, and there are medium-severity concerns regarding script efficiency, portability, and robustness. Immediate remediation of the security vulnerabilities is required by implementing strict input validation and avoiding dangerous shell constructs.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🤖 Fix all issues with AI agents
In @.github/workflows/pr-title.yml:
- Around line 12-19: The PR title is being interpolated directly into the shell
which allows command injection; instead pass the title via an environment
variable and use safe, quoted expansion when testing the regex. Update the
workflow to set an env (e.g., PR_TITLE: ${{ github.event.pull_request.title }})
and then in the script use a safe assignment like title="$PR_TITLE" (or read -r
title <<< "$PR_TITLE") and test that quoted variable against the pattern
variable (pattern='...') so no untrusted characters are executed.
In `@install.sh`:
- Around line 243-257: The installer checklist is empty because INSTALLERS is
referenced before load_installers() populates it; call load_installers() before
building dialog_opts and invoking dialog so INSTALLERS is set, then remove or
guard the later duplicate load_installers() call (or make it idempotent) to
avoid reloading; update the code around the dialog generation that uses
dialog_opts, INSTALLS_DIALOG, and INSTALLERS to rely on the pre-populated
INSTALLERS variable.
In `@README.md`:
- Around line 3-5: Update all occurrences of the old repo owner string
"jbdevprimary" to "jbcom" in the README: change the three badge URLs that
contain "jbdevprimary/get-bashed" and the install command string referenced in
the install script snippet so they point to "jbcom/get-bashed"; search for
"jbdevprimary/get-bashed" and replace with "jbcom/get-bashed" (verify badges and
the install command now resolve correctly).
🟠 Major comments (19)
bashrc.d/60-asdf.sh-6-12 (1)
6-12:⚠️ Potential issue | 🟠 MajorSource asdf.sh even when
asdfisn’t already on PATH.
The current guard prevents activation for standard~/.asdfinstalls, leaving shims/completions inactive.🛠️ Proposed fix
-# asdf: full activation for interactive shells -if command -v asdf >/dev/null 2>&1; then - if [[ -r "$HOME/.asdf/asdf.sh" ]]; then - . "$HOME/.asdf/asdf.sh" - elif [[ -r "/opt/homebrew/opt/asdf/libexec/asdf.sh" ]]; then - . "/opt/homebrew/opt/asdf/libexec/asdf.sh" - fi -fi +# asdf: full activation for interactive shells +if [[ -r "$HOME/.asdf/asdf.sh" ]]; then + . "$HOME/.asdf/asdf.sh" +elif [[ -r "/opt/homebrew/opt/asdf/libexec/asdf.sh" ]]; then + . "/opt/homebrew/opt/asdf/libexec/asdf.sh" +fi.github/workflows/release-please.yml-1-16 (1)
1-16:⚠️ Potential issue | 🟠 MajorAdd explicit workflow permissions for release-please, including all required scopes.
Release Please requires write access to create PRs, tags, and manage release metadata. The proposed permissions are incomplete—
issues: writeis also required.🔐 Proposed fix
name: Release Please on: push: branches: [ main ] + +permissions: + contents: write + pull-requests: write + issues: writeAlso ensure the repository setting is enabled: Settings → Actions → General → Workflow permissions → "Allow GitHub Actions to create and approve pull requests" (otherwise PR creation can fail despite the YAML permissions).
installers/rg.sh-18-18 (1)
18-18:⚠️ Potential issue | 🟠 MajorUse the correct package name for apt-based systems.
apt-get install rgwill fail; the correct APT package name isripgrepon Debian/Ubuntu.💡 Suggested fix
- pkg_install ripgrep rg + pkg_install ripgrep ripgrep.github/workflows/release.yml-1-21 (1)
1-21:⚠️ Potential issue | 🟠 MajorSet explicit token permissions for release asset upload.
actions/upload-release-assetrequirescontents: writepermission. Repositories created after February 2, 2023 default to read-only token permissions, and this workflow will fail without an explicit permissions block.Suggested fix
name: Release Artifacts on: release: types: [ published ] +permissions: + contents: write + jobs: build: runs-on: ubuntu-latestscripts/package.sh-9-12 (1)
9-12:⚠️ Potential issue | 🟠 MajorAdd a fallback when
git describeisn't available.With
set -euo pipefailon line 7, a missing git binary or non-git checkout will abort the packaging script. Additionally, VERSION is used directly in the filename on line 14 without sanitizing special characters that may appear in version strings (e.g., "/" in tag names).♻️ Proposed fix
-VERSION="${2:-$(git describe --tags --always --dirty)}" +VERSION="${2:-$(git describe --tags --always --dirty 2>/dev/null || echo "unknown")}" +VERSION="${VERSION//\//-}"installers/dialog.sh-19-27 (1)
19-27:⚠️ Potential issue | 🟠 MajorUse
pkg_installto cover pacman and add error handling.
INSTALL_PLATFORMSincludes linux, but this implementation omits pacman (Arch Linux) and fails silently when no package manager is found. Thepkg_installhelper ininstallers/_helpers.shalready supports pacman and provides proper error reporting—use it for consistency and better platform coverage.🐛 Proposed fix
- if _using_brew; then - brew install dialog - elif command -v apt-get >/dev/null 2>&1; then - sudo apt-get update && sudo apt-get install -y dialog - elif command -v dnf >/dev/null 2>&1; then - sudo dnf install -y dialog - elif command -v yum >/dev/null 2>&1; then - sudo yum install -y dialog - fi + pkg_install dialogbashrc.d/30-buildflags.sh-8-19 (1)
8-19:⚠️ Potential issue | 🟠 MajorAvoid emitting
-L/lib/-I/includewhen brew prefixes resolve to empty.If any brew formula is not installed,
brew --prefix <formula>fails silently (stderr redirected), leaving the variable empty. The unguarded expansion on lines 14–19 then produces flags like-L/liband-I/include, which point to system paths and can break builds by including wrong headers or linking wrong libraries.Guard each prefix with an existence check before appending to environment variables.
Proposed fix
if [[ "${GET_BASHED_BUILD_FLAGS:-0}" == "1" ]] && command -v brew >/dev/null 2>&1; then OPENSSL_PREFIX="$(brew --prefix openssl@3 2>/dev/null || brew --prefix openssl 2>/dev/null)" READLINE_PREFIX="$(brew --prefix readline 2>/dev/null)" GETTEXT_PREFIX="$(brew --prefix gettext 2>/dev/null)" ZSTD_PREFIX="$(brew --prefix zstd 2>/dev/null)" - export LDFLAGS="-L${OPENSSL_PREFIX}/lib -L${READLINE_PREFIX}/lib -L${GETTEXT_PREFIX}/lib -L${ZSTD_PREFIX}/lib ${LDFLAGS}" - export CPPFLAGS="-I${OPENSSL_PREFIX}/include -I${READLINE_PREFIX}/include -I${GETTEXT_PREFIX}/include -I${ZSTD_PREFIX}/include ${CPPFLAGS}" - export PKG_CONFIG_PATH="${OPENSSL_PREFIX}/lib/pkgconfig:${READLINE_PREFIX}/lib/pkgconfig:${GETTEXT_PREFIX}/lib/pkgconfig:${ZSTD_PREFIX}/lib/pkgconfig:${PKG_CONFIG_PATH}" - export LIBRARY_PATH="${OPENSSL_PREFIX}/lib:${READLINE_PREFIX}/lib:${GETTEXT_PREFIX}/lib:${ZSTD_PREFIX}/lib:${LIBRARY_PATH}" - export CPATH="${OPENSSL_PREFIX}/include:${READLINE_PREFIX}/include:${GETTEXT_PREFIX}/include:${ZSTD_PREFIX}/include:${CPATH}" - export PYTHON_CONFIGURE_OPTS="--with-openssl=${OPENSSL_PREFIX} --with-readline=editline" + _append_prefix() { + local prefix="$1" + [[ -n "$prefix" && -d "$prefix" ]] || return 0 + export LDFLAGS="-L${prefix}/lib ${LDFLAGS:-}" + export CPPFLAGS="-I${prefix}/include ${CPPFLAGS:-}" + export PKG_CONFIG_PATH="${prefix}/lib/pkgconfig${PKG_CONFIG_PATH:+:$PKG_CONFIG_PATH}" + export LIBRARY_PATH="${prefix}/lib${LIBRARY_PATH:+:$LIBRARY_PATH}" + export CPATH="${prefix}/include${CPATH:+:$CPATH}" + } + + _append_prefix "$OPENSSL_PREFIX" + _append_prefix "$READLINE_PREFIX" + _append_prefix "$GETTEXT_PREFIX" + _append_prefix "$ZSTD_PREFIX" + [[ -n "$OPENSSL_PREFIX" ]] && export PYTHON_CONFIGURE_OPTS="--with-openssl=${OPENSSL_PREFIX} --with-readline=editline" fiinstallers/actionlint.sh-14-19 (1)
14-19:⚠️ Potential issue | 🟠 MajorAdd fallback installation methods for non-Arch systems.
The
actionlintpackage does not exist in official apt (Debian/Ubuntu) or dnf/yum (RHEL/Fedora) repositories. Installation will fail on these systems. Consider adding fallbacks like Snap (sudo snap install actionlintfor Fedora aftersnapdsetup) or a GitHub-release binary download, similar to the pattern used inshdoc.sh.installers/bat.sh-14-18 (1)
14-18:⚠️ Potential issue | 🟠 MajorIncorrect package name for apt-based systems.
On Debian/Ubuntu, the
batpackage is namedbatbut the binary is installed asbatcat(due to a naming conflict with another package). The current idempotent check won't detect an existing installation on these systems.🐛 Proposed fix for cross-platform detection
install_bat() { - if command -v bat >/dev/null 2>&1; then + if command -v bat >/dev/null 2>&1 || command -v batcat >/dev/null 2>&1; then return 0 fi pkg_install bat }Note: Users on Debian/Ubuntu may also need to alias
battobatcatfor consistent usage across platforms.installers/fd.sh-14-18 (1)
14-18:⚠️ Potential issue | 🟠 MajorIncorrect package name for apt-based systems.
On Debian/Ubuntu, the
fdpackage is namedfd-findand the binary is installed asfdfind(notfd). The current implementation will fail to install or detectfdon these systems.🐛 Proposed fix for cross-platform package names
install_fd() { - if command -v fd >/dev/null 2>&1; then + if command -v fd >/dev/null 2>&1 || command -v fdfind >/dev/null 2>&1; then return 0 fi - pkg_install fd + pkg_install fd fd-find }Note: Users on Debian/Ubuntu may also need to alias
fdtofdfindfor consistent usage.installers/asdf.sh-7-24 (1)
7-24:⚠️ Potential issue | 🟠 MajorPlatform metadata doesn't match the install path.
The installer only supports Homebrew, yet
INSTALL_PLATFORMSadvertises Linux/WSL. This will fail on Linux/WSL without Homebrew. Official asdf documentation provides Linux/WSL installation via precompiled binaries or building from source—both bypass Homebrew. Either implement these non-Homebrew paths or narrow the platforms list to macOS (note: WSL1 is not officially supported by asdf).✅ Minimal fix if Homebrew is the only supported path
-INSTALL_PLATFORMS="macos,linux,wsl" +INSTALL_PLATFORMS="macos"bin/ram_usage-282-287 (1)
282-287:⚠️ Potential issue | 🟠 MajorGuard against total RAM == 0 to avoid downstream ZeroDivisionError.
Ifsysctl hw.memsizefails,get_total_ram()returns 0, and later divisions at lines 240 and 266 cause a crash. The current check only catches empty dicts, not dicts withtotal=0.🔧 Suggested guard
- if not memory_stats: + if not memory_stats or memory_stats.get('total', 0) == 0: print("Error: Could not get memory statistics") return 1installers/java.sh-19-25 (1)
19-25:⚠️ Potential issue | 🟠 MajorFail fast if asdf install/set fails.
Lines 23-24 lack error handling. If
asdf installorasdf setfails, the function still returns 0, silently masking installation failures. The same issue exists inpython.shandnodejs.sh.🔧 Suggested error handling
if _using_asdf; then asdf_install_plugin java https://github.com/halcyon/asdf-java.git || true latest_version="$(asdf latest java 2>/dev/null || true)" if [[ -n "$latest_version" ]]; then - asdf install java "$latest_version" - asdf set --home java "$latest_version" + asdf install java "$latest_version" || { + echo "Failed to install Java $latest_version via asdf." >&2 + return 1 + } + asdf set --home java "$latest_version" || { + echo "Failed to set Java $latest_version as home via asdf." >&2 + return 1 + } return 0 fiinstallers/python.sh-19-25 (1)
19-25:⚠️ Potential issue | 🟠 MajorFail fast if asdf install/set fails.
Lines 23–24 lack error handling; ifasdf installorasdf setfails, line 25 still returns 0, masking installation errors. This same pattern exists ininstallers/java.shandinstallers/nodejs.sh.🔧 Suggested error handling
if _using_asdf; then asdf_install_plugin python || true latest_version="$(asdf latest python 2>/dev/null || true)" if [[ -n "$latest_version" ]]; then - asdf install python "$latest_version" - asdf set --home python "$latest_version" + asdf install python "$latest_version" || { + echo "Failed to install Python $latest_version via asdf." >&2 + return 1 + } + asdf set --home python "$latest_version" || { + echo "Failed to set Python $latest_version as home via asdf." >&2 + return 1 + } return 0 fibashrc.d/95-ssh-agent.sh-7-10 (1)
7-10:⚠️ Potential issue | 🟠 MajorAvoid spawning a new SSH agent when one is already running.
Starting a fresh agent on every interactive shell invocation creates orphaned processes and accumulates duplicate SSH agent sockets. Check if an agent is already reachable before launching a new one using
ssh-add -l, which returns exit code 2 when no agent is accessible.🔧 Suggested guard to reuse existing agent
-if [[ "${GET_BASHED_SSH_AGENT:-0}" == "1" ]] && [[ -t 1 ]]; then - eval "$(ssh-agent -s)" >/dev/null +if [[ "${GET_BASHED_SSH_AGENT:-0}" == "1" ]] && [[ -t 1 ]]; then + ssh-add -l >/dev/null 2>&1 + agent_status=$? + if [[ $agent_status -eq 2 ]]; then + eval "$(ssh-agent -s)" >/dev/null + fi [[ -f "$HOME/.ssh/id_rsa" ]] && ssh-add "$HOME/.ssh/id_rsa" 2>/dev/null || true [[ -f "$HOME/.ssh/id_ed25519" ]] && ssh-add "$HOME/.ssh/id_ed25519" 2>/dev/null || true fidocs/INSTALLERS.md-1-8 (1)
1-8:⚠️ Potential issue | 🟠 MajorIncorrect document title.
The document is titled "yq" and references "Installer: yq" in the header, but it actually documents all 34+ installers in the system. This appears to be a copy-paste error or incorrect title.
📝 Proposed fix
-# yq +# Installers -Installer: yq +Installers reference for get-bashed. ## Overview -Installer script for get-bashed. +Installer scripts for get-bashed tools and dependencies.bash_profile-10-17 (1)
10-17:⚠️ Potential issue | 🟠 MajorHomebrew detection requires
brewin PATH before checking filesystem, and forces/opt/homebrewon arm64 regardless of actual installation location.Line 10's
command -v brewgate skips the entire block if Homebrew isn't already on PATH, defeating the script's purpose of setting up Homebrew paths on fresh systems. Line 11–12 also assumes all arm64 systems have Homebrew at/opt/homebrew, which fails for Linuxbrew and other non-standard installations. Probe filesystem paths first (checking existence with-x), then fall back tocommand -vas a last resort.Suggested fix
-# Homebrew shellenv (optional) -if command -v brew >/dev/null 2>&1; then - if [[ "$(uname -m)" == "arm64" ]]; then - eval "$(/opt/homebrew/bin/brew shellenv)" - elif [[ -x /usr/local/bin/brew ]]; then - eval "$(/usr/local/bin/brew shellenv)" - else - eval "$(brew shellenv)" - fi -fi +# Homebrew shellenv (optional) +BREW_BIN="" +if [[ -x /opt/homebrew/bin/brew ]]; then + BREW_BIN="/opt/homebrew/bin/brew" +elif [[ -x /usr/local/bin/brew ]]; then + BREW_BIN="/usr/local/bin/brew" +elif command -v brew >/dev/null 2>&1; then + BREW_BIN="$(command -v brew)" +fi + +if [[ -n "$BREW_BIN" ]]; then + eval "$("$BREW_BIN" shellenv)" +fiscripts/ci-setup.sh-17-19 (1)
17-19:⚠️ Potential issue | 🟠 MajorRemove
actionlintfrom the default install list or add a fallback installation method.Line 17 includes
actionlintin the default installs, and line 19 runs the installer underset -e(line 8). The actionlint installer (line 18 ofinstallers/actionlint.sh) callspkg_install actionlint, which on ubuntu-latest attemptssudo apt-get install -y actionlint. However, actionlint is not available in standard Ubuntu repositories and the installer has no error handling or fallback, causing the CI job to fail.Either remove actionlint from the default install list, provide an alternative installation method (e.g., downloading the binary directly), or add error handling with a fallback.
install.sh-459-466 (1)
459-466:⚠️ Potential issue | 🟠 MajorHardcoded paths ignore custom
--prefix.The bashrc/bash_profile snippets hardcode
$HOME/.get-bashed, but if a user specifies a custom--prefix(e.g.,/opt/get-bashed), the sourcing will fail because files were installed elsewhere.🐛 Proposed fix
BASHRC_LINE="# get-bashed: source modular bashrc" -BASHRC_SNIP='if [[ -r "$HOME/.get-bashed/bashrc" ]]; then source "$HOME/.get-bashed/bashrc"; fi' +BASHRC_SNIP="if [[ -r \"$PREFIX/bashrc\" ]]; then source \"$PREFIX/bashrc\"; fi" BASH_PROFILE_LINE="# get-bashed: source login bash_profile" -BASH_PROFILE_SNIP='if [[ -r "$HOME/.get-bashed/bash_profile" ]]; then source "$HOME/.get-bashed/bash_profile"; fi' +BASH_PROFILE_SNIP="if [[ -r \"$PREFIX/bash_profile\" ]]; then source \"$PREFIX/bash_profile\"; fi"
🟡 Minor comments (16)
profiles/README.md-10-14 (1)
10-14:⚠️ Potential issue | 🟡 MinorAdd a language to the fenced example block.
Helps markdown tooling and readability.✏️ Proposed fix
-``` +```env FEATURES=gnu_over_bsd,build_flags,auto_tools INSTALLS=brew,asdf,gnu_tools,rg,fd,bat,fzf,jq,yq,treebashrc.d/80-aliases.sh-8-8 (1)
8-8:⚠️ Potential issue | 🟡 Minor
freecommand is not available on macOS.The
freecommand is Linux-specific and doesn't exist on macOS. Since the project supports macOS (per INSTALL_PLATFORMS), this alias will fail silently or cause "command not found" errors.Consider guarding this alias or providing a macOS alternative.
🛠️ Proposed fix with platform guard
-alias free='free -m' +# free is Linux-only; on macOS use vm_stat or top +[[ "$OSTYPE" == linux* ]] && alias free='free -m'bashrc.d/90-functions.sh-7-23 (1)
7-23:⚠️ Potential issue | 🟡 MinorReturn non‑zero for unsupported archives.
The default case only echoes, so the function exits 0 even when extraction is unsupported. Make it return a failure code.
💡 Suggested fix
- *) echo "'$f' cannot be extracted via ex()" ;; + *) echo "'$f' cannot be extracted via ex()"; return 1 ;;bashrc.d/65-tools.sh-9-18 (1)
9-18:⚠️ Potential issue | 🟡 MinorGuard
asdf exec npmwhen Node.js isn't configured.If no asdf Node.js version is set, the
npm installcommands will fail when this function executes. Add a quick availability check to skip gracefully.💡 Suggested fix
install_cli_tools() { command -v asdf >/dev/null 2>&1 || return 0 + asdf which npm >/dev/null 2>&1 || { echo "npm (via asdf) not available; skipping"; return 0; } if ! command -v gemini >/dev/null 2>&1; then asdf exec npm install -g `@google/gemini-cli` fibin/gen_tool_versions-7-15 (1)
7-15:⚠️ Potential issue | 🟡 MinorNormalize
asdf listoutput before version sorting to handle version markers.
asdf listoutput includes a leading*for the currently selected version and may include thesystemspecial selector. The current code strips leading whitespace but doesn't remove the asterisk marker or filter thesystemselector, which causessort -Vto fail on marked versions and could incorrectly selectsystemas the latest version.💡 Suggested fix
- versions=$(asdf list "$plugin" | sed 's/^[[:space:]]*//' | grep -v '^latest$') + versions=$(asdf list "$plugin" \ + | sed -E 's/^[[:space:]]*\*?[[:space:]]*//' \ + | grep -Ev '^(latest|system)$')installers/doppler.sh-10-10 (1)
10-10:⚠️ Potential issue | 🟡 MinorPlatform support inconsistency.
INSTALL_PLATFORMSdeclares support forlinuxandwsl, but the implementation only supports Homebrew. While Homebrew works on Linux, the error message at Line 22 suggests otherwise ("requires Homebrew on macOS or manual install").installers/git-lfs.sh-14-18 (1)
14-18:⚠️ Potential issue | 🟡 MinorMissing post-install setup for Git LFS.
After installing the
git-lfspackage, users typically need to rungit lfs installto set up the Git hooks. Without this step, Git LFS won't automatically handle LFS-tracked files.🔧 Proposed fix to complete setup
install_git_lfs() { if command -v git-lfs >/dev/null 2>&1; then return 0 fi pkg_install git-lfs git-lfs + git lfs install }installers/awscli.sh-14-18 (1)
14-18:⚠️ Potential issue | 🟡 MinorAWS recommends against using distro package managers for AWS CLI v2 on Linux/WSL.
AWS's official documentation explicitly advises against relying on third-party distro repositories (
apt,yum,dnf) for AWS CLI v2 since they don't maintain them and versions are inconsistent. For Linux and WSL, AWS recommends using either the official bundled installer or the snap package (which auto-updates).This script uses
pkg_install awscli awscli, which relies on the OS package manager—a path AWS advises against for v2 deployments across Linux/WSL platforms.docs/CONFIG.md-42-45 (1)
42-45:⚠️ Potential issue | 🟡 MinorFix escaped quotes in the shell example.
Inside fenced code blocks, the backslashes are unnecessary and will be copied literally by users.
📝 Suggested fix
-export GET_BASHED_HOME=\"$RUNNER_TEMP/get-bashed\" +export GET_BASHED_HOME="$RUNNER_TEMP/get-bashed"installers/shdoc.sh-39-51 (1)
39-51:⚠️ Potential issue | 🟡 MinorAdd early error check when Bash < 4 and Homebrew unavailable.
The code only attempts to upgrade Bash if both
bash_major < 4and_using_breware true. If Bash is too old but Homebrew isn't available, the install continues and fails later at themake installstep (which requires Bash 4+ for;;&case syntax in the Makefile). Return early with a clear error message instead.🛠️ Suggested fix
- if [[ "$bash_major" -lt 4 ]] && _using_brew; then - brew install bash || true - if [[ -x "/opt/homebrew/bin/bash" ]]; then - bash_bin="/opt/homebrew/bin/bash" - elif [[ -x "/usr/local/bin/bash" ]]; then - bash_bin="/usr/local/bin/bash" - fi - fi + if [[ "$bash_major" -lt 4 ]]; then + if _using_brew; then + brew install bash || true + if [[ -x "/opt/homebrew/bin/bash" ]]; then + bash_bin="/opt/homebrew/bin/bash" + elif [[ -x "/usr/local/bin/bash" ]]; then + bash_bin="/usr/local/bin/bash" + fi + else + echo "shdoc build requires Bash 4+; install a newer Bash or enable Homebrew." >&2 + return 1 + fi + fibin/ram_usage-93-101 (1)
93-101:⚠️ Potential issue | 🟡 MinorAlign "available" calculation with the stated definition.
The comment at line 93 claims "Available = free + purgeable + inactive (potentially)", but the calculation omits
inactiveandspeculativepages, which are extracted fromvm_statand available inmemory_stats. Including them aligns with macOS semantics for "available" memory (quickly reclaimable pages).🔧 Suggested fix
- memory_stats['available'] = memory_stats.get('free', 0) + memory_stats.get('purgeable', 0) + memory_stats['available'] = ( + memory_stats.get('free', 0) + + memory_stats.get('purgeable', 0) + + memory_stats.get('inactive', 0) + + memory_stats.get('speculative', 0) + )secrets.d/README.md-7-9 (1)
7-9:⚠️ Potential issue | 🟡 MinorAdd language specifier to code block.
The code block is missing a language specifier, which triggers a linting warning. Since this is showing a file path example, use
textorbashas the language identifier.📝 Proposed fix
Example: -``` +```text ~/.get-bashed/secrets.d/00-local.sh</details> </blockquote></details> <details> <summary>docs/INSTALLERS.md-11-44 (1)</summary><blockquote> `11-44`: _⚠️ Potential issue_ | _🟡 Minor_ **Fix invalid link fragments in the index.** The anchor links in the index use an incorrect format. Markdown generates anchor IDs by converting headers to lowercase and replacing underscores/spaces with hyphens. The links should use `#install-actionlint` instead of `#installactionlint`. <details> <summary>🔗 Example fix for the first few entries</summary> ```diff -* [install_actionlint](`#installactionlint`) -* [install_asdf](`#installasdf`) -* [install_awscli](`#installawscli`) -* [install_bashate](`#installbashate`) +* [install_actionlint](`#install-actionlint`) +* [install_asdf](`#install-asdf`) +* [install_awscli](`#install-awscli`) +* [install_bashate](`#install-bashate`)Apply this pattern to all 34 index entries (lines 11-44).
memory-bank/progress.md-9-11 (1)
9-11:⚠️ Potential issue | 🟡 MinorMove completed item out of “What’s left.”
The “(done)” qualifier contradicts the section heading and can confuse status tracking.docs/INSTALLER.md-12-15 (1)
12-15:⚠️ Potential issue | 🟡 MinorFix invalid Markdown link fragments.
The anchor links don't match the generated heading IDs. Markdown typically converts headings like
### apply_profileto anchors like#apply_profileor#apply-profile, not#applyprofile.📝 Proposed fix
## Index -* [usage](`#usage`) -* [apply_profile](`#applyprofile`) -* [apply_feature](`#applyfeature`) -* [split_csv](`#splitcsv`) +* [usage](`#usage`) +* [apply_profile](`#apply_profile`) +* [apply_feature](`#apply_feature`) +* [split_csv](`#split_csv`)install.sh-347-366 (1)
347-366:⚠️ Potential issue | 🟡 MinorPotential infinite recursion with circular dependencies.
The
run_installfunction marks packages as done after installation completes (line 365), not when installation starts. If installer A depends on B and B depends on A, this will cause infinite recursion. Consider marking as "in-progress" at the start:🛡️ Proposed fix to detect cycles
+# `@internal` +mark_pending() { + local id="$1" + eval "PENDING_${id}=1" +} + +# `@internal` +is_pending() { + local id="$1" + eval "[[ \"\${PENDING_${id}:-0}\" == 1 ]]" +} + # `@internal` run_install() { local id="$1" dep if is_done "$id"; then return 0 fi + if is_pending "$id"; then + echo "Circular dependency detected: $id" >&2 + return 1 + fi + mark_pending "$id" for dep in $(get_deps "$id"); do run_install "$dep" done
🧹 Nitpick comments (22)
.github/workflows/dependabot-automerge.yml (1)
9-17: Consider filtering by update type before auto-merging.The workflow auto-merges all Dependabot PRs without distinguishing between patch, minor, and major updates. Major version bumps may contain breaking changes that warrant manual review.
The
dependabot/fetch-metadataaction provides outputs likeupdate-typethat can be used to conditionally enable auto-merge only for safer updates (e.g., patch and minor).♻️ Proposed enhancement to filter by update type
jobs: auto-merge: if: github.actor == 'dependabot[bot]' runs-on: ubuntu-latest steps: - - uses: dependabot/fetch-metadata@21025c705c08248db411dc16f3619e6b5f9ea21a # v2.5.0 + - name: Fetch Dependabot metadata + id: metadata + uses: dependabot/fetch-metadata@21025c705c08248db411dc16f3619e6b5f9ea21a # v2.5.0 - uses: peter-evans/enable-pull-request-automerge@a660677d5469627102a1c1e11409dd063606628d # v3.0.0 + if: steps.metadata.outputs.update-type != 'version-update:semver-major' with: merge-method: squashREADME.md (1)
19-21: Consider adding a security note for the curl | bash pattern.While
curl | bashis a common installation method, users should be encouraged to inspect the script before execution. A brief note could improve trust and transparency.📝 Suggested addition
## Quick Start +Review the installer first, then run: + ```bash curl -fsSL https://raw.githubusercontent.com/jbdevprimary/get-bashed/main/install.sh | bash
+Or clone and inspect locally:
+bash +git clone https://github.com/jbdevprimary/get-bashed && cd get-bashed && ./install.sh +</details> </blockquote></details> <details> <summary>.github/workflows/pr-title.yml (1)</summary><blockquote> `10-10`: **Checkout step is unnecessary for title validation.** This workflow only validates the PR title from the event payload—it doesn't need repository files. Removing the checkout step will speed up the workflow. <details> <summary>♻️ Remove unnecessary checkout</summary> ```diff steps: - - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 - name: Validate PR titleinstallers/_helpers.sh (2)
64-67: Use fixed-string matching for plugin names.
grep -qxtreats plugin names as regex;-Fxqavoids edge cases if names contain regex metacharacters.♻️ Proposed fix
- asdf plugin list | awk '{print $1}' | grep -qx "$plugin" + asdf plugin list | awk '{print $1}' | grep -Fxq "$plugin"
91-100: Ensure pipx installs are idempotent.
pipx installaborts with an error if the package is already installed. Usepipx install --forceto make the installer rerunnable, as pipx explicitly recommends in the error message.♻️ Proposed fix
- if _using_pipx; then - pipx install "$pkg" + if _using_pipx; then + pipx install --force "$pkg"installers/gnupg.sh (1)
18-18: Simplifypkg_installcall for consistency.The
pkg_installhelper defaults missing arguments to the first argument, so explicitly passing four identicalgnupgvalues is redundant. Other installers (curl, tree, jq) use a single argument.♻️ Proposed simplification
- pkg_install gnupg gnupg gnupg gnupg + pkg_install gnupginstallers/starship.sh (1)
19-24: Consider adding fallback to official install script for non-Homebrew systems.The installer declares support for
linuxandwslplatforms, but will fail on systems without Homebrew. Starship provides an official install script that could serve as a fallback:curl -sS https://starship.rs/install.sh | sh -s -- -yThis would broaden compatibility while keeping Homebrew as the preferred method.
♻️ Proposed enhancement
if _using_brew; then brew install starship else - echo "Starship install requires Homebrew or manual install." >&2 - return 1 + if command -v curl >/dev/null 2>&1; then + curl -sS https://starship.rs/install.sh | sh -s -- -y + else + echo "Starship install requires Homebrew or curl." >&2 + return 1 + fi fiinstallers/kubectl.sh (1)
18-18: Remove redundant argument.The second
kubectlargument is unnecessary.pkg_installalready defaults subsequent arguments to the first parameter value (apt_pkg="${2:-$1}"). Other installers in this PR use a single argument.Suggested fix
- pkg_install kubectl kubectl + pkg_install kubectlinstallers/doppler.sh (1)
19-24: Add fallback installation method for Linux systems.Doppler's official documentation supports multiple Linux installation methods. The most universal approach is the official install script, which works across all Linux distributions:
curl -Ls --tlsv1.2 --proto "=https" --retry 3 https://cli.doppler.com/install.sh | sudo shAlternatively, for Debian/Ubuntu systems, use the APT repository with GPG verification.
Suggested enhancement
if _using_brew; then brew install doppler else - echo "Doppler install requires Homebrew on macOS or manual install." >&2 - return 1 + # Official Doppler install script for Linux/WSL + curl -Ls --tlsv1.2 --proto "=https" --retry 3 https://cli.doppler.com/install.sh | sudo sh fiinstallers/brew.sh (1)
19-20: Consider non-interactive mode for automated installations.The official Homebrew installer prompts for user confirmation by default, which may cause hangs in CI or automated contexts. Consider setting
NONINTERACTIVE=1to skip prompts.♻️ Suggested change for non-interactive installs
if _using_curl; then - /bin/bash -c "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh)" + NONINTERACTIVE=1 /bin/bash -c "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh)" elseinstallers/shdoc.sh (1)
53-60: Ensure temp dir cleanup on interruption.Using a local variable and an EXIT trap avoids leaking temp dirs if the script is interrupted.
♻️ Suggested refactor
- tmp_dir="$(mktemp -d)" + local tmp_dir + tmp_dir="$(mktemp -d)" + trap 'rm -rf "$tmp_dir"' EXIT git clone --recursive https://github.com/reconquest/shdoc "$tmp_dir/shdoc" "$bash_bin" -lc "cd \"$tmp_dir/shdoc\" && make install PREFIX=\"$prefix\" BINDIR=\"$bindir\"" || { echo "Failed to install shdoc locally. See https://github.com/reconquest/shdoc" >&2 - rm -rf "$tmp_dir" return 1 } - rm -rf "$tmp_dir" + trap - EXIT + rm -rf "$tmp_dir"docs/INSTALLERS.md (1)
46-248: Consider enriching installer documentation.Each installer section contains only "Run installer." with no additional context about what the tool does, platform-specific notes, or any prerequisites beyond dependencies. While this may be intentional for auto-generated docs, adding brief descriptions would significantly improve usability.
docs/MODULES.md (1)
16-34: Consider adding function-specific descriptions.The documentation uses the generic phrase "Runtime module loaded by get-bashed in lexicographic order" for most functions, which doesn't explain what each function does. Adding brief descriptions of each function's purpose would improve usability (e.g., what
_path_add_frontdoes, whatinstall_cli_toolsinstalls, whatexextracts).bashrc (1)
16-18: Guard the module glob to avoid literal iteration when empty.
Optional, but reduces surprising behavior when no modules are present.Suggested tweak
-if [[ -r "$GET_BASHED_HOME/get-bashedrc.sh" ]]; then - source "$GET_BASHED_HOME/get-bashedrc.sh" -fi - -for f in "$GET_BASHED_RC_DIR"/[0-9][0-9]-*.sh; do - [[ -r "$f" ]] && source "$f" -done +if [[ -r "$GET_BASHED_HOME/get-bashedrc.sh" ]]; then + source "$GET_BASHED_HOME/get-bashedrc.sh" +fi + +if compgen -G "$GET_BASHED_RC_DIR"/[0-9][0-9]-*.sh > /dev/null; then + for f in "$GET_BASHED_RC_DIR"/[0-9][0-9]-*.sh; do + [[ -r "$f" ]] && source "$f" + done +fibashrc.d/99-secrets.sh (1)
10-13: Harden secrets sourcing with an ownership/permission check.
Avoid executing untrusted code if the secrets dir is accidentally group/world-writable.Suggested hardening
-if [[ -d "$GET_BASHED_SECRETS_DIR" ]]; then - for f in "$GET_BASHED_SECRETS_DIR"/*.sh; do - [[ -r "$f" ]] && source "$f" - done -fi +if [[ -d "$GET_BASHED_SECRETS_DIR" ]]; then + if find "$GET_BASHED_SECRETS_DIR" -maxdepth 0 -type d \( -perm -002 -o -perm -020 \) | grep -q .; then + echo "get-bashed: secrets dir is group/world-writable; refusing to source." >&2 + else + for f in "$GET_BASHED_SECRETS_DIR"/*.sh; do + [[ -r "$f" ]] && source "$f" + done + fi +fitests/install.bats (1)
3-15: Capture installer status and clean temp dir.
Makes failures explicit and avoids temp leakage on test errors.Suggested refactor
+setup() { + TMPDIR="$(mktemp -d)" +} + +teardown() { + rm -rf "$TMPDIR" +} + `@test` "installer writes to prefix and wires bashrc" { - TMPDIR="$(mktemp -d)" - HOME="$TMPDIR" bash ./install.sh --prefix "$TMPDIR/.get-bashed" --force + run env HOME="$TMPDIR" bash ./install.sh --prefix "$TMPDIR/.get-bashed" --force + [ "$status" -eq 0 ] [ -f "$TMPDIR/.get-bashed/bashrc" ] [ -d "$TMPDIR/.get-bashed/bashrc.d" ]TOOLS.md (1)
77-81: Clarify the difference between--listand--list-installers.Lines 77 and 80 both describe showing the installer catalog. Looking at install.sh (line 391), both flags produce the same output. Consider either:
- Removing the redundant flag from documentation
- Clarifying if there's an intended difference between these options
## Listing and Dry Run -- `./install.sh --list` shows the catalog of installers. +- `./install.sh --list` shows the catalog of installers (alias for `--list-installers`). - `./install.sh --list-profiles` shows available profiles. - `./install.sh --list-features` shows available features. -- `./install.sh --list-installers` shows the installer catalog. - `./install.sh --dry-run --install <list>` shows what would be installed.scripts/gen-docs.sh (3)
19-31: Add trap for temp file cleanup on failure.If
shdocfails, the temp file won't be cleaned up due toset -eexiting early. Use a trap to ensure cleanup regardless of exit status.🛡️ Proposed fix
+cleanup() { + rm -f "${TMP_FILE:-}" "${TMP_MODULES:-}" +} +trap cleanup EXIT + # Combine all installers into one doc TMP_FILE="$(mktemp)" for f in "$ROOT_DIR/installers"/*.sh; do [[ "$f" == "$ROOT_DIR/installers/_helpers.sh" ]] && continue echo "" >> "$TMP_FILE" cat "$f" >> "$TMP_FILE" echo "" >> "$TMP_FILE" echo "# ----" >> "$TMP_FILE" echo "" >> "$TMP_FILE" - done +done shdoc < "$TMP_FILE" > "$ROOT_DIR/docs/INSTALLERS.md" -rm -f "$TMP_FILE"Then remove the manual
rm -fcalls at lines 30 and 42 since the trap handles cleanup.
28-28: Fix inconsistent indentation.The
donekeyword has a leading space that doesn't match the rest of the script's indentation style.echo "" >> "$TMP_FILE" - done +done
40-40: Fix inconsistent indentation.Same indentation issue as line 28.
echo "" >> "$TMP_MODULES" - done +doneinstall.sh (2)
152-165: Code duplication withinstallers/dialog.sh.This
install_dialog()function duplicates the logic ininstallers/dialog.sh(lines 13-27). The only difference is checkingcommand -v brewhere vs_using_brewhelper there. Consider sourcing the helpers and reusing the installer function, or extracting to a shared location.
412-417: Backup may move entire PREFIX when user only wants to update config.If
$PREFIXexists and--forceisn't used, the entire directory is moved to a backup. This could be surprising if a user just wants to re-run the installer to update their config. Consider adding a--no-backupflag or prompting before backup in interactive mode.
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Fix all issues with AI agents
In `@bashrc.d/70-bash-it.sh`:
- Around line 24-38: The script uses `local` at top-level in the conditional
block that runs when GET_BASHED_BASH_IT_SEARCH is set (variables `action` and
`refresh`), which causes "local: can only be used in a function" errors; replace
those `local action=...` and `local refresh=""` assignments with regular
top-level assignments (e.g., `action=...` and `refresh=...`) in the same
conditional so they are valid outside any function (see the surrounding `if [[
-n "${GET_BASHED_BASH_IT_SEARCH:-}" ]]` block and the earlier
`get_bashed_component()` function for context), leaving the rest of the logic
(case for action, GET_BASHED_BASH_IT_REFRESH check, IFS read, and bash-it search
call) unchanged.
In `@bashrc.d/90-functions.sh`:
- Around line 8-23: The ex() extractor should guard against filenames beginning
with '-' by normalizing the filename before invoking extraction tools: after
validating local f, compute a safe variable (e.g., safe_f) that prefixes "./"
when f starts with '-' (test with [[ "$f" == -* ]]) and then use safe_f in all
command invocations; update each branch that calls tar, bunzip2, unrar, gunzip,
uncompress, 7z and unzip to use safe_f (instead of "$f") so tools treat the
value as a path rather than an option.
In `@bashrc.d/95-ssh-agent.sh`:
- Around line 7-11: Replace the unconditional spawn in the GET_BASHED_SSH_AGENT
block with a check for an existing agent by testing SSH_AUTH_SOCK and running
ssh-add -l (to verify the socket is usable); if that fails, start ssh-agent
using a fixed socket (ssh-agent -a "$HOME/.ssh/agent.sock") and export its
environment so child shells inherit it, then run ssh-add for id_rsa/id_ed25519
as before; additionally consider moving this logic into a login-only path
(.bash_profile) or recommend using keychain for cross-terminal persistence.
In `@docs/INSTALLERS_HELPERS.md`:
- Around line 12-24: The TOC links use incorrect anchor fragments (they omit
underscores) so update each link target to match the actual heading IDs: change
`#installtool` → `#install_tool`, `#installasdf` → `#install_asdf`, `#installgnutools` →
`#install_gnu_tools`, `#installjava` → `#install_java`, `#installnodejs` →
`#install_nodejs`, `#installpython` → `#install_python`, `#installshdoc` →
`#install_shdoc`, `#installvimrc` → `#install_vimrc`, `#installactionlint` →
`#install_actionlint`, `#pkginstall` → `#pkg_install`, `#asdfhasplugin` →
`#asdf_has_plugin`, `#asdfinstallplugin` → `#asdf_install_plugin`, and `#pipxinstall` →
`#pipx_install` so each TOC link exactly matches the corresponding heading names
(e.g., the headings install_tool, install_asdf, install_gnu_tools, install_java,
install_nodejs, install_python, install_shdoc, install_vimrc,
install_actionlint, pkg_install, asdf_has_plugin, asdf_install_plugin,
pipx_install).
In `@gitconfig`:
- Around line 1-3: When linking dotfiles with --link-dotfiles ensure the
installer checks for real git identity values before touching the gitconfig
template used by apply_gitconfig(): require --name/--email or re-prompt
interactively until non-placeholder values are provided, or skip the gitconfig
link and log a clear warning if the user declines; do not change the template
format (keep the uncommented "name = ..." and "email = ..." lines so
apply_gitconfig()'s sed substitutions still work).
In `@install.sh`:
- Around line 278-300: The run_install function currently recurses over get_deps
without detecting cycles; add cycle detection by marking an id as "in-progress"
before recursing and unmarking it after (or by passing a call-stack/visited list
into run_install) and check that mark before recurring: if the id is already
in-progress (i.e. install started but not completed) return a non-zero error and
log the circular dependency (use the same echo >&2 style). Concretely, update
run_install to set an in-progress flag (e.g., using a new
is_inprogress/mark_inprogress helper or a temporary associative array) for the
current id, check that before calling run_install on each dep, abort on
detection, and ensure mark_done/$mark_inprogress are cleared appropriately after
install_${id} / install_tool and on errors; keep existing is_valid_id, is_done,
mark_done, install_tool and install_${id} usages.
- Around line 347-364: The profile sourcing currently overwrites any
CLI-provided FEATURES; before sourcing PROFILE_FILE (the block using
PROFILE_FILE, source, FEATURES and split_csv), save the CLI-provided FEATURES
into a temporary variable (e.g., CLI_PROVIDED_FEATURES="${FEATURES:-}" or use
PROFILE_FEATURES to hold profile values), then source the profile and merge so
CLI wins: after source, if CLI_PROVIDED_FEATURES is non-empty set FEATURES to a
comma-joined merge where profile defaults come first and CLI_PROVIDED_FEATURES
are appended (e.g., if profile set FEATURES then
FEATURES="${PROFILE_FEATURES:-}${PROFILE_FEATURES:+,}${CLI_PROVIDED_FEATURES}"
else FEATURES="${CLI_PROVIDED_FEATURES}"), and then continue calling
split_csv/apply_feature; this preserves CLI --features while still applying
profile defaults and avoids shadowing by using distinct variables
(PROFILE_FEATURES / CLI_PROVIDED_FEATURES) around the apply_feature and
apply_profile logic.
- Around line 638-657: The apply_gitconfig function is vulnerable because
USER_NAME/USER_EMAIL are only escaping backslashes and pipes before being
inlined into sed regex replacements; update apply_gitconfig to avoid raw sed
regex injection by either calling git config -f "$cfg" user.name/user.email with
the variables (use git config to safely write values) or fully escape all
regex/meta characters in name_esc/email_esc before using sed (handle . * [ ] ^ $
? + ( ) { } | \ and whitespace appropriately); ensure you update both the name
and email branches (references: apply_gitconfig, USER_NAME, USER_EMAIL, cfg) and
remove reliance on manual sed escaping if switching to git config.
In `@scripts/gen-docs.sh`:
- Around line 39-44: The script currently excludes files named "index.md" (check
[[ "$base" == "index.md" ]]) but writes the index file as "INDEX.md" (redirect
to "$ROOT_DIR/docs/INDEX.md"), causing INDEX.md to be included in the generated
list; update the exclusion to match the generated filename (e.g., compare
"$base" to "INDEX.md") or change the output filename to "index.md" so the
condition and output are consistent — look for the for-loop variables (for f in
"$ROOT_DIR/docs"/*.md; do, base="$(basename "$f")") and the redirection (} >
"$ROOT_DIR/docs/INDEX.md") and make the case change accordingly.
- Around line 22-30: The for-loop over "$ROOT_DIR/bashrc.d"/*.sh can iterate the
literal pattern when no matches exist (especially with set -u); modify the loop
in scripts/gen-docs.sh to guard against unmatched globs — either enable nullglob
before the loop and restore the previous shell option or check for matching
files with a test (e.g., if compgen -G "$ROOT_DIR/bashrc.d/*.sh" >/dev/null)
before running the cat/echo block — update the loop surrounding lines where the
pattern is used to ensure it only processes real files.
🧹 Nitpick comments (3)
bashrc.d/10-helpers.sh (1)
7-8: Guard against emptyPATHto avoid inserting the current directory implicitly.If
PATHis empty/unset,PATH="$1:${PATH}"orPATH="${PATH}:$1"introduces an empty element (current directory). Consider conditional concatenation.♻️ Suggested refactor
-_path_add_front() { [[ -d "$1" ]] && PATH="$1:${PATH}"; } -_path_add_back() { [[ -d "$1" ]] && PATH="${PATH}:$1"; } +_path_add_front() { + [[ -d "$1" ]] || return 0 + if [[ -n "${PATH:-}" ]]; then + PATH="$1:$PATH" + else + PATH="$1" + fi +} +_path_add_back() { + [[ -d "$1" ]] || return 0 + if [[ -n "${PATH:-}" ]]; then + PATH="$PATH:$1" + else + PATH="$1" + fi +}installers/tools.sh (2)
167-168:dopplerhasbrewas both dependency and installation method.This creates a hard requirement on Homebrew for Doppler installation. If Homebrew isn't available and fails to install, Doppler installation will also fail despite the tool being available via other methods (official apt/dnf repos, curl installer).
♻️ Consider adding curl-based installer
-tool_register doppler "Doppler CLI" "brew" "macos,linux,wsl" "brew" -tool_pkgs doppler "doppler" "" "" "" "" +tool_register doppler "Doppler CLI" "" "macos,linux,wsl" "brew,curl" +tool_pkgs doppler "doppler" "" "" "" "" +tool_curl doppler "https://cli.doppler.com/install.sh" "sh"
152-168: Several tools claim Linux support but only have Homebrew as an installation method.Tools like
terraform,kubectl,helm,stern, anddopplerdeclareplatforms="macos,linux,wsl"but only specifymethods="brew"with noapt,dnf,yum, orpacmanentries in theirtool_pkgscalls. This means they can only be installed via Homebrew, even on Linux systems—contradicting their platform declarations.Compare with tools like
bash,git, orghwhich properly register multiple package managers (e.g.,methods="brew,apt,dnf,yum,pacman") alongside correspondingtool_pkgsentries for each manager.Add Linux package manager support (apt/dnf/yum/pacman) and corresponding package names in
tool_pkgsfor these tools, or remove them from the Linux/WSL platform list if Homebrew is the only intended installation method.Note:
starshiphas the same issue and should also be updated.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@installers/_helpers.sh`:
- Around line 494-507: The script calls python3 to fetch the latest actionlint
release tag in the tag="$(python3 - <<'PY' ... )" block without checking python3
exists; add a pre-check for python3 (similar to the existing _using_curl
pattern) before that tag assignment, and if python3 is not available print a
clear error (e.g., "python3 is required to install actionlint") to stderr and
return 1 so the installer fails fast; alternatively, if you prefer not to
require python3, replace the python3-based fetch with a curl+grep/jq fallback,
but ensure the change is applied around the tag assignment and mirrors the error
handling used elsewhere.
- Around line 270-294: The git and curl branches in installers/_helpers.sh
currently run commands but then unconditionally return success; change them to
propagate failures by chaining the critical commands with && return 0 so
non-zero exit codes bubble up: replace the unconditional "git clone --depth=1
\"$url\" \"$target\"; return 0" in the git branch with running git clone and
only returning success if it succeeds (i.e., git clone ... && return 0), and in
the curl branch ensure the download, execution, and cleanup are sequenced so
failures stop progress (e.g., curl -fsSL ... -o "$tmp_dir/install.sh" && $cmd
"$tmp_dir/install.sh" && rm -rf "$tmp_dir" && return 0), referencing
TOOL_GIT_URL, TOOL_CURL_URL, the git clone call, the curl -fsSL download, and
the $cmd "$tmp_dir/install.sh" execution points.
- Around line 196-215: The git- and curl-based install branches in
component_install currently always return success even if commands fail; modify
the git branch (checking GET_BASHED_GIT_SOURCES and _using_git) to fail-fast by
testing the exit status of git clone and the post-install script (run the post
script and if it returns non-zero, propagate that exit code instead of
unconditionally returning 0), and modify the curl branch (checking
GET_BASHED_CURL_SOURCES and _using_curl) to check curl and the invoked install
script ($cmd "$tmp_dir/install.sh") for errors and return a non-zero status on
failure (or return the actual command exit code) before cleaning up tmp_dir;
reference the symbols GET_BASHED_GIT_SOURCES, GET_BASHED_GIT_POST, git clone,
GET_BASHED_CURL_SOURCES, curl -fsSL, GET_BASHED_CURL_CMD, and the temp dir
handling when implementing these checks.
In `@tests/link_dotfiles.bats`:
- Around line 34-37: The failing test uses run with a pipe incorrectly — wrap
the entire pipeline into a single run invocation so Bats captures the pipeline
output instead of piping an empty captured output to grep; replace the current
use of run plus a separate pipe (the line containing run ls
"$HOME/.get-bashed/backup" | grep -E '^bashrc\.[0-9]+') with a single run that
executes the full pipeline via the shell (e.g., run with "sh -c" or "bash -c"),
then keep the subsequent assertion checking [ "$status" -eq 0 ] unchanged.
🧹 Nitpick comments (3)
.pre-commit-config.yaml (1)
24-28: Document rationale for bashate ignore rules.The
--ignore=E003,E006arguments suppress indent-not-multiple-of-four (E003) and line-too-long (E006) warnings. Consider adding a comment explaining why these are ignored, which helps future maintainers understand the project's style choices.📝 Suggested improvement
- repo: https://github.com/openstack/bashate rev: 2.1.1 hooks: - id: bashate + # E003: indent not multiple of 4 (project uses 2-space indent) + # E006: line too long (relaxed for readability) args: [--ignore=E003,E006]TOOLS.md (1)
111-115: Rephrase the repeated “shows” bullets.
LanguageTool flagged repetition in this list; a small wording tweak improves flow.📝 Suggested edit
-- `./install.sh --list` shows the catalog of installers. -- `./install.sh --list-profiles` shows available profiles. -- `./install.sh --list-features` shows available features. -- `./install.sh --list-installers` shows the installer catalog. -- `./install.sh --dry-run --install <list>` shows what would be installed. +- `./install.sh --list` lists the installer catalog. +- `./install.sh --list-profiles` lists available profiles. +- `./install.sh --list-features` lists available features. +- `./install.sh --list-installers` lists the installer catalog. +- `./install.sh --dry-run --install <list>` previews what would be installed.scripts/gen-docs.sh (1)
9-35: Ensure docs dir exists and temp files are cleaned up on failure.
Withset -e, a failure before cleanup can leave temp files behind; missingdocs/will also break redirects.🧹 Proposed hardening
ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +mkdir -p "$ROOT_DIR/docs" @@ -TMP_MODULES="$(mktemp)" +TMP_MODULES="$(mktemp)" +cleanup_modules() { rm -f "$TMP_MODULES"; } +trap cleanup_modules EXIT @@ -shdoc < "$TMP_MODULES" > "$ROOT_DIR/docs/MODULES.md" -rm -f "$TMP_MODULES" +shdoc < "$TMP_MODULES" > "$ROOT_DIR/docs/MODULES.md"
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 Fix all issues with AI agents
In `@bashrc.d/90-functions.sh`:
- Around line 25-26: The fallback case in the ex() function prints an
unsupported-extension message but leaves the exit status as 0; change that
branch to emit the message to stderr and return a non-zero status (e.g., echo
"'$f' cannot be extracted via ex()" >&2; return 1) so callers can detect
failure; update the case arm in ex() accordingly.
In `@bashrc.d/95-ssh-agent.sh`:
- Line 12: The command using SSH_AUTH_SOCK and SSH_AGENT_PID in 95-ssh-agent.sh
(the line invoking ssh-add -l with SSH_AUTH_SOCK="$sock" SSH_AGENT_PID= ssh-add
-l >/dev/null 2>&1) should use an explicit empty-string assignment for
SSH_AGENT_PID to satisfy ShellCheck SC1007; update the assignment to
SSH_AGENT_PID="" while preserving SSH_AUTH_SOCK="$sock" and the ssh-add -l
invocation and redirection.
In `@bin/ram_usage`:
- Around line 233-241: Guard against divide-by-zero when computing mem_percent:
check the retrieved total_ram (from get_total_ram) before dividing in the loop
over sorted_processes and in the other location that computes mem_percent (the
second block around the other 262-267 lines). If total_ram is zero or falsy, set
mem_percent to 0 or a safe placeholder (e.g., 0.0 or "N/A") instead of
performing (proc['rss'] / total_ram) * 100; otherwise compute as before. Update
the code paths that reference total_ram (the loop over sorted_processes and the
later mem_percent computation) to use this guarded check so no ZeroDivisionError
occurs.
- Around line 93-102: The available memory calculation currently sets
memory_stats['available'] = memory_stats.get('free', 0) +
memory_stats.get('purgeable', 0) which omits inactive (and optionally
speculative) pages and thus inflates used_percent; update the calculation to
include memory_stats.get('inactive', 0) and optionally
memory_stats.get('speculative', 0) (e.g., available = free + purgeable +
inactive [+ speculative]) and ensure you guard with .get(..., 0) so missing keys
don't raise, then recompute memory_stats['used'] and
memory_stats['used_percent'] as before using the new available value.
In `@install.bash`:
- Around line 505-512: The rsync call in copy_tree is using --delete which will
remove user-added files in the destination (e.g., $PREFIX/bashrc.d); update the
copy_tree function to avoid unconditional deletion by either removing --delete
or making it conditional on a FORCE flag: change copy_tree (the function that
uses rsync -a --delete) to read a global FORCE (or accept a third param) and
only include --delete when FORCE is true/explicitly set, otherwise run rsync -a
without --delete; also ensure the invocation that copies "$REPO_DIR/bashrc.d" to
"$PREFIX/bashrc.d" documents or passes the FORCE behavior so users aren’t
surprised.
- Line 43: The FORCE variable is dead code: remove the FORCE=0 declaration and
the related --force flag handling (or alternatively implement its intended
behavior); specifically either delete the FORCE variable and any option parsing
that sets it (the "--force" case) and any usage expectations, or add logic in
the installer flow (e.g., in functions that prompt or overwrite files) to check
FORCE and bypass confirmation/skip existing-file checks when FORCE is set
(update option parsing and the functions that would respect FORCE). Ensure all
references to FORCE/--force are consistently removed or implemented so no unused
variables or unreachable branches remain.
- Line 621: The unset call uses an unquoted array-subscript expression
(INSTALL_IN_PROGRESS[$id]) which can trigger glob expansion; change the unset
invocation to pass a single quoted argument for the array element (i.e., quote
the entire INSTALL_IN_PROGRESS[$id] expression) so the shell treats it as one
literal parameter and avoids globbing—update the line that references
INSTALL_IN_PROGRESS and $id accordingly.
In `@installers/_helpers.sh`:
- Around line 376-384: The install_gnu_tools function always returns 0 even if
brew_exec install fails; change it to propagate brew_exec's exit status by
capturing/returning its result (e.g., run brew_exec install coreutils findutils
gnu-sed gnu-tar and return its exit code) so failures from brew_exec are not
hidden—follow the same pattern used in install_asdf and reference the
install_gnu_tools and brew_exec calls when making the change.
- Around line 504-521: The install_vimrc function currently runs git clone and
proceeds regardless of failure; modify install_vimrc to check the exit status of
git clone (after the git clone to "$target") and abort (return non‑zero) or log
an error if clone failed so the script does not try to run
install_awesome_vimrc.sh or install_basic_vimrc.sh on a missing repo; use the
existing variables (prefix, target) and ensure the check happens before the case
that calls install_awesome_vimrc.sh/install_basic_vimrc.sh.
- Around line 591-607: The pkg_install function fails to support distinct pacman
package names and doesn't propagate installer exit codes; add a fourth or fifth
parameter (e.g., pacman_pkg="${5:-$1}" or use a dedicated pacman_pkg variable)
and use that variable when calling pacman_install instead of $brew_pkg, and
ensure every installer invocation (brew_exec install, apt_install, dnf_install,
yum_install, pacman_install) captures its exit status and returns it (e.g.,
check the command's exit code and return it) so failures bubble up from
pkg_install to callers; update references to pkg_install, pacman_install,
brew_exec, apt_install, dnf_install, and yum_install accordingly.
- Around line 574-583: The install_actionlint block currently downloads and
extracts without validating commands; modify the sequence around the url/tmp_dir
logic to check curl and tar exit statuses and handle failures: after curl -fsSL
"$url" -o "$tmp_dir/actionlint.tgz" verify the file exists and the curl exit
code is zero (otherwise clean tmp_dir and print a clear error via >&2 and exit
non‑zero), then run tar -xzf "$tmp_dir/actionlint.tgz" -C "$tmp_dir" and check
tar's exit code (on failure clean tmp_dir and error/exit), only proceed to mv
"$tmp_dir/actionlint" "$prefix/bin/actionlint" and chmod if both steps
succeeded; reference the existing symbols url, tmp_dir,
"$tmp_dir/actionlint.tgz", tar, prefix, and "$prefix/bin/actionlint" when making
these checks and error messages.
🧹 Nitpick comments (7)
SECURITY.md (2)
12-14: Consider adding a backup security contact email.While GitHub Security Advisories is an excellent primary channel, best practice for security policies includes providing a backup contact method (such as a dedicated security email address) in case GitHub is unavailable or the reporter cannot use GitHub features.
📧 Suggested addition
- Use GitHub Security Advisories for private reporting. +- For alternative contact: security@[domain] (if GitHub is unavailable). - If you cannot use GitHub, open a private discussion with the maintainer via GitHub. - Do not open a public issue for security reports.
71-71: Minor style refinement: use "outside" instead of "outside of".The phrase "outside of" can be simplified to "outside" for better conciseness.
✨ Proposed style fix
-- User-specific misconfiguration outside of get-bashed artifacts. +- User-specific misconfiguration outside get-bashed artifacts.bashrc.d/95-ssh-agent.sh (1)
28-29: Optional: Refactor to explicit conditionals (SC2015).ShellCheck notes that
A && B || Cis not if-then-else —|| trueruns both when the file doesn't exist and whenssh-addfails. The current behavior is acceptable for "best effort" key loading, but explicit conditionals would be clearer.♻️ Optional refactor for clarity
- [[ -f "$HOME/.ssh/id_rsa" ]] && ssh-add "$HOME/.ssh/id_rsa" 2>/dev/null || true - [[ -f "$HOME/.ssh/id_ed25519" ]] && ssh-add "$HOME/.ssh/id_ed25519" 2>/dev/null || true + if [[ -f "$HOME/.ssh/id_rsa" ]]; then + ssh-add "$HOME/.ssh/id_rsa" 2>/dev/null || true + fi + if [[ -f "$HOME/.ssh/id_ed25519" ]]; then + ssh-add "$HOME/.ssh/id_ed25519" 2>/dev/null || true + fiMakefile (1)
1-12: Consider adding conventionalallandcleantargets.Line 2 triggers checkmake’s minphony warnings; adding minimal targets can align with common tooling expectations.
♻️ Proposed minimal targets
.PHONY: docs lint test +.PHONY: all clean + +all: docs lint test + +clean: + rm -rf docs/*.md tests/libtests/config_output.bats (1)
5-20: Temporary directory not cleaned up after test.The test creates a temporary directory with
mktemp -dbut never removes it. While Bats may handle some cleanup, explicitly cleaning up in a teardown or at test end is best practice to avoid accumulating temp files during test runs.♻️ Suggested improvement
Add cleanup at the end of the test or use a
teardownfunction:+teardown() { + [[ -n "${TMPDIR:-}" && -d "$TMPDIR" ]] && rm -rf "$TMPDIR" +} + `@test` "installer writes get-bashedrc with git identity" { TMPDIR="$(mktemp -d)" HOME="$TMPDIR/home" mkdir -p "$HOME" TEST_HOME="$HOME" USER_NAME="Jane Doe" USER_EMAIL="jane@example.com" HOME="$TEST_HOME" bash ./install.sh --auto --name "$USER_NAME" --email "$USER_EMAIL" --prefix "$TEST_HOME/.get-bashed" --force run grep -F "GET_BASHED_USER_NAME=\"${USER_NAME}\"" "$TEST_HOME/.get-bashed/get-bashedrc.sh" assert_success run grep -F "GET_BASHED_USER_EMAIL=\"${USER_EMAIL}\"" "$TEST_HOME/.get-bashed/get-bashedrc.sh" assert_success }install.bash (2)
213-215: Declare_partsas local insplit_csv.The
_partsarray is not declared local, which could cause unintended side effects if the variable name is used elsewhere.♻️ Proposed fix
split_csv() { - local s="$1"; IFS=',' read -r -a _parts <<<"$s"; echo "${_parts[@]}"; + local s="$1" + local -a _parts + IFS=',' read -r -a _parts <<<"$s" + echo "${_parts[@]}" }
268-282:link_dotfiledoesn't verify existing symlinks point to the correct target.If
$destis already a symlink (possibly to a different location), the function returns early without verifying it points to$src. This could leave stale symlinks in place.♻️ Proposed fix to verify symlink target
link_dotfile() { local name="$1" local src="$PREFIX/$name" local dest="$HOME/.${name}" if [[ ! -e "$src" ]]; then return 0 fi if [[ -L "$dest" ]]; then + # Verify symlink points to correct target + local current_target + current_target="$(readlink "$dest")" + if [[ "$current_target" == "$src" ]]; then + return 0 + fi + # Symlink exists but points elsewhere; backup and recreate + backup_file "$dest" - return 0 fi if [[ -e "$dest" ]]; then backup_file "$dest" fi ln -s "$src" "$dest" }
| source "$REPO_DIR/installers/_helpers.sh" | ||
| source "$REPO_DIR/installers/tools.sh" | ||
| PREFIX="${GET_BASHED_HOME:-$HOME/.get-bashed}" | ||
| FORCE=0 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify FORCE usage in the codebase
echo "=== Searching for FORCE usage in install.bash ==="
rg -n '\bFORCE\b' install.bash
echo ""
echo "=== Searching for FORCE usage across the repo (shell files) ==="
rg -n '\bFORCE\b' --type=shRepository: jbcom/get-bashed
Length of output: 276
Remove unused FORCE variable.
The FORCE variable is initialized at line 43 and can be set via the --force flag at line 80, but it is never referenced anywhere in the installer logic. Either implement the functionality this flag is intended to enable, or remove the dead code.
🤖 Prompt for AI Agents
In `@install.bash` at line 43, The FORCE variable is dead code: remove the FORCE=0
declaration and the related --force flag handling (or alternatively implement
its intended behavior); specifically either delete the FORCE variable and any
option parsing that sets it (the "--force" case) and any usage expectations, or
add logic in the installer flow (e.g., in functions that prompt or overwrite
files) to check FORCE and bypass confirmation/skip existing-file checks when
FORCE is set (update option parsing and the functions that would respect FORCE).
Ensure all references to FORCE/--force are consistently removed or implemented
so no unused variables or unreachable branches remain.
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 Fix all issues with AI agents
In `@AGENTS.md`:
- Around line 31-32: Update the compound adjective in AGENTS.md by changing the
phrase "side-effect free" to the hyphenated form "side-effect-free" where it
appears in the tests guidance sentence (the line that reads "Keep tests
deterministic and side-effect free (use temp `HOME`)") so the document uses
correct hyphenation.
In `@bashrc.d/30-buildflags.sh`:
- Line 20: The script currently overwrites PYTHON_CONFIGURE_OPTS; change it to
preserve and append to any existing value by reading the current
PYTHON_CONFIGURE_OPTS and appending " --with-openssl=${OPENSSL_PREFIX}
--with-readline=editline" so user-defined options remain intact (update the
export of PYTHON_CONFIGURE_OPTS accordingly, referencing the
PYTHON_CONFIGURE_OPTS variable and OPENSSL_PREFIX).
- Around line 10-19: The current exports build flags unconditionally embed
OPENSSL_PREFIX, READLINE_PREFIX, GETTEXT_PREFIX, and ZSTD_PREFIX which can be
empty and produce invalid entries; update the logic that sets LDFLAGS, CPPFLAGS,
PKG_CONFIG_PATH, LIBRARY_PATH, and CPATH to only append a prefix if its
corresponding variable is non-empty and the expected subpath exists (e.g., check
"${OPENSSL_PREFIX}/lib" or "${OPENSSL_PREFIX}/include" before adding
"-L${OPENSSL_PREFIX}/lib" or "-I${OPENSL_PREFIX}/include"), doing this for
OPENSSL_PREFIX, READLINE_PREFIX, GETTEXT_PREFIX, and ZSTD_PREFIX so no empty
prefixes produce malformed flags in the final exported environment variables.
In `@bashrc.d/40-completions.sh`:
- Around line 1-2: Remove the duplicate shebang by keeping only a single
"#!/usr/bin/env bash" at the top of the file; edit the file to delete the second
identical shebang line so the script starts with one valid interpreter
declaration (look for the two identical "#!/usr/bin/env bash" lines and remove
the redundant one).
In `@bashrc.d/95-ssh-agent.sh`:
- Around line 17-25: The startup logic may try to bind ssh-agent to
SSH_AGENT_SOCK when a stale socket file already exists, causing “Address already
in use”; update the block that assigns SSH_AGENT_SOCK and calls ssh-agent
(referencing SSH_AGENT_SOCK, SSH_AUTH_SOCK, and _ssh_agent_usable) to check if
the socket path exists but !_ssh_agent_usable, then remove the stale socket file
(rm -f "$SSH_AGENT_SOCK") before invoking eval "$(ssh-agent -a "$SSH_AGENT_SOCK"
-s)"; keep the existing _ssh_agent_usable checks and only unlink the file when
it is present and unusable to avoid removing a live socket.
In `@bin/ram_usage`:
- Around line 43-53: In format_bytes, add a guard for negative input so
math.log(bytes, 1024) won't raise unexpectedly: check if the bytes argument is
less than 0 at the start of the function (in function format_bytes) and raise a
clear ValueError (e.g., "bytes must be non-negative") or otherwise handle it
consistently (such as normalizing to 0) before calling math.log; ensure the
error message references the invalid bytes value for easier debugging.
In `@install.bash`:
- Line 660: The unset invocation uses a double-quoted array subscript unset
"INSTALL_IN_PROGRESS[$id]" which risks glob expansion; update the unset call to
quote the array subscript with single quotes so the subscript is treated
literally (i.e., protect INSTALL_IN_PROGRESS and the $id subscript from
expansion) and ensure the change appears where the unset of
INSTALL_IN_PROGRESS[$id] occurs.
In `@installers/_helpers.sh`:
- Around line 354-357: The install_asdf path calls brew_exec install asdf but
always returns 0, hiding failures; update the install_asdf logic so the exit
status of brew_exec is propagated (e.g., test the command result and return its
non-zero code or return the command substitution result) instead of
unconditionally returning 0—locate the branch that calls brew_exec in
install_asdf and replace the unconditional "return 0" with logic that returns
brew_exec's exit status.
- Around line 359-369: The install_asdf flow doesn't check git clone's result,
so if git clone fails subsequent git -C "$HOME/.asdf" commands run against a
missing dir; after the git clone line in the block guarded by _using_git, check
the exit status (or use a conditional) and if clone failed log or return a
non-zero status and stop further processing; reference the _using_git check, the
git clone command that targets "$HOME/.asdf", and the subsequent git -C
"$HOME/.asdf" describe/checkout calls to ensure you bail out before attempting
those when clone fails.
- Around line 516-523: The case block that calls install_basic_vimrc.sh /
install_awesome_vimrc.sh doesn't check their exit status, so failures are
masked; update the install_vimrc logic (the case using GET_BASHED_VIMRC_MODE) to
capture and propagate the script exit code (e.g., run each sh invocation and if
it exits non‑zero, return that non‑zero status or exit with a clear error),
ensuring both calls to install_basic_vimrc.sh and install_awesome_vimrc.sh fail
the parent function when they fail.
In `@scripts/pre-commit-ci.sh`:
- Around line 9-19: The script uses ROOT_DIR but runs gen-docs.sh and pre-commit
from the current working directory, which can cause CWD-dependent failures;
change into ROOT_DIR before invoking "$ROOT_DIR/scripts/gen-docs.sh" and the
pre-commit command (either wrap them in a subshell like (cd "$ROOT_DIR" && ...)
or use pushd/popd) so docs generation and pre-commit run from the repository
root; update the blocks around the calls to reference ROOT_DIR and ensure the
original CWD is restored if not using a subshell.
🧹 Nitpick comments (10)
bashrc.d/99-secrets.sh (1)
10-14: Consider warning on Doppler failure.If
doppler secrets downloadfails, the script continues silently. Since the user explicitly enabled Doppler viaGET_BASHED_USE_DOPPLER=1, a silent failure could lead to missing environment variables and confusing downstream errors.💡 Optional: Add failure warning
if [[ "${GET_BASHED_USE_DOPPLER:-0}" == "1" ]] && command -v doppler >/dev/null 2>&1; then set -a - source <(doppler secrets download --no-file --format env) + if ! source <(doppler secrets download --no-file --format env 2>&1); then + echo "[get-bashed] Warning: Doppler secrets download failed" >&2 + fi set +a fibin/ram_usage (1)
24-32: Consider printing errors to stderr instead of stdout.CLI tools typically reserve stdout for data output and use stderr for diagnostics. This ensures error messages don't pollute piped output.
♻️ Suggested change
def get_command_output(command): """Run a command and return its output as a string.""" try: result = subprocess.run(command, check=True, capture_output=True, text=True) return result.stdout except subprocess.CalledProcessError as e: - print(f"Error running command: {command}") - print(f"Error message: {e.stderr}") + print(f"Error running command: {command}", file=sys.stderr) + print(f"Error message: {e.stderr}", file=sys.stderr) return ""bashrc.d/95-ssh-agent.sh (1)
28-29: Optional: de‑duplicate key loading with a loop.♻️ Suggested refactor
- [[ -f "$HOME/.ssh/id_rsa" ]] && ssh-add "$HOME/.ssh/id_rsa" 2>/dev/null || true - [[ -f "$HOME/.ssh/id_ed25519" ]] && ssh-add "$HOME/.ssh/id_ed25519" 2>/dev/null || true + for key in id_rsa id_ed25519; do + [[ -f "$HOME/.ssh/$key" ]] && ssh-add "$HOME/.ssh/$key" 2>/dev/null || true + doneTOOLS.md (1)
109-118: Consider varying sentence structure for readability.The list items at lines 112-117 all begin with the same pattern (
./install.sh --list...shows...). While functional, this was flagged by static analysis. Consider grouping or rephrasing for variety, e.g., using a table or varying the verbs.installers/_helpers.sh (1)
81-93:auto_execmay fail with empty flags array on older Bash.When
_auto_approvedreturns false,flagsremains empty and"${flags[@]}"expansion can cause issues in some Bash versions withset -u. Consider using"${flags[@]+"${flags[@]}"}"or ensuring the array is never empty.♻️ Proposed fix
- "$cmd" "${flags[@]}" + if [[ ${`#flags`[@]} -gt 0 ]]; then + "$cmd" "${flags[@]}" + else + "$cmd" + fiinstall.bash (1)
664-668: Install loop doesn't halt on failure.If
run_install "$id"fails for one installer, the loop continues to the next. This may leave the system in a partially configured state without clear indication of what succeeded.♻️ Proposed fix to fail fast
if [[ -n "$INSTALLS" ]]; then for id in $(split_csv "$INSTALLS"); do - run_install "$id" + run_install "$id" || { echo "Installation of $id failed" >&2; exit 1; } done fiAlternatively, collect failures and report at the end if partial installs are acceptable.
scripts/verify-install.sh (2)
10-12: Add cleanup trap to remove temporary directory.The script creates a temporary directory but doesn't clean it up, potentially leaving test artifacts.
♻️ Proposed fix
TMPDIR="$(mktemp -d)" +trap 'rm -rf "$TMPDIR"' EXIT TEST_HOME="$TMPDIR/home" mkdir -p "$TEST_HOME"
16-24: Silent failures provide no diagnostic context.When symlink checks fail under
set -e, the script exits without indicating which check failed. Consider adding error messages or using a helper function.♻️ Proposed fix with diagnostics
-[[ -L "$TEST_HOME/.bashrc" ]] -[[ -L "$TEST_HOME/.bash_profile" ]] -[[ -L "$TEST_HOME/.inputrc" ]] -[[ -L "$TEST_HOME/.bash_aliases" ]] -[[ -L "$TEST_HOME/.vimrc" ]] -[[ -L "$TEST_HOME/.gitconfig" ]] +check_symlink() { + [[ -L "$1" ]] || { echo "Expected symlink not found: $1" >&2; return 1; } +} + +check_symlink "$TEST_HOME/.bashrc" +check_symlink "$TEST_HOME/.bash_profile" +check_symlink "$TEST_HOME/.inputrc" +check_symlink "$TEST_HOME/.bash_aliases" +check_symlink "$TEST_HOME/.vimrc" +check_symlink "$TEST_HOME/.gitconfig"tests/link_dotfiles.bats (2)
5-14: Consider extracting common test setup to reduce duplication.Each test repeats the same TMPDIR/HOME setup pattern. A Bats
setup()function would reduce duplication and ensure consistent initialization.♻️ Proposed refactor
setup() { TMPDIR="$(mktemp -d)" HOME="$TMPDIR/home" mkdir -p "$HOME" TEST_HOME="$HOME" export HOME TEST_HOME } teardown() { rm -rf "$TMPDIR" }Then remove the setup lines from each test.
Also applies to: 35-42, 51-60
62-65: Useassert_outputfor better failure diagnostics.The direct
[[ "$output" == "..." ]]assertion won't show the actual vs expected values on failure. Usingassert_outputfrom bats-assert provides better diagnostics.♻️ Proposed fix
run readlink "$TEST_HOME/.bashrc" assert_success - [[ "$output" == "$TEST_HOME/.get-bashed/bashrc" ]] + assert_output "$TEST_HOME/.get-bashed/bashrc"
| - Tests use BATS and should only validate install behavior and module wiring. | ||
| - Keep tests deterministic and side-effect free (use temp `HOME`). |
There was a problem hiding this comment.
Hyphenate compound adjective.
Use “side-effect-free” instead of “side-effect free.”
✏️ Proposed fix
-- Keep tests deterministic and side-effect free (use temp `HOME`).
+- Keep tests deterministic and side-effect-free (use temp `HOME`).📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - Tests use BATS and should only validate install behavior and module wiring. | |
| - Keep tests deterministic and side-effect free (use temp `HOME`). | |
| - Tests use BATS and should only validate install behavior and module wiring. | |
| - Keep tests deterministic and side-effect-free (use temp `HOME`). |
🧰 Tools
🪛 LanguageTool
[grammar] ~32-~32: Use a hyphen to join words.
Context: ...Keep tests deterministic and side-effect free (use temp HOME). ## Commit & Pul...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents
In `@AGENTS.md` around lines 31 - 32, Update the compound adjective in AGENTS.md
by changing the phrase "side-effect free" to the hyphenated form
"side-effect-free" where it appears in the tests guidance sentence (the line
that reads "Keep tests deterministic and side-effect free (use temp `HOME`)") so
the document uses correct hyphenation.
| OPENSSL_PREFIX="$(brew --prefix openssl@3 2>/dev/null || brew --prefix openssl 2>/dev/null)" | ||
| READLINE_PREFIX="$(brew --prefix readline 2>/dev/null)" | ||
| GETTEXT_PREFIX="$(brew --prefix gettext 2>/dev/null)" | ||
| ZSTD_PREFIX="$(brew --prefix zstd 2>/dev/null)" | ||
|
|
||
| export LDFLAGS="-L${OPENSSL_PREFIX}/lib -L${READLINE_PREFIX}/lib -L${GETTEXT_PREFIX}/lib -L${ZSTD_PREFIX}/lib ${LDFLAGS}" | ||
| export CPPFLAGS="-I${OPENSSL_PREFIX}/include -I${READLINE_PREFIX}/include -I${GETTEXT_PREFIX}/include -I${ZSTD_PREFIX}/include ${CPPFLAGS}" | ||
| export PKG_CONFIG_PATH="${OPENSSL_PREFIX}/lib/pkgconfig:${READLINE_PREFIX}/lib/pkgconfig:${GETTEXT_PREFIX}/lib/pkgconfig:${ZSTD_PREFIX}/lib/pkgconfig:${PKG_CONFIG_PATH}" | ||
| export LIBRARY_PATH="${OPENSSL_PREFIX}/lib:${READLINE_PREFIX}/lib:${GETTEXT_PREFIX}/lib:${ZSTD_PREFIX}/lib:${LIBRARY_PATH}" | ||
| export CPATH="${OPENSSL_PREFIX}/include:${READLINE_PREFIX}/include:${GETTEXT_PREFIX}/include:${ZSTD_PREFIX}/include:${CPATH}" |
There was a problem hiding this comment.
Guard empty prefixes to avoid invalid build flags.
If any formula isn’t installed, brew --prefix returns an empty string, which yields -L/lib, -I/include, and malformed path lists. Consider appending only non-empty, existing prefixes.
🛠️ Suggested fix (skip empty prefixes)
OPENSSL_PREFIX="$(brew --prefix openssl@3 2>/dev/null || brew --prefix openssl 2>/dev/null)"
READLINE_PREFIX="$(brew --prefix readline 2>/dev/null)"
GETTEXT_PREFIX="$(brew --prefix gettext 2>/dev/null)"
ZSTD_PREFIX="$(brew --prefix zstd 2>/dev/null)"
- export LDFLAGS="-L${OPENSSL_PREFIX}/lib -L${READLINE_PREFIX}/lib -L${GETTEXT_PREFIX}/lib -L${ZSTD_PREFIX}/lib ${LDFLAGS}"
- export CPPFLAGS="-I${OPENSSL_PREFIX}/include -I${READLINE_PREFIX}/include -I${GETTEXT_PREFIX}/include -I${ZSTD_PREFIX}/include ${CPPFLAGS}"
- export PKG_CONFIG_PATH="${OPENSSL_PREFIX}/lib/pkgconfig:${READLINE_PREFIX}/lib/pkgconfig:${GETTEXT_PREFIX}/lib/pkgconfig:${ZSTD_PREFIX}/lib/pkgconfig:${PKG_CONFIG_PATH}"
- export LIBRARY_PATH="${OPENSSL_PREFIX}/lib:${READLINE_PREFIX}/lib:${GETTEXT_PREFIX}/lib:${ZSTD_PREFIX}/lib:${LIBRARY_PATH}"
- export CPATH="${OPENSSL_PREFIX}/include:${READLINE_PREFIX}/include:${GETTEXT_PREFIX}/include:${ZSTD_PREFIX}/include:${CPATH}"
+ new_ldflags=""
+ new_cppflags=""
+ new_pkgconfig=""
+ new_libpath=""
+ new_cpath=""
+ for prefix in "$OPENSSL_PREFIX" "$READLINE_PREFIX" "$GETTEXT_PREFIX" "$ZSTD_PREFIX"; do
+ [[ -n "$prefix" && -d "$prefix" ]] || continue
+ new_ldflags="${new_ldflags:+${new_ldflags} }-L${prefix}/lib"
+ new_cppflags="${new_cppflags:+${new_cppflags} }-I${prefix}/include"
+ new_pkgconfig="${new_pkgconfig:+${new_pkgconfig}:}${prefix}/lib/pkgconfig"
+ new_libpath="${new_libpath:+${new_libpath}:}${prefix}/lib"
+ new_cpath="${new_cpath:+${new_cpath}:}${prefix}/include"
+ done
+
+ if [[ -n "$new_ldflags" ]]; then
+ export LDFLAGS="${new_ldflags}${LDFLAGS:+ ${LDFLAGS}}"
+ export CPPFLAGS="${new_cppflags}${CPPFLAGS:+ ${CPPFLAGS}}"
+ export PKG_CONFIG_PATH="${new_pkgconfig}${PKG_CONFIG_PATH:+:${PKG_CONFIG_PATH}}"
+ export LIBRARY_PATH="${new_libpath}${LIBRARY_PATH:+:${LIBRARY_PATH}}"
+ export CPATH="${new_cpath}${CPATH:+:${CPATH}}"
+ fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| OPENSSL_PREFIX="$(brew --prefix openssl@3 2>/dev/null || brew --prefix openssl 2>/dev/null)" | |
| READLINE_PREFIX="$(brew --prefix readline 2>/dev/null)" | |
| GETTEXT_PREFIX="$(brew --prefix gettext 2>/dev/null)" | |
| ZSTD_PREFIX="$(brew --prefix zstd 2>/dev/null)" | |
| export LDFLAGS="-L${OPENSSL_PREFIX}/lib -L${READLINE_PREFIX}/lib -L${GETTEXT_PREFIX}/lib -L${ZSTD_PREFIX}/lib ${LDFLAGS}" | |
| export CPPFLAGS="-I${OPENSSL_PREFIX}/include -I${READLINE_PREFIX}/include -I${GETTEXT_PREFIX}/include -I${ZSTD_PREFIX}/include ${CPPFLAGS}" | |
| export PKG_CONFIG_PATH="${OPENSSL_PREFIX}/lib/pkgconfig:${READLINE_PREFIX}/lib/pkgconfig:${GETTEXT_PREFIX}/lib/pkgconfig:${ZSTD_PREFIX}/lib/pkgconfig:${PKG_CONFIG_PATH}" | |
| export LIBRARY_PATH="${OPENSSL_PREFIX}/lib:${READLINE_PREFIX}/lib:${GETTEXT_PREFIX}/lib:${ZSTD_PREFIX}/lib:${LIBRARY_PATH}" | |
| export CPATH="${OPENSSL_PREFIX}/include:${READLINE_PREFIX}/include:${GETTEXT_PREFIX}/include:${ZSTD_PREFIX}/include:${CPATH}" | |
| OPENSSL_PREFIX="$(brew --prefix openssl@3 2>/dev/null || brew --prefix openssl 2>/dev/null)" | |
| READLINE_PREFIX="$(brew --prefix readline 2>/dev/null)" | |
| GETTEXT_PREFIX="$(brew --prefix gettext 2>/dev/null)" | |
| ZSTD_PREFIX="$(brew --prefix zstd 2>/dev/null)" | |
| new_ldflags="" | |
| new_cppflags="" | |
| new_pkgconfig="" | |
| new_libpath="" | |
| new_cpath="" | |
| for prefix in "$OPENSSL_PREFIX" "$READLINE_PREFIX" "$GETTEXT_PREFIX" "$ZSTD_PREFIX"; do | |
| [[ -n "$prefix" && -d "$prefix" ]] || continue | |
| new_ldflags="${new_ldflags:+${new_ldflags} }-L${prefix}/lib" | |
| new_cppflags="${new_cppflags:+${new_cppflags} }-I${prefix}/include" | |
| new_pkgconfig="${new_pkgconfig:+${new_pkgconfig}:}${prefix}/lib/pkgconfig" | |
| new_libpath="${new_libpath:+${new_libpath}:}${prefix}/lib" | |
| new_cpath="${new_cpath:+${new_cpath}:}${prefix}/include" | |
| done | |
| if [[ -n "$new_ldflags" ]]; then | |
| export LDFLAGS="${new_ldflags}${LDFLAGS:+ ${LDFLAGS}}" | |
| export CPPFLAGS="${new_cppflags}${CPPFLAGS:+ ${CPPFLAGS}}" | |
| export PKG_CONFIG_PATH="${new_pkgconfig}${PKG_CONFIG_PATH:+:${PKG_CONFIG_PATH}}" | |
| export LIBRARY_PATH="${new_libpath}${LIBRARY_PATH:+:${LIBRARY_PATH}}" | |
| export CPATH="${new_cpath}${CPATH:+:${CPATH}}" | |
| fi |
🤖 Prompt for AI Agents
In `@bashrc.d/30-buildflags.sh` around lines 10 - 19, The current exports build
flags unconditionally embed OPENSSL_PREFIX, READLINE_PREFIX, GETTEXT_PREFIX, and
ZSTD_PREFIX which can be empty and produce invalid entries; update the logic
that sets LDFLAGS, CPPFLAGS, PKG_CONFIG_PATH, LIBRARY_PATH, and CPATH to only
append a prefix if its corresponding variable is non-empty and the expected
subpath exists (e.g., check "${OPENSSL_PREFIX}/lib" or
"${OPENSSL_PREFIX}/include" before adding "-L${OPENSSL_PREFIX}/lib" or
"-I${OPENSL_PREFIX}/include"), doing this for OPENSSL_PREFIX, READLINE_PREFIX,
GETTEXT_PREFIX, and ZSTD_PREFIX so no empty prefixes produce malformed flags in
the final exported environment variables.
| export PKG_CONFIG_PATH="${OPENSSL_PREFIX}/lib/pkgconfig:${READLINE_PREFIX}/lib/pkgconfig:${GETTEXT_PREFIX}/lib/pkgconfig:${ZSTD_PREFIX}/lib/pkgconfig:${PKG_CONFIG_PATH}" | ||
| export LIBRARY_PATH="${OPENSSL_PREFIX}/lib:${READLINE_PREFIX}/lib:${GETTEXT_PREFIX}/lib:${ZSTD_PREFIX}/lib:${LIBRARY_PATH}" | ||
| export CPATH="${OPENSSL_PREFIX}/include:${READLINE_PREFIX}/include:${GETTEXT_PREFIX}/include:${ZSTD_PREFIX}/include:${CPATH}" | ||
| export PYTHON_CONFIGURE_OPTS="--with-openssl=${OPENSSL_PREFIX} --with-readline=editline" |
There was a problem hiding this comment.
Preserve existing PYTHON_CONFIGURE_OPTS.
This currently overwrites any user-defined options. Appending keeps user settings intact.
🧩 Suggested fix (append instead of clobber)
- export PYTHON_CONFIGURE_OPTS="--with-openssl=${OPENSSL_PREFIX} --with-readline=editline"
+ export PYTHON_CONFIGURE_OPTS="${PYTHON_CONFIGURE_OPTS:+${PYTHON_CONFIGURE_OPTS} }--with-openssl=${OPENSSL_PREFIX} --with-readline=editline"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export PYTHON_CONFIGURE_OPTS="--with-openssl=${OPENSSL_PREFIX} --with-readline=editline" | |
| export PYTHON_CONFIGURE_OPTS="${PYTHON_CONFIGURE_OPTS:+${PYTHON_CONFIGURE_OPTS} }--with-openssl=${OPENSSL_PREFIX} --with-readline=editline" |
🤖 Prompt for AI Agents
In `@bashrc.d/30-buildflags.sh` at line 20, The script currently overwrites
PYTHON_CONFIGURE_OPTS; change it to preserve and append to any existing value by
reading the current PYTHON_CONFIGURE_OPTS and appending "
--with-openssl=${OPENSSL_PREFIX} --with-readline=editline" so user-defined
options remain intact (update the export of PYTHON_CONFIGURE_OPTS accordingly,
referencing the PYTHON_CONFIGURE_OPTS variable and OPENSSL_PREFIX).
| #!/usr/bin/env bash | ||
| #!/usr/bin/env bash |
There was a problem hiding this comment.
Remove duplicate shebang line.
Line 2 duplicates the shebang from line 1. This appears to be a copy-paste error.
Proposed fix
#!/usr/bin/env bash
-#!/usr/bin/env bash
# shellcheck disable=SC1090,SC1091📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #!/usr/bin/env bash | |
| #!/usr/bin/env bash | |
| #!/usr/bin/env bash |
🤖 Prompt for AI Agents
In `@bashrc.d/40-completions.sh` around lines 1 - 2, Remove the duplicate shebang
by keeping only a single "#!/usr/bin/env bash" at the top of the file; edit the
file to delete the second identical shebang line so the script starts with one
valid interpreter declaration (look for the two identical "#!/usr/bin/env bash"
lines and remove the redundant one).
| if [[ -n "${SSH_AUTH_SOCK:-}" ]] && _ssh_agent_usable "$SSH_AUTH_SOCK"; then | ||
| : | ||
| else | ||
| SSH_AGENT_SOCK="${HOME}/.ssh/agent.sock" | ||
| if _ssh_agent_usable "$SSH_AGENT_SOCK"; then | ||
| export SSH_AUTH_SOCK="$SSH_AGENT_SOCK" | ||
| else | ||
| eval "$(ssh-agent -a "$SSH_AGENT_SOCK" -s)" >/dev/null | ||
| fi |
There was a problem hiding this comment.
Handle stale socket file before starting a new agent.
If the socket path exists but is not backed by a live agent, ssh-agent -a can fail with “Address already in use,” leaving no agent running. Clear the stale socket before spawning.
🔧 Proposed fix
else
SSH_AGENT_SOCK="${HOME}/.ssh/agent.sock"
if _ssh_agent_usable "$SSH_AGENT_SOCK"; then
export SSH_AUTH_SOCK="$SSH_AGENT_SOCK"
else
+ [[ -S "$SSH_AGENT_SOCK" ]] && rm -f "$SSH_AGENT_SOCK"
eval "$(ssh-agent -a "$SSH_AGENT_SOCK" -s)" >/dev/null
fi
fi🤖 Prompt for AI Agents
In `@bashrc.d/95-ssh-agent.sh` around lines 17 - 25, The startup logic may try to
bind ssh-agent to SSH_AGENT_SOCK when a stale socket file already exists,
causing “Address already in use”; update the block that assigns SSH_AGENT_SOCK
and calls ssh-agent (referencing SSH_AGENT_SOCK, SSH_AUTH_SOCK, and
_ssh_agent_usable) to check if the socket path exists but !_ssh_agent_usable,
then remove the stale socket file (rm -f "$SSH_AGENT_SOCK") before invoking eval
"$(ssh-agent -a "$SSH_AGENT_SOCK" -s)"; keep the existing _ssh_agent_usable
checks and only unlink the file when it is present and unusable to avoid
removing a live socket.
| install_tool "$id" | ||
| fi | ||
| fi | ||
| unset "INSTALL_IN_PROGRESS[$id]" |
There was a problem hiding this comment.
Quote argument to unset to prevent glob expansion.
Per ShellCheck SC2184, the array subscript argument to unset should use single quotes to prevent potential glob expansion issues with special characters in $id.
🐛 Proposed fix
- unset "INSTALL_IN_PROGRESS[$id]"
+ unset 'INSTALL_IN_PROGRESS[$id]'🤖 Prompt for AI Agents
In `@install.bash` at line 660, The unset invocation uses a double-quoted array
subscript unset "INSTALL_IN_PROGRESS[$id]" which risks glob expansion; update
the unset call to quote the array subscript with single quotes so the subscript
is treated literally (i.e., protect INSTALL_IN_PROGRESS and the $id subscript
from expansion) and ensure the change appears where the unset of
INSTALL_IN_PROGRESS[$id] occurs.
| if _using_brew; then | ||
| brew_exec install asdf | ||
| return 0 | ||
| fi |
There was a problem hiding this comment.
install_asdf doesn't propagate brew_exec failure.
If brew_exec install asdf fails, the function still returns 0. This masks installation failures.
🐛 Proposed fix
if _using_brew; then
- brew_exec install asdf
- return 0
+ brew_exec install asdf && return 0
+ return 1
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if _using_brew; then | |
| brew_exec install asdf | |
| return 0 | |
| fi | |
| if _using_brew; then | |
| brew_exec install asdf && return 0 | |
| return 1 | |
| fi |
🤖 Prompt for AI Agents
In `@installers/_helpers.sh` around lines 354 - 357, The install_asdf path calls
brew_exec install asdf but always returns 0, hiding failures; update the
install_asdf logic so the exit status of brew_exec is propagated (e.g., test the
command result and return its non-zero code or return the command substitution
result) instead of unconditionally returning 0—locate the branch that calls
brew_exec in install_asdf and replace the unconditional "return 0" with logic
that returns brew_exec's exit status.
| if _using_git; then | ||
| if [[ -d "$HOME/.asdf" ]]; then | ||
| return 0 | ||
| fi | ||
| git clone https://github.com/asdf-vm/asdf.git "$HOME/.asdf" | ||
| if git -C "$HOME/.asdf" describe --tags --abbrev=0 >/dev/null 2>&1; then | ||
| local tag | ||
| tag="$(git -C "$HOME/.asdf" describe --tags --abbrev=0)" | ||
| git -C "$HOME/.asdf" checkout "$tag" || true | ||
| fi | ||
| return 0 |
There was a problem hiding this comment.
install_asdf git clone doesn't check exit status.
If git clone fails, the function proceeds to checkout a tag on a non-existent directory, producing confusing errors.
🐛 Proposed fix
if _using_git; then
if [[ -d "$HOME/.asdf" ]]; then
return 0
fi
- git clone https://github.com/asdf-vm/asdf.git "$HOME/.asdf"
+ if ! git clone https://github.com/asdf-vm/asdf.git "$HOME/.asdf"; then
+ echo "Failed to clone asdf repository" >&2
+ return 1
+ fi
if git -C "$HOME/.asdf" describe --tags --abbrev=0 >/dev/null 2>&1; then🤖 Prompt for AI Agents
In `@installers/_helpers.sh` around lines 359 - 369, The install_asdf flow doesn't
check git clone's result, so if git clone fails subsequent git -C "$HOME/.asdf"
commands run against a missing dir; after the git clone line in the block
guarded by _using_git, check the exit status (or use a conditional) and if clone
failed log or return a non-zero status and stop further processing; reference
the _using_git check, the git clone command that targets "$HOME/.asdf", and the
subsequent git -C "$HOME/.asdf" describe/checkout calls to ensure you bail out
before attempting those when clone fails.
| case "${GET_BASHED_VIMRC_MODE:-awesome}" in | ||
| basic) | ||
| sh "$target/install_basic_vimrc.sh" | ||
| ;; | ||
| *) | ||
| sh "$target/install_awesome_vimrc.sh" | ||
| ;; | ||
| esac |
There was a problem hiding this comment.
install_vimrc doesn't check exit status of vimrc install scripts.
If install_awesome_vimrc.sh or install_basic_vimrc.sh fails, the function returns success implicitly.
🐛 Proposed fix
case "${GET_BASHED_VIMRC_MODE:-awesome}" in
basic)
- sh "$target/install_basic_vimrc.sh"
+ sh "$target/install_basic_vimrc.sh" || return 1
;;
*)
- sh "$target/install_awesome_vimrc.sh"
+ sh "$target/install_awesome_vimrc.sh" || return 1
;;
esac
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| case "${GET_BASHED_VIMRC_MODE:-awesome}" in | |
| basic) | |
| sh "$target/install_basic_vimrc.sh" | |
| ;; | |
| *) | |
| sh "$target/install_awesome_vimrc.sh" | |
| ;; | |
| esac | |
| case "${GET_BASHED_VIMRC_MODE:-awesome}" in | |
| basic) | |
| sh "$target/install_basic_vimrc.sh" || return 1 | |
| ;; | |
| *) | |
| sh "$target/install_awesome_vimrc.sh" || return 1 | |
| ;; | |
| esac | |
| } |
🤖 Prompt for AI Agents
In `@installers/_helpers.sh` around lines 516 - 523, The case block that calls
install_basic_vimrc.sh / install_awesome_vimrc.sh doesn't check their exit
status, so failures are masked; update the install_vimrc logic (the case using
GET_BASHED_VIMRC_MODE) to capture and propagate the script exit code (e.g., run
each sh invocation and if it exits non‑zero, return that non‑zero status or exit
with a clear error), ensuring both calls to install_basic_vimrc.sh and
install_awesome_vimrc.sh fail the parent function when they fail.
| ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" | ||
|
|
||
| . "$ROOT_DIR/scripts/ci-setup.sh" "pre_commit,actionlint,shellcheck,bashate,shdoc" | ||
|
|
||
| if command -v shdoc >/dev/null 2>&1; then | ||
| "$ROOT_DIR/scripts/gen-docs.sh" | ||
| fi | ||
|
|
||
| if command -v pre-commit >/dev/null 2>&1; then | ||
| pre-commit run --all-files | ||
| else |
There was a problem hiding this comment.
Run docs and pre-commit from the repo root to avoid CWD-dependent failures.
If this script is invoked from outside the repo root, pre-commit may not find .pre-commit-config.yaml, and docs generation may write to an unexpected location. Consider changing into ROOT_DIR before running these commands.
Proposed fix
ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
+cd "$ROOT_DIR"
. "$ROOT_DIR/scripts/ci-setup.sh" "pre_commit,actionlint,shellcheck,bashate,shdoc"
if command -v shdoc >/dev/null 2>&1; then
"$ROOT_DIR/scripts/gen-docs.sh"
fi
if command -v pre-commit >/dev/null 2>&1; then
pre-commit run --all-files🤖 Prompt for AI Agents
In `@scripts/pre-commit-ci.sh` around lines 9 - 19, The script uses ROOT_DIR but
runs gen-docs.sh and pre-commit from the current working directory, which can
cause CWD-dependent failures; change into ROOT_DIR before invoking
"$ROOT_DIR/scripts/gen-docs.sh" and the pre-commit command (either wrap them in
a subshell like (cd "$ROOT_DIR" && ...) or use pushd/popd) so docs generation
and pre-commit run from the repository root; update the blocks around the calls
to reference ROOT_DIR and ensure the original CWD is restored if not using a
subshell.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@bashrc.d/70-env.sh`:
- Around line 1-2: Remove the duplicated shebang by keeping a single
"#!/usr/bin/env bash" at the top of the file and deleting the redundant second
occurrence; ensure only one shebang line remains as the very first line of the
script.
In `@bashrc.d/99-secrets.sh`:
- Around line 12-16: Capture the current allexport state before enabling it and
restore it after sourcing Doppler: just before calling set -a save the existing
allexport flag into a temp variable (e.g., old_allexport), then call set -a,
source <(doppler secrets download --no-file --format env) and finally restore
the flag by re-applying the saved state (if old_allexport was enabled leave
allexport enabled, otherwise disable it with set +a). Reference symbols:
GET_BASHED_USE_DOPPLER, doppler, set -a, set +a, and the source <(doppler
secrets download --no-file --format env) line to locate where to add the
save/restore logic.
In `@installers/_helpers.sh`:
- Around line 323-339: In the curl case inside installers/_helpers.sh (the curl)
branch using TOOL_CURL_URL and TOOL_CURL_CMD, replace the two `return 1`
statements (the one after a failed `curl -fsSL ... -o "$tmp_dir/install.sh"` and
the one after a failed `$cmd "$tmp_dir/install.sh"`) with `continue` so failures
only skip this method and allow other install methods to run; keep the existing
`rm -rf "$tmp_dir"` cleanup before continuing to ensure temporary directories
are removed.
- Around line 596-600: The mv and chmod commands that move "$tmp_dir/actionlint"
to "$prefix/bin/actionlint" and make it executable currently lack error checks;
update the block around the local prefix, mv and chmod so each critical
operation (mv and chmod) is checked and on failure logs an error and returns
non-zero (do not silently continue), referencing the same "$tmp_dir/actionlint",
"$prefix/bin/actionlint", mv and chmod calls; ensure rm -rf "$tmp_dir" still
runs appropriately (e.g., only after successful install or in a failure cleanup
path) so you don't mask or drop errors from mv/chmod.
🧹 Nitpick comments (1)
bashrc.d/80-aliases.sh (1)
7-12: Portability note:freeis Linux-specific.The
freecommand doesn't exist on macOS/BSD systems. If this project targets cross-platform environments, consider guarding the alias or documenting this limitation.Additionally, the quoting style is inconsistent across aliases (double quotes, single quotes, and no quotes on line 10). This is functional but could be unified for consistency.
🔧 Optional: Guard for portability and unify quoting
alias cp="cp -i" alias df="df -h" -alias free='free -m' -alias more=less +command -v free &>/dev/null && alias free="free -m" +alias more="less" alias lcm="git log -1 --pretty=%B" alias gcb="git branch --show-current"
| #!/usr/bin/env bash | ||
| #!/usr/bin/env bash |
There was a problem hiding this comment.
Remove the duplicate shebang.
Having two shebang lines is redundant; keep only one for clarity.
🧹 Proposed fix
-#!/usr/bin/env bash
-#!/usr/bin/env bash
+#!/usr/bin/env bash📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #!/usr/bin/env bash | |
| #!/usr/bin/env bash | |
| #!/usr/bin/env bash |
🤖 Prompt for AI Agents
In `@bashrc.d/70-env.sh` around lines 1 - 2, Remove the duplicated shebang by
keeping a single "#!/usr/bin/env bash" at the top of the file and deleting the
redundant second occurrence; ensure only one shebang line remains as the very
first line of the script.
| if [[ "${GET_BASHED_USE_DOPPLER:-0}" == "1" ]] && command -v doppler >/dev/null 2>&1; then | ||
| set -a | ||
| source <(doppler secrets download --no-file --format env) | ||
| set +a | ||
| fi |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, confirm the file exists and check the actual code
find . -name "99-secrets.sh" -o -name "bashrc.d" -type dRepository: jbcom/get-bashed
Length of output: 95
🏁 Script executed:
# List bashrc.d directory to understand structure
git ls-files bashrc.d/ | head -20Repository: jbcom/get-bashed
Length of output: 405
🏁 Script executed:
# Search for the specific file and read it
git ls-files | grep -E "bashrc\.d/99-secrets\.sh"Repository: jbcom/get-bashed
Length of output: 82
🏁 Script executed:
# Run the verification script from the review comment
rg -n --glob 'bashrc.d/*.sh' -P '\bset\s+[-+]a\b|\bset\s+[-+]o\s+allexport\b'Repository: jbcom/get-bashed
Length of output: 129
🏁 Script executed:
# Read the actual file content
cat -n bashrc.d/99-secrets.shRepository: jbcom/get-bashed
Length of output: 901
🏁 Script executed:
# Check if the grep results show ALL occurrences or just in 99-secrets
# Let's search more broadly including all bashrc.d files
rg -n 'set\s+[-+]a' bashrc.d/Repository: jbcom/get-bashed
Length of output: 129
Preserve existing allexport state around Doppler sourcing.
Line 13 enables allexport, but line 15 always disables it. While no earlier module in bashrc.d relies on allexport being enabled, this unconditional disable silently turns off the flag if a user had set -a enabled before sourcing their shell initialization. For defensive programming, capture and restore the prior state:
🔧 Proposed fix
if [[ "${GET_BASHED_USE_DOPPLER:-0}" == "1" ]] && command -v doppler >/dev/null 2>&1; then
+ _allexport_was_set=0
+ [[ $- == *a* ]] && _allexport_was_set=1
set -a
source <(doppler secrets download --no-file --format env)
- set +a
+ (( _allexport_was_set )) || set +a
+ unset _allexport_was_set
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if [[ "${GET_BASHED_USE_DOPPLER:-0}" == "1" ]] && command -v doppler >/dev/null 2>&1; then | |
| set -a | |
| source <(doppler secrets download --no-file --format env) | |
| set +a | |
| fi | |
| if [[ "${GET_BASHED_USE_DOPPLER:-0}" == "1" ]] && command -v doppler >/dev/null 2>&1; then | |
| _allexport_was_set=0 | |
| [[ $- == *a* ]] && _allexport_was_set=1 | |
| set -a | |
| source <(doppler secrets download --no-file --format env) | |
| (( _allexport_was_set )) || set +a | |
| unset _allexport_was_set | |
| fi |
🤖 Prompt for AI Agents
In `@bashrc.d/99-secrets.sh` around lines 12 - 16, Capture the current allexport
state before enabling it and restore it after sourcing Doppler: just before
calling set -a save the existing allexport flag into a temp variable (e.g.,
old_allexport), then call set -a, source <(doppler secrets download --no-file
--format env) and finally restore the flag by re-applying the saved state (if
old_allexport was enabled leave allexport enabled, otherwise disable it with set
+a). Reference symbols: GET_BASHED_USE_DOPPLER, doppler, set -a, set +a, and the
source <(doppler secrets download --no-file --format env) line to locate where
to add the save/restore logic.
| curl) | ||
| _using_curl || continue | ||
| local url="${TOOL_CURL_URL[$id]:-}" | ||
| [[ -n "$url" ]] || continue | ||
| local tmp_dir | ||
| tmp_dir="$(mktemp -d)" | ||
| if ! curl -fsSL "$url" -o "$tmp_dir/install.sh"; then | ||
| rm -rf "$tmp_dir" | ||
| return 1 | ||
| fi | ||
| local cmd="${TOOL_CURL_CMD[$id]:-bash}" | ||
| if ! $cmd "$tmp_dir/install.sh"; then | ||
| rm -rf "$tmp_dir" | ||
| return 1 | ||
| fi | ||
| rm -rf "$tmp_dir" | ||
| return 0 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's see the full context of the install_tool function
cd installers && grep -n "install_tool" _helpers.sh | head -5Repository: jbcom/get-bashed
Length of output: 80
🏁 Script executed:
#!/bin/bash
# Let's look at the full install_tool function to understand the loop structure
sed -n '280,360p' installers/_helpers.shRepository: jbcom/get-bashed
Length of output: 2177
🏁 Script executed:
#!/bin/bash
# Check if TOOL_METHODS is defined in tools.sh and see the method ordering
rg -n "TOOL_METHODS" installers/tools.sh -A 3Repository: jbcom/get-bashed
Length of output: 221
Allow other install methods to run when curl fails.
The curl branch uses return 1 on download or execution failure, which exits the entire function and prevents fallback methods from being attempted. This is inconsistent with other methods (brew, apt, dnf, yum, pacman, pip, pipx, git), which implicitly continue the loop on failure and allow subsequent methods to be tried. Replace both return 1 statements with continue to maintain consistency and enable proper fallback behavior.
Suggested fix
curl)
_using_curl || continue
local url="${TOOL_CURL_URL[$id]:-}"
[[ -n "$url" ]] || continue
local tmp_dir
tmp_dir="$(mktemp -d)"
if ! curl -fsSL "$url" -o "$tmp_dir/install.sh"; then
rm -rf "$tmp_dir"
- return 1
+ continue
fi
local cmd="${TOOL_CURL_CMD[$id]:-bash}"
if ! $cmd "$tmp_dir/install.sh"; then
rm -rf "$tmp_dir"
- return 1
+ continue
fi
rm -rf "$tmp_dir"
return 0📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| curl) | |
| _using_curl || continue | |
| local url="${TOOL_CURL_URL[$id]:-}" | |
| [[ -n "$url" ]] || continue | |
| local tmp_dir | |
| tmp_dir="$(mktemp -d)" | |
| if ! curl -fsSL "$url" -o "$tmp_dir/install.sh"; then | |
| rm -rf "$tmp_dir" | |
| return 1 | |
| fi | |
| local cmd="${TOOL_CURL_CMD[$id]:-bash}" | |
| if ! $cmd "$tmp_dir/install.sh"; then | |
| rm -rf "$tmp_dir" | |
| return 1 | |
| fi | |
| rm -rf "$tmp_dir" | |
| return 0 | |
| curl) | |
| _using_curl || continue | |
| local url="${TOOL_CURL_URL[$id]:-}" | |
| [[ -n "$url" ]] || continue | |
| local tmp_dir | |
| tmp_dir="$(mktemp -d)" | |
| if ! curl -fsSL "$url" -o "$tmp_dir/install.sh"; then | |
| rm -rf "$tmp_dir" | |
| continue | |
| fi | |
| local cmd="${TOOL_CURL_CMD[$id]:-bash}" | |
| if ! $cmd "$tmp_dir/install.sh"; then | |
| rm -rf "$tmp_dir" | |
| continue | |
| fi | |
| rm -rf "$tmp_dir" | |
| return 0 |
🤖 Prompt for AI Agents
In `@installers/_helpers.sh` around lines 323 - 339, In the curl case inside
installers/_helpers.sh (the curl) branch using TOOL_CURL_URL and TOOL_CURL_CMD,
replace the two `return 1` statements (the one after a failed `curl -fsSL ... -o
"$tmp_dir/install.sh"` and the one after a failed `$cmd "$tmp_dir/install.sh"`)
with `continue` so failures only skip this method and allow other install
methods to run; keep the existing `rm -rf "$tmp_dir"` cleanup before continuing
to ensure temporary directories are removed.
| local prefix="${GET_BASHED_HOME:-$HOME/.get-bashed}" | ||
| mkdir -p "$prefix/bin" | ||
| mv "$tmp_dir/actionlint" "$prefix/bin/actionlint" | ||
| chmod +x "$prefix/bin/actionlint" | ||
| rm -rf "$tmp_dir" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check the file exists and examine the code around lines 596-600
head -610 installers/_helpers.sh | tail -30Repository: jbcom/get-bashed
Length of output: 1060
🏁 Script executed:
# Also get the function context to understand error handling
head -650 installers/_helpers.sh | tail -70Repository: jbcom/get-bashed
Length of output: 2157
🏁 Script executed:
# Search for the function name and see its full implementation
rg -n "actionlint" installers/_helpers.sh -B 10 -A 20Repository: jbcom/get-bashed
Length of output: 3230
Add error checking for mv and chmod to prevent silent failures.
The function consistently checks for errors on previous operations (curl, tar extraction) and returns with messages on failure. However, the mv and chmod commands at lines 598-599 lack error checking. If either fails (due to permissions, disk space, or missing files), the function still returns 0 (success), misleading the caller.
Suggested fix
local prefix="${GET_BASHED_HOME:-$HOME/.get-bashed}"
mkdir -p "$prefix/bin"
- mv "$tmp_dir/actionlint" "$prefix/bin/actionlint"
- chmod +x "$prefix/bin/actionlint"
+ if ! mv "$tmp_dir/actionlint" "$prefix/bin/actionlint"; then
+ rm -rf "$tmp_dir"
+ echo "Failed to place actionlint binary." >&2
+ return 1
+ fi
+ if ! chmod +x "$prefix/bin/actionlint"; then
+ rm -rf "$tmp_dir"
+ echo "Failed to set execute bit on actionlint." >&2
+ return 1
+ fi
rm -rf "$tmp_dir"🤖 Prompt for AI Agents
In `@installers/_helpers.sh` around lines 596 - 600, The mv and chmod commands
that move "$tmp_dir/actionlint" to "$prefix/bin/actionlint" and make it
executable currently lack error checks; update the block around the local
prefix, mv and chmod so each critical operation (mv and chmod) is checked and on
failure logs an error and returns non-zero (do not silently continue),
referencing the same "$tmp_dir/actionlint", "$prefix/bin/actionlint", mv and
chmod calls; ensure rm -rf "$tmp_dir" still runs appropriately (e.g., only after
successful install or in a failure cleanup path) so you don't mask or drop
errors from mv/chmod.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@scripts/gen-docs.sh`:
- Around line 11-14: The script only checks for shdoc but later calls ensure_eof
which invokes python3; add an explicit runtime check for python3 like the
existing shdoc guard so the script fails fast with a clear message; locate the
shell snippet that checks shdoc (the block using command -v shdoc >/dev/null
2>&1) and add a similar conditional for python3, ensuring the error message
references python3 and exits with non-zero if not found, so ensure_eof can
safely rely on python3 being present.
🧹 Nitpick comments (1)
scripts/gen-docs.sh (1)
63-77: Use a trap for temp-file cleanup.If the script exits early (e.g., shdoc failure),
TMP_MODULESis left behind. A trap makes cleanup reliable.♻️ Proposed fix (trap cleanup)
# Combine all runtime modules TMP_MODULES="$(mktemp)" +cleanup_tmp_modules() { rm -f "$TMP_MODULES"; } +trap cleanup_tmp_modules EXIT shopt -s nullglob for f in "$ROOT_DIR/bashrc.d"/*.sh; do { echo "" cat "$f" echo "" echo "# ----" echo "" } >> "$TMP_MODULES" done shopt -u nullglob shdoc < "$TMP_MODULES" > "$ROOT_DIR/docs/MODULES.md" -rm -f "$TMP_MODULES"
| command -v shdoc >/dev/null 2>&1 || { | ||
| echo "shdoc is required. Install with: ./install.sh --install shdoc" >&2 | ||
| exit 1 | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find and check if scripts/gen-docs.sh exists
git ls-files | grep -E "gen-docs|scripts" | head -20Repository: jbcom/get-bashed
Length of output: 191
🏁 Script executed:
# Check the content of scripts/gen-docs.sh
if [ -f "scripts/gen-docs.sh" ]; then
wc -l scripts/gen-docs.sh
echo "---"
cat -n scripts/gen-docs.sh
fiRepository: jbcom/get-bashed
Length of output: 2951
Add a python3 dependency check (script currently assumes it).
The ensure_eof function uses python3 but the script only checks for shdoc. On systems without Python 3, the script fails unexpectedly on line 60 when ensure_eof is first called. Add an explicit python3 check alongside the existing shdoc check.
🔧 Suggested fix
command -v shdoc >/dev/null 2>&1 || {
echo "shdoc is required. Install with: ./install.sh --install shdoc" >&2
exit 1
}
+command -v python3 >/dev/null 2>&1 || {
+ echo "python3 is required to normalize EOF in docs." >&2
+ exit 1
+}🤖 Prompt for AI Agents
In `@scripts/gen-docs.sh` around lines 11 - 14, The script only checks for shdoc
but later calls ensure_eof which invokes python3; add an explicit runtime check
for python3 like the existing shdoc guard so the script fails fast with a clear
message; locate the shell snippet that checks shdoc (the block using command -v
shdoc >/dev/null 2>&1) and add a similar conditional for python3, ensuring the
error message references python3 and exits with non-zero if not found, so
ensure_eof can safely rely on python3 being present.
1 participant
Summary
Testing
Notes
Summary by CodeRabbit
New Features
Documentation
Chores
Tests