From e3e449fc4c2a35c1eba0d7a920203aab3591636a Mon Sep 17 00:00:00 2001 From: Shaojin Wen Date: Wed, 8 Jul 2026 22:18:17 +0800 Subject: [PATCH] feat(autofix): extend review loop to all dev-bot PRs, add real-time triggers (#6528) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(autofix): extend review loop to all dev-bot PRs, add real-time triggers * fix(autofix): address review feedback — trusted checkout, sender gate, in-repo check, issue comments * fix(autofix): address suggestion feedback — API warning, bot comment filter, branch prefix doc, test coverage * fix(autofix): drop pull_request_review_comment trigger to avoid redundant runs --- .github/workflows/qwen-autofix.yml | 156 ++++++++++++++++---- .qwen/skills/autofix/SKILL.md | 2 +- scripts/tests/qwen-autofix-workflow.test.js | 62 +++++++- 3 files changed, 189 insertions(+), 31 deletions(-) diff --git a/.github/workflows/qwen-autofix.yml b/.github/workflows/qwen-autofix.yml index 861a537c48..ee5d56cde3 100644 --- a/.github/workflows/qwen-autofix.yml +++ b/.github/workflows/qwen-autofix.yml @@ -8,10 +8,11 @@ name: 'Qwen Autofix' # The lifecycle is asynchronous — a PR is opened in one run and its review is # addressed in a later run once a reviewer has weighed in — so each scheduled # tick runs only the phase(s) that make sense, decided by the `route` job: -# • every 4h → review phase (sweep the bot's open PRs) -# • every 12h (00/12 UTC) → also the issue phase (locate + fix one new bug) -# • issues:labeled → issue phase when ready label, state, and sender match -# • workflow_dispatch → force a phase, an issue, or a PR +# • every 4h → review phase (sweep the bot's open PRs) +# • every 12h (00/12 UTC) → also the issue phase (locate + fix one new bug) +# • issues:labeled → issue phase when ready label, state, and sender match +# • pull_request_review → review phase (real-time feedback loop for bot PRs) +# • workflow_dispatch → force a phase, an issue, or a PR # # Every GitHub write (issue/PR comments, labels, branch push, PR create) goes # through CI_DEV_BOT_PAT so the bot acts as a single identity, qwen-code-dev-bot. @@ -21,6 +22,9 @@ on: issues: types: - 'labeled' + pull_request_review: + types: + - 'submitted' schedule: - cron: '0 0,12 * * *' # Issue + review every 12h; must match route SCHEDULE check - cron: '0 4,8,16,20 * * *' # Review only between issue runs @@ -41,7 +45,7 @@ on: required: false type: 'string' pr_number: - description: 'Force a specific autofix PR number (implies the review phase)' + description: 'Force a specific bot PR number (implies the review phase)' required: false type: 'string' dry_run: @@ -58,9 +62,11 @@ permissions: contents: 'read' env: - # Identity of the autofix bot and the branches it owns. Only its own in-repo - # PRs are ever eligible for the review phase — never a fork or another branch. + # Identity of the autofix bot. All open in-repo PRs authored by this bot are + # eligible for the review phase — not limited to autofix/issue-* branches. AUTOFIX_BOT: 'qwen-code-dev-bot' + # Branch-name prefix used by the issue phase when creating PRs. Also used + # for duplicate-PR detection and issue-number extraction from branch names. BRANCH_PREFIX: 'autofix/issue-' # The automated Qwen PR reviewer posts as this account; its review counts as # actionable feedback even though it is not a human collaborator. @@ -110,6 +116,10 @@ jobs: REPO: '${{ github.repository }}' SENDER_LOGIN: '${{ github.event.sender.login }}' SCHEDULE: '${{ github.event.schedule }}' + PR_AUTHOR: '${{ github.event.pull_request.user.login }}' + PR_NUMBER_EVENT: '${{ github.event.pull_request.number }}' + PR_HEAD_REPO: '${{ github.event.pull_request.head.repo.full_name }}' + PR_BASE_REF: '${{ github.event.pull_request.base.ref }}' run: |- DO_ISSUE=false DO_REVIEW=false @@ -141,6 +151,48 @@ jobs: if [[ "${EVENT_NAME}" == 'schedule' && "${SCHEDULE}" == '0 0,12 * * *' ]]; then DO_ISSUE=true fi + # Real-time review triggers: only process in-repo bot PRs with + # reviews from trusted senders (collaborators or the review bot). + # This prevents arbitrary commenters from forcing expensive + # review-scan runs. Only pull_request_review:submitted triggers + # (not per-comment events) to avoid redundant runs on multi-comment + # reviews. + if [[ "${EVENT_NAME}" == 'pull_request_review' ]]; then + DO_ISSUE=false + if [[ "${PR_AUTHOR}" != "${AUTOFIX_BOT}" ]]; then + echo "🧭 review event ignored: PR author '${PR_AUTHOR}' is not ${AUTOFIX_BOT}" + elif [[ "${PR_HEAD_REPO}" != "${REPO}" ]]; then + echo "🧭 review event ignored: PR is a fork (${PR_HEAD_REPO} != ${REPO})" + elif [[ "${PR_BASE_REF}" != "main" ]]; then + echo "🧭 review event ignored: PR targets '${PR_BASE_REF}' not 'main'" + else + # Verify the reviewer/commenter is trusted (prompt-injection gate). + sender_permission='' + sender_is_trusted=false + if [[ "${SENDER_LOGIN}" == "${REVIEW_BOT}" ]]; then + sender_is_trusted=true + elif [[ -n "${SENDER_LOGIN}" ]]; then + api_error_file="$(mktemp)" + if sender_permission="$(gh api "repos/${REPO}/collaborators/${SENDER_LOGIN}/permission" --jq '.permission // ""' 2>"${api_error_file}")"; then + case "${sender_permission}" in + admin|maintain|write) sender_is_trusted=true ;; + esac + else + api_error="$(tr '\r\n' ' ' < "${api_error_file}")" + echo "::warning::Permission API call failed for ${SENDER_LOGIN}: ${api_error:-unknown error}" + sender_permission='' + fi + rm -f "${api_error_file}" + fi + if [[ "${sender_is_trusted}" == "true" ]]; then + DO_REVIEW=true + ROUTE_PR="$(sanitize_number "${PR_NUMBER_EVENT}")" + echo "🧭 review event on bot PR #${PR_NUMBER_EVENT} by ${SENDER_LOGIN} (${sender_permission:-review-bot}) → review phase" + else + echo "🧭 review event ignored: sender '${SENDER_LOGIN}' permission='${sender_permission:-none}' is not trusted" + fi + fi + fi if [[ "${EVENT_NAME}" == 'issues' ]]; then DO_REVIEW=false label_is_trigger=false @@ -190,7 +242,7 @@ jobs: echo "dry_run=${DRY_RUN}" >> "${GITHUB_OUTPUT}" echo "issue_number=${ROUTE_ISSUE}" >> "${GITHUB_OUTPUT}" echo "pr_number=${ROUTE_PR}" >> "${GITHUB_OUTPUT}" - echo "🧭 phase='${PHASE:-auto}' event='${EVENT_NAME}' issue='#${ISSUE_NUMBER:-n/a}' schedule='${SCHEDULE:-n/a}' dry_run=${DRY_RUN} → issue=${DO_ISSUE} review=${DO_REVIEW}" + echo "🧭 phase='${PHASE:-auto}' event='${EVENT_NAME}' issue='#${ISSUE_NUMBER:-n/a}' pr='#${PR_NUMBER_EVENT:-n/a}' schedule='${SCHEDULE:-n/a}' dry_run=${DRY_RUN} → issue=${DO_ISSUE} review=${DO_REVIEW}" # =========================================================================== # ISSUE PHASE — locate one maintainer-ready issue, fix it, open a PR. @@ -215,7 +267,7 @@ jobs: AUTOFIX_ISSUE_EXCLUDES: 'no:assignee -linked:pr -label:autofix/skip -label:autofix/in-progress -label:status/need-information -label:status/need-retesting sort:created-desc' steps: - name: 'Checkout' - uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: fetch-depth: 0 persist-credentials: false @@ -872,33 +924,40 @@ jobs: run: |- WORKDIR="$(mktemp -d)" - # Candidate PRs: open, authored by the autofix bot, on autofix/issue-*. - # A forced PR must still pass these checks. + # Candidate PRs: open, same-repo, targeting main, authored by the + # dev-bot. A forced PR must still pass all these checks. if [[ -n "${FORCED_PR}" ]]; then META="$(gh pr view "${FORCED_PR}" --repo "${REPO}" \ - --json number,state,author,headRefName 2> /dev/null || echo '{}')" - OK="$(jq -r --arg ab "${AUTOFIX_BOT}" --arg p "${BRANCH_PREFIX}" \ + --json number,state,author,headRefName,isCrossRepository,baseRefName 2> /dev/null || echo '{}')" + OK="$(jq -r --arg ab "${AUTOFIX_BOT}" \ '(((.state // "") == "OPEN") and ((.author.login // "") == $ab) - and ((.headRefName // "") | startswith($p)))' <<< "${META}")" + and ((.isCrossRepository // true) | not) + and ((.baseRefName // "") == "main"))' <<< "${META}")" if [[ "${OK}" != "true" ]]; then - echo "❌ #${FORCED_PR} is not an open autofix PR owned by ${AUTOFIX_BOT}" + echo "❌ #${FORCED_PR} is not an open in-repo main-targeting PR owned by ${AUTOFIX_BOT}" echo "has_targets=false" >> "${GITHUB_OUTPUT}" exit 0 fi CANDIDATES="${FORCED_PR}" else gh pr list --repo "${REPO}" --state open --author "${AUTOFIX_BOT}" \ + --base main \ --limit 100 --json number,headRefName > "${WORKDIR}/bot-prs.json" - CANDIDATES="$(jq -r --arg p "${BRANCH_PREFIX}" \ - '.[] | select(.headRefName | startswith($p)) | .number' \ - "${WORKDIR}/bot-prs.json")" + # gh pr list with --author on the same repo only returns in-repo PRs, + # and --base main limits to main-targeting PRs. + CANDIDATES="$(jq -r '.[] | .number' "${WORKDIR}/bot-prs.json")" fi TARGETS='[]' for PR in ${CANDIDATES}; do BRANCH="$(gh pr view "${PR}" --repo "${REPO}" --json headRefName --jq '.headRefName')" - ISSUE="${BRANCH#"${BRANCH_PREFIX}"}" + # Extract issue number: autofix/issue- → N; otherwise use PR number. + if [[ "${BRANCH}" == "${BRANCH_PREFIX}"* ]]; then + ISSUE="${BRANCH#"${BRANCH_PREFIX}"}" + else + ISSUE="${PR}" + fi HEAD_SHA="$(gh api "repos/${REPO}/pulls/${PR}" --jq '.head.sha')" # Push watermark: the PR's last push. Feedback older than this was in @@ -942,6 +1001,19 @@ jobs: | select((.user.login // "") != $ab) | select(((.author_association // "") | IN($trust[])) or (.user.login // "") == $rb) ] | length' \ "${WORKDIR}/rc.json")" + # Issue-level PR comments (e.g. /review suggestion summaries) are + # also actionable feedback. Exclude the bot's own eval markers. + # Exclude known non-actionable bot comments (triage stages, + # coverage reports, suggestion summaries, force-push reminders). + BOT_COMMENT_FILTER='