Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 62 additions & 0 deletions .github/workflows/qwen-autofix.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand Down Expand Up @@ -3344,6 +3355,57 @@ 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 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
# 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
# #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.
# 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("<!-- autofix-eval "))
| select(
((.body // "") | contains("win=" + $win + " -->"))
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
if [[ "${H}" == *"Addressed the latest review feedback"* || "${H}" == *"no changes needed"* ]]; then
CONSEC_FAIL=1
else
CONSEC_FAIL=$(( CONSEC_FAIL + 1 ))
fi
done <<< "${PRIOR_HEADS}"
if [[ "${CONSEC_FAIL}" -ge "${CONSECUTIVE_FAILURE_CAP}" ]]; then
MARK_ROUND="${MAX_ROUNDS}"
HEADLINE="🤖 AutoFix stopped after ${CONSEC_FAIL} consecutive rounds that failed to push anything (timeouts and/or gate rejections). Retrying at the same per-round budget is not converging — this usually means the PR is too large or conflicts with a fast-moving \`main\`. A human should rebase, split, or reduce it, then comment \`${RETRY_COMMAND}\` to re-arm. Until then future scans will skip this PR."
fi
fi
{
echo "${HEADLINE}"
echo
Expand Down
150 changes: 149 additions & 1 deletion scripts/tests/qwen-autofix-workflow.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3915,8 +3915,11 @@ describe('qwen-autofix workflow', () => {
// judged the work at all; advancing there strands a fix the agent had
// already written, which is exactly how the nested-package ENOENT stranded
// #7329/#7336 until a human deleted the marker.
// Ends at the crash decision's own closing `fi`; the consecutive-failure
// block that follows is a separate unit with its own test, so anchor on it
// rather than the report `{` (which it now sits before).
const decision = reviewAddressReportStep.match(
/(GATE_CRASHED=false\n[\s\S]*?\n {12}fi)\n {12}\{/,
/(GATE_CRASHED=false\n[\s\S]*?\n {12}fi)\n\n {12}# Consecutive-failure/,
)?.[1];
expect(decision).toBeTruthy();
const SENTINEL = '9999-12-31T23:59:59Z';
Expand Down Expand Up @@ -4027,6 +4030,151 @@ describe('qwen-autofix workflow', () => {
).toContain('attempt 3/5');
});

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.
// Observed on #6723: 7 straight failed rounds (3 timeouts, 4 gate
// rejections) heading for round 100, each ~50 min. Any push or legitimate
// no-op resets the streak; only consecutive failures count.
const cap = Number(workflow.match(/CONSECUTIVE_FAILURE_CAP: '(\d+)'/)?.[1]);
expect(cap).toBeGreaterThan(0);
// The sub-cap must be below the takeover cap or it never binds there.
const takeoverCap = Number(
workflow.match(/TAKEOVER_MAX_ROUNDS: '(\d+)'/)?.[1],
);
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/,
)?.[0];
expect(block).toBeTruthy();
const script = block.replace(/^ {12}/gm, '');

const FAIL =
'🤖 Could not address the latest feedback automatically (round 3/100).';
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 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, 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<!-- autofix-eval ts=x acted=y round=z${win ? ` win=${win}` : ''} -->`,
};
}),
),
);
writeFileSync(
join(bin, 'gh'),
`#!/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\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}` },
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,
});
// 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(
[
...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', () => {
// 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
Expand Down
Loading