From da8c841ef4c887eea844b02187dbb38ed1dc57b1 Mon Sep 17 00:00:00 2001 From: Tommaso Sciortino Date: Sat, 28 Mar 2026 19:47:05 -0700 Subject: [PATCH] fix: shellcheck warnings in scripts (#24035) --- .../async-pr-review/scripts/async-review.sh | 234 +++++++++--------- .../scripts/check-async-review.sh | 46 ++-- scripts/harvest_api_reliability.sh | 54 ++-- scripts/review.sh | 93 +++---- 4 files changed, 219 insertions(+), 208 deletions(-) diff --git a/.gemini/skills/async-pr-review/scripts/async-review.sh b/.gemini/skills/async-pr-review/scripts/async-review.sh index d408c5f2f1..9977a6f907 100755 --- a/.gemini/skills/async-pr-review/scripts/async-review.sh +++ b/.gemini/skills/async-pr-review/scripts/async-review.sh @@ -1,190 +1,194 @@ #!/bin/bash notify() { - local title="$1" - local message="$2" - local pr="$3" + local title="${1}" + local message="${2}" + local pr="${3}" # Terminal escape sequence - printf "\e]9;%s | PR #%s | %s\a" "$title" "$pr" "$message" + printf "\e]9;%s | PR #%s | %s\a" "${title}" "${pr}" "${message}" # Native macOS notification - if [[ "$(uname)" == "Darwin" ]]; then - osascript -e "display notification \"$message\" with title \"$title\" subtitle \"PR #$pr\"" + os_type="$(uname || true)" + if [[ "${os_type}" == "Darwin" ]]; then + osascript -e "display notification \"${message}\" with title \"${title}\" subtitle \"PR #${pr}\"" fi } -pr_number=$1 -if [[ -z "$pr_number" ]]; then +pr_number="${1}" +if [[ -z "${pr_number}" ]]; then echo "Usage: async-review " exit 1 fi -base_dir=$(git rev-parse --show-toplevel 2>/dev/null) -if [[ -z "$base_dir" ]]; then +base_dir="$(git rev-parse --show-toplevel 2>/dev/null || true)" +if [[ -z "${base_dir}" ]]; then echo "โŒ Must be run from within a git repository." exit 1 fi # Use the repository's local .gemini/tmp directory for ephemeral worktrees and logs -pr_dir="$base_dir/.gemini/tmp/async-reviews/pr-$pr_number" -target_dir="$pr_dir/worktree" -log_dir="$pr_dir/logs" +pr_dir="${base_dir}/.gemini/tmp/async-reviews/pr-${pr_number}" +target_dir="${pr_dir}/worktree" +log_dir="${pr_dir}/logs" -cd "$base_dir" || exit 1 +cd "${base_dir}" || exit 1 -mkdir -p "$log_dir" -rm -f "$log_dir/setup.exit" "$log_dir/final-assessment.exit" "$log_dir/final-assessment.md" +mkdir -p "${log_dir}" +rm -f "${log_dir}/setup.exit" "${log_dir}/final-assessment.exit" "${log_dir}/final-assessment.md" -echo "๐Ÿงน Cleaning up previous worktree if it exists..." | tee -a "$log_dir/setup.log" -git worktree remove -f "$target_dir" >> "$log_dir/setup.log" 2>&1 || true -git branch -D "gemini-async-pr-$pr_number" >> "$log_dir/setup.log" 2>&1 || true -git worktree prune >> "$log_dir/setup.log" 2>&1 || true +echo "๐Ÿงน Cleaning up previous worktree if it exists..." | tee -a "${log_dir}/setup.log" +git worktree remove -f "${target_dir}" >> "${log_dir}/setup.log" 2>&1 || true +git branch -D "gemini-async-pr-${pr_number}" >> "${log_dir}/setup.log" 2>&1 || true +git worktree prune >> "${log_dir}/setup.log" 2>&1 || true -echo "๐Ÿ“ก Fetching PR #$pr_number..." | tee -a "$log_dir/setup.log" -if ! git fetch origin -f "pull/$pr_number/head:gemini-async-pr-$pr_number" >> "$log_dir/setup.log" 2>&1; then - echo 1 > "$log_dir/setup.exit" - echo "โŒ Fetch failed. Check $log_dir/setup.log" - notify "Async Review Failed" "Fetch failed." "$pr_number" +echo "๐Ÿ“ก Fetching PR #${pr_number}..." | tee -a "${log_dir}/setup.log" +if ! git fetch origin -f "pull/${pr_number}/head:gemini-async-pr-${pr_number}" >> "${log_dir}/setup.log" 2>&1; then + echo 1 > "${log_dir}/setup.exit" + echo "โŒ Fetch failed. Check ${log_dir}/setup.log" + notify "Async Review Failed" "Fetch failed." "${pr_number}" exit 1 fi -if [[ ! -d "$target_dir" ]]; then - echo "๐Ÿงน Pruning missing worktrees..." | tee -a "$log_dir/setup.log" - git worktree prune >> "$log_dir/setup.log" 2>&1 - echo "๐ŸŒฟ Creating worktree in $target_dir..." | tee -a "$log_dir/setup.log" - if ! git worktree add "$target_dir" "gemini-async-pr-$pr_number" >> "$log_dir/setup.log" 2>&1; then - echo 1 > "$log_dir/setup.exit" - echo "โŒ Worktree creation failed. Check $log_dir/setup.log" - notify "Async Review Failed" "Worktree creation failed." "$pr_number" +if [[ ! -d "${target_dir}" ]]; then + echo "๐Ÿงน Pruning missing worktrees..." | tee -a "${log_dir}/setup.log" + git worktree prune >> "${log_dir}/setup.log" 2>&1 + echo "๐ŸŒฟ Creating worktree in ${target_dir}..." | tee -a "${log_dir}/setup.log" + if ! git worktree add "${target_dir}" "gemini-async-pr-${pr_number}" >> "${log_dir}/setup.log" 2>&1; then + echo 1 > "${log_dir}/setup.exit" + echo "โŒ Worktree creation failed. Check ${log_dir}/setup.log" + notify "Async Review Failed" "Worktree creation failed." "${pr_number}" exit 1 fi else - echo "๐ŸŒฟ Worktree already exists." | tee -a "$log_dir/setup.log" + echo "๐ŸŒฟ Worktree already exists." | tee -a "${log_dir}/setup.log" fi -echo 0 > "$log_dir/setup.exit" +echo 0 > "${log_dir}/setup.exit" -cd "$target_dir" || exit 1 +cd "${target_dir}" || exit 1 -echo "๐Ÿš€ Launching background tasks. Logs saving to: $log_dir" +echo "๐Ÿš€ Launching background tasks. Logs saving to: ${log_dir}" echo " โ†ณ [1/5] Grabbing PR diff..." -rm -f "$log_dir/pr-diff.exit" -{ gh pr diff "$pr_number" > "$log_dir/pr-diff.diff" 2>&1; echo $? > "$log_dir/pr-diff.exit"; } & +rm -f "${log_dir}/pr-diff.exit" +{ gh pr diff "${pr_number}" > "${log_dir}/pr-diff.diff" 2>&1; echo $? > "${log_dir}/pr-diff.exit"; } & echo " โ†ณ [2/5] Starting build and lint..." -rm -f "$log_dir/build-and-lint.exit" -{ { npm run clean && npm ci && npm run format && npm run build && npm run lint:ci && npm run typecheck; } > "$log_dir/build-and-lint.log" 2>&1; echo $? > "$log_dir/build-and-lint.exit"; } & +rm -f "${log_dir}/build-and-lint.exit" +{ { npm run clean && npm ci && npm run format && npm run build && npm run lint:ci && npm run typecheck; } > "${log_dir}/build-and-lint.log" 2>&1; echo $? > "${log_dir}/build-and-lint.exit"; } & # Dynamically resolve gemini binary (fallback to your nightly path) -GEMINI_CMD=$(which gemini || echo "$HOME/.gcli/nightly/node_modules/.bin/gemini") +GEMINI_CMD="$(command -v gemini || echo "${HOME}/.gcli/nightly/node_modules/.bin/gemini")" +# shellcheck disable=SC2312 POLICY_PATH="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)/policy.toml" echo " โ†ณ [3/5] Starting Gemini code review..." -rm -f "$log_dir/review.exit" -{ "$GEMINI_CMD" --policy "$POLICY_PATH" -p "/review-frontend $pr_number" > "$log_dir/review.md" 2>&1; echo $? > "$log_dir/review.exit"; } & +rm -f "${log_dir}/review.exit" +{ "${GEMINI_CMD}" --policy "${POLICY_PATH}" -p "/review-frontend ${pr_number}" > "${log_dir}/review.md" 2>&1; echo $? > "${log_dir}/review.exit"; } & echo " โ†ณ [4/5] Starting automated tests (waiting for build and lint)..." -rm -f "$log_dir/npm-test.exit" +rm -f "${log_dir}/npm-test.exit" { - while [ ! -f "$log_dir/build-and-lint.exit" ]; do sleep 1; done - if [ "$(cat "$log_dir/build-and-lint.exit")" == "0" ]; then - gh pr checks "$pr_number" > "$log_dir/ci-checks.log" 2>&1 + while [[ ! -f "${log_dir}/build-and-lint.exit" ]]; do sleep 1; done + read -r build_exit < "${log_dir}/build-and-lint.exit" || build_exit="" + if [[ "${build_exit}" == "0" ]]; then + gh pr checks "${pr_number}" > "${log_dir}/ci-checks.log" 2>&1 ci_status=$? - if [ "$ci_status" -eq 0 ]; then - echo "CI checks passed. Skipping local npm tests." > "$log_dir/npm-test.log" - echo 0 > "$log_dir/npm-test.exit" - elif [ "$ci_status" -eq 8 ]; then - echo "CI checks are still pending. Skipping local npm tests to avoid duplicate work. Please check GitHub for final results." > "$log_dir/npm-test.log" - echo 0 > "$log_dir/npm-test.exit" + if [[ "${ci_status}" -eq 0 ]]; then + echo "CI checks passed. Skipping local npm tests." > "${log_dir}/npm-test.log" + echo 0 > "${log_dir}/npm-test.exit" + elif [[ "${ci_status}" -eq 8 ]]; then + echo "CI checks are still pending. Skipping local npm tests to avoid duplicate work. Please check GitHub for final results." > "${log_dir}/npm-test.log" + echo 0 > "${log_dir}/npm-test.exit" else - echo "CI checks failed. Failing checks:" > "$log_dir/npm-test.log" - gh pr checks "$pr_number" --json name,bucket -q '.[] | select(.bucket=="fail") | .name' >> "$log_dir/npm-test.log" 2>&1 + echo "CI checks failed. Failing checks:" > "${log_dir}/npm-test.log" + gh pr checks "${pr_number}" --json name,bucket -q '.[] | select(.bucket=="fail") | .name' >> "${log_dir}/npm-test.log" 2>&1 - echo "Attempting to extract failing test files from CI logs..." >> "$log_dir/npm-test.log" - pr_branch=$(gh pr view "$pr_number" --json headRefName -q '.headRefName' 2>/dev/null) - run_id=$(gh run list --branch "$pr_branch" --workflow ci.yml --json databaseId -q '.[0].databaseId' 2>/dev/null) + echo "Attempting to extract failing test files from CI logs..." >> "${log_dir}/npm-test.log" + pr_branch="$(gh pr view "${pr_number}" --json headRefName -q '.headRefName' 2>/dev/null || true)" + run_id="$(gh run list --branch "${pr_branch}" --workflow ci.yml --json databaseId -q '.[0].databaseId' 2>/dev/null || true)" failed_files="" - if [[ -n "$run_id" ]]; then - failed_files=$(gh run view "$run_id" --log-failed 2>/dev/null | grep -o -E '(packages/[a-zA-Z0-9_-]+|integration-tests|evals)/[a-zA-Z0-9_/-]+\.test\.ts(x)?' | sort | uniq) + if [[ -n "${run_id}" ]]; then + failed_files="$(gh run view "${run_id}" --log-failed 2>/dev/null | grep -o -E '(packages/[a-zA-Z0-9_-]+|integration-tests|evals)/[a-zA-Z0-9_/-]+\.test\.ts(x)?' | sort | uniq || true)" fi - if [[ -n "$failed_files" ]]; then - echo "Found failing test files from CI:" >> "$log_dir/npm-test.log" - for f in $failed_files; do echo " - $f" >> "$log_dir/npm-test.log"; done - echo "Running ONLY failing tests locally..." >> "$log_dir/npm-test.log" + if [[ -n "${failed_files}" ]]; then + echo "Found failing test files from CI:" >> "${log_dir}/npm-test.log" + for f in ${failed_files}; do echo " - ${f}" >> "${log_dir}/npm-test.log"; done + echo "Running ONLY failing tests locally..." >> "${log_dir}/npm-test.log" exit_code=0 - for file in $failed_files; do - if [[ "$file" == packages/* ]]; then - ws_dir=$(echo "$file" | cut -d'/' -f1,2) + for file in ${failed_files}; do + if [[ "${file}" == packages/* ]]; then + ws_dir="$(echo "${file}" | cut -d'/' -f1,2)" else - ws_dir=$(echo "$file" | cut -d'/' -f1) + ws_dir="$(echo "${file}" | cut -d'/' -f1)" fi - rel_file=${file#$ws_dir/} + rel_file="${file#"${ws_dir}"/}" - echo "--- Running $rel_file in workspace $ws_dir ---" >> "$log_dir/npm-test.log" - if ! npm run test:ci -w "$ws_dir" -- "$rel_file" >> "$log_dir/npm-test.log" 2>&1; then + echo "--- Running ${rel_file} in workspace ${ws_dir} ---" >> "${log_dir}/npm-test.log" + if ! npm run test:ci -w "${ws_dir}" -- "${rel_file}" >> "${log_dir}/npm-test.log" 2>&1; then exit_code=1 fi done - echo $exit_code > "$log_dir/npm-test.exit" + echo "${exit_code}" > "${log_dir}/npm-test.exit" else - echo "Could not extract specific failing files. Skipping full local test suite as it takes too long. Please check CI logs manually." >> "$log_dir/npm-test.log" - echo 1 > "$log_dir/npm-test.exit" + echo "Could not extract specific failing files. Skipping full local test suite as it takes too long. Please check CI logs manually." >> "${log_dir}/npm-test.log" + echo 1 > "${log_dir}/npm-test.exit" fi fi else - echo "Skipped due to build-and-lint failure" > "$log_dir/npm-test.log" - echo 1 > "$log_dir/npm-test.exit" + echo "Skipped due to build-and-lint failure" > "${log_dir}/npm-test.log" + echo 1 > "${log_dir}/npm-test.exit" fi } & echo " โ†ณ [5/5] Starting Gemini test execution (waiting for build and lint)..." -rm -f "$log_dir/test-execution.exit" +rm -f "${log_dir}/test-execution.exit" { - while [ ! -f "$log_dir/build-and-lint.exit" ]; do sleep 1; done - if [ "$(cat "$log_dir/build-and-lint.exit")" == "0" ]; then - "$GEMINI_CMD" --policy "$POLICY_PATH" -p "Analyze the diff for PR $pr_number using 'gh pr diff $pr_number'. Instead of running the project's automated test suite (like 'npm test'), physically exercise the newly changed code in the terminal (e.g., by writing a temporary script to call the new functions, or testing the CLI command directly). Verify the feature's behavior works as expected. IMPORTANT: Do NOT modify any source code to fix errors. Just exercise the code and log the results, reporting any failures clearly. Do not ask for user confirmation." > "$log_dir/test-execution.log" 2>&1; echo $? > "$log_dir/test-execution.exit" + while [[ ! -f "${log_dir}/build-and-lint.exit" ]]; do sleep 1; done + read -r build_exit < "${log_dir}/build-and-lint.exit" || build_exit="" + if [[ "${build_exit}" == "0" ]]; then + "${GEMINI_CMD}" --policy "${POLICY_PATH}" -p "Analyze the diff for PR ${pr_number} using 'gh pr diff ${pr_number}'. Instead of running the project's automated test suite (like 'npm test'), physically exercise the newly changed code in the terminal (e.g., by writing a temporary script to call the new functions, or testing the CLI command directly). Verify the feature's behavior works as expected. IMPORTANT: Do NOT modify any source code to fix errors. Just exercise the code and log the results, reporting any failures clearly. Do not ask for user confirmation." > "${log_dir}/test-execution.log" 2>&1; echo $? > "${log_dir}/test-execution.exit" else - echo "Skipped due to build-and-lint failure" > "$log_dir/test-execution.log" - echo 1 > "$log_dir/test-execution.exit" + echo "Skipped due to build-and-lint failure" > "${log_dir}/test-execution.log" + echo 1 > "${log_dir}/test-execution.exit" fi } & echo "โœ… All tasks dispatched!" -echo "You can monitor progress with: tail -f $log_dir/*.log" -echo "Read your review later at: $log_dir/review.md" +echo "You can monitor progress with: tail -f ${log_dir}/*.log" +echo "Read your review later at: ${log_dir}/review.md" # Polling loop to wait for all background tasks to finish tasks=("pr-diff" "build-and-lint" "review" "npm-test" "test-execution") log_files=("pr-diff.diff" "build-and-lint.log" "review.md" "npm-test.log" "test-execution.log") declare -A task_done -for t in "${tasks[@]}"; do task_done[$t]=0; done +for t in "${tasks[@]}"; do task_done[${t}]=0; done all_done=0 -while [[ $all_done -eq 0 ]]; do +while [[ "${all_done}" -eq 0 ]]; do clear echo "==================================================" - echo "๐Ÿš€ Async PR Review Status for PR #$pr_number" + echo "๐Ÿš€ Async PR Review Status for PR #${pr_number}" echo "==================================================" echo "" all_done=1 for i in "${!tasks[@]}"; do - t="${tasks[$i]}" + t="${tasks[${i}]}" - if [[ -f "$log_dir/$t.exit" ]]; then - exit_code=$(cat "$log_dir/$t.exit") - if [[ "$exit_code" == "0" ]]; then - echo " โœ… $t: SUCCESS" + if [[ -f "${log_dir}/${t}.exit" ]]; then + read -r task_exit < "${log_dir}/${t}.exit" || task_exit="" + if [[ "${task_exit}" == "0" ]]; then + echo " โœ… ${t}: SUCCESS" else - echo " โŒ $t: FAILED (exit code $exit_code)" + echo " โŒ ${t}: FAILED (exit code ${task_exit})" fi - task_done[$t]=1 + task_done[${t}]=1 else - echo " โณ $t: RUNNING" + echo " โณ ${t}: RUNNING" all_done=0 fi done @@ -195,47 +199,47 @@ while [[ $all_done -eq 0 ]]; do echo "==================================================" for i in "${!tasks[@]}"; do - t="${tasks[$i]}" - log_file="${log_files[$i]}" + t="${tasks[${i}]}" + log_file="${log_files[${i}]}" - if [[ ${task_done[$t]} -eq 0 ]]; then - if [[ -f "$log_dir/$log_file" ]]; then + if [[ "${task_done[${t}]}" -eq 0 ]]; then + if [[ -f "${log_dir}/${log_file}" ]]; then echo "" - echo "--- $t ---" - tail -n 5 "$log_dir/$log_file" + echo "--- ${t} ---" + tail -n 5 "${log_dir}/${log_file}" fi fi done - if [[ $all_done -eq 0 ]]; then + if [[ "${all_done}" -eq 0 ]]; then sleep 3 fi done clear echo "==================================================" -echo "๐Ÿš€ Async PR Review Status for PR #$pr_number" +echo "๐Ÿš€ Async PR Review Status for PR #${pr_number}" echo "==================================================" echo "" for t in "${tasks[@]}"; do - exit_code=$(cat "$log_dir/$t.exit") - if [[ "$exit_code" == "0" ]]; then - echo " โœ… $t: SUCCESS" + read -r task_exit < "${log_dir}/${t}.exit" || task_exit="" + if [[ "${task_exit}" == "0" ]]; then + echo " โœ… ${t}: SUCCESS" else - echo " โŒ $t: FAILED (exit code $exit_code)" + echo " โŒ ${t}: FAILED (exit code ${task_exit})" fi done echo "" echo "โณ Tasks complete! Synthesizing final assessment..." -if ! "$GEMINI_CMD" --policy "$POLICY_PATH" -p "Read the review at $log_dir/review.md, the automated test logs at $log_dir/npm-test.log, and the manual test execution logs at $log_dir/test-execution.log. Summarize the results, state whether the build and tests passed based on $log_dir/build-and-lint.exit and $log_dir/npm-test.exit, and give a final recommendation for PR $pr_number." > "$log_dir/final-assessment.md" 2>&1; then - echo $? > "$log_dir/final-assessment.exit" +if ! "${GEMINI_CMD}" --policy "${POLICY_PATH}" -p "Read the review at ${log_dir}/review.md, the automated test logs at ${log_dir}/npm-test.log, and the manual test execution logs at ${log_dir}/test-execution.log. Summarize the results, state whether the build and tests passed based on ${log_dir}/build-and-lint.exit and ${log_dir}/npm-test.exit, and give a final recommendation for PR ${pr_number}." > "${log_dir}/final-assessment.md" 2>&1; then + echo $? > "${log_dir}/final-assessment.exit" echo "โŒ Final assessment synthesis failed!" - echo "Check $log_dir/final-assessment.md for details." - notify "Async Review Failed" "Final assessment synthesis failed." "$pr_number" + echo "Check ${log_dir}/final-assessment.md for details." + notify "Async Review Failed" "Final assessment synthesis failed." "${pr_number}" exit 1 fi -echo 0 > "$log_dir/final-assessment.exit" -echo "โœ… Final assessment complete! Check $log_dir/final-assessment.md" -notify "Async Review Complete" "Review and test execution finished successfully." "$pr_number" +echo 0 > "${log_dir}/final-assessment.exit" +echo "โœ… Final assessment complete! Check ${log_dir}/final-assessment.md" +notify "Async Review Complete" "Review and test execution finished successfully." "${pr_number}" diff --git a/.gemini/skills/async-pr-review/scripts/check-async-review.sh b/.gemini/skills/async-pr-review/scripts/check-async-review.sh index fbb58c2b72..846f703237 100755 --- a/.gemini/skills/async-pr-review/scripts/check-async-review.sh +++ b/.gemini/skills/async-pr-review/scripts/check-async-review.sh @@ -1,22 +1,22 @@ #!/bin/bash -pr_number=$1 +pr_number="${1}" -if [[ -z "$pr_number" ]]; then +if [[ -z "${pr_number}" ]]; then echo "Usage: check-async-review " exit 1 fi -base_dir=$(git rev-parse --show-toplevel 2>/dev/null) -if [[ -z "$base_dir" ]]; then +base_dir="$(git rev-parse --show-toplevel 2>/dev/null || true)" +if [[ -z "${base_dir}" ]]; then echo "โŒ Must be run from within a git repository." exit 1 fi -log_dir="$base_dir/.gemini/tmp/async-reviews/pr-$pr_number/logs" +log_dir="${base_dir}/.gemini/tmp/async-reviews/pr-${pr_number}/logs" -if [[ ! -d "$log_dir" ]]; then +if [[ ! -d "${log_dir}" ]]; then echo "STATUS: NOT_FOUND" - echo "โŒ No logs found for PR #$pr_number in $log_dir" + echo "โŒ No logs found for PR #${pr_number} in ${log_dir}" exit 0 fi @@ -34,32 +34,32 @@ all_done=true echo "STATUS: CHECKING" for task_info in "${tasks[@]}"; do - IFS="|" read -r task_name log_file <<< "$task_info" + IFS="|" read -r task_name log_file <<< "${task_info}" - file_path="$log_dir/$log_file" - exit_file="$log_dir/$task_name.exit" + file_path="${log_dir}/${log_file}" + exit_file="${log_dir}/${task_name}.exit" - if [[ -f "$exit_file" ]]; then - exit_code=$(cat "$exit_file") - if [[ "$exit_code" == "0" ]]; then - echo "โœ… $task_name: SUCCESS" + if [[ -f "${exit_file}" ]]; then + read -r exit_code < "${exit_file}" || exit_code="" + if [[ "${exit_code}" == "0" ]]; then + echo "โœ… ${task_name}: SUCCESS" else - echo "โŒ $task_name: FAILED (exit code $exit_code)" - echo " Last lines of $file_path:" - tail -n 3 "$file_path" | sed 's/^/ /' + echo "โŒ ${task_name}: FAILED (exit code ${exit_code})" + echo " Last lines of ${file_path}:" + tail -n 3 "${file_path}" | sed 's/^/ /' || true fi - elif [[ -f "$file_path" ]]; then - echo "โณ $task_name: RUNNING" + elif [[ -f "${file_path}" ]]; then + echo "โณ ${task_name}: RUNNING" all_done=false else - echo "โž– $task_name: NOT STARTED" + echo "โž– ${task_name}: NOT STARTED" all_done=false fi done -if $all_done; then +if [[ "${all_done}" == "true" ]]; then echo "STATUS: COMPLETE" - echo "LOG_DIR: $log_dir" + echo "LOG_DIR: ${log_dir}" else echo "STATUS: IN_PROGRESS" -fi \ No newline at end of file +fi diff --git a/scripts/harvest_api_reliability.sh b/scripts/harvest_api_reliability.sh index 140063b8ea..383523912f 100755 --- a/scripts/harvest_api_reliability.sh +++ b/scripts/harvest_api_reliability.sh @@ -19,18 +19,20 @@ # - jq installed # Arguments & Defaults -if [[ -n "$1" && $1 =~ ^[0-9]{4}-[0-9]{2}-[0-9]{2}$ ]]; then - SINCE="$1" -elif [[ -n "$1" && $1 =~ ^([0-9]+)d$ ]]; then +if [[ -n "${1}" && "${1}" =~ ^[0-9]{4}-[0-9]{2}-[0-9]{2}$ ]]; then + SINCE="${1}" +elif [[ -n "${1}" && "${1}" =~ ^([0-9]+)d$ ]]; then DAYS="${BASH_REMATCH[1]}" - if [[ "$OSTYPE" == "darwin"* ]]; then + os_type="$(uname || true)" + if [[ "${os_type}" == "darwin"* ]]; then SINCE=$(date -u -v-"${DAYS}"d +%Y-%m-%d) else SINCE=$(date -u -d "${DAYS} days ago" +%Y-%m-%d) fi else # Default to 7 days ago in YYYY-MM-DD format (UTC) - if [[ "$OSTYPE" == "darwin"* ]]; then + os_type="$(uname || true)" + if [[ "${os_type}" == "darwin"* ]]; then SINCE=$(date -u -v-7d +%Y-%m-%d) else SINCE=$(date -u -d "7 days ago" +%Y-%m-%d) @@ -40,11 +42,11 @@ fi LIMIT=${2:-300} BRANCH=${3:-""} WORKFLOWS=("Testing: E2E (Chained)" "Evals: Nightly") -DEST_DIR=$(mktemp -d -t gemini-reliability-XXXXXX) +DEST_DIR="$(mktemp -d -t gemini-reliability-XXXXXX)" MERGED_FILE="api-reliability-summary.jsonl" # Ensure cleanup on exit -trap 'rm -rf "$DEST_DIR"' EXIT +trap 'rm -rf "${DEST_DIR}"' EXIT if ! command -v gh &> /dev/null; then echo "โŒ Error: GitHub CLI (gh) is not installed." @@ -57,55 +59,56 @@ if ! command -v jq &> /dev/null; then fi # Clean start -rm -f "$MERGED_FILE" +rm -f "${MERGED_FILE}" # gh run list --created expects a date (YYYY-MM-DD) or a range -CREATED_QUERY=">=$SINCE" +CREATED_QUERY=">=${SINCE}" for WORKFLOW in "${WORKFLOWS[@]}"; do - echo "๐Ÿ” Fetching runs for '$WORKFLOW' created since $SINCE (max $LIMIT runs, branch: ${BRANCH:-all})..." + echo "๐Ÿ” Fetching runs for '${WORKFLOW}' created since ${SINCE} (max ${LIMIT} runs, branch: ${BRANCH:-all})..." # Construct arguments for gh run list - GH_ARGS=("--workflow" "$WORKFLOW" "--created" "$CREATED_QUERY" "--limit" "$LIMIT" "--json" "databaseId" "--jq" ".[].databaseId") - if [ -n "$BRANCH" ]; then - GH_ARGS+=("--branch" "$BRANCH") + GH_ARGS=("--workflow" "${WORKFLOW}" "--created" "${CREATED_QUERY}" "--limit" "${LIMIT}" "--json" "databaseId" "--jq" ".[].databaseId") + if [[ -n "${BRANCH}" ]]; then + GH_ARGS+=("--branch" "${BRANCH}") fi RUN_IDS=$(gh run list "${GH_ARGS[@]}") exit_code=$? - if [ $exit_code -ne 0 ]; then - echo "โŒ Failed to fetch runs for '$WORKFLOW' (exit code: $exit_code). Please check 'gh auth status' and permissions." >&2 + if [[ "${exit_code}" -ne 0 ]]; then + echo "โŒ Failed to fetch runs for '${WORKFLOW}' (exit code: ${exit_code}). Please check 'gh auth status' and permissions." >&2 continue fi - if [ -z "$RUN_IDS" ]; then - echo "๐Ÿ“ญ No runs found for workflow '$WORKFLOW' since $SINCE." + if [[ -z "${RUN_IDS}" ]]; then + echo "๐Ÿ“ญ No runs found for workflow '${WORKFLOW}' since ${SINCE}." continue fi - for ID in $RUN_IDS; do + for ID in ${RUN_IDS}; do # Download artifacts named 'eval-logs-*' # Silencing output because many older runs won't have artifacts - gh run download "$ID" -p "eval-logs-*" -D "$DEST_DIR/$ID" &>/dev/null || continue + gh run download "${ID}" -p "eval-logs-*" -D "${DEST_DIR}/${ID}" &>/dev/null || continue # Append to master log # Use find to locate api-reliability.jsonl in any subdirectory of $DEST_DIR/$ID - find "$DEST_DIR/$ID" -type f -name "api-reliability.jsonl" -exec cat {} + >> "$MERGED_FILE" 2>/dev/null + find "${DEST_DIR}/${ID}" -type f -name "api-reliability.jsonl" -exec cat {} + >> "${MERGED_FILE}" 2>/dev/null done done -if [ ! -f "$MERGED_FILE" ]; then +if [[ ! -f "${MERGED_FILE}" ]]; then echo "๐Ÿ“ญ No reliability data found in the retrieved logs." exit 0 fi -echo -e "\nโœ… Harvest Complete! Data merged into: $MERGED_FILE" +echo -e "\nโœ… Harvest Complete! Data merged into: ${MERGED_FILE}" echo "------------------------------------------------" -echo "๐Ÿ“Š Gemini API Reliability Summary (Since $SINCE)" +echo "๐Ÿ“Š Gemini API Reliability Summary (Since ${SINCE})" echo "------------------------------------------------" -cat "$MERGED_FILE" | jq -s ' +# shellcheck disable=SC2312 +cat "${MERGED_FILE}" | jq -s ' group_by(.model) | map({ model: .[0].model, "500s": (map(select(.errorCode == "500")) | length), @@ -114,4 +117,5 @@ cat "$MERGED_FILE" | jq -s ' skips: (map(select(.status == "SKIP")) | length) })' -echo -e "\n๐Ÿ’ก Total events captured: $(wc -l < "$MERGED_FILE")" +# shellcheck disable=SC2312 +echo -e "\n๐Ÿ’ก Total events captured: $(wc -l < "${MERGED_FILE}")" diff --git a/scripts/review.sh b/scripts/review.sh index 9530e453a1..6e19592a9b 100755 --- a/scripts/review.sh +++ b/scripts/review.sh @@ -5,17 +5,17 @@ set -e -if [ -z "$1" ]; then - echo "Usage: $0 [model]" +if [[ -z "${1}" ]]; then + echo "Usage: ${0} [model]" exit 1 fi -pr="$1" +pr="${1}" model="${2:-gemini-3.1-pro-preview}" REPO="google-gemini/gemini-cli" -REVIEW_DIR="$HOME/git/review/gemini-cli" +REVIEW_DIR="${HOME}/git/review/gemini-cli" -if [ ! -d "$REVIEW_DIR" ]; then - echo "ERROR: Directory $REVIEW_DIR does not exist." +if [[ ! -d "${REVIEW_DIR}" ]]; then + echo "ERROR: Directory ${REVIEW_DIR} does not exist." echo "" echo "Please create a new gemini-cli clone at that directory to use for reviews." echo "Instructions:" @@ -26,54 +26,56 @@ if [ ! -d "$REVIEW_DIR" ]; then fi # 1. Check if the PR exists before doing anything else -echo "review: Validating PR $pr on $REPO..." -if ! gh pr view "$pr" -R "$REPO" > /dev/null 2>&1; then - echo "ERROR: Could not find PR #$pr in $REPO." - echo "Are you sure $pr is a Pull Request number and not an Issue number?" +echo "review: Validating PR ${pr} on ${REPO}..." +if ! gh pr view "${pr}" -R "${REPO}" > /dev/null 2>&1; then + echo "ERROR: Could not find PR #${pr} in ${REPO}." + echo "Are you sure ${pr} is a Pull Request number and not an Issue number?" exit 1 fi -echo "review: Opening PR $pr in browser..." -if [[ "$(uname)" == "Darwin" ]]; then - open "https://github.com/$REPO/pull/$pr" || true +echo "review: Opening PR ${pr} in browser..." +uname_out="$(uname || true)" +if [[ "${uname_out}" == "Darwin" ]]; then + open "https://github.com/${REPO}/pull/${pr}" || true else - xdg-open "https://github.com/$REPO/pull/$pr" || true + xdg-open "https://github.com/${REPO}/pull/${pr}" || true fi -echo "review: Changing directory to $REVIEW_DIR" -cd "$REVIEW_DIR" || exit 1 +echo "review: Changing directory to ${REVIEW_DIR}" +cd "${REVIEW_DIR}" || exit 1 # 2. Fetch latest main to ensure we have a clean starting point echo "review: Fetching latest from origin..." git fetch origin main # 3. Handle worktree creation -WORKTREE_PATH="pr_$pr" -if [ -d "$WORKTREE_PATH" ]; then - echo "review: Worktree directory $WORKTREE_PATH already exists." +WORKTREE_PATH="pr_${pr}" +if [[ -d "${WORKTREE_PATH}" ]]; then + echo "review: Worktree directory ${WORKTREE_PATH} already exists." # Check if it's actually a registered worktree - if git worktree list | grep -q "$WORKTREE_PATH"; then + # shellcheck disable=SC2312 + if git worktree list | grep -q "${WORKTREE_PATH}"; then echo "review: Reusing existing worktree..." else echo "review: Directory exists but is not a worktree. Cleaning up..." - rm -rf "$WORKTREE_PATH" + rm -rf "${WORKTREE_PATH}" fi fi -if [ ! -d "$WORKTREE_PATH" ]; then - echo "review: Adding new worktree at $WORKTREE_PATH..." +if [[ ! -d "${WORKTREE_PATH}" ]]; then + echo "review: Adding new worktree at ${WORKTREE_PATH}..." # Create a detached worktree from origin/main - git worktree add --detach "$WORKTREE_PATH" origin/main + git worktree add --detach "${WORKTREE_PATH}" origin/main fi -echo "review: Changing directory to $WORKTREE_PATH" -cd "$WORKTREE_PATH" || exit 1 +echo "review: Changing directory to ${WORKTREE_PATH}" +cd "${WORKTREE_PATH}" || exit 1 # 4. Checkout the PR -echo "review: Cleaning worktree and checking out PR $pr..." +echo "review: Cleaning worktree and checking out PR ${pr}..." git reset --hard git clean -fd -gh pr checkout "$pr" --branch "review-$pr" -f -R "$REPO" +gh pr checkout "${pr}" --branch "review-${pr}" -f -R "${REPO}" # 5. Clean and Build echo "review: Clearing possibly stale node_modules..." @@ -87,48 +89,49 @@ npm install echo "--- build ---" temp_dir_base="${TMPDIR:-/tmp}" -build_log_file=$(mktemp "${temp_dir_base}/npm_build_log.XXXXXX") || { +build_log_file="$(mktemp "${temp_dir_base}/npm_build_log.XXXXXX" || true)" +if [[ -z "${build_log_file}" || ! -f "${build_log_file}" ]]; then echo "Attempting to create temporary file in current directory as a fallback." >&2 - build_log_file=$(mktemp "./npm_build_log_fallback.XXXXXX") - if [ $? -ne 0 ] || [ -z "$build_log_file" ]; then + build_log_file="$(mktemp "./npm_build_log_fallback.XXXXXX" || true)" + if [[ -z "${build_log_file}" || ! -f "${build_log_file}" ]]; then echo "ERROR: Critical - Failed to create any temporary build log file. Aborting." >&2 exit 1 fi -} +fi build_status=0 build_command_to_run="FORCE_COLOR=1 CLICOLOR_FORCE=1 npm run build" -echo "Running build. Output (with colors) will be shown below and saved to: $build_log_file" -echo "Build command: $build_command_to_run" +echo "Running build. Output (with colors) will be shown below and saved to: ${build_log_file}" +echo "Build command: ${build_command_to_run}" -if [[ "$(uname)" == "Darwin" ]]; then - script -q "$build_log_file" /bin/sh -c "$build_command_to_run" - build_status=$? +if [[ "${uname_out}" == "Darwin" ]]; then + script -q "${build_log_file}" /bin/sh -c "${build_command_to_run}" || build_status=$? else - if script -q -e -c "$build_command_to_run" "$build_log_file"; then + if script -q -e -c "${build_command_to_run}" "${build_log_file}"; then build_status=0 else build_status=$? fi fi -if [ $build_status -ne 0 ]; then - echo "ERROR: npm build failed with exit status $build_status." >&2 - echo "Review output above. Full log (with color codes) was in $build_log_file." >&2 +if [[ "${build_status}" -ne 0 ]]; then + echo "ERROR: npm build failed with exit status ${build_status}." >&2 + echo "Review output above. Full log (with color codes) was in ${build_log_file}." >&2 exit 1 else - if grep -q -i -E "\berror\b|\bfailed\b|ERR!|FATAL|critical" "$build_log_file"; then + # shellcheck disable=SC2312 + if grep -q -i -E "\berror\b|\bfailed\b|ERR!|FATAL|critical" "${build_log_file}"; then echo "ERROR: npm build completed with exit status 0, but suspicious error patterns were found in the build output." >&2 - echo "Review output above. Full log (with color codes) was in $build_log_file." >&2 + echo "Review output above. Full log (with color codes) was in ${build_log_file}." >&2 exit 1 fi echo "npm build completed successfully (exit status 0, no critical error patterns found in log)." - rm -f "$build_log_file" + rm -f "${build_log_file}" fi echo "-- running ---" -if ! npm start -- -m "$model" -i="/review-frontend $pr"; then +if ! npm start -- -m "${model}" -i="/review-frontend ${pr}"; then echo "ERROR: npm start failed. Please check its output for details." >&2 exit 1 fi