-
Notifications
You must be signed in to change notification settings - Fork 3k
feat(autofix): auto-update a PR red only from a stale, since-fixed base #7554
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
b7666f0
feat(autofix): auto-update a PR red only from a stale, since-fixed base
0d4b312
fix(ci): move pipefail fallback outside command substitution (#7554)
d5fbb8b
fix(ci): guard stale-base update-branch with expected_head_sha (#7554)
4c12693
test(ci): pin fail-closed behavior for empty MAIN_HEAD and CMP_STATUS…
fe09456
fix(ci): guard stale-base update-branch with DRY_RUN (#7554)
965f414
merge: resolve conflicts with origin/main (stale-base + infra-rerun)
qwen-code-dev-bot 00ce360
Merge branch 'main' into ci/autofix-update-stale-base
wenshao edae6b7
test(autofix): repair the merge-resolution test breakage
d8d0f21
fix(autofix): fall through to feedback on failed update-branch; asser…
6ff5d98
fix(autofix): address review — fix stale-base gate source, add base d…
1cbb0ea
fix(autofix): drop self-contradictory predicate 2, add state/PR_HEAD_…
d8a9f43
fix(autofix): address review — identity-gate the stale-base write, co…
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1725,6 +1725,54 @@ jobs: | |
| PENDING_STALE_MIN=240 | ||
| PENDING_CUTOFF="$(date -u -d "${PENDING_STALE_MIN} minutes ago" +%Y-%m-%dT%H:%M:%SZ)" | ||
|
|
||
| # Repetition-guard cutoff for the stale-base update marker (invariant | ||
| # across candidate PRs, computed once — same reasoning as | ||
| # PENDING_CUTOFF above). A marker newer than this bounds re-updates | ||
| # to once per 2 hours (CI takes ~40 min; main moves ~13 min). | ||
| BASE_UPDATE_CUTOFF="$(date -u -d '120 minutes ago' +%Y-%m-%dT%H:%M:%SZ)" | ||
|
|
||
| # Base of the auto-update-stale-base decision below. A PR can be red | ||
| # purely because it merged a main that was BROKEN at the time and has | ||
| # since been FIXED — observed repeatedly (a web-shell TS break, an | ||
| # agent-registry test) stranding healthy PRs on a failure that has | ||
| # nothing to do with them. GitHub's "Update branch" merges current | ||
| # main in and re-runs CI, which clears it. We do that automatically | ||
| # only when the SAME failing check also passed for the PR that produced | ||
| # current main (MAIN_GREEN_CHECKS) — a necessary-but-NOT-sufficient | ||
| # signal, NOT proof that main is healthy. | ||
| # | ||
| # MAIN_GREEN_CHECKS is sourced from the last-merged PR's PRE-MERGE | ||
| # check-runs, which ran against that PR merged with main-as-of-then — | ||
| # never the tree now on main (ci.yml has no push trigger, so main's | ||
| # squash commits carry no check-runs to read). main breaks here by | ||
| # SEMANTIC CONFLICT: two PRs green apart but broken together. In exactly | ||
| # that state the last-merged PR is green, this signal reads green, and | ||
| # the update would merge a currently-broken main into a healthy PR. The | ||
| # signal also inherits the last PR's matrix shape (a SKIPPED platform | ||
| # job is absent, so a PR stranded on it is never unstuck — fail-safe, | ||
| # but non-deterministic). The blast radius stays recoverable, not zero: | ||
| # the merge (not rebase) is revertible, a marker bounds re-updates to | ||
| # once per 2h, and the CAS (expected_head_sha) rejects a concurrent | ||
| # push. A re-enabled merge queue would let us source this from a | ||
| # genuinely validated merged tree instead: ci.yml DOES have a | ||
| # merge_group trigger, so a merged tree's check-runs would land where | ||
| # we could read them. | ||
| # | ||
| # Fetch main's head and that check-name set ONCE per scan: resolve | ||
| # main's head to the PR that produced it and read check-runs from that | ||
| # PR's head SHA. | ||
| MAIN_HEAD="$(gh api "repos/${REPO}/commits/main" --jq '.sha' 2> /dev/null || echo '')" | ||
| MAIN_GREEN_CHECKS='[]' | ||
| if [[ -n "${MAIN_HEAD}" ]]; then | ||
| MAIN_PR_HEAD="$(gh api "repos/${REPO}/commits/${MAIN_HEAD}/pulls" \ | ||
| --jq '.[0].head.sha // ""' 2> /dev/null || echo '')" | ||
| if [[ -n "${MAIN_PR_HEAD}" ]]; then | ||
| MAIN_GREEN_CHECKS="$(gh api --paginate "repos/${REPO}/commits/${MAIN_PR_HEAD}/check-runs" \ | ||
| --jq '[.check_runs[] | select(.conclusion == "success") | .name]' 2> /dev/null \ | ||
| | jq -c -s 'add // [] | unique')" || MAIN_GREEN_CHECKS='[]' | ||
|
qwen-code-dev-bot marked this conversation as resolved.
|
||
| fi | ||
| fi | ||
|
|
||
| # PRs whose review-address is already RUNNING OR QUEUED in any live | ||
| # autofix run must not be re-targeted. Schedule/dispatch runs execute | ||
| # against main's SHA, so their matrix jobs never appear in the PR's | ||
|
|
@@ -1821,6 +1869,7 @@ jobs: | |
| ISSUE="${PR}" | ||
| fi | ||
| CHECKS_JSON="$(jq -c '.statusCheckRollup // []' <<< "${PR_META}")" | ||
| PR_HEAD_OID="$(jq -r '.headRefOid // ""' <<< "${PR_META}")" | ||
|
|
||
| # Auto-rerun a check that died on INFRASTRUCTURE, not the code (see | ||
| # INFRA_FAILURE_SIGNATURES). Only reached when the PR has a FAILED | ||
|
|
@@ -1831,7 +1880,6 @@ jobs: | |
| # marker needed; the attempt counter is the guard, and after a rerun | ||
| # the attempt increments so the next scan skips it. Any API failure | ||
| # here is fail-safe: it just means no rerun. | ||
| PR_HEAD_OID="$(jq -r '.headRefOid // ""' <<< "${PR_META}")" | ||
| if [[ -n "${PR_HEAD_OID}" ]] && jq -e 'any(.[]; ((.conclusion // .state // "") | IN("FAILURE","FAILED","ERROR","TIMED_OUT","ACTION_REQUIRED")) and (((.workflowName // "") != "Qwen Autofix") or ((.name // "") | startswith("review-address"))))' <<< "${CHECKS_JSON}" > /dev/null 2>&1; then | ||
| RERAN_INFRA=false | ||
| # Failed check-runs on this head, with their run id and annotation | ||
|
|
@@ -2082,6 +2130,90 @@ jobs: | |
| fi | ||
| continue | ||
| fi | ||
| # Auto-update a PR that is red ONLY because of a stale base (see the | ||
| # MAIN_GREEN_CHECKS rationale above). The gate: the failing check also | ||
| # passed for the PR that produced current main (a necessary-but-NOT- | ||
| # sufficient signal — NOT proof main is healthy), and the PR is behind | ||
| # or diverged, so it actually carries a stale base. Runs after the | ||
| # round cap and pending-checks gates but before the feedback logic, | ||
| # because a stuck-on-stale-base PR often has no NEW feedback at all (it | ||
| # just sits red), which is exactly #7490's case. | ||
| if [[ -n "${MAIN_HEAD}" && -n "${PR_HEAD_OID}" ]]; then | ||
| # STALE_BASE_REDS is pure jq over data already in memory | ||
| # (CHECKS_JSON, MAIN_GREEN_CHECKS) — free, and far more selective | ||
| # than the compare round-trip. Compute it FIRST and skip the network | ||
| # call entirely when there is no stale-base red to act on (the common | ||
| # case: a green PR, or one whose red check is also red on main). | ||
| # CANCELLED is deliberately omitted from the PR-side selector: a | ||
| # cancelled check is not evidence of a stale base. External commit | ||
| # statuses are also excluded: a StatusContext exposes .context, not | ||
| # .name/.workflowName, so it yields "" and select(. != "") drops it | ||
| # (conservative — only Actions check-runs are matched). | ||
| STALE_BASE_REDS="$(jq -c -n \ | ||
| --argjson checks "${CHECKS_JSON}" --argjson green "${MAIN_GREEN_CHECKS}" ' | ||
| [ $checks[] | ||
| | select((.conclusion // .state // "") | IN("FAILURE", "FAILED", "ERROR", "TIMED_OUT", "ACTION_REQUIRED")) | ||
| | select((.workflowName // "") != "Qwen Autofix") | ||
| | (.name // .workflowName // "") | ||
| | select(. != "" and (. as $n | $green | index($n))) ]')" || STALE_BASE_REDS='[]' | ||
| if [[ "${STALE_BASE_REDS}" != '[]' ]]; then | ||
| # --jq '.status': the compare document is ~60KB; only the | ||
| # behind/diverged/ahead status is needed. | ||
| CMP_STATUS="$(gh api "repos/${REPO}/compare/${MAIN_HEAD}...${PR_HEAD_OID}" --jq '.status // ""' 2> /dev/null || echo '')" | ||
| if [[ "${CMP_STATUS}" == 'behind' || "${CMP_STATUS}" == 'diverged' ]]; then | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] — qwen3.8-max-preview via Qwen Code /review |
||
| # Repetition guard: a marker comment bounds re-updates to | ||
| # once per 2 hours (see BASE_UPDATE_CUTOFF, hoisted above the | ||
| # loop). Without this, a still-red PR would be re-updated on | ||
| # every scan after main advances. | ||
| BASE_UPDATE_RECENT="$(jq -r --arg ab "${AUTOFIX_BOT}" \ | ||
| --arg cutoff "${BASE_UPDATE_CUTOFF}" ' | ||
| [ .[] | select((.user.login // "") == $ab) | ||
| | select((.body // "") | contains("<!-- autofix-base-updated -->")) | ||
| | select((.created_at // "") > $cutoff) ] | length > 0' "${WORKDIR}/ic.json")" | ||
| if [[ "${BASE_UPDATE_RECENT}" != "true" ]]; then | ||
| RED_NAMES="$(jq -r 'join(", ")' <<< "${STALE_BASE_REDS}")" | ||
| if [[ "${DRY_RUN}" == "true" ]]; then | ||
| echo "🧪 DRY-RUN: would update stale base on #${PR} (red [${RED_NAMES}] green on main ${MAIN_HEAD:0:9})" | ||
| fleet_row "${PR}" 'dry-run-base' "would merge main (stale-base red [${RED_NAMES}])" | ||
| continue | ||
| fi | ||
| # Convention: verify the PAT identity before ANY write (same | ||
| # as the engage ack and cap notice above). update-branch AND | ||
| # its marker are writes; a rotated PAT would do both under a | ||
| # foreign login the dedup (which counts AUTOFIX_BOT comments | ||
| # only) can never see — re-updating every scan. Memoized. | ||
| if [[ -z "${SCAN_BOT_ACTOR:-}" ]]; then | ||
| SCAN_BOT_ACTOR="$(gh api user --jq '.login' 2> /dev/null || echo 'unknown')" | ||
| fi | ||
| if [[ "${SCAN_BOT_ACTOR}" != "${AUTOFIX_BOT}" ]]; then | ||
| echo "::warning::#${PR}: stale-base update skipped: PAT authenticates as '${SCAN_BOT_ACTOR}', expected ${AUTOFIX_BOT}" | ||
| fleet_row "${PR}" 'base-update-skipped' "stale-base red [${RED_NAMES}] but PAT identity '${SCAN_BOT_ACTOR}' != ${AUTOFIX_BOT}" | ||
| # expected_head_sha makes this a compare-and-swap: if the | ||
| # author pushed between our compare read and this call, | ||
| # GitHub rejects it rather than merging main into an | ||
| # unverified head. | ||
| elif UPDATE_ERR="$(gh api -X PUT "repos/${REPO}/pulls/${PR}/update-branch" -f expected_head_sha="${PR_HEAD_OID}" 2>&1 >/dev/null)"; then | ||
| echo "🔀 #${PR}: red check(s) [${RED_NAMES}] pass on current main ${MAIN_HEAD:0:9} — merged main in via update-branch; CI will re-run" | ||
| fleet_row "${PR}" 'base-updated' "stale-base red [${RED_NAMES}] — merged current main, CI re-running" | ||
| # The marker is the ONLY repetition guard for this mutating | ||
| # action; a failed post must be loud, not swallowed, so the | ||
| # dedup gap is visible (else the next scan re-updates). | ||
| gh pr comment "${PR}" --repo "${REPO}" --body "$(printf '🔀 Base updated: red check(s) [%s] pass on current main — merged current main via update-branch; CI will re-run.\n\n<details>\n<summary>中文说明</summary>\n\n🔀 已更新 base:红色检查 [%s] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。\n\n</details>\n\n<!-- autofix-base-updated -->' "${RED_NAMES}" "${RED_NAMES}")" > /dev/null 2>&1 \ | ||
| || echo "::warning::#${PR}: base-updated marker post failed — the 2h repetition guard is NOT armed for this update" | ||
| continue | ||
| else | ||
| echo "⚠️ #${PR}: wanted to update the stale base (red [${RED_NAMES}] green on main) but update-branch failed: ${UPDATE_ERR:-unknown error}" | ||
| fleet_row "${PR}" 'base-update-failed' "stale-base red [${RED_NAMES}] but update-branch failed" | ||
| # A failed update (merge conflict, CAS rejection, or | ||
| # missing allow-edits) is a human problem, but the bot's | ||
| # review comments need not be deferred forever — fall | ||
| # through to feedback processing. | ||
| fi | ||
| fi | ||
| fi | ||
| fi | ||
| fi | ||
|
|
||
| N_FAILED_CHECKS="$(jq --arg wm "${EFF_WM}" ' | ||
| [ .[] | ||
| | select((.conclusion // .state // "") | IN("FAILURE", "FAILED", "ERROR", "TIMED_OUT", "ACTION_REQUIRED", "CANCELLED")) | ||
|
|
@@ -2144,7 +2276,7 @@ jobs: | |
| # bot's own eval markers, and known non-actionable bot comments | ||
| # (triage stages, coverage reports, legacy suggestion summaries, | ||
| # force-push reminders). | ||
| BOT_COMMENT_FILTER='<!-- (autofix-eval|autofix-rearm|qwen-triage|qwen-review-suggestion-summary|pr-force-push|qwen-review-ack) ' | ||
| BOT_COMMENT_FILTER='<!-- (autofix-eval|autofix-rearm|autofix-base-updated|qwen-triage|qwen-review-suggestion-summary|pr-force-push|qwen-review-ack) ' | ||
| # Command-style comments (@qwen-code /takeover, /triage, /review …) | ||
| # are INSTRUCTIONS to tooling, not review feedback on the diff: | ||
| # counting them as actionable burns a full agent cycle to post a | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Suggestion] MAIN_GREEN_CHECKS is based on last-merged PR's pre-merge check-runs, not main's current tree (ci.yml has no push trigger). In semantic conflict scenarios this signal may false-positive green. The comment documents this limitation well — merge queue is the proper fix.
— qwen3.8-max-preview via Qwen Code /review