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); }); });