Refactor bootstrap into array-driven step functions#17
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughRefactors Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
There was a problem hiding this comment.
Pull request overview
This PR refactors the bootstrap script to use an array-driven architecture for installation steps, improving maintainability and code organization. The refactoring extracts inline installation code into named step functions and uses a simple loop to execute them.
Changes:
- Extracted 13 installation phases into individual step functions (e.g.,
stepInstallDocker,stepSetupPython) - Introduced a
STEPSarray containing all step function names, withTOTAL_STEPSdynamically calculated from array length - Replaced sequential inline installation code with a for-loop that iterates through the
STEPSarray - Updated step counter format from
[Step N]to[N/TOTAL]for better progress visibility
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| local SPEEDTEST_BIN="${USR_BIN_DIR}/speedtest-cli" | ||
| local SPEEDTEST_REPO="sivel/speedtest-cli" | ||
| local SPEEDTEST_ASSET="speedtest.py" | ||
| if ! [ -e ${SPEEDTEST_BIN} ]; then |
There was a problem hiding this comment.
The variable ${SPEEDTEST_BIN} should be quoted in the test condition. Without quotes, the test may fail if the path contains spaces or special characters. This is inconsistent with line 319 where ${DOCKER_COMPOSE_BIN} is properly quoted as "${DOCKER_COMPOSE_BIN}". Consider changing to: if ! [ -e "${SPEEDTEST_BIN}" ]; then
| if ! [ -e ${SPEEDTEST_BIN} ]; then | |
| if ! [ -e "${SPEEDTEST_BIN}" ]; then |
| function stepSetupPython() { | ||
| logStep "Making sure python exists..." | ||
| local PYTHON_BIN=/usr/bin/python | ||
| if ! [ -x "$(command -v python)" ] || ! [ -e ${PYTHON_BIN} ]; then |
There was a problem hiding this comment.
The variable ${PYTHON_BIN} should be quoted in the test condition. Without quotes, the test may fail if the path contains spaces or special characters. This is inconsistent with line 319 where ${DOCKER_COMPOSE_BIN} is properly quoted as "${DOCKER_COMPOSE_BIN}". Consider changing to: if ! [ -x "$(command -v python)" ] || ! [ -e "${PYTHON_BIN}" ]; then
| if ! [ -x "$(command -v python)" ] || ! [ -e ${PYTHON_BIN} ]; then | |
| if ! [ -x "$(command -v python)" ] || ! [ -e "${PYTHON_BIN}" ]; then |
| runCmdAndLog update-alternatives --set vi $(which nvim) | ||
| runCmdAndLog update-alternatives --set vim $(which nvim) |
There was a problem hiding this comment.
The command substitution
| runCmdAndLog update-alternatives --set vi $(which nvim) | |
| runCmdAndLog update-alternatives --set vim $(which nvim) | |
| runCmdAndLog update-alternatives --set vi "$(which nvim)" | |
| runCmdAndLog update-alternatives --set vim "$(which nvim)" |
| if ! [ -e ${DOCKER_COMPOSE_SWITCH_BIN} ]; then | ||
| logStep "Installing Docker Switch..." | ||
| downloadBinaryLatestRelease "${DOCKER_COMPOSE_SWITCH_REPO}" "${DOCKER_COMPOSE_SWITCH_ASSET}" "${DOCKER_COMPOSE_SWITCH_BIN}" | ||
| # Set Docker Compose Switch to replace Docker Compose v1 | ||
| runCmdAndLog update-alternatives \ | ||
| --install ${USR_BIN_DIR}/docker-compose \ |
There was a problem hiding this comment.
The variable ${USR_BIN_DIR} should be quoted when used in arguments. Without quotes, the command may fail if the path contains spaces or special characters. Consider changing to: --install "${USR_BIN_DIR}/docker-compose"
| if ! [ -e ${DOCKER_COMPOSE_SWITCH_BIN} ]; then | |
| logStep "Installing Docker Switch..." | |
| downloadBinaryLatestRelease "${DOCKER_COMPOSE_SWITCH_REPO}" "${DOCKER_COMPOSE_SWITCH_ASSET}" "${DOCKER_COMPOSE_SWITCH_BIN}" | |
| # Set Docker Compose Switch to replace Docker Compose v1 | |
| runCmdAndLog update-alternatives \ | |
| --install ${USR_BIN_DIR}/docker-compose \ | |
| if ! [ -e "${DOCKER_COMPOSE_SWITCH_BIN}" ]; then | |
| logStep "Installing Docker Switch..." | |
| downloadBinaryLatestRelease "${DOCKER_COMPOSE_SWITCH_REPO}" "${DOCKER_COMPOSE_SWITCH_ASSET}" "${DOCKER_COMPOSE_SWITCH_BIN}" | |
| # Set Docker Compose Switch to replace Docker Compose v1 | |
| runCmdAndLog update-alternatives \ | |
| --install "${USR_BIN_DIR}/docker-compose" \ |
| logStep "Cleanup caches..." | ||
| runCmdAndLog ${APT_CMD} clean | ||
| for STEP in "${STEPS[@]}"; do | ||
| ${STEP} |
There was a problem hiding this comment.
The variable ${STEP} should be quoted when used in command execution. While function names are unlikely to contain spaces, quoting is a best practice for consistency and to prevent unexpected behavior. Consider changing to: "${STEP}"
| ${STEP} | |
| "${STEP}" |
| local DOCKER_COMPOSE_SWITCH_BIN="${USR_BIN_DIR}/compose-switch" | ||
| local DOCKER_COMPOSE_SWITCH_REPO="docker/compose-switch" | ||
| local DOCKER_COMPOSE_SWITCH_ASSET="docker-compose-linux-amd64" | ||
| if ! [ -e ${DOCKER_COMPOSE_SWITCH_BIN} ]; then |
There was a problem hiding this comment.
The variable ${DOCKER_COMPOSE_SWITCH_BIN} should be quoted in the test condition. Without quotes, the test may fail if the path contains spaces or special characters. This is inconsistent with line 319 where ${DOCKER_COMPOSE_BIN} is properly quoted as "${DOCKER_COMPOSE_BIN}". Consider changing to: if ! [ -e "${DOCKER_COMPOSE_SWITCH_BIN}" ]; then
| if ! [ -e ${DOCKER_COMPOSE_SWITCH_BIN} ]; then | |
| if ! [ -e "${DOCKER_COMPOSE_SWITCH_BIN}" ]; then |
| local DOCKER_COMPOSE_SWITCH_ASSET="docker-compose-linux-amd64" | ||
| if ! [ -e ${DOCKER_COMPOSE_SWITCH_BIN} ]; then | ||
| logStep "Installing Docker Switch..." | ||
| downloadBinaryLatestRelease "${DOCKER_COMPOSE_SWITCH_REPO}" "${DOCKER_COMPOSE_SWITCH_ASSET}" "${DOCKER_COMPOSE_SWITCH_BIN}" |
There was a problem hiding this comment.
The downloadBinaryLatestRelease invocation for Docker Compose Switch downloads a remote binary from the docker/compose-switch GitHub repository and marks it executable without any checksum or signature verification. If that repository or the delivery channel is compromised, attackers can replace docker-compose with a malicious binary that runs with the privileges of any user invoking it. Mitigate by pinning to a specific, trusted release/commit and verifying the downloaded binary against an expected hash or signature before installation.
| local SPEEDTEST_ASSET="speedtest.py" | ||
| if ! [ -e ${SPEEDTEST_BIN} ]; then | ||
| logStep "Installing SpeedTest CLI..." | ||
| downloadBinaryLatestRelease "${SPEEDTEST_REPO}" "${SPEEDTEST_ASSET}" "${SPEEDTEST_BIN}" "raw" |
There was a problem hiding this comment.
This downloadBinaryLatestRelease call pulls the speedtest.py asset from the third-party sivel/speedtest-cli GitHub repository and installs it as an executable without any integrity check. A compromise of that repository or the transport could lead to arbitrary code execution whenever speedtest-cli is run on systems bootstrapped by this script. To reduce this risk, pin to a specific version or commit and verify the downloaded script against a known checksum or signature before making it executable.
|
|
||
| if [ -x "${ZSH_BIN}" ]; then | ||
| if ! [ -d "${PREZTO_DIR}" ]; then | ||
| runCmdAndLog git clone --recursive "${PREZTO_REPO_URL}" "${PREZTO_DIR}" |
There was a problem hiding this comment.
This step clones the sorin-ionescu/prezto repository from GitHub at a mutable branch head and wires it into the login shell without any pinning or integrity verification. If that third-party repository is ever compromised, an attacker can execute arbitrary code in every Zsh session for this user, with access to any loaded credentials or secrets. Hardening this step requires pinning to a specific commit or signed tag and verifying it before using it as part of the login shell configuration.
| if ! [ -e "${DOCKER_COMPOSE_BIN}" ]; then | ||
| mkdir -p "${DOCKER_CLI_PLUGINS_DIR}" | ||
| logStep "Installing Docker Compose..." | ||
| downloadBinaryLatestRelease "${DOCKER_COMPOSE_REPO}" "${DOCKER_COMPOSE_ASSET}" "${DOCKER_COMPOSE_BIN}" |
There was a problem hiding this comment.
The call to downloadBinaryLatestRelease here fetches and installs the Docker Compose binary from GitHub releases without any integrity verification before making it executable via downloadLatestReleaseArtifact/chmod +x. If the docker/compose repository or the network path is compromised, a malicious binary could be installed and later executed with the same privileges as this bootstrap script. Consider pinning to a specific release (or commit) and validating the downloaded artifact against a known checksum or signature before using it.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Fix all issues with AI agents
In `@bootstrap.sh`:
- Around line 248-253: Add a standard bash doc block before each new step
function (e.g., stepUpgradePackages and the other new step functions)
documenting usage, arguments, return values, and examples; specifically insert a
comment block immediately above the function definition that includes a brief
description, a "Usage:" line showing the function call, an "Args:" section (or
"None" if no args), a "Returns:" section describing exit codes, and an
"Example:" demonstrating invocation so it matches the repository's doc block
template and coding guidelines.
- Around line 313-326: In stepInstallDockerCompose, ensure all actions are
executed through runCmdAndLog and add a completion log: replace direct calls to
mkdir -p and downloadBinaryLatestRelease with runCmdAndLog invocations (or
update downloadBinaryLatestRelease to internally call runCmdAndLog) so the
directory creation and the download are logged, and after a successful download
call logStep (e.g., "Docker Compose installed.") to mirror the install-complete
pattern used by other installers.
- Around line 361-372: The stepInstallSpeedTest function currently calls
downloadBinaryLatestRelease directly and lacks a completion log; change it to
invoke downloadBinaryLatestRelease through runCmdAndLog (so output is captured)
when installing SPEEDTEST_BIN and then add a logStep call after successful
download to indicate completion; locate the stepInstallSpeedTest function and
replace the direct downloadBinaryLatestRelease invocation with runCmdAndLog
calling downloadBinaryLatestRelease (preserving arguments SPEEDTEST_REPO,
SPEEDTEST_ASSET, SPEEDTEST_BIN, "raw") and add a final logStep "SpeedTest CLI
installed." (or similar) in the else branch pattern.
- Around line 290-311: The install branch of function stepInstallDocker logs the
start but never logs successful completion; add a completion log after the final
runCmdAndLog call. Specifically, inside stepInstallDocker, after the block that
runs ${APT_INSTALL} docker-ce docker-ce-cli containerd.io docker-compose-plugin,
call logStep (or the project's standard success log function) with a message
like "Docker installation complete" so the install pattern (check, start log,
runCmdAndLog..., completion log) is followed; ensure you reference the same
symbols (stepInstallDocker, runCmdAndLog, logStep) when making the change.
- Around line 374-385: In stepSetupPython(), the symlink creation currently
calls ln -s directly and isn't logged; replace the direct ln invocation with
runCmdAndLog so the command output is captured and add a completion log after
successful symlink creation. Specifically, in function stepSetupPython(), when
detecting python3 at PYTHON3_BIN, call runCmdAndLog to create the symlink from
PYTHON3_BIN to PYTHON_BIN (instead of ln -s) and then call log (or logStep) to
record completion of the symlink step.
- Around line 271-288: The stepInstallTools function installs many packages
without checking for existing installs or logging completion; update it to
follow the required pattern by checking each tool before installing (use command
-v or similar), log start for each tool, call runCmdAndLog with APT_INSTALL for
only missing tools (referencing stepInstallTools and runCmdAndLog), and add a
final logStep or logInfo message indicating completion for each installed tool
and for the entire step. Ensure you reference the same APT_INSTALL variable and
keep the list of packages but iterate/guard per-package so already-installed
tools are skipped and successful installs are logged.
- Around line 328-344: The stepInstallDockerSwitch function currently calls
downloadBinaryLatestRelease directly and lacks a completion log; change the
download invocation to run via runCmdAndLog (e.g., wrap the
downloadBinaryLatestRelease call so failures are logged/handled) and after the
update-alternatives runCmdAndLog call add an explicit success log (use logStep
or similar) referencing DOCKER_COMPOSE_SWITCH_BIN and the function name
stepInstallDockerSwitch so the install start, command execution, and completion
are all logged.
- Around line 387-415: In stepInstallZshPrezto, route the raw git/curl/chsh
invocations through runCmdAndLog and add a completion log: replace the direct
git clone, ${CURL_CMD} downloads and chsh call inside the if ! [ -d
"${PREZTO_DIR}" ] block with runCmdAndLog git clone --recursive
"${PREZTO_REPO_URL}" "${PREZTO_DIR}", runCmdAndLog ${CURL_CMD} "${PREZTORC_URL}"
-o "${PREZTO_DIR}/runcoms/zpreztorc", runCmdAndLog ${CURL_CMD} "${P10K_URL}" -o
"${HOME}/.p10k.zsh", and runCmdAndLog ${ZSH_BIN} -c '...' (or runCmdAndLog chsh
-s /bin/zsh); then add a log (e.g., log "Prezto installation complete.") after
successful setup and keep the existing "Prezto already installed." path
unchanged.
- Around line 346-359: In stepInstallNeoVim, update the unquoted command
substitution used in the update-alternatives calls to be quoted (e.g., use
"$(which nvim)" or better "$(command -v nvim)") to fix ShellCheck SC2046, and
add a completion logStep (e.g., logStep "NeoVim installed.") after the
installation/update-alternatives steps so the function follows the
check/start/install/complete logging pattern; adjust the two lines referencing
update-alternatives and append the completion log inside stepInstallNeoVim.
- Around line 262-269: In stepSetLocales the locale-gen invocation and command
check use unquoted expansions which can cause word-splitting (SC2068); update
usages of LOCALE_GEN and LOCALES to use quoted expansions: quote the command
lookup check (e.g. command -v "${LOCALE_GEN}"), and pass the generator and the
array as separate quoted args to runCmdAndLog and the locale-gen invocation by
using "${LOCALE_GEN}" and "${LOCALES[@]}" so each locale element is preserved;
keep runCmdAndLog and APT_INSTALL usage the same but quote their arguments where
applicable.
| function stepInstallTools() { | ||
| logStep "Installing tools..." | ||
| runCmdAndLog ${APT_INSTALL} \ | ||
| byobu \ | ||
| curl \ | ||
| fd-find \ | ||
| fzf \ | ||
| git \ | ||
| htop \ | ||
| ripgrep \ | ||
| silversearcher-ag \ | ||
| software-properties-common \ | ||
| tig \ | ||
| unzip \ | ||
| vim \ | ||
| wget \ | ||
| zip | ||
| } |
There was a problem hiding this comment.
Guard tool installs and log completion.
This step installs a large tool bundle without an “already installed” check or a completion log, which breaks the required install pattern.
✅ Suggested structure
function stepInstallTools() {
logStep "Installing tools..."
- runCmdAndLog ${APT_INSTALL} \
- byobu \
- curl \
- fd-find \
- fzf \
- git \
- htop \
- ripgrep \
- silversearcher-ag \
- software-properties-common \
- tig \
- unzip \
- vim \
- wget \
- zip
+ local TOOL_PACKAGES=(
+ byobu
+ curl
+ fd-find
+ fzf
+ git
+ htop
+ ripgrep
+ silversearcher-ag
+ software-properties-common
+ tig
+ unzip
+ vim
+ wget
+ zip
+ )
+ if ! dpkg -s "${TOOL_PACKAGES[@]}" >/dev/null 2>&1; then
+ runCmdAndLog ${APT_INSTALL} "${TOOL_PACKAGES[@]}"
+ log "Tools installed."
+ else
+ log "Tools already installed."
+ fi
}Based on learnings: Each tool installation must follow the pattern: check if installed, log start, run installation via runCmdAndLog, log completion.
🤖 Prompt for AI Agents
In `@bootstrap.sh` around lines 271 - 288, The stepInstallTools function installs
many packages without checking for existing installs or logging completion;
update it to follow the required pattern by checking each tool before installing
(use command -v or similar), log start for each tool, call runCmdAndLog with
APT_INSTALL for only missing tools (referencing stepInstallTools and
runCmdAndLog), and add a final logStep or logInfo message indicating completion
for each installed tool and for the entire step. Ensure you reference the same
APT_INSTALL variable and keep the list of packages but iterate/guard per-package
so already-installed tools are skipped and successful installs are logged.
| function stepInstallDockerCompose() { | ||
| # https://github.com/docker/compose | ||
| local DOCKER_CLI_PLUGINS_DIR="/usr/local/lib/docker/cli-plugins" | ||
| local DOCKER_COMPOSE_BIN="${DOCKER_CLI_PLUGINS_DIR}/docker-compose" | ||
| local DOCKER_COMPOSE_REPO="docker/compose" | ||
| local DOCKER_COMPOSE_ASSET="docker-compose-linux-$(uname -m)" | ||
| if ! [ -e "${DOCKER_COMPOSE_BIN}" ]; then | ||
| mkdir -p "${DOCKER_CLI_PLUGINS_DIR}" | ||
| logStep "Installing Docker Compose..." | ||
| downloadBinaryLatestRelease "${DOCKER_COMPOSE_REPO}" "${DOCKER_COMPOSE_ASSET}" "${DOCKER_COMPOSE_BIN}" | ||
| else | ||
| logStep "Docker Compose already installed." | ||
| fi | ||
| } |
There was a problem hiding this comment.
Log Docker Compose install actions via runCmdAndLog and add completion log.
mkdir -p and the download path bypass runCmdAndLog, so logs are incomplete. Add completion logging and consider routing the download through runCmdAndLog (or update the helper to use it internally).
✅ Suggested fix (partial)
if ! [ -e "${DOCKER_COMPOSE_BIN}" ]; then
- mkdir -p "${DOCKER_CLI_PLUGINS_DIR}"
+ runCmdAndLog mkdir -p "${DOCKER_CLI_PLUGINS_DIR}"
logStep "Installing Docker Compose..."
downloadBinaryLatestRelease "${DOCKER_COMPOSE_REPO}" "${DOCKER_COMPOSE_ASSET}" "${DOCKER_COMPOSE_BIN}"
+ log "Docker Compose installed."
else
logStep "Docker Compose already installed."
fiBased on learnings: Each tool installation must follow the pattern: check if installed, log start, run installation via runCmdAndLog, log completion.
📝 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.
| function stepInstallDockerCompose() { | |
| # https://github.com/docker/compose | |
| local DOCKER_CLI_PLUGINS_DIR="/usr/local/lib/docker/cli-plugins" | |
| local DOCKER_COMPOSE_BIN="${DOCKER_CLI_PLUGINS_DIR}/docker-compose" | |
| local DOCKER_COMPOSE_REPO="docker/compose" | |
| local DOCKER_COMPOSE_ASSET="docker-compose-linux-$(uname -m)" | |
| if ! [ -e "${DOCKER_COMPOSE_BIN}" ]; then | |
| mkdir -p "${DOCKER_CLI_PLUGINS_DIR}" | |
| logStep "Installing Docker Compose..." | |
| downloadBinaryLatestRelease "${DOCKER_COMPOSE_REPO}" "${DOCKER_COMPOSE_ASSET}" "${DOCKER_COMPOSE_BIN}" | |
| else | |
| logStep "Docker Compose already installed." | |
| fi | |
| } | |
| function stepInstallDockerCompose() { | |
| # https://github.com/docker/compose | |
| local DOCKER_CLI_PLUGINS_DIR="/usr/local/lib/docker/cli-plugins" | |
| local DOCKER_COMPOSE_BIN="${DOCKER_CLI_PLUGINS_DIR}/docker-compose" | |
| local DOCKER_COMPOSE_REPO="docker/compose" | |
| local DOCKER_COMPOSE_ASSET="docker-compose-linux-$(uname -m)" | |
| if ! [ -e "${DOCKER_COMPOSE_BIN}" ]; then | |
| runCmdAndLog mkdir -p "${DOCKER_CLI_PLUGINS_DIR}" | |
| logStep "Installing Docker Compose..." | |
| downloadBinaryLatestRelease "${DOCKER_COMPOSE_REPO}" "${DOCKER_COMPOSE_ASSET}" "${DOCKER_COMPOSE_BIN}" | |
| log "Docker Compose installed." | |
| else | |
| logStep "Docker Compose already installed." | |
| fi | |
| } |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 318-318: Declare and assign separately to avoid masking return values.
(SC2155)
🤖 Prompt for AI Agents
In `@bootstrap.sh` around lines 313 - 326, In stepInstallDockerCompose, ensure all
actions are executed through runCmdAndLog and add a completion log: replace
direct calls to mkdir -p and downloadBinaryLatestRelease with runCmdAndLog
invocations (or update downloadBinaryLatestRelease to internally call
runCmdAndLog) so the directory creation and the download are logged, and after a
successful download call logStep (e.g., "Docker Compose installed.") to mirror
the install-complete pattern used by other installers.
| function stepInstallDockerSwitch() { | ||
| local DOCKER_COMPOSE_SWITCH_BIN="${USR_BIN_DIR}/compose-switch" | ||
| local DOCKER_COMPOSE_SWITCH_REPO="docker/compose-switch" | ||
| local DOCKER_COMPOSE_SWITCH_ASSET="docker-compose-linux-amd64" | ||
| if ! [ -e ${DOCKER_COMPOSE_SWITCH_BIN} ]; then | ||
| logStep "Installing Docker Switch..." | ||
| downloadBinaryLatestRelease "${DOCKER_COMPOSE_SWITCH_REPO}" "${DOCKER_COMPOSE_SWITCH_ASSET}" "${DOCKER_COMPOSE_SWITCH_BIN}" | ||
| # Set Docker Compose Switch to replace Docker Compose v1 | ||
| runCmdAndLog update-alternatives \ | ||
| --install ${USR_BIN_DIR}/docker-compose \ | ||
| docker-compose \ | ||
| "${DOCKER_COMPOSE_SWITCH_BIN}" \ | ||
| 99 | ||
| else | ||
| logStep "Docker Switch already installed." | ||
| fi | ||
| } |
There was a problem hiding this comment.
Ensure Docker Switch install is logged and completes with explicit success log.
The download path isn’t routed through runCmdAndLog, and there’s no completion log after the install.
✅ Suggested fix (add completion log)
runCmdAndLog update-alternatives \
--install ${USR_BIN_DIR}/docker-compose \
docker-compose \
"${DOCKER_COMPOSE_SWITCH_BIN}" \
99
+ log "Docker Switch installed."
else
logStep "Docker Switch already installed."
fiBased on learnings: Each tool installation must follow the pattern: check if installed, log start, run installation via runCmdAndLog, log completion.
📝 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.
| function stepInstallDockerSwitch() { | |
| local DOCKER_COMPOSE_SWITCH_BIN="${USR_BIN_DIR}/compose-switch" | |
| local DOCKER_COMPOSE_SWITCH_REPO="docker/compose-switch" | |
| local DOCKER_COMPOSE_SWITCH_ASSET="docker-compose-linux-amd64" | |
| if ! [ -e ${DOCKER_COMPOSE_SWITCH_BIN} ]; then | |
| logStep "Installing Docker Switch..." | |
| downloadBinaryLatestRelease "${DOCKER_COMPOSE_SWITCH_REPO}" "${DOCKER_COMPOSE_SWITCH_ASSET}" "${DOCKER_COMPOSE_SWITCH_BIN}" | |
| # Set Docker Compose Switch to replace Docker Compose v1 | |
| runCmdAndLog update-alternatives \ | |
| --install ${USR_BIN_DIR}/docker-compose \ | |
| docker-compose \ | |
| "${DOCKER_COMPOSE_SWITCH_BIN}" \ | |
| 99 | |
| else | |
| logStep "Docker Switch already installed." | |
| fi | |
| } | |
| function stepInstallDockerSwitch() { | |
| local DOCKER_COMPOSE_SWITCH_BIN="${USR_BIN_DIR}/compose-switch" | |
| local DOCKER_COMPOSE_SWITCH_REPO="docker/compose-switch" | |
| local DOCKER_COMPOSE_SWITCH_ASSET="docker-compose-linux-amd64" | |
| if ! [ -e ${DOCKER_COMPOSE_SWITCH_BIN} ]; then | |
| logStep "Installing Docker Switch..." | |
| downloadBinaryLatestRelease "${DOCKER_COMPOSE_SWITCH_REPO}" "${DOCKER_COMPOSE_SWITCH_ASSET}" "${DOCKER_COMPOSE_SWITCH_BIN}" | |
| # Set Docker Compose Switch to replace Docker Compose v1 | |
| runCmdAndLog update-alternatives \ | |
| --install ${USR_BIN_DIR}/docker-compose \ | |
| docker-compose \ | |
| "${DOCKER_COMPOSE_SWITCH_BIN}" \ | |
| 99 | |
| log "Docker Switch installed." | |
| else | |
| logStep "Docker Switch already installed." | |
| fi | |
| } |
🤖 Prompt for AI Agents
In `@bootstrap.sh` around lines 328 - 344, The stepInstallDockerSwitch function
currently calls downloadBinaryLatestRelease directly and lacks a completion log;
change the download invocation to run via runCmdAndLog (e.g., wrap the
downloadBinaryLatestRelease call so failures are logged/handled) and after the
update-alternatives runCmdAndLog call add an explicit success log (use logStep
or similar) referencing DOCKER_COMPOSE_SWITCH_BIN and the function name
stepInstallDockerSwitch so the install start, command execution, and completion
are all logged.
| function stepInstallSpeedTest() { | ||
| # https://github.com/sivel/speedtest-cli | ||
| local SPEEDTEST_BIN="${USR_BIN_DIR}/speedtest-cli" | ||
| local SPEEDTEST_REPO="sivel/speedtest-cli" | ||
| local SPEEDTEST_ASSET="speedtest.py" | ||
| if ! [ -e ${SPEEDTEST_BIN} ]; then | ||
| logStep "Installing SpeedTest CLI..." | ||
| downloadBinaryLatestRelease "${SPEEDTEST_REPO}" "${SPEEDTEST_ASSET}" "${SPEEDTEST_BIN}" "raw" | ||
| else | ||
| logStep "SpeedTest CLI already installed." | ||
| fi | ||
| } |
There was a problem hiding this comment.
Log SpeedTest install completion (and keep download output in logs).
The download path bypasses runCmdAndLog, and there’s no completion log.
✅ Suggested fix (add completion log)
if ! [ -e ${SPEEDTEST_BIN} ]; then
logStep "Installing SpeedTest CLI..."
downloadBinaryLatestRelease "${SPEEDTEST_REPO}" "${SPEEDTEST_ASSET}" "${SPEEDTEST_BIN}" "raw"
+ log "SpeedTest CLI installed."
else
logStep "SpeedTest CLI already installed."
fiBased on learnings: Each tool installation must follow the pattern: check if installed, log start, run installation via runCmdAndLog, log completion.
📝 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.
| function stepInstallSpeedTest() { | |
| # https://github.com/sivel/speedtest-cli | |
| local SPEEDTEST_BIN="${USR_BIN_DIR}/speedtest-cli" | |
| local SPEEDTEST_REPO="sivel/speedtest-cli" | |
| local SPEEDTEST_ASSET="speedtest.py" | |
| if ! [ -e ${SPEEDTEST_BIN} ]; then | |
| logStep "Installing SpeedTest CLI..." | |
| downloadBinaryLatestRelease "${SPEEDTEST_REPO}" "${SPEEDTEST_ASSET}" "${SPEEDTEST_BIN}" "raw" | |
| else | |
| logStep "SpeedTest CLI already installed." | |
| fi | |
| } | |
| function stepInstallSpeedTest() { | |
| # https://github.com/sivel/speedtest-cli | |
| local SPEEDTEST_BIN="${USR_BIN_DIR}/speedtest-cli" | |
| local SPEEDTEST_REPO="sivel/speedtest-cli" | |
| local SPEEDTEST_ASSET="speedtest.py" | |
| if ! [ -e ${SPEEDTEST_BIN} ]; then | |
| logStep "Installing SpeedTest CLI..." | |
| downloadBinaryLatestRelease "${SPEEDTEST_REPO}" "${SPEEDTEST_ASSET}" "${SPEEDTEST_BIN}" "raw" | |
| log "SpeedTest CLI installed." | |
| else | |
| logStep "SpeedTest CLI already installed." | |
| fi | |
| } |
🤖 Prompt for AI Agents
In `@bootstrap.sh` around lines 361 - 372, The stepInstallSpeedTest function
currently calls downloadBinaryLatestRelease directly and lacks a completion log;
change it to invoke downloadBinaryLatestRelease through runCmdAndLog (so output
is captured) when installing SPEEDTEST_BIN and then add a logStep call after
successful download to indicate completion; locate the stepInstallSpeedTest
function and replace the direct downloadBinaryLatestRelease invocation with
runCmdAndLog calling downloadBinaryLatestRelease (preserving arguments
SPEEDTEST_REPO, SPEEDTEST_ASSET, SPEEDTEST_BIN, "raw") and add a final logStep
"SpeedTest CLI installed." (or similar) in the else branch pattern.
Extract each installation phase into a named function and define a STEPS array that drives execution via a simple loop. TOTAL_STEPS is derived from the array length, so adding or removing a step automatically keeps the [N/TOTAL] progress counter accurate. This replaces the previous inline script structure where the step count had to be manually maintained as a hardcoded constant. https://claude.ai/code/session_01Uic8AmN9C1xoxUT38FPMAy
yorch
force-pushed
the
claude/add-processing-progress-feedback-34Ai1
branch
from
5971e9b to
4d7f5a8
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@bootstrap.sh`:
- Around line 450-485: The stepInstallZshPrezto function currently calls the
installer unconditionally; add a pre-check that uses command -v zsh (ZSH_BIN) to
detect an existing zsh before running runCmdAndLog ${APT_INSTALL} zsh, and only
proceed with cloning Prezto and fetching configs if zsh is missing;
specifically, move or add the ZSH_BIN assignment and an early-if that logs "ZSH
already installed." and returns when found, otherwise log the start, run
runCmdAndLog ${APT_INSTALL} zsh, re-evaluate ZSH_BIN and continue with the
existing Prezto install logic (references: stepInstallZshPrezto, ZSH_BIN,
APT_INSTALL, runCmdAndLog).
🧹 Nitpick comments (1)
bootstrap.sh (1)
162-171: Use${*}inlogStepto match the interpolation guideline.
$*should be braced for consistency with the${VAR}rule.🛠️ Suggested tweak
- log "[${CURRENT_STEP}/${TOTAL_STEPS}] $*" + log "[${CURRENT_STEP}/${TOTAL_STEPS}] ${*}"As per coding guidelines: Use
SCREAMING_SNAKE_CASEfor bash variable names and${VAR}interpolation instead of$VAR.
| # Install ZSH with Prezto framework and Powerlevel10k theme | ||
| # Usage: stepInstallZshPrezto | ||
| # Arguments: none | ||
| # Returns: 0 on success, non-zero on error | ||
| # Example: stepInstallZshPrezto | ||
| function stepInstallZshPrezto() { | ||
| # https://github.com/sorin-ionescu/prezto | ||
| logStep "Installing ZSH and Prezto..." | ||
| runCmdAndLog ${APT_INSTALL} zsh | ||
| local ZSH_BIN | ||
| ZSH_BIN=$(command -v zsh) | ||
| local PREZTO_DIR="${HOME}/.zprezto" | ||
| local PREZTORC_URL="https://raw.githubusercontent.com/yorch/ubuntu-server-bootstrap/main/config/zpreztorc" | ||
| local P10K_URL="https://raw.githubusercontent.com/yorch/ubuntu-server-bootstrap/main/config/p10k.zsh" | ||
| local PREZTO_REPO_URL="https://github.com/sorin-ionescu/prezto.git" | ||
|
|
||
| if [ -x "${ZSH_BIN}" ]; then | ||
| if ! [ -d "${PREZTO_DIR}" ]; then | ||
| runCmdAndLog git clone --recursive "${PREZTO_REPO_URL}" "${PREZTO_DIR}" | ||
| runCmdAndLog ${CURL_CMD} "${PREZTORC_URL}" -o "${PREZTO_DIR}/runcoms/zpreztorc" | ||
| runCmdAndLog ${CURL_CMD} "${P10K_URL}" -o "${HOME}/.p10k.zsh" | ||
| runCmdAndLog ${ZSH_BIN} -c " | ||
| setopt EXTENDED_GLOB | ||
| for rcfile in \"\${HOME}\"/.zprezto/runcoms/^README.md(.N); do | ||
| ln -s \"\$rcfile\" \"\${HOME}/.\${rcfile:t}\" | ||
| done | ||
| " | ||
| runCmdAndLog chsh -s /bin/zsh | ||
| log "ZSH and Prezto installed." | ||
| else | ||
| log "Prezto already installed." | ||
| fi | ||
| else | ||
| log "ERROR - Could not find ZSH even though we tried to install it" | ||
| fi | ||
| } |
There was a problem hiding this comment.
Add a pre-check before installing ZSH.
The step installs ZSH unconditionally; the required pattern calls for checking whether it’s already present.
✅ Possible fix
function stepInstallZshPrezto() {
# https://github.com/sorin-ionescu/prezto
logStep "Installing ZSH and Prezto..."
- runCmdAndLog ${APT_INSTALL} zsh
+ if ! command -v zsh >/dev/null 2>&1; then
+ log "Installing ZSH..."
+ runCmdAndLog ${APT_INSTALL} zsh
+ log "ZSH installed."
+ else
+ log "ZSH already installed."
+ fi
local ZSH_BIN
ZSH_BIN=$(command -v zsh)
local PREZTO_DIR="${HOME}/.zprezto"Based on learnings: Each tool installation must follow the pattern: check if installed, log start, run installation via runCmdAndLog, log completion.
🤖 Prompt for AI Agents
In `@bootstrap.sh` around lines 450 - 485, The stepInstallZshPrezto function
currently calls the installer unconditionally; add a pre-check that uses command
-v zsh (ZSH_BIN) to detect an existing zsh before running runCmdAndLog
${APT_INSTALL} zsh, and only proceed with cloning Prezto and fetching configs if
zsh is missing; specifically, move or add the ZSH_BIN assignment and an early-if
that logs "ZSH already installed." and returns when found, otherwise log the
start, run runCmdAndLog ${APT_INSTALL} zsh, re-evaluate ZSH_BIN and continue
with the existing Prezto install logic (references: stepInstallZshPrezto,
ZSH_BIN, APT_INSTALL, runCmdAndLog).
Summary
stepInstallDocker,stepInstallNeoVim)STEPSarray listing all step functions;TOTAL_STEPSis derived from${#STEPS[@]}for STEP in "${STEPS[@]}"loop[N/TOTAL]progress format — the total is now always accurate since it is computed from the array lengthAdding or removing a step now only requires defining the function and adding/removing one line in the
STEPSarray.Test plan
bash -n bootstrap.shpasses (syntax check)bash run-tests.sh ubuntu:24.04to confirm full bootstrap succeeds[N/13]format throughoutSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.