From 4bd69a8deb9b0eb269e5312da3e5db0c2def49b3 Mon Sep 17 00:00:00 2001 From: ndycode Date: Mon, 15 Jun 2026 05:36:35 +0800 Subject: [PATCH] fix(rotation): persist rate-limit window in short-retry 429 path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The short-retry branch of the runtime fetch loop in index.ts marks the account rate-limited via `markRateLimitedWithReason` (which mutates the disk-serialized `rateLimitResetTimes`) and then sleeps + retries, but never called `saveToDiskDebounced()` — unlike the sibling full-rotation branch directly below it, which persists at line ~2327. A crash during the retry sleep (or before any later save) lost the rate-limit reset time; on restart the account was immediately re-selected, defeating the cooldown. This is the same durability gap class as PR #608 (runtime-rotation-proxy.ts) and PR #607, in a third location. Add the missing `saveToDiskDebounced()` after `recordRateLimit()` in the short-retry branch, mirroring the full-rotation branch. Found by a pre-release deep stress-test sweep. Regression test drives a 429 with a sub-threshold cooldown into the short-retry path and asserts the save is scheduled; it fails without the fix (verified by mutation). Co-Authored-By: Claude Opus 4.8 (1M context) --- index.ts | 6 +++ test/index.test.ts | 100 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+) diff --git a/index.ts b/index.ts index 20a536a83..95c7fbd51 100644 --- a/index.ts +++ b/index.ts @@ -2282,6 +2282,12 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { modelFamily, model, ); + // Persist the rate-limit window like the full-rotation + // branch below (line ~2327). markRateLimitedWithReason + // mutates `rateLimitResetTimes`, which is serialized to + // disk; without this a crash during the retry sleep loses + // the cooldown and the account is re-selected on restart. + accountManager.saveToDiskDebounced(); if ( accountManager.shouldShowAccountToast( diff --git a/test/index.test.ts b/test/index.test.ts index 7922edd5d..9e40b6c75 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -5546,6 +5546,106 @@ describe("OpenAIOAuthPlugin runtime toast forwarding", () => { expect(globalThis.fetch).toHaveBeenCalledTimes(2); }); + it("persists rate-limit window to disk on short-cooldown 429 (saveToDiskDebounced regression)", async () => { + const { AccountManager } = await import("../lib/accounts.js"); + const fetchHelpersModule = await import("../lib/request/fetch-helpers.js"); + const rateLimitBackoffModule = await import( + "../lib/request/rate-limit-backoff.js" + ); + + const saveToDiskDebounced = vi.fn(); + const manager = { + getAccountCount: () => 1, + getCurrentOrNextForFamilyHybrid: () => ({ + index: 0, + accountId: "acc-1", + email: "alpha@example.com", + refreshToken: "refresh-1", + }), + getCurrentOrNextForFamily: () => ({ + index: 0, + accountId: "acc-1", + email: "alpha@example.com", + refreshToken: "refresh-1", + }), + getCurrentWorkspace: () => null, + getAccountByIndex: () => null, + getAccountsSnapshot: () => [], + isAccountAvailableForFamily: () => true, + toAuthDetails: () => ({ + type: "oauth" as const, + access: "access-token", + refresh: "refresh-1", + expires: Date.now() + 60_000, + }), + hasRefreshToken: () => true, + saveToDiskDebounced, + updateFromAuth: () => {}, + clearAuthFailures: () => {}, + incrementAuthFailures: () => 1, + saveToDisk: async () => {}, + markAccountCoolingDown: () => {}, + markRateLimited: () => {}, + markRateLimitedWithReason: () => {}, + consumeToken: () => true, + refundToken: () => {}, + syncCodexCliActiveSelectionForIndex: async () => {}, + markSwitched: () => {}, + removeAccount: () => {}, + recordFailure: () => {}, + recordSuccess: () => {}, + recordRateLimit: () => {}, + getMinWaitTimeForFamily: () => 0, + shouldShowAccountToast: () => false, + markToastShown: () => {}, + setActiveIndex: () => null, + }; + vi.spyOn(AccountManager, "loadFromDisk").mockResolvedValue( + manager as never, + ); + // Short cooldown: 1000ms < 5000ms threshold -> short-retry branch + vi.mocked(fetchHelpersModule.handleErrorResponse).mockResolvedValueOnce({ + response: new Response("rate limited", { status: 429 }), + rateLimit: { retryAfterMs: 1000, code: "rate_limit_exceeded" }, + errorBody: "rate limited", + } as never); + vi.mocked(rateLimitBackoffModule.getRateLimitBackoff).mockReturnValueOnce({ + attempt: 1, + delayMs: 500, + }); + globalThis.fetch = vi + .fn() + .mockResolvedValueOnce(new Response("rate limited", { status: 429 })) + .mockResolvedValueOnce( + new Response(JSON.stringify({ content: "ok" }), { status: 200 }), + ); + + const mockClient = createMockClient(); + const { OpenAIOAuthPlugin } = await import("../index.js"); + const plugin = (await OpenAIOAuthPlugin({ + client: mockClient, + } as never)) as unknown as PluginType; + const sdk = await plugin.auth.loader(getOAuthAuth, { + options: {}, + models: {}, + }); + const response = await sdk.fetch!( + "https://api.openai.com/v1/chat/completions", + { + method: "POST", + body: JSON.stringify({ model: "gpt-5.1" }), + }, + ); + + // The request should ultimately succeed (short-retry -> 200) + expect(response.status).toBe(200); + expect(globalThis.fetch).toHaveBeenCalledTimes(2); + // Regression guard: the short-retry branch MUST call saveToDiskDebounced so + // the rateLimitResetTimes mutation from markRateLimitedWithReason survives a + // crash during the retry sleep (mirrors the full-rotation branch below it). + expect(saveToDiskDebounced).toHaveBeenCalledTimes(1); + }); + it("does not rotate on 404 with unrelated body (not a usage limit)", async () => { const { AccountManager } = await import("../lib/accounts.js"); const fetchHelpersModule = await import("../lib/request/fetch-helpers.js");