From a49a502bc70ce93776e8665fae69a293e184695b Mon Sep 17 00:00:00 2001 From: mkorwel Date: Mon, 9 Mar 2026 17:13:39 -0700 Subject: [PATCH] feat(skills): improve async-pr-review workflow and logging --- .gemini/skills/async-pr-review/SKILL.md | 2 +- .../async-pr-review/scripts/async-review.sh | 157 ++++++++++++++++-- .../scripts/check-async-review.sh | 5 +- 3 files changed, 144 insertions(+), 20 deletions(-) diff --git a/.gemini/skills/async-pr-review/SKILL.md b/.gemini/skills/async-pr-review/SKILL.md index 975f7f90fc..3719cd4b7b 100644 --- a/.gemini/skills/async-pr-review/SKILL.md +++ b/.gemini/skills/async-pr-review/SKILL.md @@ -41,5 +41,5 @@ If the user wants to check the status or view the final assessment of a previous ``` 3. **Evaluate Output**: Read the output from the script. * If the output contains `STATUS: IN_PROGRESS`, tell the user which tasks are still running. - * If the output contains `STATUS: COMPLETE`, use your file reading tools (`read_file`) to retrieve the contents of the `review.md` and `test-execution.log` files from the `LOG_DIR` specified in the output. + * If the output contains `STATUS: COMPLETE`, use your file reading tools (`read_file`) to retrieve the contents of `final-assessment.md`, `review.md`, `npm-test.log`, and `test-execution.log` files from the `LOG_DIR` specified in the output. * **Final Assessment**: Read those files, synthesize their results, and give the user a concise recommendation on whether the PR builds successfully, passes tests, and if you recommend they approve it based on the review. \ No newline at end of file diff --git a/.gemini/skills/async-pr-review/scripts/async-review.sh b/.gemini/skills/async-pr-review/scripts/async-review.sh index 05799c1f17..d371f8cc86 100755 --- a/.gemini/skills/async-pr-review/scripts/async-review.sh +++ b/.gemini/skills/async-pr-review/scripts/async-review.sh @@ -17,40 +17,161 @@ target_dir="$pr_dir/worktree" log_dir="$pr_dir/logs" cd "$base_dir" || exit 1 -echo "๐Ÿ“ก Fetching PR #$pr_number..." -# Fetch the PR into a local branch to avoid detached head / namespace issues -git fetch origin -f "pull/$pr_number/head:pr-$pr_number" -if [[ ! -d "$target_dir" ]]; then - echo "๐Ÿงน Pruning missing worktrees..." - git worktree prune - echo "๐ŸŒฟ Creating worktree in $target_dir..." - mkdir -p "$pr_dir" - git worktree add "$target_dir" "pr-$pr_number" -else - echo "๐ŸŒฟ Worktree already exists." +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 "๐Ÿ“ก 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" + printf "\e]9;Async Review Failed | PR #%s | Fetch failed.\a" "$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" + printf "\e]9;Async Review Failed | PR #%s | Worktree creation failed.\a" "$pr_number" + exit 1 + fi +else + echo "๐ŸŒฟ Worktree already exists." | tee -a "$log_dir/setup.log" +fi +echo 0 > "$log_dir/setup.exit" + cd "$target_dir" || exit 1 -mkdir -p "$log_dir" echo "๐Ÿš€ Launching background tasks. Logs saving to: $log_dir" -echo " โ†ณ [1/3] Starting npm run preflight..." -rm -f "$log_dir/preflight.exit" -{ npm run preflight > "$log_dir/preflight.log" 2>&1; echo $? > "$log_dir/preflight.exit"; } & +echo " โ†ณ [1/4] 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"; } & # Dynamically resolve gemini binary (fallback to your nightly path) GEMINI_CMD=$(which gemini || echo "$HOME/.gcli/nightly/node_modules/.bin/gemini") -echo " โ†ณ [2/3] Starting Gemini code review..." +echo " โ†ณ [2/4] Starting Gemini code review..." rm -f "$log_dir/review.exit" { "$GEMINI_CMD" --approval-mode=yolo /review-frontend "$pr_number" > "$log_dir/review.md" 2>&1; echo $? > "$log_dir/review.exit"; } & -echo " โ†ณ [3/3] Starting Gemini test execution..." +echo " โ†ณ [3/4] Starting automated tests (waiting for build and lint)..." +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 + npm run test:ci > "$log_dir/npm-test.log" 2>&1; echo $? > "$log_dir/npm-test.exit" + else + echo "Skipped due to build-and-lint failure" > "$log_dir/npm-test.log" + echo 1 > "$log_dir/npm-test.exit" + fi +} & + +echo " โ†ณ [4/4] Starting Gemini test execution (waiting for build and lint)..." rm -f "$log_dir/test-execution.exit" -{ "$GEMINI_CMD" --approval-mode=yolo "Analyze the diff for PR $pr_number using 'gh pr diff $pr_number'. Formulate a test plan for the changes, and then autonomously execute the test commands in the terminal to verify the feature. Do not ask for user confirmation, just run the tests and log the results." > "$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 + if [ "$(cat "$log_dir/build-and-lint.exit")" == "0" ]; then + "$GEMINI_CMD" --approval-mode=yolo "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" + 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" + +# Polling loop to wait for all background tasks to finish +tasks=("build-and-lint" "review" "npm-test" "test-execution") +log_files=("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 + +all_done=0 +while [[ $all_done -eq 0 ]]; do + clear + echo "==================================================" + echo "๐Ÿš€ Async PR Review Status for PR #$pr_number" + echo "==================================================" + echo "" + + all_done=1 + for i in "${!tasks[@]}"; do + 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" + else + echo " โŒ $t: FAILED (exit code $exit_code)" + fi + task_done[$t]=1 + else + echo " โณ $t: RUNNING" + all_done=0 + fi + done + + echo "" + echo "==================================================" + echo "๐Ÿ“ Live Logs (Last 5 lines of running tasks)" + echo "==================================================" + + for i in "${!tasks[@]}"; do + t="${tasks[$i]}" + log_file="${log_files[$i]}" + + if [[ ${task_done[$t]} -eq 0 ]]; then + if [[ -f "$log_dir/$log_file" ]]; then + echo "" + echo "--- $t ---" + tail -n 5 "$log_dir/$log_file" + fi + fi + done + + if [[ $all_done -eq 0 ]]; then + sleep 3 + fi +done + +clear +echo "==================================================" +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" + else + echo " โŒ $t: FAILED (exit code $exit_code)" + fi +done +echo "" + +echo "โณ Tasks complete! Synthesizing final assessment..." +if ! "$GEMINI_CMD" --approval-mode=yolo -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." + printf "\e]9;Async Review Failed | PR #%s | Final assessment synthesis failed.\a" "$pr_number" + exit 1 +fi + +echo 0 > "$log_dir/final-assessment.exit" +echo "โœ… Final assessment complete! Check $log_dir/final-assessment.md" +printf "\e]9;Async Review Complete | PR #%s | Review and test execution finished successfully.\a" "$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 8ac72be0bb..7ba52bd926 100755 --- a/.gemini/skills/async-pr-review/scripts/check-async-review.sh +++ b/.gemini/skills/async-pr-review/scripts/check-async-review.sh @@ -21,9 +21,12 @@ if [[ ! -d "$log_dir" ]]; then fi tasks=( - "preflight|preflight.log" + "setup|setup.log" + "build-and-lint|build-and-lint.log" "review|review.md" + "npm-test|npm-test.log" "test-execution|test-execution.log" + "final-assessment|final-assessment.md" ) all_done=true