diff --git a/.github/workflows/claude-review.yml b/.github/workflows/claude-review.yml index 1d9c5917c..1056e6dab 100644 --- a/.github/workflows/claude-review.yml +++ b/.github/workflows/claude-review.yml @@ -79,14 +79,31 @@ jobs: uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: fetch-depth: 0 + # On issue_comment events github.ref is the default branch, not the PR + # head, so checkout would fetch main. Check out the PR merge ref instead + # so the review actually runs against the PR code. This ref works for + # fork PRs with a read token; the fork guard below still governs whether + # AI review runs. pull_request events keep default behavior (no ref), + # which already checks out the PR merge commit. + ref: ${{ github.event_name == 'issue_comment' && format('refs/pull/{0}/merge', github.event.issue.number) || '' }} - name: Check PR size (cost control) if: steps.fork_check.outputs.is_fork != 'true' id: pr_size run: | - BASE_REF="${{ github.base_ref || github.event.repository.default_branch || 'main' }}" - FILES_CHANGED=$(git diff --name-only origin/${BASE_REF}...HEAD | wc -l) - LINES_CHANGED=$(git diff --stat origin/${BASE_REF}...HEAD | tail -1 | awk '{print $4+$6}') + # On issue_comment events github.base_ref is empty and the checked-out + # HEAD is the PR merge ref (refs/pull/N/merge), whose first parent is + # the PR's base tip — so diff HEAD^...HEAD to measure the PR changes. + # On pull_request events diff against the base branch as before. + if [ "${{ github.event_name }}" == "issue_comment" ]; then + DIFF_ARGS="HEAD^...HEAD" + else + BASE_REF="${{ github.base_ref || github.event.repository.default_branch || 'main' }}" + DIFF_ARGS="origin/${BASE_REF}...HEAD" + fi + + FILES_CHANGED=$(git diff --name-only ${DIFF_ARGS} | wc -l) + LINES_CHANGED=$(git diff --stat ${DIFF_ARGS} | tail -1 | awk '{print $4+$6}') echo "files_changed=$FILES_CHANGED" >> $GITHUB_OUTPUT echo "lines_changed=$LINES_CHANGED" >> $GITHUB_OUTPUT diff --git a/ods/tests/test-claude-review-pr-ref.sh b/ods/tests/test-claude-review-pr-ref.sh new file mode 100644 index 000000000..02bf80473 --- /dev/null +++ b/ods/tests/test-claude-review-pr-ref.sh @@ -0,0 +1,82 @@ +#!/bin/bash +# ============================================================================ +# claude-review.yml PR-head checkout test +# ============================================================================ +# Regression for a subtle CI bug: on issue_comment events github.ref points at +# the default branch and github.base_ref is empty, so the checkout fetched +# main and the review silently reviewed main instead of the PR head. +# +# The workflow must: +# - check out the PR merge ref (refs/pull/N/merge) on issue_comment events +# via the checkout `ref:` expression, and +# - use HEAD^...HEAD for the size check on issue_comment (where base_ref is +# empty), keeping origin/...HEAD for pull_request events. +# +# Usage: ./tests/test-claude-review-pr-ref.sh +# ============================================================================ + +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +ROOT_DIR="$(cd "$SCRIPT_DIR/.." && pwd)" +WORKFLOW="$ROOT_DIR/../.github/workflows/claude-review.yml" + +GREEN='\033[0;32m' +RED='\033[0;31m' +NC='\033[0m' + +PASSED=0 +FAILED=0 + +pass() { echo -e " ${GREEN}✓ PASS${NC} $1"; PASSED=$((PASSED + 1)); } +fail() { echo -e " ${RED}✗ FAIL${NC} $1"; FAILED=$((FAILED + 1)); } + +echo "" +echo "╔═══════════════════════════════════════════════╗" +echo "║ claude-review.yml PR-head checkout test ║" +echo "╚═══════════════════════════════════════════════╝" +echo "" + +if [[ ! -f "$WORKFLOW" ]]; then + fail "claude-review.yml not found at $WORKFLOW" + echo ""; echo "Result: $PASSED passed, $FAILED failed"; exit 1 +fi +pass "claude-review.yml exists" + +# --- 1. issue_comment path checks out the PR merge ref ------------------------ +if grep -q "format('refs/pull/{0}/merge', github.event.issue.number)" "$WORKFLOW"; then + pass "checkout ref resolves the PR merge ref on issue_comment events" +else + fail "checkout must use format('refs/pull/{0}/merge', github.event.issue.number) on issue_comment" +fi + +if grep -q "ref: \${{ github.event_name == 'issue_comment' && format('refs/pull/{0}/merge', github.event.issue.number) || '' }}" "$WORKFLOW"; then + pass "checkout ref is conditional on issue_comment and empty otherwise" +else + fail "checkout ref must be conditional: refs/pull/N/merge on issue_comment, '' otherwise" +fi + +# --- 2. Size check uses HEAD^...HEAD on issue_comment, base diff otherwise ----- +if grep -q 'DIFF_ARGS="HEAD^...HEAD"' "$WORKFLOW"; then + pass "issue_comment path diffs HEAD^...HEAD for the PR size check" +else + fail "issue_comment path must set DIFF_ARGS=HEAD^...HEAD" +fi + +if grep -q 'DIFF_ARGS="origin/\${BASE_REF}...HEAD"' "$WORKFLOW"; then + pass "pull_request path still diffs against origin/\${BASE_REF}...HEAD" +else + fail "pull_request path must keep DIFF_ARGS=origin/\${BASE_REF}...HEAD" +fi + +# --- 3. The two branches are keyed on github.event_name ------------------------ +if grep -q 'if \[ "\${{ github.event_name }}" == "issue_comment" \]; then' "$WORKFLOW"; then + pass "diff-arg selection branches on github.event_name" +else + fail "diff-arg selection must branch on github.event_name == issue_comment" +fi + +echo "" +echo "Result: $PASSED passed, $FAILED failed" +[[ $FAILED -eq 0 ]] || exit 1 +exit 0