From 94e7df2b26bdcca98c3d95325a753f87fbcf2023 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Sun, 12 Jul 2026 15:28:44 -0700 Subject: [PATCH 1/2] fix(review): stop retrying an AI provider 429 immediately with zero backoff runWorkersOpinion, runDualAiTieBreakJudgeCall, runWorkersSlopOpinion, and runPlannerModel each retry a failed model call up to 3x (2x for the planner) before falling through to the fallback model -- but a rate-limit error (claude_code_error_429 / ai_http_429 / anthropic_http_429) will not have cleared by the next attempt a few hundred ms later, so retrying the SAME model burns the whole per-model attempt budget for zero additional chance of success, delaying (or in the tie-break/slop/planner loops, which had no special-casing at all, fully exhausting before) the fallback model ever gets a turn. Adds isRateLimitError (exported from ai-review.ts, shared by ai-slop.ts and planner.ts) and applies the same short-circuit runWorkersOpinion already uses for a non-transient CLI timeout: break out of the current model's retry loop on a 429 and move straight to the fallback, which may be on a different provider/account entirely. Fixes GITTENSORY-K Fixes GITTENSORY-8 --- src/review/planner.ts | 9 ++++++--- src/services/ai-review.ts | 23 ++++++++++++++++++++--- src/services/ai-slop.ts | 8 ++++++-- test/unit/ai-review.test.ts | 32 +++++++++++++++++++++++++++++++- test/unit/ai-slop.test.ts | 14 ++++++++++++++ test/unit/planner.test.ts | 10 ++++++++++ 6 files changed, 87 insertions(+), 9 deletions(-) diff --git a/src/review/planner.ts b/src/review/planner.ts index 3959cfa4fd..9563704f52 100644 --- a/src/review/planner.ts +++ b/src/review/planner.ts @@ -10,7 +10,7 @@ // legacy Workers-AI pair); the output is public-safe-sanitized before posting; any model/error degrades to // a no-plan no-op. -import { type AiReviewActualUsage, BEST_REVIEW_MODELS, clampNumber, coerceAiText, coerceAiUsage, estimateNeurons, RELIABLE_FALLBACK_MODELS, utcDayStartIso } from "../services/ai-review"; +import { type AiReviewActualUsage, BEST_REVIEW_MODELS, clampNumber, coerceAiText, coerceAiUsage, estimateNeurons, isRateLimitError, RELIABLE_FALLBACK_MODELS, utcDayStartIso } from "../services/ai-review"; import { recordAiUsageEvent, sumAiEstimatedNeuronsSince } from "../db/repositories"; import { sanitizePublicComment } from "../github/commands"; import { AGENT_COMMAND_COMMENT_MARKER } from "../github/comments"; @@ -123,8 +123,11 @@ async function runPlannerModel(env: Env, system: string, user: string): Promise< const result = await ai.run(model, { max_tokens: PLANNER_MAX_TOKENS, temperature: 0.2, messages: [{ role: "system", content: system }, { role: "user", content: user }], finalAttempt: attempt === 1 && modelIndex === models.length - 1 }, extra); const text = coerceAiText(result).trim(); if (text) return { text, usage: coerceAiUsage(result) }; - } catch { - /* retry, then fall through to the fallback model */ + } catch (error) { + // #5385-sentry (GITTENSORY-K/8): a 429 will not have cleared by the next attempt a few hundred ms + // later, so retrying THIS model burns the remaining budget for zero additional chance of success -- + // move straight to the fallback model instead (same guard as runWorkersOpinion in ai-review.ts). + if (isRateLimitError(error)) break; } } } diff --git a/src/services/ai-review.ts b/src/services/ai-review.ts index 0cd4faa72c..16f4b6034d 100644 --- a/src/services/ai-review.ts +++ b/src/services/ai-review.ts @@ -1007,6 +1007,19 @@ function isSubscriptionCliTimeout(error: unknown): boolean { return error instanceof Error && error.message === "subscription_cli_timeout"; } +/** True for a provider's own HTTP-429 signal (`src/selfhost/ai.ts`'s `claude_code_error_429` / + * `ai_http_429` / `anthropic_http_429`, and the generic Workers-AI equivalent). #5385-sentry + * (GITTENSORY-K/8): an immediate same-model retry against a rate limit that is still in its window has + * near-zero chance of success -- unlike a transient network blip, a 429 will not clear in the handful of + * milliseconds between attempts. Mirrors {@link isSubscriptionCliTimeout}'s identical non-transient-error + * short-circuit: stop burning the remaining per-model retry budget and move straight to the fallback model + * (which may be a different provider/account entirely, and so isn't necessarily still rate-limited). + * Exported so every independent AI-calling retry loop (ai-slop.ts, planner.ts) can share this one + * definition instead of each re-deriving its own copy of the error-shape regex. */ +export function isRateLimitError(error: unknown): boolean { + return error instanceof Error && /_(?:http|error)_429$/.test(error.message); +} + /** Cap on the diagnostic prefix logged for an unparseable model response (#observability-unparseable) -- long * enough to tell a markdown-fenced/truncated-mid-JSON/plain-prose response apart, short enough to never dump * a large chunk of model output into Sentry/audit context. */ @@ -1131,7 +1144,11 @@ async function runWorkersOpinion( // budget, since a different model/config may not share the same timeout) instead of burning up to 3x // the full effort-timeout in subprocess time for zero additional chance of success (#gaming-tactic-draft-cycle // audit finding: this inner retry count is distinct from c7073949's outer cross-sweep-tick cap). - if (isSubscriptionCliTimeout(error)) break; + // A 429 is the same story (#5385-sentry, GITTENSORY-K/8): the rate-limit window that just rejected + // this attempt will not have cleared by the next attempt a few hundred ms later, so an immediate + // same-model retry burns the remaining budget for zero additional chance of success -- move straight + // to the fallback model instead, which may be on a different account/provider entirely. + if (isSubscriptionCliTimeout(error) || isRateLimitError(error)) break; } } } @@ -1842,8 +1859,8 @@ async function runDualAiTieBreakJudgeCall( status: "provider_error", error: errorMessage(error), }); - // See runWorkersOpinion's identical guard: a CLI timeout will not resolve by retrying the same model. - if (isSubscriptionCliTimeout(error)) break; + // See runWorkersOpinion's identical guard: a CLI timeout or 429 will not resolve by retrying the same model. + if (isSubscriptionCliTimeout(error) || isRateLimitError(error)) break; } } } diff --git a/src/services/ai-slop.ts b/src/services/ai-slop.ts index b37081120f..1a9615c37c 100644 --- a/src/services/ai-slop.ts +++ b/src/services/ai-slop.ts @@ -32,6 +32,7 @@ import { coerceAiUsage, estimateNeurons, isEnabled, + isRateLimitError, toPublicSafe, utcDayStartIso, } from "./ai-review"; @@ -165,8 +166,11 @@ async function runWorkersSlopOpinion(env: Env, system: string, user: string, max ); const parsed = parseSlopOpinion(coerceAiText(result)); if (parsed) return { opinion: parsed, usage: coerceAiUsage(result) }; - } catch { - /* retry / fall through to fallback */ + } catch (error) { + // #5385-sentry (GITTENSORY-K/8): a 429 will not have cleared by the next attempt a few hundred ms + // later, so retrying THIS model burns the remaining budget for zero additional chance of success -- + // move straight to the fallback model instead (same guard as runWorkersOpinion in ai-review.ts). + if (isRateLimitError(error)) break; } } } diff --git a/test/unit/ai-review.test.ts b/test/unit/ai-review.test.ts index bb7c02fa65..98e97f98f5 100644 --- a/test/unit/ai-review.test.ts +++ b/test/unit/ai-review.test.ts @@ -2862,6 +2862,21 @@ describe("pure helpers", () => { expect(run).toHaveBeenCalledTimes(2); // 1 primary (timed out) + 1 fallback (succeeded on its first try). }); + it("REGRESSION (#5385-sentry, GITTENSORY-K/8): runDualAiTieBreakJudgeCall stops retrying a model after ONE 429 rate-limit error, same as a CLI timeout", async () => { + let primaryAttempts = 0; + const run = vi.fn(async (model: string) => { + if (model === "fallback") return { response: '{"favored":"reviewer_1"}' }; + primaryAttempts += 1; + throw new Error("claude_code_error_429"); + }); + const env = createTestEnv({ AI: { run } as unknown as Ai }); + const diagnostics: Array<{ status: string; model: string }> = []; + const parsed = await runDualAiTieBreakJudgeCall(env, "primary", "fallback", blockedA, clean, false, diagnostics as never); + expect(parsed?.verdict).toBe("reviewer_1"); + expect(primaryAttempts).toBe(1); // NOT 3 -- the 429 short-circuits further retries of this model. + expect(run).toHaveBeenCalledTimes(2); // 1 primary (rate-limited) + 1 fallback (succeeded on its first try). + }); + it("resolveDualAiTieBreakWithOrderStability returns inconclusive when judge output never parses", async () => { const run = vi.fn(async () => ({ response: "not-json" })); const env = createTestEnv({ AI: { run } as unknown as Ai }); @@ -3013,7 +3028,22 @@ describe("pure helpers", () => { expect(run).toHaveBeenCalledTimes(2); // 1 primary (timed out) + 1 fallback (succeeded on its first try). }); - it("runWorkersOpinion still retries a genuinely transient (non-timeout) error up to the full budget", async () => { + it("REGRESSION (#5385-sentry, GITTENSORY-K/8): runWorkersOpinion stops retrying a model after ONE 429 rate-limit error, same as a CLI timeout", async () => { + let primaryAttempts = 0; + const run = vi.fn(async (model: string) => { + if (model === "fallback") return { response: reviewJson() }; + primaryAttempts += 1; + throw new Error("claude_code_error_429"); + }); + const env = createTestEnv({ AI: { run } as unknown as Ai }); + const diagnostics: Array<{ status: string; model: string }> = []; + const parsed = await runWorkersOpinion(env, "primary", "fallback", "sys", "user", 256, diagnostics as never); + expect(parsed.review?.assessment).toContain("reasonable"); + expect(primaryAttempts).toBe(1); // NOT 3 -- the 429 short-circuits further retries of this model. + expect(run).toHaveBeenCalledTimes(2); // 1 primary (rate-limited) + 1 fallback (succeeded on its first try). + }); + + it("runWorkersOpinion still retries a genuinely transient (non-timeout, non-429) error up to the full budget", async () => { let attempts = 0; const run = vi.fn(async () => { attempts += 1; diff --git a/test/unit/ai-slop.test.ts b/test/unit/ai-slop.test.ts index 71005ebe4a..e51ed9ab2b 100644 --- a/test/unit/ai-slop.test.ts +++ b/test/unit/ai-slop.test.ts @@ -271,6 +271,20 @@ describe("runGittensoryAiSlopAdvisory gating + fail-safe", () => { expect(run).toHaveBeenCalled(); // it tried (3× primary + fallback) and gave up cleanly }); + it("REGRESSION (#5385-sentry, GITTENSORY-K/8): stops retrying a model after ONE 429 rate-limit error instead of burning all 3 attempts, unlike a genuinely transient error", async () => { + const run = vi.fn(async () => { + throw new Error("claude_code_error_429"); + }); + const result = await runGittensoryAiSlopAdvisory(enabledEnv(run), baseInput); + expect(result.status).toBe("ok"); + if (result.status !== "ok") throw new Error("unreachable"); + expect(result.finding).toBeNull(); + // 1 attempt per model (2 models), NOT the full 6-call budget a non-429 error would burn (see the + // "is fail-safe: a throwing model" test above, which uses toHaveBeenCalled() precisely because it burns + // the whole budget) -- the 429 short-circuits each model's remaining retries. + expect(run).toHaveBeenCalledTimes(2); + }); + it("falls back to the reliable model when the primary keeps returning garbage", async () => { const run = vi.fn(async (model: string) => ({ response: model.includes("gpt-oss") ? "not json" : slopJson({ band: "low" }) })); const result = await runGittensoryAiSlopAdvisory(enabledEnv(run), baseInput); diff --git a/test/unit/planner.test.ts b/test/unit/planner.test.ts index 04108985f5..91bfa3da4b 100644 --- a/test/unit/planner.test.ts +++ b/test/unit/planner.test.ts @@ -76,6 +76,16 @@ describe("generateIssuePlan (#issue-coding-plan)", () => { throw new Error("ai down"); }); expect(await generateIssuePlan(createTestEnv({ AI: { run: throwRun } as unknown as Ai }), { title: "T", body: "B" })).toBeNull(); + expect(throwRun).toHaveBeenCalledTimes(4); // 2 models x 2 attempts each -- a non-429 error burns the full budget. + }); + + it("REGRESSION (#5385-sentry, GITTENSORY-K/8): stops retrying a model after ONE 429 rate-limit error instead of burning its full attempt budget", async () => { + const throwRun = vi.fn(async () => { + throw new Error("claude_code_error_429"); + }); + const env = createTestEnv({ AI: { run: throwRun } as unknown as Ai }); + expect(await generateIssuePlan(env, { title: "T", body: "B" })).toBeNull(); + expect(throwRun).toHaveBeenCalledTimes(2); // 1 attempt per model (2 models), not the full 4-call budget. }); }); From 51fda975b7ad33c0dd762229c66cd83ebc48c4bf Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Sun, 12 Jul 2026 16:15:19 -0700 Subject: [PATCH 2/2] fix(review): apply the 429 short-circuit to runWorkersSatisfactionOpinion too Gittensory review flagged that linked-issue-satisfaction-run.ts's own env.AI.run() retry loop was missed by the original fix -- it still burned its full per-model attempt budget on a claude_code_error_429 before falling back, same as the four loops already fixed. --- src/services/linked-issue-satisfaction-run.ts | 4 +++- test/unit/linked-issue-satisfaction-run.test.ts | 11 +++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/services/linked-issue-satisfaction-run.ts b/src/services/linked-issue-satisfaction-run.ts index 0b3d8dcaa6..54131a4976 100644 --- a/src/services/linked-issue-satisfaction-run.ts +++ b/src/services/linked-issue-satisfaction-run.ts @@ -26,6 +26,7 @@ import { coerceAiUsage, estimateNeurons, isEnabled, + isRateLimitError, utcDayStartIso, } from "./ai-review"; @@ -86,7 +87,8 @@ async function runWorkersSatisfactionOpinion( ); const result = buildLinkedIssueSatisfactionResult(issueText, coerceAiText(raw)); if (result) return { result, usage: coerceAiUsage(raw) }; - } catch { + } catch (error) { + if (isRateLimitError(error)) break; /* retry / fall through to fallback */ } } diff --git a/test/unit/linked-issue-satisfaction-run.test.ts b/test/unit/linked-issue-satisfaction-run.test.ts index 49cefd52e7..e35e4cc4d8 100644 --- a/test/unit/linked-issue-satisfaction-run.test.ts +++ b/test/unit/linked-issue-satisfaction-run.test.ts @@ -191,6 +191,17 @@ describe("runGittensoryLinkedIssueSatisfaction gating + fail-safe", () => { expect(run).toHaveBeenCalled(); }); + it("REGRESSION (#5385-sentry, GITTENSORY-K/8): stops retrying a model after ONE 429 rate-limit error instead of burning its full attempt budget", async () => { + const run = vi.fn(async () => { + throw new Error("claude_code_error_429"); + }); + const result = await runGittensoryLinkedIssueSatisfaction(enabledEnv(run), baseInput); + expect(result.status).toBe("ok"); + if (result.status !== "ok") throw new Error("unreachable"); + expect(result.result).toBeNull(); + expect(run).toHaveBeenCalledTimes(2); // 1 attempt per model (2 models), not the full 6-call budget + }); + it("falls back to the reliable model when the primary keeps returning garbage", async () => { const run = vi.fn(async (model: string) => ({ response: model.includes("gpt-oss") ? "not json" : satisfactionJson({ status: "partial" }) })); const result = await runGittensoryLinkedIssueSatisfaction(enabledEnv(run), baseInput);