fix(autofix): retry a model API error instead of stranding the PR

.github/workflows/qwen-autofix.yml

@@ -108,6 +108,12 @@ env:
   # Hard cap on automated review-address rounds per PR. After this the bot stops
   # and leaves the PR for a human.
   MAX_ROUNDS: '5'
+  # An auth/access model error (401/402/403, "no access"/"does not exist")
+  # never self-heals - only a maintainer can fix the key - and every retry
+  # costs an agent run AND a PR comment. Cap those attempts far below
+  # MAX_ROUNDS so the actionable "check the model key" message lands in an
+  # hour instead of a day. Transient (429/5xx) errors keep the full budget.
+  API_AUTH_MAX_ROUNDS: '3'
   # Upper bound on review targets emitted per scan (fan-out defense-in-depth;
   # excess is logged and deferred to the next scan).
   MAX_TARGETS_PER_SCAN: '10'
@@ -2705,7 +2711,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 resolved-comments.txt pr.diff; do
+          for f in feedback.md address-summary.md no-action.md failure.md handoff.md agent-api-error agent-api-error-kind resolved-comments.txt pr.diff; do
             if [[ -f "${WORKDIR}/${f}" ]]; then
               echo "=============== ${f} ==============="
               cat "${WORKDIR}/${f}"
@@ -2964,6 +2970,30 @@ jobs:
             for f in failure.md handoff.md address-summary.md no-action.md; do
               if [[ -s "${WORKDIR}/${f}" ]]; then DETAIL_FILE="${WORKDIR}/${f}"; break; fi
             done
+            # run-agent.mjs drops this marker when qwen died on a model-side
+            # [API Error: 4xx/5xx] (access denied, quota, a 5xx). The agent never
+            # evaluated the feedback, so this is treated as a retry (sentinel ts,
+            # no watermark advance) below — a model access/quota blip must not
+            # strand the PR the way a real evaluated handoff would.
+            API_ERROR_DETAIL=''
+            API_ERROR_KIND=''
+            if [[ -s "${WORKDIR}/agent-api-error" ]]; then
+              # First line only, comment-opener escaped (agent stdout can echo
+              # external PR-comment text and the marker regex spans '<!-- ... -->'
+              # happily), and capped so a long span can't bloat the headline.
+              # `cut -c` counts BYTES, so the cap can split a multi-byte
+              # character - and the classifier deliberately matches CJK renders,
+              # so a >200-byte Chinese error is a supported input, not a
+              # hypothetical. iconv -c drops the dangling bytes so the headline
+              # stays valid UTF-8; it EXITS 1 when it discards one, which under
+              # this step's `set -eo pipefail` would abort before the marker and
+              # the gh pr comment - hence the `|| true`, same as the sibling
+              # publish site below.
+              API_ERROR_DETAIL="$(head -n 1 "${WORKDIR}/agent-api-error" | sed 's/<!--/<!\\-\\-/g' | cut -c1-200 | iconv -f utf-8 -t utf-8 -c || true)"
+            fi
+            if [[ -s "${WORKDIR}/agent-api-error-kind" ]]; then
+              API_ERROR_KIND="$(head -n 1 "${WORKDIR}/agent-api-error-kind" | tr -cd 'a-z')"
+            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
@@ -2991,41 +3021,71 @@ jobs:
             MARK_TS="${NEWEST:-${WATERMARK:-9999-12-31T23:59:59Z}}"
             if [[ -n "${NEWEST:-}" ]]; then
               MARK_ROUND="$(( ROUND + 1 ))"
