feat(autofix): extend review loop to all dev-bot PRs, add real-time triggers (#6528)

* 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
This commit is contained in:
Shaojin Wen 2026-07-08 22:18:17 +08:00 committed by GitHub
parent 7281f0de8c
commit e3e449fc4c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 189 additions and 31 deletions

View file

@ -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> → 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='<!-- (autofix-eval|qwen-triage|qwen-review-suggestion-summary|pr-force-push|qwen-review-ack) '
N_ISSUE_COMMENTS="$(jq --arg wm "${EFF_WM}" --arg rb "${REVIEW_BOT}" --arg ab "${AUTOFIX_BOT}" \
--argjson trust "${TRUSTED_ASSOC}" --arg bf "${BOT_COMMENT_FILTER}" '
[ .[]
| select((.created_at // "") > $wm)
| select((.user.login // "") != $ab)
| select(((.author_association // "") | IN($trust[])) or (.user.login // "") == $rb)
| select((.body // "") | test($bf) | not) ] | length' \
"${WORKDIR}/ic.json")"
# mergeable: GitHub may report UNKNOWN until it recomputes; treat only
# an explicit CONFLICTING as a conflict so we never block on UNKNOWN.
@ -949,12 +1021,12 @@ jobs:
HAS_CONFLICT='false'
if [[ "${MERGEABLE}" == "CONFLICTING" ]]; then HAS_CONFLICT='true'; fi
if [[ "${N_REVIEWS}" -eq 0 && "${N_COMMENTS}" -eq 0 && "${HAS_CONFLICT}" != "true" ]]; then
if [[ "${N_REVIEWS}" -eq 0 && "${N_COMMENTS}" -eq 0 && "${N_ISSUE_COMMENTS}" -eq 0 && "${HAS_CONFLICT}" != "true" ]]; then
echo "✅ #${PR}: nothing new since ${EFF_WM} (conflict=${HAS_CONFLICT})"
continue
fi
echo "🔎 #${PR}: ${N_REVIEWS} review(s) + ${N_COMMENTS} comment(s) new, conflict=${HAS_CONFLICT}, round=${ROUND}"
echo "🔎 #${PR}: ${N_REVIEWS} review(s) + ${N_COMMENTS} inline + ${N_ISSUE_COMMENTS} issue comment(s) new, conflict=${HAS_CONFLICT}, round=${ROUND}"
TARGETS="$(jq -c \
--arg pr "${PR}" --arg branch "${BRANCH}" --arg issue "${ISSUE}" \
--arg round "${ROUND}" --arg wm "${EFF_WM}" \
@ -996,9 +1068,15 @@ jobs:
ROUND: '${{ matrix.target.round }}'
WATERMARK: '${{ matrix.target.watermark }}'
steps:
- name: 'Checkout'
uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3
# SECURITY: checkout trusted base code first. The PR branch is checked
# out later in "Prepare branch and feedback" after the trusted CLI bundle
# is built from this base. Without this pin, pull_request_review events
# would check out the PR merge ref by default, letting PR-controlled
# code influence the secret-bearing address run.
- name: 'Checkout trusted base'
uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3
with:
ref: '${{ github.event.repository.default_branch }}'
fetch-depth: 0
persist-credentials: false
@ -1102,9 +1180,12 @@ jobs:
gh api "repos/${REPO}/pulls/${PR}/reviews" --paginate > "${WORKDIR}/rv.json"
gh api "repos/${REPO}/pulls/${PR}/comments" --paginate > "${WORKDIR}/rc.json"
gh api "repos/${REPO}/issues/${PR}/comments" --paginate > "${WORKDIR}/ic.json"
# Newest actionable feedback timestamp — stamped into the eval marker so
# the next scan knows everything up to here has been considered.
# Includes reviews, inline review comments, and issue-level PR comments
# (excluding the bot's own eval markers).
NEWEST="$(jq -rs \
--arg wm "${WATERMARK}" --arg rb "${REVIEW_BOT}" --arg ab "${AUTOFIX_BOT}" \
--argjson trust "${TRUSTED_ASSOC}" '
@ -1115,13 +1196,19 @@ jobs:
+ (.[1] | map(select((.created_at // "") > $wm)
| select((.user.login // "") != $ab)
| select(((.author_association // "") | IN($trust[])) or (.user.login // "") == $rb) | .created_at))
| max // ""' "${WORKDIR}/rv.json" "${WORKDIR}/rc.json")"
+ (.[2] | map(select((.created_at // "") > $wm)
| select((.user.login // "") != $ab)
| select(((.author_association // "") | IN($trust[])) or (.user.login // "") == $rb)
| select((.body // "") | test("<!-- (autofix-eval|qwen-triage|qwen-review-suggestion-summary|pr-force-push|qwen-review-ack) ") | not) | .created_at))
| max // ""' "${WORKDIR}/rv.json" "${WORKDIR}/rc.json" "${WORKDIR}/ic.json")"
[[ -z "${NEWEST}" ]] && NEWEST="${WATERMARK}"
echo "newest=${NEWEST}" >> "${GITHUB_OUTPUT}"
# Render the actionable feedback into one prompt-ready file.
{
echo "# Review feedback to triage on PR #${PR} (issue #${ISSUE})"
ISSUE_REF=""
[[ "${ISSUE}" != "${PR}" ]] && ISSUE_REF=" (issue #${ISSUE})"
echo "# Review feedback to triage on PR #${PR}${ISSUE_REF}"
echo
echo "Only feedback newer than the last evaluation (${WATERMARK}) from"
echo "trusted maintainers or the automated reviewer is listed."
@ -1146,6 +1233,17 @@ jobs:
| select(((.author_association // "") | IN($trust[])) or (.user.login // "") == $rb)
| "- \(.path // "?"):\(.line // "?") @\(.user.login): \(.body // "" | gsub("\r"; ""))"' \
"${WORKDIR}/rc.json"
echo
echo "## Issue-level comments"
jq -r --arg wm "${WATERMARK}" --arg rb "${REVIEW_BOT}" --arg ab "${AUTOFIX_BOT}" \
--argjson trust "${TRUSTED_ASSOC}" '
.[]
| select((.created_at // "") > $wm)
| select((.user.login // "") != $ab)
| select(((.author_association // "") | IN($trust[])) or (.user.login // "") == $rb)
| select((.body // "") | test("<!-- (autofix-eval|qwen-triage|qwen-review-suggestion-summary|pr-force-push|qwen-review-ack) ") | not)
| "- @\(.user.login): \(.body // "" | gsub("\r"; ""))"' \
"${WORKDIR}/ic.json"
} > "${WORKDIR}/feedback.md"
echo '--- feedback.md ---'
cat "${WORKDIR}/feedback.md"
@ -1364,7 +1462,9 @@ jobs:
gh pr comment "${PR}" --repo "${REPO}" --body-file "${WORKDIR}/report.md"
{
echo "### PR #${PR} (issue #${ISSUE}) — ${STATUS}"
ISSUE_REF=""
[[ "${ISSUE}" != "${PR}" ]] && ISSUE_REF=" (issue #${ISSUE})"
echo "### PR #${PR}${ISSUE_REF} — ${STATUS}"
echo "- Base conflict: ${CONFLICT}"
echo
if [[ "${OUTCOME}" == "fixed" ]]; then
@ -1388,7 +1488,9 @@ jobs:
SUFFIX=''
[[ "${DRY_RUN}" == "true" ]] && SUFFIX=' (dry-run, nothing pushed)'
{
echo "### PR #${PR} (issue #${ISSUE}) — outcome=${OUTCOME:-unknown}${SUFFIX}"
ISSUE_REF=""
[[ "${ISSUE}" != "${PR}" ]] && ISSUE_REF=" (issue #${ISSUE})"
echo "### PR #${PR}${ISSUE_REF} — outcome=${OUTCOME:-unknown}${SUFFIX}"
echo "- Base conflict: ${CONFLICT:-unknown}"
echo
for f in address-summary.md no-action.md failure.md handoff.md; do

View file

@ -98,7 +98,7 @@ blocked, write `<workdir>/failure.md` and do not commit.
Inputs: `--pr`, `--issue`, `<workdir>/feedback.md`, `--conflict`, and `--base`.
The workflow already checked out `autofix/issue-<issue>`. Stay on that branch.
The workflow already checked out the PR's head branch. Stay on it.
Read `git diff origin/<base>...HEAD` first, then `<workdir>/feedback.md`.
Classify every feedback point:

View file

@ -271,12 +271,11 @@ describe('qwen-autofix workflow', () => {
expect(workflow).not.toContain(
"issue_comment:\n types:\n - 'created'",
);
// pull_request_review_comment triggers are NOT used to avoid redundant
// runs on multi-comment reviews; only pull_request_review:submitted.
expect(workflow).not.toContain(
"pull_request_review_comment:\n types:\n - 'created'",
);
expect(workflow).not.toContain(
"pull_request_review:\n types:\n - 'submitted'",
);
expect(workflow).not.toContain(
"COMMENT_BODY: '${{ github.event.comment.body }}'",
);
@ -289,6 +288,63 @@ describe('qwen-autofix workflow', () => {
expect(routeStep).not.toContain('ROUTE_ISSUE="${ISSUE_NUMBER}"');
});
it('gates real-time review triggers on bot author, trusted sender, and in-repo PR', () => {
// Route step must check PR author against AUTOFIX_BOT for review events.
expect(routeStep).toContain('"${PR_AUTHOR}" != "${AUTOFIX_BOT}"');
// Must verify sender is trusted (collaborator or review bot).
expect(routeStep).toContain('"${SENDER_LOGIN}" == "${REVIEW_BOT}"');
expect(routeStep).toContain(
'gh api "repos/${REPO}/collaborators/${SENDER_LOGIN}/permission"',
);
// Must reject fork PRs and non-main targets.
expect(routeStep).toContain('"${PR_HEAD_REPO}" != "${REPO}"');
expect(routeStep).toContain('"${PR_BASE_REF}" != "main"');
// Must set ROUTE_PR from the event payload.
expect(routeStep).toContain(
'ROUTE_PR="$(sanitize_number "${PR_NUMBER_EVENT}")"',
);
// Review-scan must also verify in-repo and base-ref for forced PRs.
const reviewScanStep =
workflow.match(
/- name: 'Scan for PRs with new feedback'[\s\S]*?(?=\n[ ]{6}- name: )/,
)?.[0] ?? '';
expect(reviewScanStep).toContain('isCrossRepository');
expect(reviewScanStep).toContain('(.baseRefName // "") == "main"');
expect(reviewScanStep).toContain('--base main');
// review-address must check out trusted base, not PR merge ref.
expect(workflow).toContain("'Checkout trusted base'");
expect(workflow).toContain(
"ref: '${{ github.event.repository.default_branch }}'",
);
});
it('includes issue-level comments in review feedback scanning', () => {
const reviewScanStep =
workflow.match(
/- name: 'Scan for PRs with new feedback'[\s\S]*?(?=\n[ ]{6}- name: )/,
)?.[0] ?? '';
// Must count issue-level comments separately from inline review comments.
expect(reviewScanStep).toContain('N_ISSUE_COMMENTS=');
// Must fetch issue comments for the count (already fetched for markers).
expect(reviewScanStep).toContain('ic.json');
// Must exclude known non-actionable bot comments.
expect(reviewScanStep).toContain('qwen-triage');
expect(reviewScanStep).toContain('qwen-review-suggestion-summary');
// The "nothing new" gate must check all three feedback sources.
expect(reviewScanStep).toContain('"${N_ISSUE_COMMENTS}" -eq 0');
// review-address must also fetch ic.json and render issue-level comments.
expect(workflow).toContain(
'repos/${REPO}/issues/${PR}/comments" --paginate > "${WORKDIR}/ic.json"',
);
expect(workflow).toContain('## Issue-level comments');
// NEWEST watermark must consider issue-level comment timestamps.
expect(workflow).toContain('.[2] | map(select((.created_at // "")');
// Permission API failures in the review-trigger path must be logged.
expect(routeStep).toContain(
'::warning::Permission API call failed for ${SENDER_LOGIN}',
);
});
it('keeps forced issue routing bounded to open issues', () => {
expect(workflow).toContain(
'--json number,title,body,labels,createdAt,url,state',