From 5df71c8fd1cf2d0bed6de63441b27f37f8955164 Mon Sep 17 00:00:00 2001 From: Shaojin Wen Date: Thu, 23 Jul 2026 13:13:35 +0800 Subject: [PATCH] fix(autofix): retry an agent timeout instead of advancing past its feedback (#7563) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A timeout evaluated NOTHING — the agent ran out of budget before finishing, so nothing was committed and the feedback is unaddressed. It was treated as an evaluated verdict (real ts, watermark advances), which strands that feedback: the next scan sees "nothing new" and never retries. Observed on #7471 (round 13/100), a heavily-reviewed 1871-line PR: rounds 11 and 13 timed out, but round 12 pushed — so a timeout is transient far more often than not, and advancing past it left the round-13 feedback unhandled. run-agent.mjs now drops an `agent-timeout` signal on result.timedOut, and the handoff routes it like a pre-verdict crash: sentinel ts (feedback stays live) and a retry, with a headline that names the real fix at the cap (split the PR or raise the budget). A PR that PERSISTENTLY times out is bounded by the round cap and the consecutive-failure cap, so this cannot loop forever — it just stops treating a one-off budget blip as a verdict. The loop guard stays terminal (a tool-call loop is a real defect, not a budget blip). An API error still routes to its own model-key handoff; the timeout signal is written only when NOT an API error. Co-authored-by: wenshao --- .github/workflows/qwen-autofix.yml | 34 ++++++++++++++++----- .qwen/skills/autofix/scripts/run-agent.mjs | 23 +++++++++++--- scripts/tests/qwen-autofix-workflow.test.js | 31 +++++++++++++++++++ 3 files changed, 75 insertions(+), 13 deletions(-) diff --git a/.github/workflows/qwen-autofix.yml b/.github/workflows/qwen-autofix.yml index 14ae8d62fb..e41fa511b8 100644 --- a/.github/workflows/qwen-autofix.yml +++ b/.github/workflows/qwen-autofix.yml @@ -2962,7 +2962,7 @@ jobs: if git rev-parse --verify "${BRANCH}" > /dev/null 2>&1; then git diff "origin/main...${BRANCH}" > "${WORKDIR}/pr.diff" || true fi - for f in feedback.md address-summary.md no-action.md failure.md handoff.md gate-rejection.md agent-api-error agent-api-error-kind resolved-comments.txt pr.diff; do + for f in feedback.md address-summary.md no-action.md failure.md handoff.md gate-rejection.md agent-api-error agent-api-error-kind agent-timeout resolved-comments.txt pr.diff; do if [[ -f "${WORKDIR}/${f}" ]]; then echo "=============== ${f} ===============" cat "${WORKDIR}/${f}" @@ -3264,6 +3264,16 @@ jobs: if [[ -s "${WORKDIR}/agent-api-error-kind" ]]; then API_ERROR_KIND="$(head -n 1 "${WORKDIR}/agent-api-error-kind" | tr -cd 'a-z')" fi + # A timeout means the agent evaluated NOTHING before its budget ran + # out (run-agent.mjs writes this signal). Routed to retry, not an + # evaluated advance — same as a pre-verdict crash. Bounded by the + # round cap and the consecutive-failure cap, so a PR that keeps + # timing out still stops; a one-off (usually followed by a good + # round) recovers on the next scan instead of stranding its feedback. + AGENT_TIMEOUT='' + if [[ -s "${WORKDIR}/agent-timeout" ]]; then + AGENT_TIMEOUT="$(head -n 1 "${WORKDIR}/agent-timeout" | cut -c1-120 | iconv -f utf-8 -t utf-8 -c || true)" + fi # If feedback was actually read (prepare ran), stamp its newest ts so # the watermark advances and the same feedback is not re-selected next # scan. If the crash happened before prepare, NEWEST is empty and the @@ -3291,13 +3301,14 @@ jobs: MARK_TS="${NEWEST:-${WATERMARK:-9999-12-31T23:59:59Z}}" if [[ -n "${NEWEST:-}" ]]; then MARK_ROUND="$(( ROUND + 1 ))" - if [[ -z "${DETAIL_FILE}" || -n "${API_ERROR_DETAIL}" || "${GATE_CRASHED}" == 'true' ]]; then - # Prepare ran (NEWEST is set) but no verdict was reached. Three - # ways that happens, and in ALL of them the agent evaluated - # NOTHING: it produced no output at all (crashed before any - # verdict — a staged runner that fails to boot), it died on a - # model [API Error] (access/quota/5xx/transport), or the gate - # crashed after the agent wrote its summary. So the watermark + if [[ -z "${DETAIL_FILE}" || -n "${API_ERROR_DETAIL}" || -n "${AGENT_TIMEOUT}" || "${GATE_CRASHED}" == 'true' ]]; then + # Prepare ran (NEWEST is set) but no verdict was reached. Ways + # that happens, and in ALL of them the agent evaluated NOTHING: + # it produced no output at all (crashed before any verdict — a + # staged runner that fails to boot), it died on a model + # [API Error] (access/quota/5xx/transport), it TIMED OUT before + # finishing, or the gate crashed after the agent wrote its + # summary. So the watermark # must NOT advance past this feedback: an advance makes the next # scan see "nothing new" and never retry, stranding the PR on a # transient failure (an infra blip, a quota reset minutes away, a @@ -3319,6 +3330,13 @@ jobs: if [[ -n "${API_ERROR_DETAIL}" ]]; then CAUSE="could not reach the model — ${API_ERROR_DETAIL}" LAST_FIX="a maintainer should check the autofix model key/access, then re-arm" + elif [[ -n "${AGENT_TIMEOUT}" ]]; then + # A timeout evaluated nothing, so the feedback is unaddressed + # and stays live for the retry. On a big / heavily-reviewed PR + # this is usually a one-off; the last automatic attempt names + # the real fix (split the PR or raise the budget). + CAUSE="ran out of time before finishing (${AGENT_TIMEOUT})" + LAST_FIX="a human should split the PR or raise the agent time budget, then re-arm" elif [[ -z "${DETAIL_FILE}" ]]; then CAUSE="crashed before it could evaluate the feedback" LAST_FIX="a human should take over this PR" diff --git a/.qwen/skills/autofix/scripts/run-agent.mjs b/.qwen/skills/autofix/scripts/run-agent.mjs index bf876f0488..7884108f6b 100755 --- a/.qwen/skills/autofix/scripts/run-agent.mjs +++ b/.qwen/skills/autofix/scripts/run-agent.mjs @@ -315,11 +315,24 @@ if (result.error || result.signal || result.status !== 0) { result.apiError ? ` ${result.apiError}` : '' }`, ); - // Only a BARE, un-evaluated API failure is retryable. A loop guard, a - // timeout, or an agent-written failure.md is a real verdict — leave - // those terminal even if an API-error string appears in the output tail. - // Signals the workflow to retry (sentinel ts) instead of advancing the - // watermark and stranding the PR. + // A TIMEOUT evaluated NOTHING — the agent ran out of budget before + // finishing, so nothing was committed and the feedback is UNaddressed. + // Treat it like any other pre-verdict failure and RETRY (the workflow + // stamps a sentinel ts) rather than advancing the watermark and + // stranding the feedback the loop never actually handled. This is + // transient far more often than not — on a heavily-reviewed PR a + // timed-out round is usually followed by a successful one (#7471: + // rounds 11/13 timed out, 12 pushed) — and a PR that PERSISTENTLY times + // out is bounded by the consecutive-failure cap, not by one-shot + // terminal. A loop guard stays terminal (a tool-call loop is a real + // defect, not a budget blip), handled by the loopDetected branch above. + if (result.timedOut) { + writeFileSync(file(options.workdir, 'agent-timeout'), `${detail}\n`); + } + // Only a BARE, un-evaluated API failure is retryable. An agent-written + // failure.md is a real verdict — left terminal even if an API-error + // string appears in the output tail. Signals the workflow to retry + // (sentinel ts) instead of advancing the watermark and stranding the PR. if (result.apiError && !result.timedOut) { writeFileSync( file(options.workdir, 'agent-api-error'), diff --git a/scripts/tests/qwen-autofix-workflow.test.js b/scripts/tests/qwen-autofix-workflow.test.js index 9ad88a70bd..d8ef54371a 100644 --- a/scripts/tests/qwen-autofix-workflow.test.js +++ b/scripts/tests/qwen-autofix-workflow.test.js @@ -4006,6 +4006,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' }); @@ -5687,6 +5708,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); }); });