-              if [[ -z "${DETAIL_FILE}" || "${GATE_CRASHED}" == 'true' ]]; then
-                # Prepare ran (NEWEST is set) but no verdict was reached — either
-                # the agent produced NO output at all (crashed before writing any
-                # verdict, e.g. a staged runner that fails to boot), or the gate
-                # crashed after the agent wrote its summary. It evaluated NOTHING,
-                # 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 crash (a deploy, an infra blip,
-                # a base-image bug fixed minutes later). Stamp the sentinel ts
-                # (excluded from EVAL_WM) so the feedback stays live and the next
-                # scan retries; the incremented round still bounds retries to
-                # MAX_ROUNDS before a terminal handoff, so a PERSISTENT crash
-                # cannot loop forever.
+              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
+                # 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
+                # model-access grant, a base-image bug fixed minutes later).
+                # Stamp the sentinel ts (excluded from EVAL_WM) so the feedback
+                # stays live and the next scan retries; the incremented round
+                # still bounds retries before a terminal handoff, so a PERSISTENT
+                # failure cannot loop forever.
                 MARK_TS='9999-12-31T23:59:59Z'
-                # Only promise a retry when one will actually happen: at
-                # MARK_ROUND == MAX_ROUNDS the next scan's round-cap gate skips
-                # the PR, and the cap-reached notice is takeover-only — so the
-                # final attempt must say so itself, or the maintainer waits for
-                # a retry that never comes. No Run log here: the report block
-                # below appends it to every handoff (avoid a duplicate URL).
-                # Name the real cause: a gate crash points the maintainer at
-                # the gate logs (the agent's commit is discarded with the runner,
-                # but the feedback watermark is preserved so the retry re-attempts
-                # the same feedback), while a no-output crash points at the run.
-                if [[ -z "${DETAIL_FILE}" ]]; then
-                  CAUSE='crashed before it could evaluate the feedback'
-                  LAST_FIX='a human should take over this PR'
+                # Cause-aware wording, most specific first — a model error and a
+                # gate crash each name the operator fix, while a bare no-output
+                # crash points at a human. (The API clause runs first as
+                # defense-in-depth: today run-agent writes failure.md on the
+                # API-death path and the gate converts that to an explicit
+                # outcome=failed, so GATE_CRASHED is false — but if the gate
+                # ever changes, a model blip must not be reported as a gate
+                # problem.) No Run log here — the report block below appends
+                # it (avoid a duplicate).
+                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 [[ -z "${DETAIL_FILE}" ]]; then
+                  CAUSE="crashed before it could evaluate the feedback"
+                  LAST_FIX="a human should take over this PR"
                 else
-                  CAUSE='hit a verification-gate error before reaching a verdict'
-                  LAST_FIX='a maintainer should check the gate logs, then re-arm'
+                  # A gate crash points the maintainer at the gate logs: the
+                  # agent's commit is discarded with the runner, but the
+                  # feedback watermark is preserved, so the retry re-attempts
+                  # the same feedback rather than skipping it.
+                  CAUSE="hit a verification-gate error before reaching a verdict"
+                  LAST_FIX="a maintainer should check the gate logs, then re-arm"
+                fi
+                # Budget retries by CAUSE. A transient 429/5xx self-heals, so
+                # it keeps the full round budget. An auth/access error that
+                # only a maintainer can fix never self-heals, and every attempt
+                # costs an agent run AND a PR comment - on a takeover PR that
+                # is ~100 comments over ~17h before the useful message lands.
+                CAUSE_MAX="${MAX_ROUNDS}"
+                if [[ "${API_ERROR_KIND}" == 'auth' && "${API_AUTH_MAX_ROUNDS}" -lt "${MAX_ROUNDS}" ]]; then
+                  CAUSE_MAX="${API_AUTH_MAX_ROUNDS}"
                 fi
-                if [[ "${MARK_ROUND}" -lt "${MAX_ROUNDS}" ]]; then
-                  HEADLINE="🤖 AutoFix ${CAUSE} (attempt ${MARK_ROUND}/${MAX_ROUNDS}) — it will retry on the next scan."
+                # Only promise a retry when one will actually happen: at the
+                # cap the next scan's round gate skips the PR, and the
+                # cap-reached notice is takeover-only, so the final attempt
+                # must say so itself. Clamp the displayed numerator:
+                # MARK_ROUND counts ALL rounds in the window, so earlier real
+                # attempts can push it past CAUSE_MAX — "attempt 4/3" reads
+                # as a bug.
+                DISPLAY_ROUND=$(( MARK_ROUND < CAUSE_MAX ? MARK_ROUND : CAUSE_MAX ))
+                if [[ "${MARK_ROUND}" -lt "${CAUSE_MAX}" ]]; then
+                  HEADLINE="🤖 AutoFix ${CAUSE} (attempt ${DISPLAY_ROUND}/${CAUSE_MAX}) — it will retry on the next scan."
                 else
