diff --git a/.github/workflows/qwen-autofix.yml b/.github/workflows/qwen-autofix.yml index 215db9ed37e..6ca4b8b19e5 100644 --- a/.github/workflows/qwen-autofix.yml +++ b/.github/workflows/qwen-autofix.yml @@ -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='[]' + 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 + # 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("")) + | 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
\n中文说明\n\n🔀 已更新 base：红色检查 [%s] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。\n\n
\n\n' "${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='', + created_at: new Date().toISOString(), + }, + ]) + : '[]'; + writeFileSync(join(dir, 'ic.json'), icJson); + // Wrap in a for-loop so the block's `continue` is legal; a sentinel after + // the loop body tells us whether `continue` fired (stale-base path) or the + // block fell through (no update). Production runs -eo pipefail; match it. + const out = execFileSync( + 'bash', + [ + '-c', + `set -eo pipefail\nfleet_row(){ :; }\nfor _ in x; do\n${script}\nprintf 'FELL_THROUGH'\ndone`, + ], + { + env: { + ...process.env, + REPO: 'o/r', + PR: '1', + MAIN_HEAD: mainHead, + MAIN_GREEN_CHECKS: JSON.stringify(mainGreen), + CHECKS_JSON: JSON.stringify(prChecks), + PR_HEAD_OID: prHeadOid, + DRY_RUN: dryRun ? 'true' : 'false', + AUTOFIX_BOT: 'autofix-bot', + WORKDIR: dir, + PATH: `${bin}:${process.env.PATH}`, + }, + encoding: 'utf8', + }, + ); + const calls = existsSync(join(dir, 'calls.log')) + ? readFileSync(join(dir, 'calls.log'), 'utf8') + : ''; + rmSync(dir, { recursive: true, force: true }); + return { + updated: /pulls\/1\/update-branch/.test(calls), + cas: /expected_head_sha=prhead123/.test(calls), + continued: !out.includes('FELL_THROUGH'), + markerPosted: /autofix-base-updated/.test(calls), + }; + }; + const FAIL = (name) => ({ name, conclusion: 'FAILURE' }); + const FAIL_STATE = (name) => ({ name, state: 'FAILURE' }); + const OK = (name) => ({ name, conclusion: 'SUCCESS' }); + + // Stale-base red: fails here, passes on main, and the PR is behind → + // update & skip. + expect( + run({ + prChecks: [FAIL('Test')], + mainGreen: ['Test'], + }), + ).toEqual({ + updated: true, + cas: true, + continued: true, + markerPosted: true, + }); + // The .conclusion // .state // "" fallback: a check reported with only + // state (no conclusion) is still matched. + expect( + run({ + prChecks: [FAIL_STATE('Test')], + mainGreen: ['Test'], + }), + ).toEqual({ + updated: true, + cas: true, + continued: true, + markerPosted: true, + }); + // Red on the PR AND red on main (main is also broken) → not stale-base. + expect( + run({ + prChecks: [FAIL('Test')], + mainGreen: [], + }), + ).toEqual({ + updated: false, + cas: false, + continued: false, + markerPosted: false, + }); + // Red but the PR already contains main (ahead) → no-op skip. + expect( + run({ + prChecks: [FAIL('Test')], + mainGreen: ['Test'], + cmp: 'ahead', + }), + ).toEqual({ + updated: false, + cas: false, + continued: false, + markerPosted: false, + }); + // Diverged also counts as behind (has commits main lacks AND vice versa). + expect( + run({ + prChecks: [FAIL('Lint')], + mainGreen: ['Lint'], + cmp: 'diverged', + }), + ).toEqual({ + updated: true, + cas: true, + continued: true, + markerPosted: true, + }); + // No red at all → nothing to do. + expect( + run({ + prChecks: [OK('Test')], + mainGreen: ['Test'], + }), + ).toEqual({ + updated: false, + cas: false, + continued: false, + markerPosted: false, + }); + // update-branch fails (a merge conflict): still attempted and logged, but + // the scan falls through to feedback processing rather than skipping the PR. + expect( + run({ + prChecks: [FAIL('Test')], + mainGreen: ['Test'], + updateOk: false, + }), + ).toEqual({ + updated: true, + cas: true, + continued: false, + markerPosted: false, + }); + // DRY_RUN: the scan must NOT call update-branch — it logs and skips. + expect( + run({ + prChecks: [FAIL('Test')], + mainGreen: ['Test'], + dryRun: true, + }), + ).toEqual({ + updated: false, + cas: false, + continued: true, + markerPosted: false, + }); + // A Qwen Autofix check that fails on the PR and passes on main must NOT + // be treated as stale-base red — the exclusion filter keeps the workflow's + // own failing checks from triggering an update-branch. + expect( + run({ + prChecks: [ + { + name: 'Build', + conclusion: 'FAILURE', + workflowName: 'Qwen Autofix', + }, + ], + mainGreen: ['Build'], + }), + ).toEqual({ + updated: false, + cas: false, + continued: false, + markerPosted: false, + }); + // review-address is also a Qwen Autofix check and is excluded (the old + // carve-out was inverted relative to the description's intent). + expect( + run({ + prChecks: [ + { + name: 'review-address (1)', + conclusion: 'FAILURE', + workflowName: 'Qwen Autofix', + }, + ], + mainGreen: ['review-address (1)'], + }), + ).toEqual({ + updated: false, + cas: false, + continued: false, + markerPosted: false, + }); + // MAIN_HEAD empty (initial gh api failure): the -n guard prevents any + // update-branch call. + expect( + run({ + prChecks: [FAIL('Test')], + mainGreen: ['Test'], + mainHead: '', + }), + ).toEqual({ + updated: false, + cas: false, + continued: false, + markerPosted: false, + }); + // CMP_STATUS empty (compare API failure): empty falls through (no update). + expect( + run({ + prChecks: [FAIL('Test')], + mainGreen: ['Test'], + cmp: '', + }), + ).toEqual({ + updated: false, + cas: false, + continued: false, + markerPosted: false, + }); + // Repetition guard: a recent base-updated marker prevents re-updating. + expect( + run({ + prChecks: [FAIL('Test')], + mainGreen: ['Test'], + hasMarker: true, + }), + ).toEqual({ + updated: false, + cas: false, + continued: false, + markerPosted: false, + }); + // PR_HEAD_OID empty (headRefOid missing from PR metadata): the -n guard + // prevents any update-branch call. + expect( + run({ + prChecks: [FAIL('Test')], + mainGreen: ['Test'], + prHeadOid: '', + }), + ).toEqual({ + updated: false, + cas: false, + continued: false, + markerPosted: false, + }); + // Wrong PAT identity: the mutating update-branch and its marker are + // skipped (convention: verify identity before ANY write), and the scan + // falls through to feedback processing rather than updating under a + // foreign login the dedup could never see. + expect( + run({ + prChecks: [FAIL('Test')], + mainGreen: ['Test'], + actor: 'some-other-bot', + }), + ).toEqual({ + updated: false, + cas: false, + continued: false, + markerPosted: false, + }); + }); + + it('resolves MAIN_GREEN_CHECKS from the last-merged PR check-runs, once per scan', () => { + // The stale-base gate reads MAIN_GREEN_CHECKS, but the test above injects it + // as a pre-built env var, sidestepping the three gh calls and the jq + // aggregation that populate it. Exercise that population block end-to-end: + // resolve main's head -> the PR that produced it -> that PR's check-runs, + // then concatenate pages and unique them. Every failure path must fail safe + // to an empty set (no update-branch can then trigger). + const popBlock = reviewScanJob.match( + /( {10}MAIN_HEAD="\$\(gh api "repos\/\$\{REPO\}\/commits\/main" --jq '\.sha'[\s\S]*?\n {10}fi\n)/, + )?.[1]; + expect(popBlock).toBeTruthy(); + // The server-side filter keeps only successfully-concluded check-runs, by + // name; pin it so a change to the conclusion predicate is caught (the stub + // below cannot execute this --jq itself). + expect(popBlock).toContain('select(.conclusion == "success") | .name'); + const popScript = popBlock.replace(/^ {10}/gm, ''); + + const runPop = ({ + mainSha = 'mainsha123', + prHeadSha = 'prheadsha456', + greenPages = [ + ['Test', 'Lint'], + ['Lint', 'Build'], + ], + mainFails = false, + pullsFails = false, + checkRunsFails = false, + }) => { + const dir = mkdtempSync(join(tmpdir(), 'mgc-')); + const bin = join(dir, 'bin'); + mkdirSync(bin); + const pageArgs = greenPages + .map((p) => `'${JSON.stringify(p)}'`) + .join(' '); + writeFileSync( + join(bin, 'gh'), + [ + '#!/usr/bin/env bash', + 'for a in "$@"; do case "$a" in', + ` *commits/main) ${mainFails ? 'exit 1' : `printf '%s' '${mainSha}'`}; exit 0;;`, + ` */pulls) ${pullsFails ? 'exit 1' : `printf '%s' '${prHeadSha}'`}; exit 0;;`, + ` *check-runs) ${checkRunsFails ? 'exit 1' : `printf '%s\\n' ${pageArgs}`}; exit 0;;`, + 'esac; done', + 'exit 0', + ].join('\n'), + ); + chmodSync(join(bin, 'gh'), 0o755); + const out = execFileSync( + 'bash', + [ + '-c', + `set -eo pipefail\n${popScript}\njq -n --arg h "$MAIN_HEAD" --argjson g "$MAIN_GREEN_CHECKS" '{mainHead:$h, green:$g}'`, + ], + { + env: { + ...process.env, + REPO: 'o/r', + PATH: `${bin}:${process.env.PATH}`, + }, + encoding: 'utf8', + }, + ); + rmSync(dir, { recursive: true, force: true }); + return JSON.parse(out); + }; + + // Two pages aggregate (concatenate then unique, lexical order). + expect(runPop({})).toEqual({ + mainHead: 'mainsha123', + green: ['Build', 'Lint', 'Test'], + }); + // A single page with a duplicate also uniques. + expect(runPop({ greenPages: [['Test', 'Test', 'Lint']] })).toEqual({ + mainHead: 'mainsha123', + green: ['Lint', 'Test'], + }); + // commits/main fails -> fail-safe empty. + expect(runPop({ mainFails: true })).toEqual({ mainHead: '', green: [] }); + // The /pulls resolution fails -> fail-safe empty. + expect(runPop({ pullsFails: true })).toEqual({ + mainHead: 'mainsha123', + green: [], + }); + // check-runs fails -> fail-safe empty. + expect(runPop({ checkRunsFails: true })).toEqual({ + mainHead: 'mainsha123', + green: [], + }); + }); + it('auto-reruns a check that died on infrastructure, once, guarded by run_attempt', () => { // A self-hosted runner losing the server (or the disk filling) reds a check // for a reason unrelated to the PR; it clears on a rerun (#7490's E2E: @@ -408,11 +818,20 @@ describe('qwen-autofix workflow', () => { // failed job ONCE, and run_attempt is the guard: a run already at attempt 2 // and still infra-failing is persistent and left alone — no infinite loop. const block = reviewScanJob.match( - /( {12}PR_HEAD_OID="\$\(jq -r '\.headRefOid[\s\S]*?\n {12}fi\n)\n {12}# startedAt is the only staleness/, + /( {12}# Auto-rerun a check that died on INFRASTRUCTURE[\s\S]*?\n {12}fi\n)\n {12}# startedAt is the only staleness/, )?.[1]; expect(block).toBeTruthy(); const script = block.replace(/^ {12}/gm, ''); + // Single-source the signature list from the workflow rather than re-typing + // it here, so the production value and this test can never drift out of + // sync (same extract-from-source idiom as NON_BLOCKING_CHECKS above). The + // toContain guard fails loudly if the env is renamed or the regex breaks — + // otherwise an empty pattern would match every line and silently pass. + const INFRA_SIGNATURES = + workflow.match(/INFRA_FAILURE_SIGNATURES: '([^']*)'/)?.[1] ?? ''; + expect(INFRA_SIGNATURES).toContain('lost communication with the server'); + const run = ({ checks, annotations, @@ -465,9 +884,9 @@ describe('qwen-autofix workflow', () => { REPO: 'o/r', PR: '1', PR_META: JSON.stringify({ headRefOid: 'headSHA' }), + PR_HEAD_OID: 'headSHA', CHECKS_JSON: JSON.stringify(checks), - INFRA_FAILURE_SIGNATURES: - 'lost communication with the server|No space left on device|ENOSPC|received a shutdown signal|The runner has received|Failed to initialize container|runner (was|has been) (lost|terminated)|invalid index-pack output|RPC failed', + INFRA_FAILURE_SIGNATURES: INFRA_SIGNATURES, PATH: `${bin}:${process.env.PATH}`, }, encoding: 'utf8', @@ -4878,10 +5297,15 @@ describe('qwen-autofix workflow', () => { ); expect(workflow).toContain("RETRY_COMMAND: '@qwen-code /retry'"); expect(workflow).toContain(''); - expect(workflow).toContain('