install-mysql-cli-linux.sh - #3
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 24 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a Linux installer for the GameAP MySQL CLI. It supports configurable options, architecture detection, root-aware paths, concurrent mirror probing, download fallback, executable installation, state directory creation, and CLI verification. ChangesMySQL CLI installer
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Installer as install-mysql-cli-linux.sh
participant Mirrors as Configured mirrors
participant Filesystem
participant CLI as gameap-mysql
Installer->>Mirrors: Probe HTTPS latency
Mirrors-->>Installer: Return mirror results
Installer->>Mirrors: Download release artifact with fallback
Mirrors-->>Installer: Return binary
Installer->>Filesystem: Install executable and create state directory
Installer->>CLI: Verify installation
CLI-->>Installer: Return verification result
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@mysql/install-mysql-cli-linux.sh`:
- Around line 215-234: Update the curl invocation in the mirror download loop to
include a finite --max-time limit, alongside the existing --connect-timeout
option, so stalled transfers eventually fail and fallback proceeds to the next
mirror.
- Around line 70-84: Add explicit OS validation immediately after computing OS
in the installer: accept only Linux, and print a clear unsupported-OS message
followed by exit 1 for any other value. Keep the existing ARCH case validation
and downstream download flow unchanged.
- Around line 86-105: Update resolve_install_dir to record whether the selected
rootless directory is the daemon tools directory or the separate
${HOME}/.local/bin fallback, using a distinct state variable for the two
branches. Update the final PATH note to check that state and only claim
gameap-daemon resolves the binary through its tools PATH when script_dir was
selected; provide the appropriate guidance for the home-local fallback.
- Around line 236-248: Update the artifact installation flow before the install
command in install-mysql-cli-linux.sh to verify TMP_FILE against a trusted
release-channel checksum or signature, sourced independently of the selected
mirror. Abort on verification failure and only proceed to install after
successful integrity validation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6c917ec0-2708-4a81-8a37-3c546e84f5a3
📒 Files selected for processing (1)
mysql/install-mysql-cli-linux.sh
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
gameap/gameap.github.io(manual)
| OS=$(uname -s | tr '[:upper:]' '[:lower:]') | ||
| ARCH=$(uname -m) | ||
|
|
||
| case $ARCH in | ||
| x86_64) | ||
| ARCH="amd64" | ||
| ;; | ||
| aarch64|arm64) | ||
| ARCH="arm64" | ||
| ;; | ||
| *) | ||
| echo "Unsupported architecture: $ARCH" | ||
| exit 1 | ||
| ;; | ||
| esac |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add explicit OS validation.
ARCH is validated with an explicit case/exit 1 (lines 73-84), but OS (line 70) is never checked. If this script runs on a non-Linux platform, it silently builds a BINARY_FILE name for that OS and only fails later with a generic "Failed to download" message (lines 230-234), instead of a clear "unsupported OS" error.
The review-stack context for this layer states the installer "validates the runtime platform." Add the same style of explicit check used for ARCH.
🔧 Proposed fix
OS=$(uname -s | tr '[:upper:]' '[:lower:]')
ARCH=$(uname -m)
+
+if [ "$OS" != "linux" ]; then
+ echo "Unsupported OS: $OS (this script installs the Linux build only)"
+ exit 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.
| OS=$(uname -s | tr '[:upper:]' '[:lower:]') | |
| ARCH=$(uname -m) | |
| case $ARCH in | |
| x86_64) | |
| ARCH="amd64" | |
| ;; | |
| aarch64|arm64) | |
| ARCH="arm64" | |
| ;; | |
| *) | |
| echo "Unsupported architecture: $ARCH" | |
| exit 1 | |
| ;; | |
| esac | |
| OS=$(uname -s | tr '[:upper:]' '[:lower:]') | |
| ARCH=$(uname -m) | |
| if [ "$OS" != "linux" ]; then | |
| echo "Unsupported OS: $OS (this script installs the Linux build only)" | |
| exit 1 | |
| fi | |
| case $ARCH in | |
| x86_64) | |
| ARCH="amd64" | |
| ;; | |
| aarch64|arm64) | |
| ARCH="arm64" | |
| ;; | |
| *) | |
| echo "Unsupported architecture: $ARCH" | |
| exit 1 | |
| ;; | |
| esac |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mysql/install-mysql-cli-linux.sh` around lines 70 - 84, Add explicit OS
validation immediately after computing OS in the installer: accept only Linux,
and print a clear unsupported-OS message followed by exit 1 for any other value.
Keep the existing ARCH case validation and downstream download flow unchanged.
| resolve_install_dir() { | ||
| if [ -n "$INSTALL_DIR" ]; then | ||
| return | ||
| fi | ||
|
|
||
| if [ -w /usr/local/bin ]; then | ||
| INSTALL_DIR="/usr/local/bin" | ||
| return | ||
| fi | ||
|
|
||
| script_dir=$(cd "$(dirname "$0")" && pwd) | ||
| if [ -w "$script_dir" ]; then | ||
| # get-tool drops this script into the daemon tools directory, which | ||
| # gameap-daemon prepends to its PATH — the natural rootless target. | ||
| INSTALL_DIR="$script_dir" | ||
| return | ||
| fi | ||
|
|
||
| INSTALL_DIR="${HOME}/.local/bin" | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Distinguish the two rootless fallback targets before printing the PATH note.
resolve_install_dir has three outcomes: /usr/local/bin, script_dir (the daemon tools directory, which the header comment says gameap-daemon puts on its PATH), and ${HOME}/.local/bin (not documented anywhere as being on the daemon's PATH). The function does not record which of the last two was chosen.
Downstream, the final note (lines 246-248) only checks [ "$INSTALL_DIR" != "/usr/local/bin" ] and prints the same "gameap-daemon resolves the binary by name via its tools PATH" message for both non-root outcomes. That claim is incorrect when INSTALL_DIR falls back to ${HOME}/.local/bin}, since that directory is not the daemon tools directory. An operator relying on this message could believe the daemon will find the binary when it will not.
Track which fallback branch is taken so the downstream message can differentiate the two cases.
🔧 Proposed fix
+INSTALL_DIR_IS_DAEMON_TOOLS_DIR=""
resolve_install_dir() {
if [ -n "$INSTALL_DIR" ]; then
return
fi
if [ -w /usr/local/bin ]; then
INSTALL_DIR="/usr/local/bin"
return
fi
script_dir=$(cd "$(dirname "$0")" && pwd)
if [ -w "$script_dir" ]; then
# get-tool drops this script into the daemon tools directory, which
# gameap-daemon prepends to its PATH — the natural rootless target.
INSTALL_DIR="$script_dir"
+ INSTALL_DIR_IS_DAEMON_TOOLS_DIR="1"
return
fi
INSTALL_DIR="${HOME}/.local/bin"
}Then at lines 246-248:
-if [ "$INSTALL_DIR" != "/usr/local/bin" ]; then
- echo "Note: rootless install — gameap-daemon resolves the binary by name via its tools PATH."
-fi
+if [ "$INSTALL_DIR" = "/usr/local/bin" ] || [ -n "$INSTALL_DIR_IS_DAEMON_TOOLS_DIR" ]; then
+ :
+else
+ echo "Note: rootless install to ${INSTALL_DIR}, which is not on gameap-daemon's tools PATH."
+ echo "Add it to PATH, or re-run with --install-dir set to the daemon tools directory."
+fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mysql/install-mysql-cli-linux.sh` around lines 86 - 105, Update
resolve_install_dir to record whether the selected rootless directory is the
daemon tools directory or the separate ${HOME}/.local/bin fallback, using a
distinct state variable for the two branches. Update the final PATH note to
check that state and only claim gameap-daemon resolves the binary through its
tools PATH when script_dir was selected; provide the appropriate guidance for
the home-local fallback.
| TMP_FILE=$(mktemp /tmp/gameap-mysql.XXXXXX) | ||
| trap 'rm -f "$TMP_FILE"' EXIT | ||
|
|
||
| echo "Downloading gameap-mysql v${GAMEAP_MYSQL_VERSION} (${OS}-${ARCH})..." | ||
|
|
||
| downloaded="" | ||
| for mirror_url in "${ordered_mirrors[@]}"; do | ||
| echo "Downloading from $(_url_host "${mirror_url}")..." | ||
| if curl -fsSL --connect-timeout 10 -o "$TMP_FILE" "${mirror_url}"; then | ||
| downloaded="1" | ||
| break | ||
| fi | ||
| echo "Failed to download from ${mirror_url}, trying the next mirror..." >&2 | ||
| done | ||
|
|
||
| if [[ -z "${downloaded}" ]]; then | ||
| echo "Failed to download gameap-mysql. Mirrors tried:" >&2 | ||
| printf ' - %s\n' "${ordered_mirrors[@]}" >&2 | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Add a total time limit to the download request.
The probe call (_probe_mirror_latency, line 139) sets both --connect-timeout 5 and --max-time 10. The actual download at line 223 sets only --connect-timeout 10. --connect-timeout bounds connection setup only; a mirror that accepts the connection but stalls or trickles data mid-transfer can hang this call indefinitely, blocking the whole install (and the daemon task chain that invokes it), since sequential fallback to the next mirror never triggers.
Add --max-time to the download call as well.
🔧 Proposed fix
- if curl -fsSL --connect-timeout 10 -o "$TMP_FILE" "${mirror_url}"; then
+ if curl -fsSL --connect-timeout 10 --max-time 120 -o "$TMP_FILE" "${mirror_url}"; then📝 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.
| TMP_FILE=$(mktemp /tmp/gameap-mysql.XXXXXX) | |
| trap 'rm -f "$TMP_FILE"' EXIT | |
| echo "Downloading gameap-mysql v${GAMEAP_MYSQL_VERSION} (${OS}-${ARCH})..." | |
| downloaded="" | |
| for mirror_url in "${ordered_mirrors[@]}"; do | |
| echo "Downloading from $(_url_host "${mirror_url}")..." | |
| if curl -fsSL --connect-timeout 10 -o "$TMP_FILE" "${mirror_url}"; then | |
| downloaded="1" | |
| break | |
| fi | |
| echo "Failed to download from ${mirror_url}, trying the next mirror..." >&2 | |
| done | |
| if [[ -z "${downloaded}" ]]; then | |
| echo "Failed to download gameap-mysql. Mirrors tried:" >&2 | |
| printf ' - %s\n' "${ordered_mirrors[@]}" >&2 | |
| exit 1 | |
| fi | |
| TMP_FILE=$(mktemp /tmp/gameap-mysql.XXXXXX) | |
| trap 'rm -f "$TMP_FILE"' EXIT | |
| echo "Downloading gameap-mysql v${GAMEAP_MYSQL_VERSION} (${OS}-${ARCH})..." | |
| downloaded="" | |
| for mirror_url in "${ordered_mirrors[@]}"; do | |
| echo "Downloading from $(_url_host "${mirror_url}")..." | |
| if curl -fsSL --connect-timeout 10 --max-time 120 -o "$TMP_FILE" "${mirror_url}"; then | |
| downloaded="1" | |
| break | |
| fi | |
| echo "Failed to download from ${mirror_url}, trying the next mirror..." >&2 | |
| done | |
| if [[ -z "${downloaded}" ]]; then | |
| echo "Failed to download gameap-mysql. Mirrors tried:" >&2 | |
| printf ' - %s\n' "${ordered_mirrors[@]}" >&2 | |
| exit 1 | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mysql/install-mysql-cli-linux.sh` around lines 215 - 234, Update the curl
invocation in the mirror download loop to include a finite --max-time limit,
alongside the existing --connect-timeout option, so stalled transfers eventually
fail and fallback proceeds to the next mirror.
| mkdir -p "$INSTALL_DIR" | ||
| install -m 0755 "$TMP_FILE" "${INSTALL_DIR}/gameap-mysql" | ||
|
|
||
| install -d -m 0700 "$STATE_DIR" | ||
|
|
||
| echo "Verifying installation..." | ||
| "${INSTALL_DIR}/gameap-mysql" version --json | ||
|
|
||
| echo "gameap-mysql v${GAMEAP_MYSQL_VERSION} installed to ${INSTALL_DIR}/gameap-mysql (state: ${STATE_DIR})" | ||
|
|
||
| if [ "$INSTALL_DIR" != "/usr/local/bin" ]; then | ||
| echo "Note: rootless install — gameap-daemon resolves the binary by name via its tools PATH." | ||
| fi |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
# Example verification pattern once a checksum is published alongside the binary
curl -fsSL --connect-timeout 10 --max-time 30 -o "${TMP_FILE}.sha256" "${mirror_url}.sha256"
echo "$(cat "${TMP_FILE}.sha256") ${TMP_FILE}" | sha256sum -c -Repository: gameap/scripts
Length of output: 288
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- installer context ---'
sed -n '1,280p' mysql/install-mysql-cli-linux.sh
printf '%s\n' '--- release and checksum references ---'
rg -n -i 'gameap-mysql|sha256|checksum|checksums|signature|cosign|gpg|release|mirror|cdn' \
--glob '!*.min.*' --glob '!node_modules/**' . || true
printf '%s\n' '--- repository files related to release automation ---'
git ls-files | rg -i '(^|/)(release|publish|build|package|artifact|workflow)|mysql|checksum|sha256|sig|\.github' || trueRepository: gameap/scripts
Length of output: 14440
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- related repository references ---'
rg -n -i 'gameap-mysql|sha256|checksum|checksums|signature|cosign|gpg|release|mirror|cdn' \
--glob '!*.min.*' --glob '!node_modules/**' . || trueRepository: gameap/gameap.github.io
Length of output: 2259
🏁 Script executed (no clone):
#!/bin/bash
set -u
printf '%s\n' '--- public release metadata ---'
curl -fsSL --connect-timeout 10 --max-time 30 \
https://api.github.com/repos/gameap/gameap/releases?per_page=10 \
| jq -r '.[] | [.tag_name, .name, (.assets[]?.name // ""), (.assets[]?.browser_download_url // "")] | `@tsv`' || true
printf '%s\n' '--- public repository metadata ---'
curl -fsSL --connect-timeout 10 --max-time 30 \
https://api.github.com/repos/gameap/scripts/contents/mysql/install-mysql-cli-linux.sh \
| jq -r '.download_url, .html_url' || true
Length of output: 18901
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- installer-specific URLs and artifact handling ---'
rg -n -C 8 'TMP_FILE|mirror_url|curl|wget|GAMEAP_MYSQL_VERSION|INSTALL_DIR|STATE_DIR|install ' \
mysql/install-mysql-cli-linux.sh || true
printf '%s\n' '--- workflow and release files ---'
find .github -maxdepth 3 -type f -print 2>/dev/null | sort | while read -r f; do
printf '\n### %s\n' "$f"
sed -n '1,260p' "$f"
doneRepository: gameap/scripts
Length of output: 8278
🏁 Script executed (no clone):
#!/bin/bash
set -u
repo='gameap/gameap-mysql'
printf '%s\n' '--- gameap-mysql releases and assets ---'
if curl -fsSL --connect-timeout 10 --max-time 30 \
"https://api.github.com/repos/${repo}/releases?per_page=10" |
jq -r '.[] | [.tag_name, .name, (.assets[]?.name // "")] | `@tsv`'; then
:
else
printf '%s\n' 'Release API request failed'
fi
printf '%s\n' '--- repository and tag metadata ---'
curl -fsSL --connect-timeout 10 --max-time 30 \
"https://api.github.com/repos/${repo}" |
jq -r '[.html_url, .default_branch, .archived] | `@tsv`' || true
curl -fsSL --connect-timeout 10 --max-time 30 \
"https://api.github.com/repos/${repo}/tags?per_page=10" |
jq -r '.[] | [.name, .commit.sha] | `@tsv`' || true
printf '%s\n' '--- expected checksum endpoints for version 0.1.0 ---'
for host in \
'https://github.com/gameap/gameap-mysql/releases/download/0.1.0' \
'https://cdn.gameap.com/gameap-mysql/0.1.0' \
'https://cdn.gameap.ru/gameap-mysql/0.1.0'
do
url="${host}/gameap-mysql-0.1.0-linux-amd64.sha256"
status=$(curl -sS -o /tmp/gameap-mysql-checksum.$$ -w '%{http_code}' \
--connect-timeout 10 --max-time 30 "$url" || true)
printf '%s\t%s\t' "$status" "$url"
if [ -s /tmp/gameap-mysql-checksum.$$ ]; then
tr '\n' ' ' < /tmp/gameap-mysql-checksum.$$
fi
printf '\n'
done
rm -f /tmp/gameap-mysql-checksum.$$
Length of output: 946
Add trusted artifact integrity verification before installation
The script executes the first successful artifact from a mirror without verification. Publish a checksum or signature in a trusted release channel and verify it before install. Do not obtain the verification value only from the selected mirror.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mysql/install-mysql-cli-linux.sh` around lines 236 - 248, Update the artifact
installation flow before the install command in install-mysql-cli-linux.sh to
verify TMP_FILE against a trusted release-channel checksum or signature, sourced
independently of the selected mirror. Abort on verification failure and only
proceed to install after successful integrity validation.
Summary by CodeRabbit