-                  HEADLINE="🤖 AutoFix ${CAUSE} (attempt ${MARK_ROUND}/${MAX_ROUNDS}) — this was the last automatic attempt; ${LAST_FIX}."
+                  HEADLINE="🤖 AutoFix ${CAUSE} (attempt ${DISPLAY_ROUND}/${CAUSE_MAX}) — this was the last automatic attempt; ${LAST_FIX}."
+                  # Stamp the terminal round so the scan's max-round gate skips
+                  # this PR instead of re-handing-off every tick; the sentinel
+                  # ts keeps the feedback live for a re-arm once the key is
+                  # fixed. (No-op when CAUSE_MAX is already MAX_ROUNDS.)
+                  MARK_ROUND="${MAX_ROUNDS}"
                 fi
               else
                 HEADLINE="🤖 Could not address the latest feedback automatically (round ${MARK_ROUND}/${MAX_ROUNDS}). A human should take over this PR."

.qwen/skills/autofix/scripts/run-agent.mjs

@@ -67,6 +67,85 @@ function writeFailure(workdir, message) {
   );
 }
 
+// Classify a model-side [API Error] render as recoverable, and by CAUSE:
+//   'transient' - 429 / 5xx / rate-limit / overload / quota: self-heals on its
+//                 own once the limit resets, so it earns the full retry budget.
+//   'auth'      - 401 / 402 / 403, or a render saying the model does not exist
+//                 or the key has no access: ONLY a maintainer can fix it, so
+//                 the workflow caps these retries low and then goes terminal
+//                 with the operator fix (each attempt costs an agent run AND a
+//                 PR comment; a hundred of them help nobody).
+//   ''          - terminal: reproduces identically forever (a malformed 400).
+//
+// The status code is read from its POSITION in the render (`[API Error: <code>`)
+// rather than matched anywhere in the text. Matching anywhere made permanent
+// failures look retryable: `400 Invalid value for max_tokens: must be <= 512`
+// matched a bare \b5\d\d\b, and `400 context length exceeded` matched a bare
+// `exceeded` - both reproduce forever. `exceeded` therefore only counts as part
+// of `quota`. The match is single-line ([^\]\n]) so a multi-line render cannot
+// smuggle a newline into the marker or the PR-comment headline.
+const TRANSIENT_API_ERROR =
+  /rate.?limit|quota|RESOURCE_EXHAUSTED|overloaded|temporarily|too many requests|速率限制|配额|服务(?:繁忙|不可用)/i;
+// Transport-level failures carry no HTTP status at all - the request never got
+// far enough to have one. They are unambiguously retryable, and leaving them
+// out stranded #7365 at round 2/100 on a bare
+// `[API Error: terminated (cause: read ECONNRESET)]`. ENOTFOUND is deliberately
+// excluded: a hostname that does not resolve is a misconfigured endpoint, which
+// repeats forever like a bad model name.
+const TRANSPORT_API_ERROR =
+  /ECONNRESET|ECONNREFUSED|ETIMEDOUT|EPIPE|EAI_AGAIN|socket hang up|fetch failed|terminated/i;
+// "does not exist or you do not have access to it" is the OpenAI-compatible
+// render of the same condition a 403 reports - same root cause, same fix.
+const AUTH_API_ERROR =
+  /api key|do not have access|does not exist|unauthorized|forbidden/i;
+
+function classifyApiError(render) {
+  const code = render.match(/\[API Error:\s*(\d{3})\b/)?.[1];
+  if (code) {
+    const status = Number(code);
+    if (status === 429 || (status >= 500 && status <= 599)) return 'transient';
+    if (status === 401 || status === 402 || status === 403) return 'auth';
+    // 400 is always a malformed client request — it never self-heals by retry,
+    // regardless of what the message says. Route it terminal unconditionally
+    // so a 'does not exist' in the body (a tool name, a field name) cannot
+    // trigger the auth/access retry path.
+    if (status === 400) return '';
+    // Any other code (404, ...) is permanent UNLESS the message itself names
+    // an access/existence problem.
+    return AUTH_API_ERROR.test(render) ? 'auth' : '';
+  }
+  // Code-less render: fall back to the keyword arms.
+  if (AUTH_API_ERROR.test(render)) return 'auth';
+  if (TRANSPORT_API_ERROR.test(render)) return 'transient';
+  return TRANSIENT_API_ERROR.test(render) ? 'transient' : '';
+}
+
+// Returns { error, kind }; error is '' when nothing recoverable was found.
+// NOTE: the caller passes the last 20 KB of output, so an API error emitted
+// early in a long run can scroll out and be classified terminal. That is the
+// fail-safe direction (a missed retry, never a wrongly-retried permanent
+// failure), but detection is best-effort rather than guaranteed.
+function recoverableApiError(output) {
+  const wrapped = output.match(/\[API Error:[^\]\n]*\]/g) || [];
+  if (wrapped.length > 0) {
+    // Classify only the LAST render — it represents the terminal state of the
+    // run. An earlier transient error followed by a permanent one must not
+    // retry: the permanent error reproduces identically on every attempt.
+    const last = wrapped[wrapped.length - 1];
+    const kind = classifyApiError(last);
+    if (kind) return { error: last, kind };
+    // Terminal wrapped error — do NOT fall through to the OAuth fallback.
+    // The standalone quota form only appears when there is NO [API Error:]
+    // wrapper at all; matching it here would override a terminal verdict.
+    return { error: '', kind: '' };
+  }
+  // Some quota errors are never wrapped in [API Error: ...] (e.g. Qwen OAuth
+  // quota returns early before formatting) - catch the known standalone form.
+  const oauth = output.match(/Qwen OAuth quota exceeded[^\n]*/i);
+  if (oauth) return { error: `[API Error: ${oauth[0]}]`, kind: 'transient' };
+  return { error: '', kind: '' };
+}
+
 function writeHandoff(workdir, message) {
   mkdirSync(workdir, { recursive: true });
   writeFileSync(file(workdir, 'handoff.md'), `${message}\n`);
@@ -111,10 +190,15 @@ function runQwen(options, prompt) {
       settled = true;
       clearTimeout(timer);
       clearTimeout(killTimer);
+      const apiErrorInfo = recoverableApiError(outputTail);
       const payload = {
         ...result,
         timedOut,
         loopDetected: loopDetected || isLoopGuardOutput(outputTail),
+        // A RECOVERABLE model error means qwen never evaluated the feedback —
+        // the workflow retries it rather than advancing the watermark.
+        apiError: apiErrorInfo.error,
+        apiErrorKind: apiErrorInfo.kind,
       };
       if (log.destroyed) {
         resolve(payload);
@@ -227,8 +311,28 @@ if (result.error || result.signal || result.status !== 0) {
     } else {
       writeFailure(
         options.workdir,
-        `Qwen failed during ${options.mode}: ${detail}.`,
+        `Qwen failed during ${options.mode}: ${detail}.${
+          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.
+      if (result.apiError && !result.timedOut) {
+        writeFileSync(
+          file(options.workdir, 'agent-api-error'),
+          `${result.apiError}\n`,
+        );
+        // Cause class ("transient" | "auth") — the handoff step gives a
+        // self-healing transient error the full round budget, but caps an
+        // auth/access error that only a maintainer can fix.
+        writeFileSync(
+          file(options.workdir, 'agent-api-error-kind'),
+          `${result.apiErrorKind}\n`,
+        );
+      }
     }
   } else {
     writeHandoff(