From b1eba443f56031f9c457d2e4f3d401a883beb32d Mon Sep 17 00:00:00 2001 From: wenshao Date: Wed, 22 Jul 2026 13:33:04 +0800 Subject: [PATCH 1/3] feat(autofix): stop a PR that fails to push for N rounds in a row MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Under takeover the round cap is 100, which is right for a PR that needs many PRODUCTIVE rounds. It is wrong for one that fails every round: #6723 ran 7 consecutive failed rounds (3 agent timeouts at 50 min, 4 gate rejections whose fix broke tests) over 8 hours, heading for round 100, because it is a 5700-line, 47-file, 5-day-old PR racing a fast-moving main — every round re-resolves a conflict it cannot finish or that fails the gate. Retrying at the same per-round budget will not converge; a human has to rebase or split it. Adds CONSECUTIVE_FAILURE_CAP (5), distinct from the total round cap. The handoff step already runs only when a round did NOT push, so it counts the unbroken run of prior failure markers — stopping at the first push ("Addressed the latest review feedback") or legitimate no-op ("no changes needed"), either of which proves progress and resets the streak. At the cap it forces the terminal round even under takeover, with a handoff that names the real fix (rebase/split, then /retry). Cause- agnostic: a timeout and a gate rejection both count. --- .github/workflows/qwen-autofix.yml | 46 ++++++++++ scripts/tests/qwen-autofix-workflow.test.js | 97 ++++++++++++++++++++- 2 files changed, 142 insertions(+), 1 deletion(-) diff --git a/.github/workflows/qwen-autofix.yml b/.github/workflows/qwen-autofix.yml index 2d528dbf93f..5a2fbe4a8d8 100644 --- a/.github/workflows/qwen-autofix.yml +++ b/.github/workflows/qwen-autofix.yml @@ -161,6 +161,17 @@ env: # explicitly delegated work; removing the label restores the strict cap, # and re-engaging opens a fresh counting window (see REARM_KEY below). TAKEOVER_MAX_ROUNDS: '100' + # Consecutive-failure sub-cap, distinct from the total round cap above. The + # total cap bounds how many PRODUCTIVE rounds a PR may take; this bounds how + # many rounds may fail IN A ROW with nothing pushed. Under takeover a PR gets + # up to 100 rounds, but a PR that fails to push this many times running is not + # iterating, it is stuck — a too-large / fast-conflicting PR whose fix keeps + # timing out or failing the gate. Retrying at the same budget will not fix + # that; a human has to rebase or split it. Any pushed round OR a legitimate + # "no changes needed" no-op resets the streak, so this only ever fires on an + # unbroken run of failures. Observed on #6723: 7 straight failed rounds (3 + # timeouts, 4 gate rejections) over 8 hours, heading for 100. + CONSECUTIVE_FAILURE_CAP: '5' # Do not claim more issues when too many existing autofix PRs are still open. MAX_OPEN_AUTOFIX_PRS: '5' @@ -3344,6 +3355,41 @@ jobs: MARK_ROUND="${MAX_ROUNDS}" HEADLINE="🤖 AutoFix could not start evaluation — it crashed or timed out before reading the feedback, so no fix was attempted. This PR is now marked terminal and future scans (including forced dispatch) will skip it. To recover: delete this bot's terminal \`autofix-eval\` marker comment, then re-trigger if the failure looked transient." fi + + # Consecutive-failure circuit breaker, distinct from the round cap. + # Reaching this step at all means this round did NOT push (the push + # and no-op paths report from "Push and report"), so this round is a + # failure. Count how many failures precede it WITHOUT a break: walk + # the bot's prior eval markers newest-first and stop at the first + # one that pushed ("Addressed the latest review feedback") or was a + # deliberate no-op ("no changes needed") — either proves the loop + # was making progress, so the streak resets there. If the unbroken + # streak (this round included) reaches the cap, stop retrying even + # under takeover: a PR that fails this many times running is stuck + # on something a re-run at the same budget will not fix (observed on + # #6723: 7 straight failures, 3 timeouts + 4 gate rejections). Only + # overrides a would-be RETRY — a round already terminal for another + # reason keeps its own headline. + if [[ "${MARK_ROUND}" != "${MAX_ROUNDS}" ]]; then + CONSEC_FAIL=1 + PRIOR_HEADS="$(gh api "repos/${REPO}/issues/${PR}/comments" --paginate 2> /dev/null \ + | jq -r --arg ab "${AUTOFIX_BOT}" '.[] + | select((.user.login // "") == $ab) + | select((.body // "") | contains("`, + })), + ), + ); + writeFileSync( + join(bin, 'gh'), + `#!/usr/bin/env bash\ncat ${JSON.stringify(join(dir, 'comments.json'))}\n`, + ); + chmodSync(join(bin, 'gh'), 0o755); + const out = execFileSync( + 'bash', + [ + '-c', + `set -uo pipefail\nMARK_ROUND=7\nMAX_ROUNDS=100\nCONSECUTIVE_FAILURE_CAP=${cap}\nREPO=o/r\nPR=1\nAUTOFIX_BOT=qwen-code-dev-bot\nRETRY_COMMAND='@qwen-code /retry'\nHEADLINE=orig\n${script}\nprintf '%s|%s|%s' "$MARK_ROUND" "${'${CONSEC_FAIL}'}" "$HEADLINE"`, + ], + { + env: { ...process.env, PATH: `${bin}:${process.env.PATH}` }, + encoding: 'utf8', + }, + ); + rmSync(dir, { recursive: true, force: true }); + const [mark, consec, headline] = out.split('|'); + return { + mark, + consec: Number(consec), + terminal: mark === '100', + headline, + }; + }; + + // This round alone (no prior failures) never terminates. + expect(run([])).toMatchObject({ consec: 1, terminal: false }); + // cap-1 prior failures + this round = cap → terminal, with the structural + // handoff, not the ordinary "could not address". + const capped = run(Array(cap - 1).fill(FAIL)); + expect(capped).toMatchObject({ consec: cap, terminal: true }); + expect(capped.headline).toContain('consecutive'); + expect(capped.headline).toContain('/retry'); + // One short of the cap keeps retrying. + expect(run(Array(cap - 2).fill(FAIL))).toMatchObject({ terminal: false }); + // A push resets the streak — failures before it do not count. + expect(run([FAIL, FAIL, PUSH, FAIL, FAIL])).toMatchObject({ + consec: 3, + terminal: false, + }); + // A legitimate no-op resets it too (the loop was caught up, not stuck). + expect(run([...Array(cap).fill(FAIL), NOOP, FAIL])).toMatchObject({ + consec: 2, + terminal: false, + }); + // Timeouts and gate rejections both count — the streak is cause-agnostic. + expect(run([FAIL, FAIL_TIMEOUT, FAIL, FAIL_TIMEOUT])).toMatchObject({ + consec: cap, + terminal: true, + }); + }); + 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 From 067afd780e8882b0ec9dec14bd8401b459e20599 Mon Sep 17 00:00:00 2001 From: qwen-code-dev-bot Date: Wed, 22 Jul 2026 07:13:59 +0000 Subject: [PATCH 2/3] fix(autofix): address review feedback on consecutive-failure circuit breaker (#7482) - Fix misleading comment: the walk is oldest-first (API order) with reset-on-success, not newest-first with early stop - Prefer the already-fetched ic.json over a redundant gh api call, falling back to the API only when the file is missing - Filter eval markers by re-arm window (win=) so pre-re-arm failures do not immediately re-terminate a re-armed PR - Add test coverage for the MARK_ROUND == MAX_ROUNDS guard and for window-scoped streak counting --- .github/workflows/qwen-autofix.yml | 26 +++++++++------ scripts/tests/qwen-autofix-workflow.test.js | 35 ++++++++++++++++----- 2 files changed, 44 insertions(+), 17 deletions(-) diff --git a/.github/workflows/qwen-autofix.yml b/.github/workflows/qwen-autofix.yml index 5a2fbe4a8d8..f2dee6d2ab5 100644 --- a/.github/workflows/qwen-autofix.yml +++ b/.github/workflows/qwen-autofix.yml @@ -3360,10 +3360,11 @@ jobs: # Reaching this step at all means this round did NOT push (the push # and no-op paths report from "Push and report"), so this round is a # failure. Count how many failures precede it WITHOUT a break: walk - # the bot's prior eval markers newest-first and stop at the first - # one that pushed ("Addressed the latest review feedback") or was a - # deliberate no-op ("no changes needed") — either proves the loop - # was making progress, so the streak resets there. If the unbroken + # the bot's prior eval markers in API order (oldest-first) and + # reset the streak at each push ("Addressed the latest review + # feedback") or deliberate no-op ("no changes needed"). After the + # full walk, CONSEC_FAIL holds failures since the last progress + # point plus one for this round. If the unbroken # streak (this round included) reaches the cap, stop retrying even # under takeover: a PR that fails this many times running is stuck # on something a re-run at the same budget will not fix (observed on @@ -3372,11 +3373,18 @@ jobs: # reason keeps its own headline. if [[ "${MARK_ROUND}" != "${MAX_ROUNDS}" ]]; then CONSEC_FAIL=1 - PRIOR_HEADS="$(gh api "repos/${REPO}/issues/${PR}/comments" --paginate 2> /dev/null \ - | jq -r --arg ab "${AUTOFIX_BOT}" '.[] - | select((.user.login // "") == $ab) - | select((.body // "") | contains("")) + or ($win == "none" and (((.body // "") | contains("win=")) | not))) + | (.body | gsub("\r"; "") | split("\n")[0])' <<< "${COMMENTS_JSON}" 2> /dev/null || true)" while IFS= read -r H; do [[ -n "${H}" ]] || continue if [[ "${H}" == *"Addressed the latest review feedback"* || "${H}" == *"no changes needed"* ]]; then diff --git a/scripts/tests/qwen-autofix-workflow.test.js b/scripts/tests/qwen-autofix-workflow.test.js index 71b28069372..4510cb041ee 100644 --- a/scripts/tests/qwen-autofix-workflow.test.js +++ b/scripts/tests/qwen-autofix-workflow.test.js @@ -4056,29 +4056,33 @@ describe('qwen-autofix workflow', () => { const PUSH = '🤖 Addressed the latest review feedback (round 2/100).'; const NOOP = '🤖 Reviewed the latest feedback — no changes needed.'; - const run = (priorHeadlines) => { + const run = (priorHeadlines, { window, markRound = 7 } = {}) => { const dir = mkdtempSync(join(tmpdir(), 'consec-')); const bin = join(dir, 'bin'); mkdirSync(bin); writeFileSync( - join(dir, 'comments.json'), + join(dir, 'ic.json'), JSON.stringify( - priorHeadlines.map((h) => ({ - user: { login: 'qwen-code-dev-bot' }, - body: `${h}\n`, - })), + priorHeadlines.map((h) => { + const headline = typeof h === 'string' ? h : h.headline; + const win = typeof h === 'string' ? undefined : h.win; + return { + user: { login: 'qwen-code-dev-bot' }, + body: `${headline}\n`, + }; + }), ), ); writeFileSync( join(bin, 'gh'), - `#!/usr/bin/env bash\ncat ${JSON.stringify(join(dir, 'comments.json'))}\n`, + `#!/usr/bin/env bash\ncat ${JSON.stringify(join(dir, 'ic.json'))}\n`, ); chmodSync(join(bin, 'gh'), 0o755); const out = execFileSync( 'bash', [ '-c', - `set -uo pipefail\nMARK_ROUND=7\nMAX_ROUNDS=100\nCONSECUTIVE_FAILURE_CAP=${cap}\nREPO=o/r\nPR=1\nAUTOFIX_BOT=qwen-code-dev-bot\nRETRY_COMMAND='@qwen-code /retry'\nHEADLINE=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'\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}` }, @@ -4120,6 +4124,21 @@ describe('qwen-autofix workflow', () => { consec: cap, terminal: true, }); + // Already-terminal rounds skip the circuit breaker entirely. + expect(run(Array(cap).fill(FAIL), { markRound: 100 })).toMatchObject({ + terminal: true, + headline: 'orig', + }); + // Window filtering: pre-re-arm failures don't count after a re-arm. + expect( + run( + [ + ...Array(cap - 1).fill({ headline: FAIL, win: 'old-window' }), + { headline: FAIL, win: 'current-window' }, + ], + { window: 'current-window' }, + ), + ).toMatchObject({ consec: 2, terminal: false }); }); it('makes every known gate rejection declare its verdict', () => { From 7af973fa9b304dedd80e5822d82bb923db3cdf29 Mon Sep 17 00:00:00 2001 From: qwen-code-dev-bot Date: Wed, 22 Jul 2026 10:50:14 +0000 Subject: [PATCH 3/3] fix(autofix): exempt transient model errors from consecutive-failure breaker (#7482) --- .github/workflows/qwen-autofix.yml | 24 +++++++---- scripts/tests/qwen-autofix-workflow.test.js | 44 ++++++++++++++++++--- 2 files changed, 55 insertions(+), 13 deletions(-) diff --git a/.github/workflows/qwen-autofix.yml b/.github/workflows/qwen-autofix.yml index f2dee6d2ab5..5e498ba004d 100644 --- a/.github/workflows/qwen-autofix.yml +++ b/.github/workflows/qwen-autofix.yml @@ -3360,7 +3360,8 @@ jobs: # Reaching this step at all means this round did NOT push (the push # and no-op paths report from "Push and report"), so this round is a # failure. Count how many failures precede it WITHOUT a break: walk - # the bot's prior eval markers in API order (oldest-first) and + # the bot's prior eval markers in API order (oldest-first, pinned + # by sort_by so a stray reorder cannot corrupt the streak) and # reset the streak at each push ("Addressed the latest review # feedback") or deliberate no-op ("no changes needed"). After the # full walk, CONSEC_FAIL holds failures since the last progress @@ -3371,19 +3372,26 @@ jobs: # #6723: 7 straight failures, 3 timeouts + 4 gate rejections). Only # overrides a would-be RETRY — a round already terminal for another # reason keeps its own headline. - if [[ "${MARK_ROUND}" != "${MAX_ROUNDS}" ]]; then + # Transient model errors (429/5xx) are exempt: the CAUSE_MAX logic + # above deliberately gives them the full round budget because they + # self-heal once the provider recovers. Letting the breaker override + # that would mark every in-flight PR terminal at once during a + # provider outage — the failures are not the PR's fault and DO + # self-heal. Auth errors are NOT exempt (they never self-heal). + if [[ "${MARK_ROUND}" != "${MAX_ROUNDS}" ]] && { [[ -z "${API_ERROR_DETAIL}" ]] || [[ "${API_ERROR_KIND}" == 'auth' ]]; }; then CONSEC_FAIL=1 if [[ -f "${WORKDIR}/ic.json" ]]; then COMMENTS_JSON="$(cat "${WORKDIR}/ic.json")" else COMMENTS_JSON="$(gh api "repos/${REPO}/issues/${PR}/comments" --paginate 2> /dev/null || true)" fi - PRIOR_HEADS="$(jq -r --arg ab "${AUTOFIX_BOT}" --arg win "${WINDOW:-none}" '.[] - | select((.user.login // "") == $ab) - | select((.body // "") | contains("")) - or ($win == "none" and (((.body // "") | contains("win=")) | not))) + PRIOR_HEADS="$(jq -r --arg ab "${AUTOFIX_BOT}" --arg win "${WINDOW:-none}" ' + [.[] | select((.user.login // "") == $ab) + | select((.body // "") | contains("")) + or ($win == "none" and (((.body // "") | contains("win=")) | not)))] + | sort_by(.created_at) | .[] | (.body | gsub("\r"; "") | split("\n")[0])' <<< "${COMMENTS_JSON}" 2> /dev/null || true)" while IFS= read -r H; do [[ -n "${H}" ]] || continue diff --git a/scripts/tests/qwen-autofix-workflow.test.js b/scripts/tests/qwen-autofix-workflow.test.js index 4510cb041ee..9f7aedb498d 100644 --- a/scripts/tests/qwen-autofix-workflow.test.js +++ b/scripts/tests/qwen-autofix-workflow.test.js @@ -4045,7 +4045,7 @@ describe('qwen-autofix workflow', () => { expect(cap).toBeLessThan(takeoverCap); const block = reviewAddressReportStep.match( - /if \[\[ "\$\{MARK_ROUND\}" != "\$\{MAX_ROUNDS\}" \]\]; then\n {14}CONSEC_FAIL=1\n[\s\S]*?\n {14}fi\n {12}fi\n/, + /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/, )?.[0]; expect(block).toBeTruthy(); const script = block.replace(/^ {12}/gm, ''); @@ -4056,18 +4056,22 @@ describe('qwen-autofix workflow', () => { const PUSH = '🤖 Addressed the latest review feedback (round 2/100).'; const NOOP = '🤖 Reviewed the latest feedback — no changes needed.'; - const run = (priorHeadlines, { window, markRound = 7 } = {}) => { + const run = ( + priorHeadlines, + { window, markRound = 7, apiErrorDetail = '', apiErrorKind = '' } = {}, + ) => { const dir = mkdtempSync(join(tmpdir(), 'consec-')); const bin = join(dir, 'bin'); mkdirSync(bin); writeFileSync( join(dir, 'ic.json'), JSON.stringify( - priorHeadlines.map((h) => { + priorHeadlines.map((h, i) => { const headline = typeof h === 'string' ? h : h.headline; const win = typeof h === 'string' ? undefined : h.win; return { user: { login: 'qwen-code-dev-bot' }, + created_at: `2026-01-01T00:${String(i).padStart(2, '0')}:00Z`, body: `${headline}\n`, }; }), @@ -4082,7 +4086,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'\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}'\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}` }, @@ -4119,16 +4123,46 @@ describe('qwen-autofix workflow', () => { consec: 2, terminal: false, }); - // Timeouts and gate rejections both count — the streak is cause-agnostic. + // Prior-round headlines are cause-agnostic: timeouts and gate rejections + // both count toward the streak. expect(run([FAIL, FAIL_TIMEOUT, FAIL, FAIL_TIMEOUT])).toMatchObject({ consec: cap, terminal: true, }); + // A transient (non-auth) model error on the CURRENT round skips the + // breaker entirely — the CAUSE_MAX logic above gives it the full budget + // because it self-heals, and the breaker must not override that. + expect( + run(Array(cap - 1).fill(FAIL), { + apiErrorDetail: 'terminated', + apiErrorKind: 'transient', + }), + ).toMatchObject({ terminal: false, headline: 'orig' }); + // An auth error on the current round is NOT exempt — it never self-heals. + expect( + run(Array(cap - 1).fill(FAIL), { + apiErrorDetail: 'access denied', + apiErrorKind: 'auth', + }), + ).toMatchObject({ consec: cap, terminal: true }); // Already-terminal rounds skip the circuit breaker entirely. expect(run(Array(cap).fill(FAIL), { markRound: 100 })).toMatchObject({ terminal: true, headline: 'orig', }); + // The reset detector keys on literal substrings; pin them to the actual + // "Push and report" emit lines so a reword breaks this test, not silently + // the streak reset in production. + const pushEmit = pushAndReportStep.match( + /echo "(🤖 Addressed the latest review feedback[^"]*)"/, + ); + expect(pushEmit).toBeTruthy(); + expect(pushEmit[1]).toContain('Addressed the latest review feedback'); + const noopEmit = pushAndReportStep.match( + /echo "(🤖 Reviewed the latest feedback — no changes needed[^"]*)"/, + ); + expect(noopEmit).toBeTruthy(); + expect(noopEmit[1]).toContain('no changes needed'); // Window filtering: pre-re-arm failures don't count after a re-arm. expect( run(