Skip to content

Commit da8c841

Browse files
authored
fix: shellcheck warnings in scripts (#24035)
1 parent b7c86b5 commit da8c841

File tree

4 files changed

+219
-208
lines changed

4 files changed

+219
-208
lines changed
Lines changed: 119 additions & 115 deletions
Original file line numberDiff line numberDiff line change
@@ -1,190 +1,194 @@
11
#!/bin/bash
22

33
notify() {
4-
local title="$1"
5-
local message="$2"
6-
local pr="$3"
4+
local title="${1}"
5+
local message="${2}"
6+
local pr="${3}"
77
# Terminal escape sequence
8-
printf "\e]9;%s | PR #%s | %s\a" "$title" "$pr" "$message"
8+
printf "\e]9;%s | PR #%s | %s\a" "${title}" "${pr}" "${message}"
99
# Native macOS notification
10-
if [[ "$(uname)" == "Darwin" ]]; then
11-
osascript -e "display notification \"$message\" with title \"$title\" subtitle \"PR #$pr\""
10+
os_type="$(uname || true)"
11+
if [[ "${os_type}" == "Darwin" ]]; then
12+
osascript -e "display notification \"${message}\" with title \"${title}\" subtitle \"PR #${pr}\""
1213
fi
1314
}
1415

15-
pr_number=$1
16-
if [[ -z "$pr_number" ]]; then
16+
pr_number="${1}"
17+
if [[ -z "${pr_number}" ]]; then
1718
echo "Usage: async-review <pr_number>"
1819
exit 1
1920
fi
2021

21-
base_dir=$(git rev-parse --show-toplevel 2>/dev/null)
22-
if [[ -z "$base_dir" ]]; then
22+
base_dir="$(git rev-parse --show-toplevel 2>/dev/null || true)"
23+
if [[ -z "${base_dir}" ]]; then
2324
echo "❌ Must be run from within a git repository."
2425
exit 1
2526
fi
2627

2728
# Use the repository's local .gemini/tmp directory for ephemeral worktrees and logs
28-
pr_dir="$base_dir/.gemini/tmp/async-reviews/pr-$pr_number"
29-
target_dir="$pr_dir/worktree"
30-
log_dir="$pr_dir/logs"
29+
pr_dir="${base_dir}/.gemini/tmp/async-reviews/pr-${pr_number}"
30+
target_dir="${pr_dir}/worktree"
31+
log_dir="${pr_dir}/logs"
3132

32-
cd "$base_dir" || exit 1
33+
cd "${base_dir}" || exit 1
3334

34-
mkdir -p "$log_dir"
35-
rm -f "$log_dir/setup.exit" "$log_dir/final-assessment.exit" "$log_dir/final-assessment.md"
35+
mkdir -p "${log_dir}"
36+
rm -f "${log_dir}/setup.exit" "${log_dir}/final-assessment.exit" "${log_dir}/final-assessment.md"
3637

37-
echo "🧹 Cleaning up previous worktree if it exists..." | tee -a "$log_dir/setup.log"
38-
git worktree remove -f "$target_dir" >> "$log_dir/setup.log" 2>&1 || true
39-
git branch -D "gemini-async-pr-$pr_number" >> "$log_dir/setup.log" 2>&1 || true
40-
git worktree prune >> "$log_dir/setup.log" 2>&1 || true
38+
echo "🧹 Cleaning up previous worktree if it exists..." | tee -a "${log_dir}/setup.log"
39+
git worktree remove -f "${target_dir}" >> "${log_dir}/setup.log" 2>&1 || true
40+
git branch -D "gemini-async-pr-${pr_number}" >> "${log_dir}/setup.log" 2>&1 || true
41+
git worktree prune >> "${log_dir}/setup.log" 2>&1 || true
4142

42-
echo "📡 Fetching PR #$pr_number..." | tee -a "$log_dir/setup.log"
43-
if ! git fetch origin -f "pull/$pr_number/head:gemini-async-pr-$pr_number" >> "$log_dir/setup.log" 2>&1; then
44-
echo 1 > "$log_dir/setup.exit"
45-
echo "❌ Fetch failed. Check $log_dir/setup.log"
46-
notify "Async Review Failed" "Fetch failed." "$pr_number"
43+
echo "📡 Fetching PR #${pr_number}..." | tee -a "${log_dir}/setup.log"
44+
if ! git fetch origin -f "pull/${pr_number}/head:gemini-async-pr-${pr_number}" >> "${log_dir}/setup.log" 2>&1; then
45+
echo 1 > "${log_dir}/setup.exit"
46+
echo "❌ Fetch failed. Check ${log_dir}/setup.log"
47+
notify "Async Review Failed" "Fetch failed." "${pr_number}"
4748
exit 1
4849
fi
4950

50-
if [[ ! -d "$target_dir" ]]; then
51-
echo "🧹 Pruning missing worktrees..." | tee -a "$log_dir/setup.log"
52-
git worktree prune >> "$log_dir/setup.log" 2>&1
53-
echo "🌿 Creating worktree in $target_dir..." | tee -a "$log_dir/setup.log"
54-
if ! git worktree add "$target_dir" "gemini-async-pr-$pr_number" >> "$log_dir/setup.log" 2>&1; then
55-
echo 1 > "$log_dir/setup.exit"
56-
echo "❌ Worktree creation failed. Check $log_dir/setup.log"
57-
notify "Async Review Failed" "Worktree creation failed." "$pr_number"
51+
if [[ ! -d "${target_dir}" ]]; then
52+
echo "🧹 Pruning missing worktrees..." | tee -a "${log_dir}/setup.log"
53+
git worktree prune >> "${log_dir}/setup.log" 2>&1
54+
echo "🌿 Creating worktree in ${target_dir}..." | tee -a "${log_dir}/setup.log"
55+
if ! git worktree add "${target_dir}" "gemini-async-pr-${pr_number}" >> "${log_dir}/setup.log" 2>&1; then
56+
echo 1 > "${log_dir}/setup.exit"
57+
echo "❌ Worktree creation failed. Check ${log_dir}/setup.log"
58+
notify "Async Review Failed" "Worktree creation failed." "${pr_number}"
5859
exit 1
5960
fi
6061
else
61-
echo "🌿 Worktree already exists." | tee -a "$log_dir/setup.log"
62+
echo "🌿 Worktree already exists." | tee -a "${log_dir}/setup.log"
6263
fi
63-
echo 0 > "$log_dir/setup.exit"
64+
echo 0 > "${log_dir}/setup.exit"
6465

65-
cd "$target_dir" || exit 1
66+
cd "${target_dir}" || exit 1
6667

67-
echo "🚀 Launching background tasks. Logs saving to: $log_dir"
68+
echo "🚀 Launching background tasks. Logs saving to: ${log_dir}"
6869

6970
echo " ↳ [1/5] Grabbing PR diff..."
70-
rm -f "$log_dir/pr-diff.exit"
71-
{ gh pr diff "$pr_number" > "$log_dir/pr-diff.diff" 2>&1; echo $? > "$log_dir/pr-diff.exit"; } &
71+
rm -f "${log_dir}/pr-diff.exit"
72+
{ gh pr diff "${pr_number}" > "${log_dir}/pr-diff.diff" 2>&1; echo $? > "${log_dir}/pr-diff.exit"; } &
7273

7374
echo " ↳ [2/5] Starting build and lint..."
74-
rm -f "$log_dir/build-and-lint.exit"
75-
{ { 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"; } &
75+
rm -f "${log_dir}/build-and-lint.exit"
76+
{ { 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"; } &
7677

7778
# Dynamically resolve gemini binary (fallback to your nightly path)
78-
GEMINI_CMD=$(which gemini || echo "$HOME/.gcli/nightly/node_modules/.bin/gemini")
79+
GEMINI_CMD="$(command -v gemini || echo "${HOME}/.gcli/nightly/node_modules/.bin/gemini")"
80+
# shellcheck disable=SC2312
7981
POLICY_PATH="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)/policy.toml"
8082

8183
echo " ↳ [3/5] Starting Gemini code review..."
82-
rm -f "$log_dir/review.exit"
83-
{ "$GEMINI_CMD" --policy "$POLICY_PATH" -p "/review-frontend $pr_number" > "$log_dir/review.md" 2>&1; echo $? > "$log_dir/review.exit"; } &
84+
rm -f "${log_dir}/review.exit"
85+
{ "${GEMINI_CMD}" --policy "${POLICY_PATH}" -p "/review-frontend ${pr_number}" > "${log_dir}/review.md" 2>&1; echo $? > "${log_dir}/review.exit"; } &
8486

8587
echo " ↳ [4/5] Starting automated tests (waiting for build and lint)..."
86-
rm -f "$log_dir/npm-test.exit"
88+
rm -f "${log_dir}/npm-test.exit"
8789
{
88-
while [ ! -f "$log_dir/build-and-lint.exit" ]; do sleep 1; done
89-
if [ "$(cat "$log_dir/build-and-lint.exit")" == "0" ]; then
90-
gh pr checks "$pr_number" > "$log_dir/ci-checks.log" 2>&1
90+
while [[ ! -f "${log_dir}/build-and-lint.exit" ]]; do sleep 1; done
91+
read -r build_exit < "${log_dir}/build-and-lint.exit" || build_exit=""
92+
if [[ "${build_exit}" == "0" ]]; then
93+
gh pr checks "${pr_number}" > "${log_dir}/ci-checks.log" 2>&1
9194
ci_status=$?
9295

93-
if [ "$ci_status" -eq 0 ]; then
94-
echo "CI checks passed. Skipping local npm tests." > "$log_dir/npm-test.log"
95-
echo 0 > "$log_dir/npm-test.exit"
96-
elif [ "$ci_status" -eq 8 ]; then
97-
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"
98-
echo 0 > "$log_dir/npm-test.exit"
96+
if [[ "${ci_status}" -eq 0 ]]; then
97+
echo "CI checks passed. Skipping local npm tests." > "${log_dir}/npm-test.log"
98+
echo 0 > "${log_dir}/npm-test.exit"
99+
elif [[ "${ci_status}" -eq 8 ]]; then
100+
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"
101+
echo 0 > "${log_dir}/npm-test.exit"
99102
else
100-
echo "CI checks failed. Failing checks:" > "$log_dir/npm-test.log"
101-
gh pr checks "$pr_number" --json name,bucket -q '.[] | select(.bucket=="fail") | .name' >> "$log_dir/npm-test.log" 2>&1
103+
echo "CI checks failed. Failing checks:" > "${log_dir}/npm-test.log"
104+
gh pr checks "${pr_number}" --json name,bucket -q '.[] | select(.bucket=="fail") | .name' >> "${log_dir}/npm-test.log" 2>&1
102105

103-
echo "Attempting to extract failing test files from CI logs..." >> "$log_dir/npm-test.log"
104-
pr_branch=$(gh pr view "$pr_number" --json headRefName -q '.headRefName' 2>/dev/null)
105-
run_id=$(gh run list --branch "$pr_branch" --workflow ci.yml --json databaseId -q '.[0].databaseId' 2>/dev/null)
106+
echo "Attempting to extract failing test files from CI logs..." >> "${log_dir}/npm-test.log"
107+
pr_branch="$(gh pr view "${pr_number}" --json headRefName -q '.headRefName' 2>/dev/null || true)"
108+
run_id="$(gh run list --branch "${pr_branch}" --workflow ci.yml --json databaseId -q '.[0].databaseId' 2>/dev/null || true)"
106109

107110
failed_files=""
108-
if [[ -n "$run_id" ]]; then
109-
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)
111+
if [[ -n "${run_id}" ]]; then
112+
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)"
110113
fi
111114

112-
if [[ -n "$failed_files" ]]; then
113-
echo "Found failing test files from CI:" >> "$log_dir/npm-test.log"
114-
for f in $failed_files; do echo " - $f" >> "$log_dir/npm-test.log"; done
115-
echo "Running ONLY failing tests locally..." >> "$log_dir/npm-test.log"
115+
if [[ -n "${failed_files}" ]]; then
116+
echo "Found failing test files from CI:" >> "${log_dir}/npm-test.log"
117+
for f in ${failed_files}; do echo " - ${f}" >> "${log_dir}/npm-test.log"; done
118+
echo "Running ONLY failing tests locally..." >> "${log_dir}/npm-test.log"
116119

117120
exit_code=0
118-
for file in $failed_files; do
119-
if [[ "$file" == packages/* ]]; then
120-
ws_dir=$(echo "$file" | cut -d'/' -f1,2)
121+
for file in ${failed_files}; do
122+
if [[ "${file}" == packages/* ]]; then
123+
ws_dir="$(echo "${file}" | cut -d'/' -f1,2)"
121124
else
122-
ws_dir=$(echo "$file" | cut -d'/' -f1)
125+
ws_dir="$(echo "${file}" | cut -d'/' -f1)"
123126
fi
124-
rel_file=${file#$ws_dir/}
127+
rel_file="${file#"${ws_dir}"/}"
125128

126-
echo "--- Running $rel_file in workspace $ws_dir ---" >> "$log_dir/npm-test.log"
127-
if ! npm run test:ci -w "$ws_dir" -- "$rel_file" >> "$log_dir/npm-test.log" 2>&1; then
129+
echo "--- Running ${rel_file} in workspace ${ws_dir} ---" >> "${log_dir}/npm-test.log"
130+
if ! npm run test:ci -w "${ws_dir}" -- "${rel_file}" >> "${log_dir}/npm-test.log" 2>&1; then
128131
exit_code=1
129132
fi
130133
done
131-
echo $exit_code > "$log_dir/npm-test.exit"
134+
echo "${exit_code}" > "${log_dir}/npm-test.exit"
132135
else
133-
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"
134-
echo 1 > "$log_dir/npm-test.exit"
136+
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"
137+
echo 1 > "${log_dir}/npm-test.exit"
135138
fi
136139
fi
137140
else
138-
echo "Skipped due to build-and-lint failure" > "$log_dir/npm-test.log"
139-
echo 1 > "$log_dir/npm-test.exit"
141+
echo "Skipped due to build-and-lint failure" > "${log_dir}/npm-test.log"
142+
echo 1 > "${log_dir}/npm-test.exit"
140143
fi
141144
} &
142145

143146
echo " ↳ [5/5] Starting Gemini test execution (waiting for build and lint)..."
144-
rm -f "$log_dir/test-execution.exit"
147+
rm -f "${log_dir}/test-execution.exit"
145148
{
146-
while [ ! -f "$log_dir/build-and-lint.exit" ]; do sleep 1; done
147-
if [ "$(cat "$log_dir/build-and-lint.exit")" == "0" ]; then
148-
"$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"
149+
while [[ ! -f "${log_dir}/build-and-lint.exit" ]]; do sleep 1; done
150+
read -r build_exit < "${log_dir}/build-and-lint.exit" || build_exit=""
151+
if [[ "${build_exit}" == "0" ]]; then
152+
"${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"
149153
else
150-
echo "Skipped due to build-and-lint failure" > "$log_dir/test-execution.log"
151-
echo 1 > "$log_dir/test-execution.exit"
154+
echo "Skipped due to build-and-lint failure" > "${log_dir}/test-execution.log"
155+
echo 1 > "${log_dir}/test-execution.exit"
152156
fi
153157
} &
154158

155159
echo "✅ All tasks dispatched!"
156-
echo "You can monitor progress with: tail -f $log_dir/*.log"
157-
echo "Read your review later at: $log_dir/review.md"
160+
echo "You can monitor progress with: tail -f ${log_dir}/*.log"
161+
echo "Read your review later at: ${log_dir}/review.md"
158162

159163
# Polling loop to wait for all background tasks to finish
160164
tasks=("pr-diff" "build-and-lint" "review" "npm-test" "test-execution")
161165
log_files=("pr-diff.diff" "build-and-lint.log" "review.md" "npm-test.log" "test-execution.log")
162166

163167
declare -A task_done
164-
for t in "${tasks[@]}"; do task_done[$t]=0; done
168+
for t in "${tasks[@]}"; do task_done[${t}]=0; done
165169

166170
all_done=0
167-
while [[ $all_done -eq 0 ]]; do
171+
while [[ "${all_done}" -eq 0 ]]; do
168172
clear
169173
echo "=================================================="
170-
echo "🚀 Async PR Review Status for PR #$pr_number"
174+
echo "🚀 Async PR Review Status for PR #${pr_number}"
171175
echo "=================================================="
172176
echo ""
173177

174178
all_done=1
175179
for i in "${!tasks[@]}"; do
176-
t="${tasks[$i]}"
180+
t="${tasks[${i}]}"
177181

178-
if [[ -f "$log_dir/$t.exit" ]]; then
179-
exit_code=$(cat "$log_dir/$t.exit")
180-
if [[ "$exit_code" == "0" ]]; then
181-
echo "$t: SUCCESS"
182+
if [[ -f "${log_dir}/${t}.exit" ]]; then
183+
read -r task_exit < "${log_dir}/${t}.exit" || task_exit=""
184+
if [[ "${task_exit}" == "0" ]]; then
185+
echo "${t}: SUCCESS"
182186
else
183-
echo "$t: FAILED (exit code $exit_code)"
187+
echo "${t}: FAILED (exit code ${task_exit})"
184188
fi
185-
task_done[$t]=1
189+
task_done[${t}]=1
186190
else
187-
echo "$t: RUNNING"
191+
echo "${t}: RUNNING"
188192
all_done=0
189193
fi
190194
done
@@ -195,47 +199,47 @@ while [[ $all_done -eq 0 ]]; do
195199
echo "=================================================="
196200

197201
for i in "${!tasks[@]}"; do
198-
t="${tasks[$i]}"
199-
log_file="${log_files[$i]}"
202+
t="${tasks[${i}]}"
203+
log_file="${log_files[${i}]}"
200204

201-
if [[ ${task_done[$t]} -eq 0 ]]; then
202-
if [[ -f "$log_dir/$log_file" ]]; then
205+
if [[ "${task_done[${t}]}" -eq 0 ]]; then
206+
if [[ -f "${log_dir}/${log_file}" ]]; then
203207
echo ""
204-
echo "--- $t ---"
205-
tail -n 5 "$log_dir/$log_file"
208+
echo "--- ${t} ---"
209+
tail -n 5 "${log_dir}/${log_file}"
206210
fi
207211
fi
208212
done
209213

210-
if [[ $all_done -eq 0 ]]; then
214+
if [[ "${all_done}" -eq 0 ]]; then
211215
sleep 3
212216
fi
213217
done
214218

215219
clear
216220
echo "=================================================="
217-
echo "🚀 Async PR Review Status for PR #$pr_number"
221+
echo "🚀 Async PR Review Status for PR #${pr_number}"
218222
echo "=================================================="
219223
echo ""
220224
for t in "${tasks[@]}"; do
221-
exit_code=$(cat "$log_dir/$t.exit")
222-
if [[ "$exit_code" == "0" ]]; then
223-
echo "$t: SUCCESS"
225+
read -r task_exit < "${log_dir}/${t}.exit" || task_exit=""
226+
if [[ "${task_exit}" == "0" ]]; then
227+
echo "${t}: SUCCESS"
224228
else
225-
echo "$t: FAILED (exit code $exit_code)"
229+
echo "${t}: FAILED (exit code ${task_exit})"
226230
fi
227231
done
228232
echo ""
229233

230234
echo "⏳ Tasks complete! Synthesizing final assessment..."
231-
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
232-
echo $? > "$log_dir/final-assessment.exit"
235+
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
236+
echo $? > "${log_dir}/final-assessment.exit"
233237
echo "❌ Final assessment synthesis failed!"
234-
echo "Check $log_dir/final-assessment.md for details."
235-
notify "Async Review Failed" "Final assessment synthesis failed." "$pr_number"
238+
echo "Check ${log_dir}/final-assessment.md for details."
239+
notify "Async Review Failed" "Final assessment synthesis failed." "${pr_number}"
236240
exit 1
237241
fi
238242

239-
echo 0 > "$log_dir/final-assessment.exit"
240-
echo "✅ Final assessment complete! Check $log_dir/final-assessment.md"
241-
notify "Async Review Complete" "Review and test execution finished successfully." "$pr_number"
243+
echo 0 > "${log_dir}/final-assessment.exit"
244+
echo "✅ Final assessment complete! Check ${log_dir}/final-assessment.md"
245+
notify "Async Review Complete" "Review and test execution finished successfully." "${pr_number}"

0 commit comments

Comments
 (0)