From b7666f080c6c89df26b3fe2c633ed7b0fbb32fad Mon Sep 17 00:00:00 2001 From: wenshao Date: Thu, 23 Jul 2026 10:47:12 +0800 Subject: [PATCH 01/10] feat(autofix): auto-update a PR red only from a stale, since-fixed base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A PR can be red purely because it merged a main that was broken then and is fixed now — observed twice today: a web-shell TS break and an agent-registry test, each stranding healthy PRs on a failure with nothing to do with them. The recovery was manual: merge current main and let CI re-run. The scan now does that automatically via GitHub's update-branch (a merge, never a rebase, so no force-push and no dismissed history). The single safety gate is that the SAME failing check is passing on current main. That one condition proves both halves at once: the red is base-inherited (green on main = not the PR's own bug) AND main is healthy on that check right now (so the merge cannot import a fresh breakage). It acts only when the PR is also BEHIND main (compare status behind/diverged) — otherwise the update is a no-op and the red is not stale-base after all. Self-limiting: after the update the PR contains main's head, so it is no longer behind and the next scan will not re-update. A failed update (merge conflict) is logged and the PR is left for a human. Runs 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 what stranded #7490. --- .github/workflows/qwen-autofix.yml | 54 ++++++++++++ scripts/tests/qwen-autofix-workflow.test.js | 95 +++++++++++++++++++++ 2 files changed, 149 insertions(+) diff --git a/.github/workflows/qwen-autofix.yml b/.github/workflows/qwen-autofix.yml index 4c8d9ef1579..5b909f3e7ba 100644 --- a/.github/workflows/qwen-autofix.yml +++ b/.github/workflows/qwen-autofix.yml @@ -1713,6 +1713,26 @@ jobs: PENDING_STALE_MIN=240 PENDING_CUTOFF="$(date -u -d "${PENDING_STALE_MIN} 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, + # but ONLY when the specific failing check is GREEN on current main — + # that is the single safety gate: it proves the red is base-inherited + # (not the PR's own bug) AND that main is healthy on that check right + # now (so the update cannot pull a NEW breakage in). Fetch main's head + # and the set of check names currently passing on it, ONCE per scan. + DEFAULT_BRANCH="${DEFAULT_BRANCH:-main}" + MAIN_HEAD="$(gh api "repos/${REPO}/commits/${DEFAULT_BRANCH}" --jq '.sha' 2> /dev/null || echo '')" + MAIN_GREEN_CHECKS='[]' + if [[ -n "${MAIN_HEAD}" ]]; then + MAIN_GREEN_CHECKS="$(gh api --paginate "repos/${REPO}/commits/${MAIN_HEAD}/check-runs" \ + --jq '[.check_runs[] | select(.conclusion == "success") | .name]' 2> /dev/null \ + | jq -c -s 'add // []' || echo '[]')" + 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 @@ -1809,6 +1829,40 @@ jobs: ISSUE="${PR}" fi CHECKS_JSON="$(jq -c '.statusCheckRollup // []' <<< "${PR_META}")" + + # Auto-update a PR that is red ONLY because of a stale base (see the + # MAIN_GREEN_CHECKS rationale above). Act only when a check that is + # FAILING on this PR is passing on current main by the SAME name — + # that gate proves it is base-inherited and main is healthy on it, so + # the merge cannot import a fresh breakage. Runs 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. + 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") or (((.name // "") | startswith("review-address")))) + | (.name // .workflowName // "") + | select(. != "" and (. as $n | $green | index($n))) ]')" + PR_HEAD_OID="$(jq -r '.headRefOid // ""' <<< "${PR_META}")" + if [[ "${STALE_BASE_REDS}" != '[]' && -n "${MAIN_HEAD}" && -n "${PR_HEAD_OID}" ]]; then + # Only when the PR does not already contain main's head — else the + # update is a no-op (422) and the red is NOT stale-base after all. + # Compare by SHA so a fork head needs no owner:branch qualifier. + 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 + RED_NAMES="$(jq -r 'join(", ")' <<< "${STALE_BASE_REDS}")" + if gh api -X PUT "repos/${REPO}/pulls/${PR}/update-branch" > /dev/null 2>&1; then + echo "🔀 #${PR}: red check(s) [${RED_NAMES}] pass on current main ${MAIN_HEAD:0:9} but fail here on a stale base — 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" + else + echo "⚠️ #${PR}: wanted to update the stale base (red [${RED_NAMES}] green on main) but update-branch failed (likely a merge conflict) — leaving for a human" + fleet_row "${PR}" 'base-update-failed' "stale-base red [${RED_NAMES}] but update-branch failed (conflict?)" + fi + continue + fi + fi + # startedAt is the only staleness clock: a check blocks only if it # started within the bound; one with no startedAt (queued, not yet # running) is not blocking (the next scan re-checks once it starts). diff --git a/scripts/tests/qwen-autofix-workflow.test.js b/scripts/tests/qwen-autofix-workflow.test.js index f344e75b413..d08797e1a77 100644 --- a/scripts/tests/qwen-autofix-workflow.test.js +++ b/scripts/tests/qwen-autofix-workflow.test.js @@ -401,6 +401,101 @@ describe('qwen-autofix workflow', () => { expect(run([{ ...llm, name: 'resolve-pr' }])).toBe('true'); }); + it('auto-updates a PR red only from a stale base, gated on the check being green on main', () => { + // A PR can be red purely because it merged a main that was broken then and + // is fixed now (a web-shell TS break, an agent-registry test — both stranded + // healthy PRs today). The one safety gate is: the SAME failing check passes + // on current main. That proves the red is base-inherited AND main is healthy + // on it, so merging main in cannot import a fresh breakage. + const block = reviewScanJob.match( + /( {12}STALE_BASE_REDS="\$\(jq[\s\S]*?\n {12}fi\n)\n {12}# startedAt is the only staleness/, + )?.[1]; + expect(block).toBeTruthy(); + const script = block.replace(/^ {12}/gm, ''); + + const run = ({ prChecks, mainGreen, cmp = 'behind', updateOk = true }) => { + const dir = mkdtempSync(join(tmpdir(), 'ub-')); + const bin = join(dir, 'bin'); + mkdirSync(bin); + writeFileSync( + join(bin, 'gh'), + [ + '#!/usr/bin/env bash', + `echo "$*" >> ${JSON.stringify(join(dir, 'calls.log'))}`, + 'for a in "$@"; do case "$a" in', + ` */compare/*) printf '${cmp}'; exit 0;;`, + ` */update-branch) exit ${updateOk ? 0 : 1};;`, + 'esac; done', + 'exit 0', + ].join('\n'), + ); + chmodSync(join(bin, 'gh'), 0o755); + // 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). + const out = execFileSync( + 'bash', + [ + '-c', + `set -uo pipefail\nfleet_row(){ :; }\nfor _ in x; do\n${script}\nprintf 'FELL_THROUGH'\ndone`, + ], + { + env: { + ...process.env, + REPO: 'o/r', + PR: '1', + MAIN_HEAD: 'mainhead999', + MAIN_GREEN_CHECKS: JSON.stringify(mainGreen), + CHECKS_JSON: JSON.stringify(prChecks), + PR_META: JSON.stringify({ headRefOid: 'prhead123' }), + 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), + continued: !out.includes('FELL_THROUGH'), + }; + }; + const FAIL = (name) => ({ name, conclusion: 'FAILURE' }); + const OK = (name) => ({ name, conclusion: 'SUCCESS' }); + + // Base-inherited red (fails here, passes on main) + behind → update & skip. + expect(run({ prChecks: [FAIL('Test')], mainGreen: ['Test'] })).toEqual({ + updated: true, + continued: true, + }); + // Red on the PR AND on main (not base-inherited — the PR's own bug) → never + // touch it. This is the gate that stops churning a genuinely-broken PR. + expect(run({ prChecks: [FAIL('Test')], mainGreen: [] })).toEqual({ + updated: false, + continued: false, + }); + // Base-inherited red but the PR already contains main (ahead) → no-op skip. + expect( + run({ prChecks: [FAIL('Test')], mainGreen: ['Test'], cmp: 'ahead' }), + ).toEqual({ updated: false, continued: 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, continued: true }); + // No red at all → nothing to do. + expect(run({ prChecks: [OK('Test')], mainGreen: ['Test'] })).toEqual({ + updated: false, + continued: false, + }); + // update-branch fails (a merge conflict): still attempted, logged, and the + // PR is skipped this scan rather than crashing the loop. + expect( + run({ prChecks: [FAIL('Test')], mainGreen: ['Test'], updateOk: false }), + ).toEqual({ updated: true, continued: true }); + }); + it('keeps a still-red check visible, but only once per head', () => { // A red check is a STATE, not the instant it turned red. Counting only // "failed since the watermark" made a still-failing PR invisible the From 0d4b3124f93dd2394f97255b8c32e116cc936281 Mon Sep 17 00:00:00 2001 From: Qwen Code Autofix Date: Thu, 23 Jul 2026 03:58:08 +0000 Subject: [PATCH 02/10] fix(ci): move pipefail fallback outside command substitution (#7554) --- .github/workflows/qwen-autofix.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/qwen-autofix.yml b/.github/workflows/qwen-autofix.yml index 5b909f3e7ba..3c23f427bdf 100644 --- a/.github/workflows/qwen-autofix.yml +++ b/.github/workflows/qwen-autofix.yml @@ -1730,7 +1730,7 @@ jobs: if [[ -n "${MAIN_HEAD}" ]]; then MAIN_GREEN_CHECKS="$(gh api --paginate "repos/${REPO}/commits/${MAIN_HEAD}/check-runs" \ --jq '[.check_runs[] | select(.conclusion == "success") | .name]' 2> /dev/null \ - | jq -c -s 'add // []' || echo '[]')" + | jq -c -s 'add // []')" || MAIN_GREEN_CHECKS='[]' fi # PRs whose review-address is already RUNNING OR QUEUED in any live From d5fbb8badb97bcb2bff6fb5926c349fe8067efa6 Mon Sep 17 00:00:00 2001 From: Qwen Code Autofix Date: Thu, 23 Jul 2026 04:51:44 +0000 Subject: [PATCH 03/10] fix(ci): guard stale-base update-branch with expected_head_sha (#7554) --- .github/workflows/qwen-autofix.yml | 5 +++- scripts/tests/qwen-autofix-workflow.test.js | 30 +++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/.github/workflows/qwen-autofix.yml b/.github/workflows/qwen-autofix.yml index 3c23f427bdf..411ecca30c6 100644 --- a/.github/workflows/qwen-autofix.yml +++ b/.github/workflows/qwen-autofix.yml @@ -1852,7 +1852,10 @@ jobs: 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 RED_NAMES="$(jq -r 'join(", ")' <<< "${STALE_BASE_REDS}")" - if gh api -X PUT "repos/${REPO}/pulls/${PR}/update-branch" > /dev/null 2>&1; then + # 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. + if gh api -X PUT "repos/${REPO}/pulls/${PR}/update-branch" -f expected_head_sha="${PR_HEAD_OID}" > /dev/null 2>&1; then echo "🔀 #${PR}: red check(s) [${RED_NAMES}] pass on current main ${MAIN_HEAD:0:9} but fail here on a stale base — 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" else diff --git a/scripts/tests/qwen-autofix-workflow.test.js b/scripts/tests/qwen-autofix-workflow.test.js index d08797e1a77..fef25998a6f 100644 --- a/scripts/tests/qwen-autofix-workflow.test.js +++ b/scripts/tests/qwen-autofix-workflow.test.js @@ -494,6 +494,36 @@ describe('qwen-autofix workflow', () => { expect( run({ prChecks: [FAIL('Test')], mainGreen: ['Test'], updateOk: false }), ).toEqual({ updated: true, continued: true }); + // A Qwen Autofix check (not review-address) 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. A logic + // inversion in that filter would pass the cases above (none set workflowName). + expect( + run({ + prChecks: [ + { + name: 'Build', + conclusion: 'FAILURE', + workflowName: 'Qwen Autofix', + }, + ], + mainGreen: ['Build'], + }), + ).toEqual({ updated: false, continued: false }); + // ...but a review-address check IS eligible (it is the workflow's signal that + // the previous address round needs a fresh base), so it still updates. + expect( + run({ + prChecks: [ + { + name: 'review-address (1)', + conclusion: 'FAILURE', + workflowName: 'Qwen Autofix', + }, + ], + mainGreen: ['review-address (1)'], + }), + ).toEqual({ updated: true, continued: true }); }); it('keeps a still-red check visible, but only once per head', () => { From 4c12693b253e85d394b5f5c37816ec63fbe26031 Mon Sep 17 00:00:00 2001 From: Qwen Code Autofix Date: Thu, 23 Jul 2026 05:53:53 +0000 Subject: [PATCH 04/10] test(ci): pin fail-closed behavior for empty MAIN_HEAD and CMP_STATUS (#7554) --- scripts/tests/qwen-autofix-workflow.test.js | 22 +++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/scripts/tests/qwen-autofix-workflow.test.js b/scripts/tests/qwen-autofix-workflow.test.js index fef25998a6f..ec00d964f4d 100644 --- a/scripts/tests/qwen-autofix-workflow.test.js +++ b/scripts/tests/qwen-autofix-workflow.test.js @@ -413,7 +413,13 @@ describe('qwen-autofix workflow', () => { expect(block).toBeTruthy(); const script = block.replace(/^ {12}/gm, ''); - const run = ({ prChecks, mainGreen, cmp = 'behind', updateOk = true }) => { + const run = ({ + prChecks, + mainGreen, + cmp = 'behind', + updateOk = true, + mainHead = 'mainhead999', + }) => { const dir = mkdtempSync(join(tmpdir(), 'ub-')); const bin = join(dir, 'bin'); mkdirSync(bin); @@ -444,7 +450,7 @@ describe('qwen-autofix workflow', () => { ...process.env, REPO: 'o/r', PR: '1', - MAIN_HEAD: 'mainhead999', + MAIN_HEAD: mainHead, MAIN_GREEN_CHECKS: JSON.stringify(mainGreen), CHECKS_JSON: JSON.stringify(prChecks), PR_META: JSON.stringify({ headRefOid: 'prhead123' }), @@ -524,6 +530,18 @@ describe('qwen-autofix workflow', () => { mainGreen: ['review-address (1)'], }), ).toEqual({ updated: true, continued: true }); + // MAIN_HEAD empty (initial gh api failure): the -n guard prevents any + // update-branch call — a future refactor removing that guard would silently + // allow updates with a null compare baseline. + expect( + run({ prChecks: [FAIL('Test')], mainGreen: ['Test'], mainHead: '' }), + ).toEqual({ updated: false, continued: false }); + // CMP_STATUS empty (compare API failure): empty falls through today (no + // update), but a future change treating empty as "assume behind" would + // auto-merge main into PRs blindly. + expect( + run({ prChecks: [FAIL('Test')], mainGreen: ['Test'], cmp: '' }), + ).toEqual({ updated: false, continued: false }); }); it('keeps a still-red check visible, but only once per head', () => { From fe0945611cc8db30656b7df0be13f8a7ef958df0 Mon Sep 17 00:00:00 2001 From: Qwen Code Autofix Date: Thu, 23 Jul 2026 07:19:58 +0000 Subject: [PATCH 05/10] fix(ci): guard stale-base update-branch with DRY_RUN (#7554) --- .github/workflows/qwen-autofix.yml | 5 +++++ scripts/tests/qwen-autofix-workflow.test.js | 6 ++++++ 2 files changed, 11 insertions(+) diff --git a/.github/workflows/qwen-autofix.yml b/.github/workflows/qwen-autofix.yml index 411ecca30c6..ab4209b43da 100644 --- a/.github/workflows/qwen-autofix.yml +++ b/.github/workflows/qwen-autofix.yml @@ -1852,6 +1852,11 @@ jobs: 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 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 # 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. diff --git a/scripts/tests/qwen-autofix-workflow.test.js b/scripts/tests/qwen-autofix-workflow.test.js index ec00d964f4d..dd31c1a03aa 100644 --- a/scripts/tests/qwen-autofix-workflow.test.js +++ b/scripts/tests/qwen-autofix-workflow.test.js @@ -419,6 +419,7 @@ describe('qwen-autofix workflow', () => { cmp = 'behind', updateOk = true, mainHead = 'mainhead999', + dryRun = false, }) => { const dir = mkdtempSync(join(tmpdir(), 'ub-')); const bin = join(dir, 'bin'); @@ -454,6 +455,7 @@ describe('qwen-autofix workflow', () => { MAIN_GREEN_CHECKS: JSON.stringify(mainGreen), CHECKS_JSON: JSON.stringify(prChecks), PR_META: JSON.stringify({ headRefOid: 'prhead123' }), + DRY_RUN: dryRun ? 'true' : 'false', PATH: `${bin}:${process.env.PATH}`, }, encoding: 'utf8', @@ -500,6 +502,10 @@ describe('qwen-autofix workflow', () => { expect( run({ prChecks: [FAIL('Test')], mainGreen: ['Test'], updateOk: false }), ).toEqual({ updated: true, continued: true }); + // 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, continued: true }); // A Qwen Autofix check (not review-address) 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. A logic From edae6b74ac5fb55523e0a31747f9865c47923a6d Mon Sep 17 00:00:00 2001 From: wenshao Date: Thu, 23 Jul 2026 17:41:55 +0800 Subject: [PATCH 06/10] test(autofix): repair the merge-resolution test breakage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolving the base conflict kept this branch's older CONSECUTIVE_FAILURE and handoff-decision tests (which predate main's PREPARE_OUTCOME env plumbing), so both broke, while it correctly re-anchored the stale-base and infra block extractions. Take main's test file wholesale — its consec-fail, handoff, infra and bilingual tests are all current — then re-add this PR's one intentional test (the stale-base auto-update), and re-anchor the infra test's block extraction onto the "# Auto-rerun a check that died on INFRASTRUCTURE" comment so it stops at that block instead of over-extracting past the now-adjacent stale-base block. Full suite green bar the pre-existing load flakes (eligibility recheck, permanent API failures terminal). --- scripts/tests/qwen-autofix-workflow.test.js | 246 +++++++++++++++++++- 1 file changed, 236 insertions(+), 10 deletions(-) diff --git a/scripts/tests/qwen-autofix-workflow.test.js b/scripts/tests/qwen-autofix-workflow.test.js index 070144b391e..b55a30f4661 100644 --- a/scripts/tests/qwen-autofix-workflow.test.js +++ b/scripts/tests/qwen-autofix-workflow.test.js @@ -562,6 +562,15 @@ describe('qwen-autofix workflow', () => { 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, @@ -615,8 +624,7 @@ describe('qwen-autofix workflow', () => { PR: '1', PR_META: JSON.stringify({ headRefOid: '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', @@ -4367,6 +4375,27 @@ describe('qwen-autofix workflow', () => { expect(noOutput.split('|')[0]).toBe(SENTINEL); expect(noOutput).toContain('crashed before it could evaluate the feedback'); + // A TIMEOUT evaluated nothing → retry (sentinel), not an evaluated advance + // that would strand the unaddressed feedback. Even with OUTCOME=failed set + // by the gate (so GATE_CRASHED is false), the agent-timeout signal wins. + const timedOut = run({ + OUTCOME: 'failed', + AGENT_TIMEOUT: 'timeout (3000000ms)', + }); + expect(timedOut.split('|')[0]).toBe(SENTINEL); + expect(timedOut).toContain('ran out of time before finishing'); + expect(timedOut).toContain('it will retry on the next scan'); + // At the cap it names the real fix instead of promising a refused retry. + const timedOutCapped = run({ + OUTCOME: 'failed', + AGENT_TIMEOUT: 'timeout (3000000ms)', + ROUND: '4', + }); + expect(timedOutCapped).toContain('this was the last automatic attempt'); + expect(timedOutCapped).toContain( + 'split the PR or raise the agent time budget', + ); + // At the cap the gate crash names the operator fix rather than promising a // retry the scan's round gate would refuse. const capped = run({ OUTCOME: '', ROUND: '4' }); @@ -4430,6 +4459,93 @@ describe('qwen-autofix workflow', () => { ).toContain('attempt 3/5'); }); + it('retries a skipped-Prepare (base/infra failure) instead of stranding it terminal', () => { + // NEWEST empty has two meanings, and the fix is to stop conflating them: + // - Prepare RAN but the agent crashed/timed out before reading → terminal + // - Prepare was SKIPPED because an earlier step failed (base install/ + // build) → infra/base, transient → RETRY. + // Observed: a web-shell TS break on `main` failed the trusted-base build + // across a whole scan batch, skipping Prepare, and the old code stranded + // SIX healthy PRs (one at round 11) terminally at round=100. + // End at the decision block's own closing `fi`, anchored on the + // consecutive-failure block that follows (not the report `{`): that block + // was inserted between this decision and the `{`, and it calls `gh api`, so + // a `{`-anchored match over-captures it and fails when gh is unstubbed. + const block = reviewAddressReportStep.match( + /(GATE_CRASHED=false\n[\s\S]*?\n {12}fi)\n\n {12}# Consecutive-failure/, + )?.[1]; + expect(block).toBeTruthy(); + const script = block.replace(/^ {12}/gm, ''); + const SENTINEL = '9999-12-31T23:59:59Z'; + const run = (env) => { + const out = execFileSync( + 'bash', + [ + '-c', + `set -uo pipefail\n${script}\nprintf '%s|%s|%s' "$MARK_TS" "$MARK_ROUND" "$HEADLINE"`, + ], + { + env: { + ...process.env, + NEWEST: '', + WATERMARK: '2026-07-20T09:00:00Z', + ROUND: '3', + MAX_ROUNDS: '100', + OUTCOME: '', + JOB_STATUS: 'failure', + DETAIL_FILE: '', + API_ERROR_DETAIL: '', + API_ERROR_KIND: '', + API_AUTH_MAX_ROUNDS: '3', + PREPARE_OUTCOME: 'skipped', + RETRY_COMMAND: '@qwen-code /retry', + ...env, + }, + encoding: 'utf8', + }, + ); + const [ts, round, headline] = out.split('|'); + return { ts, round, terminal: round === '100', headline }; + }; + + // Prepare skipped, early round → retry: sentinel ts (feedback stays live), + // round increments, NOT terminal, and the headline names infra/base. + const early = run({ PREPARE_OUTCOME: 'skipped', ROUND: '3' }); + expect(early).toMatchObject({ ts: SENTINEL, round: '4', terminal: false }); + expect(early.headline).toContain('setup step'); + expect(early.headline).toContain('retry on the next scan'); + // A PERSISTENTLY broken base is still bounded: at the cap it goes terminal + // (so it cannot loop forever) but keeps the sentinel ts so /retry recovers. + const persistent = run({ PREPARE_OUTCOME: 'skipped', ROUND: '99' }); + expect(persistent).toMatchObject({ ts: SENTINEL, terminal: true }); + expect(persistent.headline).toContain('/retry'); + // A CANCELLED job (concurrency/manual cancel) is a DISTINCT outcome value + // from 'skipped', and a job stopped before Prepare enters the step context + // reports outcome ''. Both are pre-agent and transient, so both must also + // retry — matching only 'skipped' sent them to the terminal branch. + const cancelled = run({ PREPARE_OUTCOME: 'cancelled', ROUND: '3' }); + expect(cancelled).toMatchObject({ + ts: SENTINEL, + round: '4', + terminal: false, + }); + const emptyOutcome = run({ PREPARE_OUTCOME: '', ROUND: '3' }); + expect(emptyOutcome).toMatchObject({ + ts: SENTINEL, + round: '4', + terminal: false, + }); + // Prepare RAN to a verdict (success/failure) and produced no feedback → a + // genuine pre-read agent crash: unchanged terminal behaviour. Both real-run + // outcomes stay terminal; only they do. + for (const outcome of ['success', 'failure']) { + const crashed = run({ PREPARE_OUTCOME: outcome, ROUND: '3' }); + expect(crashed).toMatchObject({ terminal: true }); + expect(crashed.headline).toContain('crashed or timed out before reading'); + expect(crashed.headline).not.toContain('setup step'); + } + }); + it('stops a PR that fails to push for CONSECUTIVE_FAILURE_CAP rounds in a row', () => { // The total round cap bounds productive iteration; this bounds an UNBROKEN // run of failures under takeover, where the strict cap does not apply. @@ -4445,7 +4561,7 @@ describe('qwen-autofix workflow', () => { expect(cap).toBeLessThan(takeoverCap); const block = reviewAddressReportStep.match( - /if \[\[ "\$\{MARK_ROUND\}" != "\$\{MAX_ROUNDS\}" \]\] && \{ \[\[ -z "\$\{API_ERROR_DETAIL\}" \]\] \|\| \[\[ "\$\{API_ERROR_KIND\}" == 'auth' \]\]; \}; then\n {14}CONSEC_FAIL=1\n[\s\S]*?\n {14}fi\n {12}fi\n/, + /if \[\[ "\$\{MARK_ROUND\}" != "\$\{MAX_ROUNDS\}" \]\] && \[\[ "\$\{PREPARE_OUTCOME\}" == 'success' \|\| "\$\{PREPARE_OUTCOME\}" == 'failure' \]\] && \{ \[\[ -z "\$\{API_ERROR_DETAIL\}" \]\] \|\| \[\[ "\$\{API_ERROR_KIND\}" == 'auth' \]\]; \}; then\n {14}CONSEC_FAIL=1\n[\s\S]*?\n {14}fi\n {12}fi\n/, )?.[0]; expect(block).toBeTruthy(); const script = block.replace(/^ {12}/gm, ''); @@ -4455,10 +4571,22 @@ describe('qwen-autofix workflow', () => { const FAIL_TIMEOUT = '🤖 AutoFix could not reach the model (attempt 2/3)'; const PUSH = '🤖 Addressed the latest review feedback (round 2/100).'; const NOOP = '🤖 Reviewed the latest feedback — no changes needed.'; + const INFRA_FAIL = + '🤖 AutoFix could not start — a setup step failed (or the run was cancelled) before the agent ran.'; + const INFRA_FAIL_CAP = + '🤖 AutoFix could not start — reached the round cap (100) because a setup step (base install/build) kept failing.'; + const CRASH_TERMINAL = + '🤖 AutoFix could not start evaluation — it crashed or timed out before reading the feedback.'; const run = ( priorHeadlines, - { window, markRound = 7, apiErrorDetail = '', apiErrorKind = '' } = {}, + { + window, + markRound = 7, + apiErrorDetail = '', + apiErrorKind = '', + prepareOutcome = 'success', + } = {}, ) => { const dir = mkdtempSync(join(tmpdir(), 'consec-')); const bin = join(dir, 'bin'); @@ -4486,7 +4614,7 @@ describe('qwen-autofix workflow', () => { 'bash', [ '-c', - `set -uo pipefail\nWORKDIR='${dir}'\nMARK_ROUND=${markRound}\nMAX_ROUNDS=100\nCONSECUTIVE_FAILURE_CAP=${cap}\nCONSEC_FAIL=0\nREPO=o/r\nPR=1\nAUTOFIX_BOT=qwen-code-dev-bot\nRETRY_COMMAND='@qwen-code /retry'\nAPI_ERROR_DETAIL='${apiErrorDetail}'\nAPI_ERROR_KIND='${apiErrorKind}'\n${window !== undefined ? `WINDOW='${window}'\n` : ''}HEADLINE=orig\n${script}\nprintf '%s|%s|%s' "$MARK_ROUND" "${'${CONSEC_FAIL}'}" "$HEADLINE"`, + `set -uo pipefail\nWORKDIR='${dir}'\nMARK_ROUND=${markRound}\nMAX_ROUNDS=100\nCONSECUTIVE_FAILURE_CAP=${cap}\nCONSEC_FAIL=0\nREPO=o/r\nPR=1\nAUTOFIX_BOT=qwen-code-dev-bot\nRETRY_COMMAND='@qwen-code /retry'\nAPI_ERROR_DETAIL='${apiErrorDetail}'\nAPI_ERROR_KIND='${apiErrorKind}'\nPREPARE_OUTCOME='${prepareOutcome}'\n${window !== undefined ? `WINDOW='${window}'\n` : ''}HEADLINE=orig\n${script}\nprintf '%s|%s|%s' "$MARK_ROUND" "${'${CONSEC_FAIL}'}" "$HEADLINE"`, ], { env: { ...process.env, PATH: `${bin}:${process.env.PATH}` }, @@ -4545,6 +4673,39 @@ describe('qwen-autofix workflow', () => { apiErrorKind: 'auth', }), ).toMatchObject({ consec: cap, terminal: true }); + // A skipped-Prepare (pre-agent infra failure) is exempt from the breaker — + // same failure class as transient 429/5xx: not the PR's fault, self-heals, + // hits the whole scan batch. The round cap + sentinel-ts /retry already + // bounds a persistently broken base; the breaker must not override that + // and re-introduce the mass-stranding this retry path exists to prevent. + expect( + run(Array(cap - 1).fill(FAIL), { prepareOutcome: 'skipped' }), + ).toMatchObject({ terminal: false, headline: 'orig' }); + expect( + run(Array(cap - 1).fill(FAIL), { prepareOutcome: 'cancelled' }), + ).toMatchObject({ terminal: false, headline: 'orig' }); + expect( + run(Array(cap - 1).fill(FAIL), { prepareOutcome: '' }), + ).toMatchObject({ terminal: false, headline: 'orig' }); + // Prior infra-failure headlines reset the streak too — a broken base + // build is not the PR's fault, same class as the current-round exemption + // above. Without this, 3 real failures + 3 infra rounds + 1 more real + // failure would trip the cap-5 breaker even though only 4 rounds were the + // PR's fault. + expect(run([FAIL, FAIL, INFRA_FAIL, FAIL, FAIL])).toMatchObject({ + consec: 3, + terminal: false, + }); + expect(run([FAIL, FAIL, INFRA_FAIL_CAP, FAIL, FAIL])).toMatchObject({ + consec: 3, + terminal: false, + }); + // The genuine agent-crash headline must NOT reset the streak — it is a + // real failure, not infra. + expect(run([FAIL, FAIL, CRASH_TERMINAL, FAIL])).toMatchObject({ + consec: cap, + terminal: true, + }); // Already-terminal rounds skip the circuit breaker entirely. expect(run(Array(cap).fill(FAIL), { markRound: 100 })).toMatchObject({ terminal: true, @@ -4563,6 +4724,25 @@ describe('qwen-autofix workflow', () => { ); expect(noopEmit).toBeTruthy(); expect(noopEmit[1]).toContain('no changes needed'); + // The infra-failure reset strings must match the actual retry/cap + // headlines emitted in this same step, so a reword breaks this test, + // not silently the streak reset. + const infraRetryEmit = reviewAddressReportStep.match( + /HEADLINE="(🤖 AutoFix could not start — [^"]*)"/, + ); + expect(infraRetryEmit).toBeTruthy(); + expect(infraRetryEmit[1]).toContain('AutoFix could not start —'); + const infraCapEmit = reviewAddressReportStep.match( + /HEADLINE="(🤖 AutoFix could not start — reached the round cap[^"]*)"/, + ); + expect(infraCapEmit).toBeTruthy(); + expect(infraCapEmit[1]).toContain('AutoFix could not start —'); + // The crash headline must NOT match the infra reset patterns. + const crashEmit = reviewAddressReportStep.match( + /HEADLINE="(🤖 AutoFix could not start evaluation[^"]*)"/, + ); + expect(crashEmit).toBeTruthy(); + expect(crashEmit[1]).not.toContain('AutoFix could not start —'); // Window filtering: pre-re-arm failures don't count after a re-arm. expect( run( @@ -4575,6 +4755,33 @@ describe('qwen-autofix workflow', () => { ).toMatchObject({ consec: 2, terminal: false }); }); + it('posts the review-address report wrapper lines bilingually', () => { + // The agent's own address-summary.md / no-action.md ends with a collapsed + // Chinese block, but these workflow-appended wrapper lines sit OUTSIDE it — + // so each must carry its own inline translation (the `model/模型` footer in + // this same step is the idiom) or the posted comment is only half in + // Chinese. Pin the English↔Chinese pairs so a reword that drops the Chinese + // fails here. The English halves are load-bearing elsewhere too: the streak + // reset detector globs `*"Addressed the latest review feedback"*` and + // `*"no changes needed"*`, so they must stay verbatim. + for (const [en, zh] of [ + ['Addressed the latest review feedback', '已处理最新评审反馈'], + ['Re-review when you have a moment', '有空请复审'], + ['Reviewed the latest feedback — no changes needed', '无需改动'], + ['conflicted with main — resolved in this push', '已在本次推送中解决'], + ['conflicts with main (no review fix needed', '合并前需 rebase/merge'], + ['no conflict with main', '与 main 无冲突'], + ]) { + expect(pushAndReportStep, `English anchor missing: ${en}`).toContain(en); + expect(pushAndReportStep, `Chinese missing for: ${en}`).toContain(zh); + } + // Every posted line in the step is either bilingual, the agent's own + // (already-bilingual) markdown, a structural token (---), or the footer + // (model/模型). Guard specifically that no Base-conflict label is emitted + // English-only. + expect(pushAndReportStep).not.toMatch(/echo "Base-conflict check:/); + }); + it('makes every known gate rejection declare its verdict', () => { // The retry/advance split above is only sound while each real rejection // writes outcome=failed; an unwired check would read as a gate crash and be @@ -5270,12 +5477,21 @@ describe('qwen-autofix workflow', () => { expect(runMark({ NEWEST: '2026-07-16T00:00:00Z', DETAIL_FILE: '' })).toBe( `${SENTINEL}|3`, ); - // 3. Crash before prepare (NEWEST empty): terminal round so the scan skips + // 3. NEWEST empty but Prepare RAN to a verdict (outcome success/failure) + // and the agent crashed before reading: terminal round so the scan skips // instead of re-handing-off forever; ts falls back to WATERMARK/sentinel. - expect(runMark({ NEWEST: '', WATERMARK: '2026-07-10T00:00:00Z' })).toBe( - '2026-07-10T00:00:00Z|5', - ); - expect(runMark({ NEWEST: '', WATERMARK: '' })).toBe(`${SENTINEL}|5`); + // (An empty/skipped/cancelled Prepare — the agent never ran — now retries + // instead; that is the dedicated skipped-Prepare test above.) + expect( + runMark({ + NEWEST: '', + WATERMARK: '2026-07-10T00:00:00Z', + PREPARE_OUTCOME: 'success', + }), + ).toBe('2026-07-10T00:00:00Z|5'); + expect( + runMark({ NEWEST: '', WATERMARK: '', PREPARE_OUTCOME: 'failure' }), + ).toBe(`${SENTINEL}|5`); // The no-output-crash HEADLINE must only promise a retry when one will // actually happen: at the final attempt (MARK_ROUND == MAX_ROUNDS) the @@ -5888,6 +6104,16 @@ describe('qwen-autofix workflow', () => { expect(readFileSync(join(dir, 'failure.md'), 'utf8')).toContain( 'timeout (100ms)', ); + // A timeout drops the agent-timeout signal so the handoff routes it to a + // RETRY (sentinel ts), not an evaluated advance that strands the feedback + // the agent never finished addressing. + expect(existsSync(join(dir, 'agent-timeout'))).toBe(true); + expect(readFileSync(join(dir, 'agent-timeout'), 'utf8')).toContain( + 'timeout (100ms)', + ); + // It is NOT an API error — the api-error signal must stay absent so the + // model-key handoff is not shown for a budget timeout. + expect(existsSync(join(dir, 'agent-api-error'))).toBe(false); }); }); From d8d0f2179197111a19d9e9cc6202fb29d5b0eba6 Mon Sep 17 00:00:00 2001 From: Qwen Code Autofix Date: Thu, 23 Jul 2026 10:52:17 +0000 Subject: [PATCH 07/10] fix(autofix): fall through to feedback on failed update-branch; assert CAS param (#7554) --- .github/workflows/qwen-autofix.yml | 5 ++++- scripts/tests/qwen-autofix-workflow.test.js | 24 ++++++++++++--------- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/.github/workflows/qwen-autofix.yml b/.github/workflows/qwen-autofix.yml index 913bfdaae0f..1c371bd2976 100644 --- a/.github/workflows/qwen-autofix.yml +++ b/.github/workflows/qwen-autofix.yml @@ -1875,11 +1875,14 @@ jobs: if gh api -X PUT "repos/${REPO}/pulls/${PR}/update-branch" -f expected_head_sha="${PR_HEAD_OID}" > /dev/null 2>&1; then echo "🔀 #${PR}: red check(s) [${RED_NAMES}] pass on current main ${MAIN_HEAD:0:9} but fail here on a stale base — 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" + continue else echo "⚠️ #${PR}: wanted to update the stale base (red [${RED_NAMES}] green on main) but update-branch failed (likely a merge conflict) — leaving for a human" fleet_row "${PR}" 'base-update-failed' "stale-base red [${RED_NAMES}] but update-branch failed (conflict?)" + # A failed update (merge conflict) is a human problem, but + # the bot's review comments need not be deferred forever — + # fall through to feedback processing. fi - continue fi fi diff --git a/scripts/tests/qwen-autofix-workflow.test.js b/scripts/tests/qwen-autofix-workflow.test.js index b55a30f4661..efba8bdcccf 100644 --- a/scripts/tests/qwen-autofix-workflow.test.js +++ b/scripts/tests/qwen-autofix-workflow.test.js @@ -467,6 +467,7 @@ describe('qwen-autofix workflow', () => { 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'), }; }; @@ -476,36 +477,39 @@ describe('qwen-autofix workflow', () => { // Base-inherited red (fails here, passes on main) + behind → update & skip. expect(run({ prChecks: [FAIL('Test')], mainGreen: ['Test'] })).toEqual({ updated: true, + cas: true, continued: true, }); // Red on the PR AND on main (not base-inherited — the PR's own bug) → never // touch it. This is the gate that stops churning a genuinely-broken PR. expect(run({ prChecks: [FAIL('Test')], mainGreen: [] })).toEqual({ updated: false, + cas: false, continued: false, }); // Base-inherited red but the PR already contains main (ahead) → no-op skip. expect( run({ prChecks: [FAIL('Test')], mainGreen: ['Test'], cmp: 'ahead' }), - ).toEqual({ updated: false, continued: false }); + ).toEqual({ updated: false, cas: false, continued: 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, continued: true }); + ).toEqual({ updated: true, cas: true, continued: true }); // No red at all → nothing to do. expect(run({ prChecks: [OK('Test')], mainGreen: ['Test'] })).toEqual({ updated: false, + cas: false, continued: false, }); - // update-branch fails (a merge conflict): still attempted, logged, and the - // PR is skipped this scan rather than crashing the loop. + // 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, continued: true }); + ).toEqual({ updated: true, cas: true, continued: 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, continued: true }); + ).toEqual({ updated: false, cas: false, continued: true }); // A Qwen Autofix check (not review-address) 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. A logic @@ -521,7 +525,7 @@ describe('qwen-autofix workflow', () => { ], mainGreen: ['Build'], }), - ).toEqual({ updated: false, continued: false }); + ).toEqual({ updated: false, cas: false, continued: false }); // ...but a review-address check IS eligible (it is the workflow's signal that // the previous address round needs a fresh base), so it still updates. expect( @@ -535,19 +539,19 @@ describe('qwen-autofix workflow', () => { ], mainGreen: ['review-address (1)'], }), - ).toEqual({ updated: true, continued: true }); + ).toEqual({ updated: true, cas: true, continued: true }); // MAIN_HEAD empty (initial gh api failure): the -n guard prevents any // update-branch call — a future refactor removing that guard would silently // allow updates with a null compare baseline. expect( run({ prChecks: [FAIL('Test')], mainGreen: ['Test'], mainHead: '' }), - ).toEqual({ updated: false, continued: false }); + ).toEqual({ updated: false, cas: false, continued: false }); // CMP_STATUS empty (compare API failure): empty falls through today (no // update), but a future change treating empty as "assume behind" would // auto-merge main into PRs blindly. expect( run({ prChecks: [FAIL('Test')], mainGreen: ['Test'], cmp: '' }), - ).toEqual({ updated: false, continued: false }); + ).toEqual({ updated: false, cas: false, continued: false }); }); it('auto-reruns a check that died on infrastructure, once, guarded by run_attempt', () => { From 6ff5d989310ef08ba9e3b0a148f78ee6f8ad4c50 Mon Sep 17 00:00:00 2001 From: Qwen Code Bot Date: Thu, 23 Jul 2026 11:56:23 +0000 Subject: [PATCH 08/10] =?UTF-8?q?fix(autofix):=20address=20review=20?= =?UTF-8?q?=E2=80=94=20fix=20stale-base=20gate=20source,=20add=20base=20di?= =?UTF-8?q?mension,=20bound=20repetition=20(#7554)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .github/workflows/qwen-autofix.yml | 143 ++++++----- scripts/tests/qwen-autofix-workflow.test.js | 250 ++++++++++++++++---- 2 files changed, 288 insertions(+), 105 deletions(-) diff --git a/.github/workflows/qwen-autofix.yml b/.github/workflows/qwen-autofix.yml index 1c371bd2976..b881df25128 100644 --- a/.github/workflows/qwen-autofix.yml +++ b/.github/workflows/qwen-autofix.yml @@ -1731,18 +1731,25 @@ jobs: # 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, - # but ONLY when the specific failing check is GREEN on current main — - # that is the single safety gate: it proves the red is base-inherited - # (not the PR's own bug) AND that main is healthy on that check right - # now (so the update cannot pull a NEW breakage in). Fetch main's head + # but ONLY when TWO predicates hold for the specific failing check: + # (1) it is GREEN on current main — main is healthy on it right now, + # so the update cannot pull a NEW breakage in; AND (2) it was NOT + # green on the merge-base commit the PR carries — the red is + # base-inherited, not the PR's own regression. Fetch main's head # and the set of check names currently passing on it, ONCE per scan. - DEFAULT_BRANCH="${DEFAULT_BRANCH:-main}" - MAIN_HEAD="$(gh api "repos/${REPO}/commits/${DEFAULT_BRANCH}" --jq '.sha' 2> /dev/null || echo '')" + # ci.yml has no push trigger, so its check-runs never land on main's + # squash commits; resolve main's head to the PR that produced it and + # read check-runs from that PR's head SHA instead. + MAIN_HEAD="$(gh api "repos/${REPO}/commits/main" --jq '.sha' 2> /dev/null || echo '')" MAIN_GREEN_CHECKS='[]' if [[ -n "${MAIN_HEAD}" ]]; then - MAIN_GREEN_CHECKS="$(gh api --paginate "repos/${REPO}/commits/${MAIN_HEAD}/check-runs" \ - --jq '[.check_runs[] | select(.conclusion == "success") | .name]' 2> /dev/null \ - | jq -c -s 'add // []')" || MAIN_GREEN_CHECKS='[]' + 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 @@ -1841,50 +1848,7 @@ jobs: ISSUE="${PR}" fi CHECKS_JSON="$(jq -c '.statusCheckRollup // []' <<< "${PR_META}")" - - # Auto-update a PR that is red ONLY because of a stale base (see the - # MAIN_GREEN_CHECKS rationale above). Act only when a check that is - # FAILING on this PR is passing on current main by the SAME name — - # that gate proves it is base-inherited and main is healthy on it, so - # the merge cannot import a fresh breakage. Runs 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. - 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") or (((.name // "") | startswith("review-address")))) - | (.name // .workflowName // "") - | select(. != "" and (. as $n | $green | index($n))) ]')" PR_HEAD_OID="$(jq -r '.headRefOid // ""' <<< "${PR_META}")" - if [[ "${STALE_BASE_REDS}" != '[]' && -n "${MAIN_HEAD}" && -n "${PR_HEAD_OID}" ]]; then - # Only when the PR does not already contain main's head — else the - # update is a no-op (422) and the red is NOT stale-base after all. - # Compare by SHA so a fork head needs no owner:branch qualifier. - 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 - 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 - # 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. - if gh api -X PUT "repos/${REPO}/pulls/${PR}/update-branch" -f expected_head_sha="${PR_HEAD_OID}" > /dev/null 2>&1; then - echo "🔀 #${PR}: red check(s) [${RED_NAMES}] pass on current main ${MAIN_HEAD:0:9} but fail here on a stale base — 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" - continue - else - echo "⚠️ #${PR}: wanted to update the stale base (red [${RED_NAMES}] green on main) but update-branch failed (likely a merge conflict) — leaving for a human" - fleet_row "${PR}" 'base-update-failed' "stale-base red [${RED_NAMES}] but update-branch failed (conflict?)" - # A failed update (merge conflict) is a human problem, but - # the bot's review comments need not be deferred forever — - # fall through to feedback processing. - fi - fi - fi # Auto-rerun a check that died on INFRASTRUCTURE, not the code (see # INFRA_FAILURE_SIGNATURES). Only reached when the PR has a FAILED @@ -1895,7 +1859,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 @@ -2146,6 +2109,78 @@ jobs: fi continue fi + # Auto-update a PR that is red ONLY because of a stale base (see the + # MAIN_GREEN_CHECKS rationale above). Two predicates must hold for a + # failing check: (1) it is GREEN on current main (main is healthy on + # it), AND (2) it was NOT green on the merge-base commit the PR + # carries (the red is base-inherited, not the PR's own regression). + # 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 + CMP="$(gh api "repos/${REPO}/compare/${MAIN_HEAD}...${PR_HEAD_OID}" 2> /dev/null || echo '{}')" + CMP_STATUS="$(jq -r '.status // ""' <<< "${CMP}")" + if [[ "${CMP_STATUS}" == 'behind' || "${CMP_STATUS}" == 'diverged' ]]; then + # Resolve the merge-base to the PR that produced it and read + # its check-runs, so we can require the failing check to have + # been NOT green at the base the PR carries (predicate 2). + BASE_SHA="$(jq -r '.merge_base_commit.sha // ""' <<< "${CMP}")" + BASE_NOT_GREEN='[]' + if [[ -n "${BASE_SHA}" ]]; then + BASE_PR_HEAD="$(gh api "repos/${REPO}/commits/${BASE_SHA}/pulls" \ + --jq '.[0].head.sha // ""' 2> /dev/null || echo '')" + if [[ -n "${BASE_PR_HEAD}" ]]; then + BASE_NOT_GREEN="$(gh api --paginate "repos/${REPO}/commits/${BASE_PR_HEAD}/check-runs" \ + --jq '[.check_runs[] | select(.conclusion | IN("failure","error","timed_out","action_required","cancelled","stale")) | .name]' 2> /dev/null \ + | jq -c -s 'add // [] | unique')" || BASE_NOT_GREEN='[]' + fi + fi + STALE_BASE_REDS="$(jq -c -n \ + --argjson checks "${CHECKS_JSON}" --argjson green "${MAIN_GREEN_CHECKS}" --argjson basered "${BASE_NOT_GREEN}" ' + [ $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)) and (. as $n | $basered | index($n))) ]')" + if [[ "${STALE_BASE_REDS}" != '[]' ]]; then + # Repetition guard: a marker comment bounds re-updates to + # once per 2 hours (CI takes ~40 min; main moves ~13 min). + # 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 "$(date -u -d '120 minutes ago' +%Y-%m-%dT%H:%M:%SZ)" ' + [ .[] | 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}, red at base)" + fleet_row "${PR}" 'dry-run-base' "would merge main (stale-base red [${RED_NAMES}])" + continue + fi + # 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. + if 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} but failed at the base this PR carries — 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" + gh pr comment "${PR}" --repo "${REPO}" --body "$(printf '🔀 Base updated: red check(s) [%s] pass on current main but failed at the stale base this PR carried — merged current main via update-branch; CI will re-run.\n\n
\n中文说明\n\n🔀 已更新 base：红色检查 [%s] 在当前 main 上通过，但在本 PR 携带的旧 base 上失败 —— 已通过 update-branch 合入当前 main，CI 将重新运行。\n\n
\n\n' "${RED_NAMES}" "${RED_NAMES}")" > /dev/null 2>&1 || true + 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")) @@ -2208,7 +2243,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). + // block fell through (no update). Production runs -eo pipefail; match it. const out = execFileSync( 'bash', [ '-c', - `set -uo pipefail\nfleet_row(){ :; }\nfor _ in x; do\n${script}\nprintf 'FELL_THROUGH'\ndone`, + `set -eo pipefail\nfleet_row(){ :; }\nfor _ in x; do\n${script}\nprintf 'FELL_THROUGH'\ndone`, ], { env: { @@ -454,8 +480,10 @@ describe('qwen-autofix workflow', () => { MAIN_HEAD: mainHead, MAIN_GREEN_CHECKS: JSON.stringify(mainGreen), CHECKS_JSON: JSON.stringify(prChecks), - PR_META: JSON.stringify({ headRefOid: 'prhead123' }), + PR_HEAD_OID: 'prhead123', DRY_RUN: dryRun ? 'true' : 'false', + AUTOFIX_BOT: 'autofix-bot', + WORKDIR: dir, PATH: `${bin}:${process.env.PATH}`, }, encoding: 'utf8', @@ -469,51 +497,122 @@ describe('qwen-autofix workflow', () => { 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 OK = (name) => ({ name, conclusion: 'SUCCESS' }); - // Base-inherited red (fails here, passes on main) + behind → update & skip. - expect(run({ prChecks: [FAIL('Test')], mainGreen: ['Test'] })).toEqual({ + // Base-inherited red: fails here, passes on main, was red at the merge-base, + // and the PR is behind → update & skip. + expect( + run({ + prChecks: [FAIL('Test')], + mainGreen: ['Test'], + baseNotGreen: ['Test'], + }), + ).toEqual({ updated: true, cas: true, continued: true, + markerPosted: true, }); - // Red on the PR AND on main (not base-inherited — the PR's own bug) → never - // touch it. This is the gate that stops churning a genuinely-broken PR. - expect(run({ prChecks: [FAIL('Test')], mainGreen: [] })).toEqual({ + // Red on the PR but GREEN at the merge-base → the PR introduced the + // regression, NOT a stale base. Must NOT update. + expect( + run({ prChecks: [FAIL('Test')], mainGreen: ['Test'], baseNotGreen: [] }), + ).toEqual({ updated: false, cas: false, continued: false, + markerPosted: false, + }); + // Red on the PR AND red on main (main is also broken) → not stale-base. + expect( + run({ + prChecks: [FAIL('Test')], + mainGreen: [], + baseNotGreen: ['Test'], + }), + ).toEqual({ + updated: false, + cas: false, + continued: false, + markerPosted: false, }); // Base-inherited 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 }); + run({ + prChecks: [FAIL('Test')], + mainGreen: ['Test'], + baseNotGreen: ['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 }); + run({ + prChecks: [FAIL('Lint')], + mainGreen: ['Lint'], + baseNotGreen: ['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({ + expect( + run({ + prChecks: [OK('Test')], + mainGreen: ['Test'], + baseNotGreen: ['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 }); + run({ + prChecks: [FAIL('Test')], + mainGreen: ['Test'], + baseNotGreen: ['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 }); - // A Qwen Autofix check (not review-address) 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. A logic - // inversion in that filter would pass the cases above (none set workflowName). + run({ + prChecks: [FAIL('Test')], + mainGreen: ['Test'], + baseNotGreen: ['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: [ @@ -524,10 +623,16 @@ describe('qwen-autofix workflow', () => { }, ], mainGreen: ['Build'], + baseNotGreen: ['Build'], }), - ).toEqual({ updated: false, cas: false, continued: false }); - // ...but a review-address check IS eligible (it is the workflow's signal that - // the previous address round needs a fresh base), so it still updates. + ).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: [ @@ -538,20 +643,57 @@ describe('qwen-autofix workflow', () => { }, ], mainGreen: ['review-address (1)'], + baseNotGreen: ['review-address (1)'], }), - ).toEqual({ updated: true, cas: true, continued: true }); + ).toEqual({ + updated: false, + cas: false, + continued: false, + markerPosted: false, + }); // MAIN_HEAD empty (initial gh api failure): the -n guard prevents any - // update-branch call — a future refactor removing that guard would silently - // allow updates with a null compare baseline. + // update-branch call. + expect( + run({ + prChecks: [FAIL('Test')], + mainGreen: ['Test'], + baseNotGreen: ['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'], mainHead: '' }), - ).toEqual({ updated: false, cas: false, continued: false }); - // CMP_STATUS empty (compare API failure): empty falls through today (no - // update), but a future change treating empty as "assume behind" would - // auto-merge main into PRs blindly. + run({ + prChecks: [FAIL('Test')], + mainGreen: ['Test'], + baseNotGreen: ['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'], cmp: '' }), - ).toEqual({ updated: false, cas: false, continued: false }); + run({ + prChecks: [FAIL('Test')], + mainGreen: ['Test'], + baseNotGreen: ['Test'], + hasMarker: true, + }), + ).toEqual({ + updated: false, + cas: false, + continued: false, + markerPosted: false, + }); }); it('auto-reruns a check that died on infrastructure, once, guarded by run_attempt', () => { @@ -627,6 +769,7 @@ 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: INFRA_SIGNATURES, PATH: `${bin}:${process.env.PATH}`, @@ -5039,10 +5182,15 @@ describe('qwen-autofix workflow', () => { ); expect(workflow).toContain("RETRY_COMMAND: '@qwen-code /retry'"); expect(workflow).toContain(''); - expect(workflow).toContain('' "${RED_NAMES}" "${RED_NAMES}")" > /dev/null 2>&1 || true + 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 || true continue else echo "⚠️ #${PR}: wanted to update the stale base (red [${RED_NAMES}] green on main) but update-branch failed: ${UPDATE_ERR:-unknown error}" diff --git a/scripts/tests/qwen-autofix-workflow.test.js b/scripts/tests/qwen-autofix-workflow.test.js index 5642f8678bb..248da732d72 100644 --- a/scripts/tests/qwen-autofix-workflow.test.js +++ b/scripts/tests/qwen-autofix-workflow.test.js @@ -407,13 +407,12 @@ describe('qwen-autofix workflow', () => { expect(run([{ ...llm, name: 'resolve-pr' }])).toBe('true'); }); - it('auto-updates a PR red only from a stale base, gated on green-on-main AND red-at-merge-base', () => { + it('auto-updates a PR red only from a stale base, gated on green-on-main', () => { // A PR can be red purely because it merged a main that was broken then and // is fixed now (a web-shell TS break, an agent-registry test — both stranded - // healthy PRs today). Two predicates gate the update: (1) the SAME failing - // check passes on current main (main is healthy), AND (2) it was NOT green - // at the merge-base the PR carries (the red is base-inherited, not the PR's - // own regression). A marker comment bounds repetition. + // healthy PRs today). The gate: the SAME failing check passes on current + // main (main is healthy), so merging main in cannot pull a NEW breakage. + // A marker comment bounds repetition. const block = reviewScanJob.match( /( {12}# Auto-update a PR that is red ONLY because of a stale base[\s\S]*?\n {12}fi\n)\n {12}N_FAILED_CHECKS=/, )?.[1]; @@ -423,27 +422,23 @@ describe('qwen-autofix workflow', () => { const run = ({ prChecks, mainGreen, - baseNotGreen = [], cmp = 'behind', updateOk = true, mainHead = 'mainhead999', + prHeadOid = 'prhead123', dryRun = false, hasMarker = false, }) => { const dir = mkdtempSync(join(tmpdir(), 'ub-')); const bin = join(dir, 'bin'); mkdirSync(bin); - const baseSha = 'basesha123'; - const basePrHead = 'baseprhead456'; writeFileSync( join(bin, 'gh'), [ '#!/usr/bin/env bash', `echo "$*" >> ${JSON.stringify(join(dir, 'calls.log'))}`, 'for a in "$@"; do case "$a" in', - ` *compare*) printf '{"status":"%s","merge_base_commit":{"sha":"%s"}}' "${cmp}" "${baseSha}"; exit 0;;`, - ` *commits*pulls*) printf '%s' "${basePrHead}"; exit 0;;`, - ` *check-runs*) printf '%s' '${JSON.stringify(baseNotGreen)}'; exit 0;;`, + ` *compare*) printf '{"status":"%s"}' "${cmp}"; exit 0;;`, ` *update-branch*) ${updateOk ? '' : `printf 'HTTP 409: merge conflict' >&2; `}exit ${updateOk ? 0 : 1};;`, 'esac; done', 'exit 0', @@ -480,7 +475,7 @@ describe('qwen-autofix workflow', () => { MAIN_HEAD: mainHead, MAIN_GREEN_CHECKS: JSON.stringify(mainGreen), CHECKS_JSON: JSON.stringify(prChecks), - PR_HEAD_OID: 'prhead123', + PR_HEAD_OID: prHeadOid, DRY_RUN: dryRun ? 'true' : 'false', AUTOFIX_BOT: 'autofix-bot', WORKDIR: dir, @@ -501,15 +496,15 @@ describe('qwen-autofix workflow', () => { }; }; const FAIL = (name) => ({ name, conclusion: 'FAILURE' }); + const FAIL_STATE = (name) => ({ name, state: 'FAILURE' }); const OK = (name) => ({ name, conclusion: 'SUCCESS' }); - // Base-inherited red: fails here, passes on main, was red at the merge-base, - // and the PR is behind → update & skip. + // Stale-base red: fails here, passes on main, and the PR is behind → + // update & skip. expect( run({ prChecks: [FAIL('Test')], mainGreen: ['Test'], - baseNotGreen: ['Test'], }), ).toEqual({ updated: true, @@ -517,22 +512,24 @@ describe('qwen-autofix workflow', () => { continued: true, markerPosted: true, }); - // Red on the PR but GREEN at the merge-base → the PR introduced the - // regression, NOT a stale base. Must NOT update. + // The .conclusion // .state // "" fallback: a check reported with only + // state (no conclusion) is still matched. expect( - run({ prChecks: [FAIL('Test')], mainGreen: ['Test'], baseNotGreen: [] }), + run({ + prChecks: [FAIL_STATE('Test')], + mainGreen: ['Test'], + }), ).toEqual({ - updated: false, - cas: false, - continued: false, - markerPosted: false, + 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: [], - baseNotGreen: ['Test'], }), ).toEqual({ updated: false, @@ -540,12 +537,11 @@ describe('qwen-autofix workflow', () => { continued: false, markerPosted: false, }); - // Base-inherited red but the PR already contains main (ahead) → no-op skip. + // Red but the PR already contains main (ahead) → no-op skip. expect( run({ prChecks: [FAIL('Test')], mainGreen: ['Test'], - baseNotGreen: ['Test'], cmp: 'ahead', }), ).toEqual({ @@ -559,7 +555,6 @@ describe('qwen-autofix workflow', () => { run({ prChecks: [FAIL('Lint')], mainGreen: ['Lint'], - baseNotGreen: ['Lint'], cmp: 'diverged', }), ).toEqual({ @@ -573,7 +568,6 @@ describe('qwen-autofix workflow', () => { run({ prChecks: [OK('Test')], mainGreen: ['Test'], - baseNotGreen: ['Test'], }), ).toEqual({ updated: false, @@ -587,7 +581,6 @@ describe('qwen-autofix workflow', () => { run({ prChecks: [FAIL('Test')], mainGreen: ['Test'], - baseNotGreen: ['Test'], updateOk: false, }), ).toEqual({ @@ -601,7 +594,6 @@ describe('qwen-autofix workflow', () => { run({ prChecks: [FAIL('Test')], mainGreen: ['Test'], - baseNotGreen: ['Test'], dryRun: true, }), ).toEqual({ @@ -623,7 +615,6 @@ describe('qwen-autofix workflow', () => { }, ], mainGreen: ['Build'], - baseNotGreen: ['Build'], }), ).toEqual({ updated: false, @@ -643,7 +634,6 @@ describe('qwen-autofix workflow', () => { }, ], mainGreen: ['review-address (1)'], - baseNotGreen: ['review-address (1)'], }), ).toEqual({ updated: false, @@ -657,7 +647,6 @@ describe('qwen-autofix workflow', () => { run({ prChecks: [FAIL('Test')], mainGreen: ['Test'], - baseNotGreen: ['Test'], mainHead: '', }), ).toEqual({ @@ -671,7 +660,6 @@ describe('qwen-autofix workflow', () => { run({ prChecks: [FAIL('Test')], mainGreen: ['Test'], - baseNotGreen: ['Test'], cmp: '', }), ).toEqual({ @@ -685,7 +673,6 @@ describe('qwen-autofix workflow', () => { run({ prChecks: [FAIL('Test')], mainGreen: ['Test'], - baseNotGreen: ['Test'], hasMarker: true, }), ).toEqual({ @@ -694,6 +681,20 @@ describe('qwen-autofix workflow', () => { 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, + }); }); it('auto-reruns a check that died on infrastructure, once, guarded by run_attempt', () => { From d8a9f4331631a98ea37aac51930d92bce5167f6e Mon Sep 17 00:00:00 2001 From: Qwen Code Bot Date: Thu, 23 Jul 2026 14:37:19 +0000 Subject: [PATCH 10/10] =?UTF-8?q?fix(autofix):=20address=20review=20?= =?UTF-8?q?=E2=80=94=20identity-gate=20the=20stale-base=20write,=20correct?= =?UTF-8?q?=20the=20green-checks=20safety=20claim,=20per-selector=20guard?= =?UTF-8?q?=20test,=20gate=20the=20compare=20call=20(#7554)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .github/workflows/qwen-autofix.yml | 116 +++++++++++------ scripts/tests/qwen-autofix-workflow.test.js | 136 ++++++++++++++++++-- 2 files changed, 205 insertions(+), 47 deletions(-) diff --git a/.github/workflows/qwen-autofix.yml b/.github/workflows/qwen-autofix.yml index 4d625c557ee..6ca4b8b19e5 100644 --- a/.github/workflows/qwen-autofix.yml +++ b/.github/workflows/qwen-autofix.yml @@ -1725,22 +1725,42 @@ 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 - # when the failing check is GREEN on current main — main is healthy - # on it right now, so the update cannot pull a NEW breakage in. This - # cannot distinguish a base-inherited red from the PR's own regression - # (both are red-on-PR / green-on-main), but the safety bounds are - # tight: the merge is recoverable, a marker limits re-updates to once - # per 2h, and the CAS (expected_head_sha) rejects a concurrent push. - # Fetch main's head and the set of check names currently passing on - # it, ONCE per scan. ci.yml has no push trigger, so its check-runs - # never land on main's squash commits; resolve main's head to the PR - # that produced it and read check-runs from that PR's head SHA. + # 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 @@ -2111,33 +2131,42 @@ jobs: 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 is - # GREEN on current main, so merging main in cannot pull a NEW - # breakage — 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. + # 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 - CMP="$(gh api "repos/${REPO}/compare/${MAIN_HEAD}...${PR_HEAD_OID}" 2> /dev/null || echo '{}')" - CMP_STATUS="$(jq -r '.status // ""' <<< "${CMP}")" - if [[ "${CMP_STATUS}" == 'behind' || "${CMP_STATUS}" == 'diverged' ]]; then - # CANCELLED is deliberately omitted from the PR-side selector: - # a cancelled check is not evidence of a stale base. - 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 + # 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 (CI takes ~40 min; main moves ~13 min). - # Without this, a still-red PR would be re-updated on every - # scan after main advances. + # 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 "$(date -u -d '120 minutes ago' +%Y-%m-%dT%H:%M:%SZ)" ' + --arg cutoff "${BASE_UPDATE_CUTOFF}" ' [ .[] | select((.user.login // "") == $ab) | select((.body // "") | contains("")) | select((.created_at // "") > $cutoff) ] | length > 0' "${WORKDIR}/ic.json")" @@ -2148,14 +2177,29 @@ jobs: 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. - if UPDATE_ERR="$(gh api -X PUT "repos/${REPO}/pulls/${PR}/update-branch" -f expected_head_sha="${PR_HEAD_OID}" 2>&1 >/dev/null)"; then + 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" - 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 || true + # 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}" diff --git a/scripts/tests/qwen-autofix-workflow.test.js b/scripts/tests/qwen-autofix-workflow.test.js index 248da732d72..2f681d08781 100644 --- a/scripts/tests/qwen-autofix-workflow.test.js +++ b/scripts/tests/qwen-autofix-workflow.test.js @@ -297,19 +297,25 @@ describe('qwen-autofix workflow', () => { // Every failed-check selector must guard against the loop reading its OWN // runs as feedback about the PR. Most selectors carry the review-address // carve-out; the stale-base selector instead excludes ALL Qwen Autofix - // checks (no carve-out), which is strictly narrower. Assert that every - // selector has one guard or the other. + // checks (no carve-out), which is strictly narrower. Assert PER SELECTOR + // that its own text carries one guard or the other — a global count is + // vacuous here because `!= "Qwen Autofix"` is a substring of the carve-out + // expression, so every carve-out selector increments BOTH counters and a + // guardless selector slips through (proven by A/B mutation). const scanCheckSelectors = reviewScanJob.match(/IN\("(?:FAILURE|QUEUED)"/g) ?? []; expect(scanCheckSelectors.length).toBeGreaterThanOrEqual(3); - const carveOutCount = ( - reviewScanJob.match(/startswith\("review-address"\)/g) ?? [] - ).length; - const fullExclusionCount = (reviewScanJob.match(/!= "Qwen Autofix"/g) ?? []) - .length; - expect(carveOutCount + fullExclusionCount).toBeGreaterThanOrEqual( - scanCheckSelectors.length, - ); + const guardlessSelectors = reviewScanJob + .split(/(?=IN\("(?:FAILURE|QUEUED)")/) + .slice(1) + .filter((seg) => { + const sel = seg.slice(0, 400); + return ( + !/startswith\("review-address"\)/.test(sel) && + !/!= "Qwen Autofix"/.test(sel) + ); + }); + expect(guardlessSelectors).toEqual([]); expect(reviewScanJob).toContain('"${N_FAILED_CHECKS}" -eq 0'); expect(reviewScanJob).toContain('${N_FAILED_CHECKS} failed check(s) new'); expect(reviewScanJob).toContain('.completedAt // .updatedAt // ""'); @@ -428,6 +434,7 @@ describe('qwen-autofix workflow', () => { prHeadOid = 'prhead123', dryRun = false, hasMarker = false, + actor = 'autofix-bot', }) => { const dir = mkdtempSync(join(tmpdir(), 'ub-')); const bin = join(dir, 'bin'); @@ -438,7 +445,8 @@ describe('qwen-autofix workflow', () => { '#!/usr/bin/env bash', `echo "$*" >> ${JSON.stringify(join(dir, 'calls.log'))}`, 'for a in "$@"; do case "$a" in', - ` *compare*) printf '{"status":"%s"}' "${cmp}"; exit 0;;`, + ` user) printf '%s' ${JSON.stringify(actor)}; exit 0;;`, + ` *compare*) printf '%s' "${cmp}"; exit 0;;`, ` *update-branch*) ${updateOk ? '' : `printf 'HTTP 409: merge conflict' >&2; `}exit ${updateOk ? 0 : 1};;`, 'esac; done', 'exit 0', @@ -695,6 +703,112 @@ describe('qwen-autofix workflow', () => { 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', () => {