diff --git a/.github/workflows/on-review-feedback.yml b/.github/workflows/on-review-feedback.yml index 71e5f1dc46..64a72d1ac8 100644 --- a/.github/workflows/on-review-feedback.yml +++ b/.github/workflows/on-review-feedback.yml @@ -1,12 +1,13 @@ name: "egg: Address Review Feedback" -# Trigger when the review bot posts feedback on a PR. -# This enables the loop: PR opened → review → address feedback → re-review → ... +# Trigger when the review bot posts feedback on a PR, or when a human @mentions +# the bot in a PR comment. # -# Handles two cases: +# Handles three cases: # - pull_request_review: formal review posted via gh pr review -# - issue_comment: self-review posted as comment (GitHub blocks bots from +# - issue_comment (bot): self-review posted as comment (GitHub blocks bots from # reviewing their own PRs via the Reviews API) +# - issue_comment (human): human @mentions the bot on a bot-authored PR on: pull_request_review: types: [submitted] @@ -38,7 +39,7 @@ on: type: string default: "jwbron/egg/action@main" authorized_users: - description: 'Comma-separated list of authorized GitHub usernames (for bot-authored PR check bypass)' + description: 'Comma-separated list of GitHub usernames authorized to trigger the bot (via review or @mention)' required: false type: string default: "jwbron" @@ -105,6 +106,7 @@ jobs: bot_username: ${{ steps.resolve.outputs.bot_username }} branch_prefix: ${{ steps.resolve.outputs.branch_prefix }} reviewer_username: ${{ steps.resolve.outputs.reviewer_username }} + authorized_users: ${{ steps.resolve.outputs.authorized_users }} max_feedback_rounds: ${{ steps.resolve.outputs.max_feedback_rounds }} pr_number: ${{ steps.resolve.outputs.pr_number }} should_check_trigger: ${{ steps.resolve.outputs.should_check_trigger }} @@ -117,10 +119,12 @@ jobs: echo "bot_username=${{ inputs.bot_username }}" >> "$GITHUB_OUTPUT" echo "branch_prefix=${{ inputs.branch_prefix }}" >> "$GITHUB_OUTPUT" echo "reviewer_username=${{ inputs.reviewer_username || '' }}" >> "$GITHUB_OUTPUT" + echo "authorized_users=${{ inputs.authorized_users || 'jwbron' }}" >> "$GITHUB_OUTPUT" else echo "bot_username=${{ vars.EGG_BOT_USERNAME }}" >> "$GITHUB_OUTPUT" echo "branch_prefix=${{ vars.EGG_BRANCH_PREFIX }}" >> "$GITHUB_OUTPUT" echo "reviewer_username=${{ vars.EGG_REVIEWER_USERNAME || '' }}" >> "$GITHUB_OUTPUT" + echo "authorized_users=${{ vars.EGG_AUTHORIZED_USERS || 'jwbron' }}" >> "$GITHUB_OUTPUT" fi echo "max_feedback_rounds=${{ inputs.max_feedback_rounds || '3' }}" >> "$GITHUB_OUTPUT" @@ -144,12 +148,14 @@ jobs: runs-on: ubuntu-latest outputs: should_run: ${{ steps.check.outputs.should_run }} + trigger_type: ${{ steps.check.outputs.trigger_type }} steps: - name: Check if triggered by our bot or reviewer id: check env: BOT_USERNAME: ${{ needs.resolve-inputs.outputs.bot_username }} REVIEWER_USERNAME: ${{ needs.resolve-inputs.outputs.reviewer_username }} + AUTHORIZED_USERS: ${{ needs.resolve-inputs.outputs.authorized_users }} EVENT_NAME: ${{ github.event_name }} REVIEW_USER: ${{ github.event.review.user.login }} REVIEW_BODY: ${{ github.event.review.body }} @@ -172,6 +178,18 @@ jobs: [[ "$PR_AUTHOR" == "$BOT_USERNAME" || "$PR_AUTHOR" == "${BOT_USERNAME}[bot]" ]] } + # Helper: check if a user is in the authorized_users list (comma-separated) + is_authorized_user() { + local user="$1" + local IFS=',' + for authorized in $AUTHORIZED_USERS; do + # Trim whitespace + authorized=$(echo "$authorized" | xargs) + [[ "$user" == "$authorized" ]] && return 0 + done + return 1 + } + # For pull_request_review events if [[ "$EVENT_NAME" == "pull_request_review" ]]; then # Case 1: Automated review (bot/reviewer with marker) @@ -180,30 +198,44 @@ jobs: [[ "$REVIEW_BODY" == *"egg-automated-review"* ]] && \ [[ "$REVIEW_STATE" != "approved" || "$REVIEW_BODY" == *"verdict=approve-with-suggestions"* ]]; then echo "should_run=true" >> "$GITHUB_OUTPUT" + echo "trigger_type=review" >> "$GITHUB_OUTPUT" # Case 2: Human review on bot-authored PR - # Trigger for non-approvals (changes_requested, commented) + # Trigger for non-approvals (changes_requested, commented) from authorized users only. # Note: Human approvals with suggestions won't trigger because humans # don't add the verdict=approve-with-suggestions marker. This is an # accepted limitation - non-blocking suggestions in human approvals # are considered optional and don't require automated follow-up. - elif ! is_review_bot "$REVIEW_USER" && is_bot_authored_pr && \ + elif ! is_review_bot "$REVIEW_USER" && is_authorized_user "$REVIEW_USER" && is_bot_authored_pr && \ [[ "$REVIEW_STATE" != "approved" ]]; then echo "should_run=true" >> "$GITHUB_OUTPUT" + echo "trigger_type=review" >> "$GITHUB_OUTPUT" else echo "should_run=false" >> "$GITHUB_OUTPUT" fi - # For issue_comment events (self-review as comment - legacy, less common with reviewer account) + # For issue_comment events elif [[ "$EVENT_NAME" == "issue_comment" ]]; then # Extract verdict using regex to distinguish approve from approve-with-suggestions comment_verdict="" if [[ "$COMMENT_BODY" =~ verdict=([a-z-]+) ]]; then comment_verdict="${BASH_REMATCH[1]}" fi + # Case 3: Self-review as comment (GitHub blocks bots from reviewing own PRs) if is_review_bot "$COMMENT_USER" && \ [[ "$IS_PR" == "true" && \ "$COMMENT_BODY" == *"egg-automated-review"* && \ "$comment_verdict" != "approve" ]]; then echo "should_run=true" >> "$GITHUB_OUTPUT" + echo "trigger_type=review" >> "$GITHUB_OUTPUT" + # Case 4: Human @mentions the bot in a PR comment + # Only authorized users can trigger via @mention. + # The should-run step validates PR authorship; this just checks the mention. + # Use regex with leading and trailing boundaries to avoid matching + # substrings like @egg-bot-staging or email-like foo@egg-bot + elif ! is_review_bot "$COMMENT_USER" && is_authorized_user "$COMMENT_USER" && \ + [[ "$IS_PR" == "true" ]] && \ + [[ "$COMMENT_BODY" =~ (^|[[:space:]]|[^a-zA-Z0-9_])@${BOT_USERNAME}($|[^a-zA-Z0-9_-]) ]]; then + echo "should_run=true" >> "$GITHUB_OUTPUT" + echo "trigger_type=mention" >> "$GITHUB_OUTPUT" else echo "should_run=false" >> "$GITHUB_OUTPUT" fi @@ -346,7 +378,7 @@ jobs: # This ensures we don't run multiple times when multiple reviewers trigger concurrently. - name: Wait for all reviewer checks to complete id: wait-for-reviewers - if: steps.should-run.outputs.run == 'true' + if: steps.should-run.outputs.run == 'true' && needs.check-trigger.outputs.trigger_type != 'mention' env: GH_TOKEN: ${{ steps.bot-token.outputs.token }} run: | @@ -409,11 +441,25 @@ jobs: sleep "$POLL_INTERVAL" done + # Compute a single proceed flag to avoid repeating the long condition + # on every downstream step. Mentions skip wait-for-reviewers, so we + # also proceed when trigger_type is 'mention'. + - name: Compute should-proceed + id: should-proceed + if: steps.should-run.outputs.run == 'true' + run: | + if [[ "${{ steps.wait-for-reviewers.outputs.proceed }}" == "true" || \ + "${{ needs.check-trigger.outputs.trigger_type }}" == "mention" ]]; then + echo "ok=true" >> "$GITHUB_OUTPUT" + else + echo "ok=false" >> "$GITHUB_OUTPUT" + fi + # Minimize previous feedback status comments to reduce clutter # Uses semantic marker instead of content patterns per #363 - name: Minimize previous feedback comments id: minimize - if: steps.should-run.outputs.run == 'true' && steps.wait-for-reviewers.outputs.proceed == 'true' + if: steps.should-proceed.outputs.ok == 'true' run: | set -euo pipefail HIDDEN_COUNT=0 @@ -439,7 +485,7 @@ jobs: # Post acknowledgment comment with marker for iteration tracking - name: Post starting comment - if: steps.should-run.outputs.run == 'true' && steps.wait-for-reviewers.outputs.proceed == 'true' + if: steps.should-proceed.outputs.ok == 'true' run: | set -euo pipefail gh api "repos/${{ github.repository }}/issues/${{ env.PR_NUMBER }}/comments" \ @@ -451,21 +497,21 @@ jobs: # This prevents malicious PRs from injecting content via .egg/feedback-rules.md. # The agent will check out the PR branch itself after the prompt is built. - name: Checkout main (trusted) - if: steps.should-run.outputs.run == 'true' && steps.wait-for-reviewers.outputs.proceed == 'true' + if: steps.should-proceed.outputs.ok == 'true' uses: actions/checkout@v4 with: ref: main persist-credentials: false - name: Build feedback prompt - if: steps.should-run.outputs.run == 'true' && steps.wait-for-reviewers.outputs.proceed == 'true' + if: steps.should-proceed.outputs.ok == 'true' id: prompt run: bash ${{ inputs.prompt_script || 'action/build-feedback-prompt.sh' }} env: PR_NUMBER: ${{ env.PR_NUMBER }} - name: Checkout PR branch - if: steps.should-run.outputs.run == 'true' && steps.wait-for-reviewers.outputs.proceed == 'true' + if: steps.should-proceed.outputs.ok == 'true' uses: actions/checkout@v4 with: repository: ${{ steps.pr-meta.outputs.head-repo }} @@ -473,7 +519,7 @@ jobs: persist-credentials: false - name: Run egg to address feedback - if: steps.should-run.outputs.run == 'true' && steps.wait-for-reviewers.outputs.proceed == 'true' + if: steps.should-proceed.outputs.ok == 'true' id: egg uses: jwbron/egg/action@main with: @@ -491,7 +537,7 @@ jobs: EGG_BOT_NAME: feedback - name: Post result comment - if: always() && !cancelled() && steps.should-run.outputs.run == 'true' && steps.wait-for-reviewers.outputs.proceed == 'true' + if: always() && !cancelled() && steps.should-proceed.outputs.ok == 'true' run: | set -euo pipefail RUN_URL="${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}"