fix(admin): lift the bench when the operator fixes the shared provider

The health ledger records failures per account and provider, and since
background work leaves out a benched provider, an operator who fixed the
instance-wide provider in the admin AI settings still saw every account's
background work sit out the pause the broken setup had earned, up to a day.
Only a person's own provider settings and the Codex sign-ins forgot the
record.

A successful save of the admin AI settings now forgets what the ledger
recorded for admin-openai on every account and closes the status notes'
negative windows, the same reset a person's own save makes for theirs
(clearStatusNegativeWindows without an account covers every record). The
ledger comments that still said a benched row only deprioritises now say
what it does for background work.

Refs #1279
This commit is contained in:
Marc-André Bombeck committed 2026-10-25 00:42:20 +02:00
1 parent f1163d5efb
commit 402a4a546c
5 files changed
+72 -16

No files matched your search

@@ -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();
});
});
/**
+12
View File
@@ -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({
+16 -13
View File
@@ -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).
}
}
@@ -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 },
});
});
});
+5 -3
View File
@@ -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<void> {
try {
await prisma.insightStatusCache.updateMany({
where: {
userId,
...(userId ? { userId } : {}),
OR: [{ retryAt: { not: null } }, { failureStreak: { gt: 0 } }],
},
data: { retryAt: null, negativeReason: null, failureStreak: 0 },