diff --git a/src/app/api/admin/ai-settings/__tests__/route.test.ts b/src/app/api/admin/ai-settings/__tests__/route.test.ts index f776098b5..2a64f5d60 100644 --- a/src/app/api/admin/ai-settings/__tests__/route.test.ts +++ b/src/app/api/admin/ai-settings/__tests__/route.test.ts @@ -16,6 +16,10 @@ vi.mock("@/lib/db", () => ({ upsert: vi.fn(), }, user: { findMany: vi.fn().mockResolvedValue([]) }, + providerHealth: { deleteMany: vi.fn().mockResolvedValue({ count: 0 }) }, + insightStatusCache: { + updateMany: vi.fn().mockResolvedValue({ count: 0 }), + }, }, })); @@ -217,6 +221,31 @@ describe("PUT /api/admin/ai-settings", () => { statusCode: 429, }); }); + + // #1279 — background work leaves a provider the health ledger benched out + // of every account's chain. A fixed operator provider must not sit out a + // pause the broken one earned, so a saved change forgets what the ledger + // recorded for it on every account, and lets their notes try again. + it("forgets the operator provider's recorded failures on every account", async () => { + upsertEcho(); + await PUT(jsonReq({ model: "gpt-4o-mini" })); + expect(prisma.providerHealth.deleteMany).toHaveBeenCalledWith({ + where: { providerType: "admin-openai" }, + }); + expect(prisma.insightStatusCache.updateMany).toHaveBeenCalledWith({ + where: { + OR: [{ retryAt: { not: null } }, { failureStreak: { gt: 0 } }], + }, + data: { retryAt: null, negativeReason: null, failureStreak: 0 }, + }); + }); + + it("forgets nothing when the change is refused", async () => { + await expect( + PUT(jsonReq({ baseUrl: "https://attacker.example/v1" })), + ).rejects.toMatchObject({ statusCode: 422 }); + expect(prisma.providerHealth.deleteMany).not.toHaveBeenCalled(); + }); }); /** diff --git a/src/app/api/admin/ai-settings/route.ts b/src/app/api/admin/ai-settings/route.ts index aafbd9b7a..a413d0bb9 100644 --- a/src/app/api/admin/ai-settings/route.ts +++ b/src/app/api/admin/ai-settings/route.ts @@ -8,6 +8,8 @@ import { encrypt, decrypt } from "@/lib/crypto"; import { checkRateLimit } from "@/lib/rate-limit"; import { annotate } from "@/lib/logging/context"; import { legacyPrivateHostReport } from "@/lib/ai/legacy-private-host-report"; +import { forgetProviderHealth } from "@/lib/ai/provider-health-ledger"; +import { clearStatusNegativeWindows } from "@/lib/insights/status-negative-window"; export const dynamic = "force-dynamic"; @@ -134,6 +136,16 @@ export const PUT = apiHandler(async (request: NextRequest) => { details: auditDetails, }); + // #1279 — background work leaves out a provider the health ledger benched, + // on every account whose chain falls back on this one, and a status note + // waits out its negative window. A fixed operator provider must not sit out + // a pause the broken one earned, so what was recorded against it is + // forgotten on every account, as a person's own save does for theirs. + await Promise.all([ + forgetProviderHealth({ providerType: "admin-openai" }), + clearStatusNegativeWindows(), + ]); + const hasKey = Boolean(settings.adminAiKeyEncrypted); return apiSuccess({ diff --git a/src/lib/ai/provider-health-ledger.ts b/src/lib/ai/provider-health-ledger.ts index 8368aadab..4ddafe397 100644 --- a/src/lib/ai/provider-health-ledger.ts +++ b/src/lib/ai/provider-health-ledger.ts @@ -64,12 +64,14 @@ export const HARD_FAILURE_SKIP_THRESHOLD = 3; * From `PROVIDER_PAUSE_THRESHOLD` consecutive hard failures on, each failure * re-arms the skip window at `PROVIDER_PAUSE_BASE_MS` doubled per further * failure, capped at `PROVIDER_PAUSE_MAX_MS`. Paused is the same skip as - * backoff (the provider moves to the tail of the chain, behind everything - * healthy, and is still walked as a last resort, so a chain never loses its - * only provider). When the window lifts, the next call tries it again: one - * success clears the count and resumes it in place, one more failure doubles - * the pause. Nothing needs to be reset by hand. The admin provider-health card - * shows a paused provider and until when. + * backoff: an interactive call moves the provider to the tail of the chain, + * behind everything healthy, and still walks it as a last resort, so a person + * never loses their only provider; background work leaves it out until the + * window lifts (#1279, `background-bench.ts`). When the window lifts, the next + * call tries it again: one success clears the count and resumes it in place, + * one more failure doubles the pause. Saving a fixed provider forgets its + * record (`forgetProviderHealth`). The admin provider-health card shows a + * paused provider and until when. * * Conservative on purpose: five failures in a row is far past a brown-out, * the first pause is half an hour, and the ceiling is a day, so a provider @@ -283,12 +285,12 @@ export const postgresProviderHealthLedger: ProviderHealthLedger = { * Drop the recorded outcomes of a credential that has just been replaced. * * The ledger describes a credential, not a provider slot: once the person - * (or, for `admin-codex`, the operator) signs in again, the `auth_failed` - * row of the refused credential would otherwise keep the fresh one at the - * tail of the chain and reported as expired for the rest of its cooldown. - * Without `userId` the rows of every account are dropped — the operator's - * central connection is one credential shared by all of them. Best effort, - * like every ledger write. + * (or, for the `admin-*` providers, the operator) signs in again or saves a + * fixed setup, the row of the refused one would otherwise keep the fresh one + * at the tail of every interactive chain, out of background work entirely + * (#1279), and reported as expired for the rest of its window. Without + * `userId` the rows of every account are dropped: an operator provider is one + * credential shared by all of them. Best effort, like every ledger write. */ export async function forgetProviderHealth(target: { providerType: ProviderChainType; @@ -302,7 +304,8 @@ export async function forgetProviderHealth(target: { }, }); } catch { - // A stale row only deprioritises; it never blocks. + // A stale row lapses with its window; until then it keeps background + // work off the replaced credential (#1279). } } diff --git a/src/lib/insights/__tests__/status-negative-window.test.ts b/src/lib/insights/__tests__/status-negative-window.test.ts index 56b253fc6..fbc8c9263 100644 --- a/src/lib/insights/__tests__/status-negative-window.test.ts +++ b/src/lib/insights/__tests__/status-negative-window.test.ts @@ -209,4 +209,14 @@ describe("clearStatusNegativeWindows", () => { updateMany.mockRejectedValue(new Error("pool exhausted")); await expect(clearStatusNegativeWindows("u1")).resolves.toBeUndefined(); }); + + it("closes them for every record when the operator's provider changed", async () => { + await clearStatusNegativeWindows(); + expect(updateMany).toHaveBeenCalledWith({ + where: { + OR: [{ retryAt: { not: null } }, { failureStreak: { gt: 0 } }], + }, + data: { retryAt: null, negativeReason: null, failureStreak: 0 }, + }); + }); }); diff --git a/src/lib/insights/status-negative-window.ts b/src/lib/insights/status-negative-window.ts index d6d740337..e748f89cd 100644 --- a/src/lib/insights/status-negative-window.ts +++ b/src/lib/insights/status-negative-window.ts @@ -136,15 +136,17 @@ export async function statusNoteCoolingDown( /** * Close every open window on one record's notes, streaks included: the person * asked for fresh notes (the regenerate action) or changed what answers them. - * Best-effort: a failed write leaves windows that lapse on their own. + * Without `userId`, on every record's notes: the operator changed the + * provider every account may fall back on. Best-effort: a failed write leaves + * windows that lapse on their own. */ export async function clearStatusNegativeWindows( - userId: string, + userId?: string, ): Promise { try { await prisma.insightStatusCache.updateMany({ where: { - userId, + ...(userId ? { userId } : {}), OR: [{ retryAt: { not: null } }, { failureStreak: { gt: 0 } }], }, data: { retryAt: null, negativeReason: null, failureStreak: 0 },