diff --git a/src/services/ai-review.ts b/src/services/ai-review.ts index e9c3ba3aea..35b5ff5f1d 100644 --- a/src/services/ai-review.ts +++ b/src/services/ai-review.ts @@ -876,6 +876,14 @@ type AiRunCorrelation = { anthropicModel?: string | undefined; }; +/** True for the self-host CLI adapter's own non-transient timeout signal (`src/selfhost/ai.ts`'s + * `throw new Error("subscription_cli_timeout")`, thrown after a `claude-code`/`codex` subprocess is SIGKILLed + * at its effort-based deadline). Distinguishes it from a genuinely transient failure (a dropped connection, a + * malformed response) that's still worth retrying up to the full budget. */ +function isSubscriptionCliTimeout(error: unknown): boolean { + return error instanceof Error && error.message === "subscription_cli_timeout"; +} + /** One reviewer opinion (whichever provider `env.AI` resolves to — self-host Codex/Claude Code/etc, or the * legacy Workers-AI pair) with a per-slot reliable fallback and a 3× retry on the primary. */ async function runWorkersOpinion( @@ -976,6 +984,12 @@ async function runWorkersOpinion( }), ); lastError = error; + // A CLI timeout is not transient -- the same model retrying the same oversized/complex diff will almost + // certainly time out again. Stop retrying THIS model (the fallback below still gets its own full retry + // 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; } } } @@ -1616,6 +1630,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; } } } diff --git a/test/unit/ai-review.test.ts b/test/unit/ai-review.test.ts index 4717ec7f91..43b647c9cb 100644 --- a/test/unit/ai-review.test.ts +++ b/test/unit/ai-review.test.ts @@ -2675,6 +2675,21 @@ describe("pure helpers", () => { expect(diagnostics.some((d) => d.status === "provider_error")).toBe(true); }); + it("runDualAiTieBreakJudgeCall stops retrying a model after ONE subscription_cli_timeout, but the fallback still gets its full retry budget (#gaming-tactic-draft-cycle)", async () => { + let primaryAttempts = 0; + const run = vi.fn(async (model: string) => { + if (model === "fallback") return { response: '{"favored":"reviewer_1"}' }; + primaryAttempts += 1; + throw new Error("subscription_cli_timeout"); + }); + 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 timeout short-circuits further retries of this model. + expect(run).toHaveBeenCalledTimes(2); // 1 primary (timed out) + 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 }); @@ -2793,6 +2808,34 @@ describe("pure helpers", () => { expect(run).toHaveBeenCalledTimes(1); }); + it("runWorkersOpinion stops retrying a model after ONE subscription_cli_timeout, but the fallback still gets its full retry budget (#gaming-tactic-draft-cycle)", async () => { + let primaryAttempts = 0; + const run = vi.fn(async (model: string) => { + if (model === "fallback") return { response: reviewJson() }; + primaryAttempts += 1; + throw new Error("subscription_cli_timeout"); + }); + 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 timeout short-circuits further retries of this model. + 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 () => { + let attempts = 0; + const run = vi.fn(async () => { + attempts += 1; + if (attempts < 3) throw new Error("connection reset"); + return { response: reviewJson() }; + }); + const env = createTestEnv({ AI: { run } as unknown as Ai }); + const parsed = await runWorkersOpinion(env, "m", "m", "sys", "user", 256); + expect(parsed.review?.assessment).toContain("reasonable"); + expect(attempts).toBe(3); + }); + it("forwards correlation + self-host ai_model override fields into ai.run's options, omitting absent ones (#selfhost-ai-model-override)", async () => { let seenOptions: Record = {}; const run = vi.fn(async (_model: string, options: Record) => {