From d7a8a4de46ee1f953a481172f9863e686e56493e Mon Sep 17 00:00:00 2001 From: ndycode Date: Wed, 4 Mar 2026 16:39:08 +0800 Subject: [PATCH 1/8] feat(reliability): add retry governor controls and telemetry Adds a pure retry governor for all-rate-limited flows, introduces an absolute wait ceiling setting with env override, and wires decision-based retry gating into the request loop. Also exposes retry ceiling in Settings Hub (Rotation & Quota), and adds structured codex-metrics counters for retry governor stop reasons. Validation: - npm run typecheck - npm run lint - npm run build - npm test - npm run clean:repo:check - npm run audit:ci Co-authored-by: Codex --- docs/development/CONFIG_FIELDS.md | 2 + docs/reference/settings.md | 4 +- index.ts | 86 ++++++++++++++++++-- lib/codex-manager/settings-hub.ts | 19 +++++ lib/config.ts | 10 +++ lib/request/retry-governor.ts | 72 +++++++++++++++++ lib/schemas.ts | 1 + test/codex-manager-cli.test.ts | 2 + test/index-retry.test.ts | 30 +++++++ test/index.test.ts | 1 + test/plugin-config.test.ts | 18 +++++ test/retry-governor.test.ts | 127 ++++++++++++++++++++++++++++++ test/schemas.test.ts | 3 + test/settings-hub-utils.test.ts | 4 + 14 files changed, 370 insertions(+), 9 deletions(-) create mode 100644 lib/request/retry-governor.ts create mode 100644 test/retry-governor.test.ts diff --git a/docs/development/CONFIG_FIELDS.md b/docs/development/CONFIG_FIELDS.md index 9a3ee4cf3..df82442d7 100644 --- a/docs/development/CONFIG_FIELDS.md +++ b/docs/development/CONFIG_FIELDS.md @@ -62,6 +62,7 @@ Used only for host plugin mode through the host runtime config file. | `retryAllAccountsRateLimited` | `true` | | `retryAllAccountsMaxWaitMs` | `0` | | `retryAllAccountsMaxRetries` | `Infinity` | +| `retryAllAccountsAbsoluteCeilingMs` | `0` | | `unsupportedCodexPolicy` | `strict` | | `fallbackOnUnsupportedCodexModel` | `false` | | `fallbackToGpt52OnUnsupportedGpt53` | `true` | @@ -193,6 +194,7 @@ Used only for host plugin mode through the host runtime config file. | `CODEX_TUI_V2` | Toggle TUI v2 | | `CODEX_TUI_COLOR_PROFILE` | TUI color profile | | `CODEX_TUI_GLYPHS` | TUI glyph mode | +| `CODEX_AUTH_RETRY_ALL_ABSOLUTE_CEILING_MS` | Absolute wait ceiling for retry-all-on-rate-limit loop | | `CODEX_AUTH_FETCH_TIMEOUT_MS` | Request timeout override | | `CODEX_AUTH_STREAM_STALL_TIMEOUT_MS` | Stream stall timeout override | | `CODEX_MULTI_AUTH_SYNC_CODEX_CLI` | Toggle Codex CLI state sync | diff --git a/docs/reference/settings.md b/docs/reference/settings.md index 1466374b9..0accb9739 100644 --- a/docs/reference/settings.md +++ b/docs/reference/settings.md @@ -86,6 +86,7 @@ Examples: - `retryAllAccountsRateLimited` - `retryAllAccountsMaxWaitMs` - `retryAllAccountsMaxRetries` +- `retryAllAccountsAbsoluteCeilingMs` ### Refresh and Recovery @@ -126,6 +127,7 @@ Common operator overrides: - `CODEX_TUI_V2` - `CODEX_TUI_COLOR_PROFILE` - `CODEX_TUI_GLYPHS` +- `CODEX_AUTH_RETRY_ALL_ABSOLUTE_CEILING_MS` - `CODEX_AUTH_FETCH_TIMEOUT_MS` - `CODEX_AUTH_STREAM_STALL_TIMEOUT_MS` @@ -175,4 +177,4 @@ codex auth forecast --live - [commands.md](commands.md) - [storage-paths.md](storage-paths.md) -- [../configuration.md](../configuration.md) \ No newline at end of file +- [../configuration.md](../configuration.md) diff --git a/index.ts b/index.ts index 7db88088a..55c8e52f6 100644 --- a/index.ts +++ b/index.ts @@ -44,6 +44,7 @@ import { getFastSessionMaxInputItems, getRateLimitToastDebounceMs, getRetryAllAccountsMaxRetries, + getRetryAllAccountsAbsoluteCeilingMs, getRetryAllAccountsMaxWaitMs, getRetryAllAccountsRateLimited, getFallbackToGpt52OnUnsupportedGpt53, @@ -156,6 +157,10 @@ import { } from "./lib/request/rate-limit-backoff.js"; import { isEmptyResponse } from "./lib/request/response-handler.js"; import { addJitter } from "./lib/rotation.js"; +import { + decideRetryAllAccountsRateLimited, + type RetryAllAccountsRateLimitDecisionReason, +} from "./lib/request/retry-governor.js"; import { SessionAffinityStore } from "./lib/session-affinity.js"; import { LiveAccountSync } from "./lib/live-account-sync.js"; import { RefreshGuardian } from "./lib/refresh-guardian.js"; @@ -344,6 +349,9 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { streamFailoverAttempts: number; streamFailoverRecoveries: number; streamFailoverCrossAccountRecoveries: number; + retryGovernorStopsWaitExceedsMax: number; + retryGovernorStopsRetryLimitReached: number; + retryGovernorStopsAbsoluteCeilingExceeded: number; cumulativeLatencyMs: number; lastRequestAt: number | null; lastError: string | null; @@ -365,11 +373,32 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { streamFailoverAttempts: 0, streamFailoverRecoveries: 0, streamFailoverCrossAccountRecoveries: 0, + retryGovernorStopsWaitExceedsMax: 0, + retryGovernorStopsRetryLimitReached: 0, + retryGovernorStopsAbsoluteCeilingExceeded: 0, cumulativeLatencyMs: 0, lastRequestAt: null, lastError: null, }; + const recordRetryGovernorStopReason = ( + reason: RetryAllAccountsRateLimitDecisionReason, + ): void => { + switch (reason) { + case "wait-exceeds-max": + runtimeMetrics.retryGovernorStopsWaitExceedsMax += 1; + return; + case "retry-limit-reached": + runtimeMetrics.retryGovernorStopsRetryLimitReached += 1; + return; + case "absolute-ceiling-exceeded": + runtimeMetrics.retryGovernorStopsAbsoluteCeilingExceeded += 1; + return; + default: + return; + } + }; + type TokenSuccess = Extract; type TokenSuccessWithAccount = TokenSuccess & { accountIdOverride?: string; @@ -1124,6 +1153,8 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { const retryAllAccountsRateLimited = getRetryAllAccountsRateLimited(pluginConfig); const retryAllAccountsMaxWaitMs = getRetryAllAccountsMaxWaitMs(pluginConfig); const retryAllAccountsMaxRetries = getRetryAllAccountsMaxRetries(pluginConfig); + const retryAllAccountsAbsoluteCeilingMs = + getRetryAllAccountsAbsoluteCeilingMs(pluginConfig); const unsupportedCodexPolicy = getUnsupportedCodexPolicy(pluginConfig); const fallbackOnUnsupportedCodexModel = unsupportedCodexPolicy === "fallback"; const fallbackToGpt52OnUnsupportedGpt53 = @@ -1397,6 +1428,7 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { }; let allRateLimitedRetries = 0; + let accumulatedAllRateLimitedWaitMs = 0; let emptyResponseRetries = 0; const attemptedUnsupportedFallbackModels = new Set(); if (model) { @@ -2368,20 +2400,37 @@ while (attempted.size < Math.max(1, accountCount)) { const waitMs = accountManager.getMinWaitTimeForFamily(modelFamily, model); const count = accountManager.getAccountCount(); + const retryDecision = decideRetryAllAccountsRateLimited({ + enabled: retryAllAccountsRateLimited, + accountCount: count, + waitMs, + maxWaitMs: retryAllAccountsMaxWaitMs, + currentRetryCount: allRateLimitedRetries, + maxRetries: retryAllAccountsMaxRetries, + accumulatedWaitMs: accumulatedAllRateLimitedWaitMs, + absoluteCeilingMs: retryAllAccountsAbsoluteCeilingMs, + }); - if ( - retryAllAccountsRateLimited && - count > 0 && - waitMs > 0 && - (retryAllAccountsMaxWaitMs === 0 || - waitMs <= retryAllAccountsMaxWaitMs) && - allRateLimitedRetries < retryAllAccountsMaxRetries - ) { + if (retryDecision.shouldRetry) { const countdownMessage = `All ${count} account(s) rate-limited. Waiting`; await sleepWithCountdown(addJitter(waitMs, 0.2), countdownMessage); allRateLimitedRetries++; + accumulatedAllRateLimitedWaitMs += waitMs; continue; } + recordRetryGovernorStopReason(retryDecision.reason); + if (retryDecision.reason !== "disabled" && retryDecision.reason !== "no-accounts") { + logDebug("Retry governor blocked all-rate-limited retry", { + reason: retryDecision.reason, + accountCount: count, + waitMs, + retryCount: allRateLimitedRetries, + accumulatedWaitMs: accumulatedAllRateLimitedWaitMs, + maxWaitMs: retryAllAccountsMaxWaitMs, + maxRetries: retryAllAccountsMaxRetries, + absoluteCeilingMs: retryAllAccountsAbsoluteCeilingMs, + }); + } const waitLabel = waitMs > 0 ? formatWaitTime(waitMs) : "a bit"; const message = @@ -3763,6 +3812,9 @@ while (attempted.size < Math.max(1, accountCount)) { `Stream failover attempts: ${runtimeMetrics.streamFailoverAttempts}`, `Stream failover recoveries: ${runtimeMetrics.streamFailoverRecoveries}`, `Stream failover cross-account recoveries: ${runtimeMetrics.streamFailoverCrossAccountRecoveries}`, + `Retry governor stops (wait>max): ${runtimeMetrics.retryGovernorStopsWaitExceedsMax}`, + `Retry governor stops (retry limit): ${runtimeMetrics.retryGovernorStopsRetryLimitReached}`, + `Retry governor stops (absolute ceiling): ${runtimeMetrics.retryGovernorStopsAbsoluteCeilingExceeded}`, `Empty-response retries: ${runtimeMetrics.emptyResponseRetries}`, `Session affinity entries: ${sessionAffinityEntries}`, `Live sync: ${liveSyncSnapshot?.running ? "on" : "off"} (${liveSyncSnapshot?.reloadCount ?? 0} reloads)`, @@ -3798,6 +3850,24 @@ while (attempted.size < Math.max(1, accountCount)) { String(runtimeMetrics.streamFailoverCrossAccountRecoveries), "accent", ), + formatUiKeyValue( + ui, + "Retry governor stops (wait>max)", + String(runtimeMetrics.retryGovernorStopsWaitExceedsMax), + "warning", + ), + formatUiKeyValue( + ui, + "Retry governor stops (retry limit)", + String(runtimeMetrics.retryGovernorStopsRetryLimitReached), + "warning", + ), + formatUiKeyValue( + ui, + "Retry governor stops (absolute ceiling)", + String(runtimeMetrics.retryGovernorStopsAbsoluteCeilingExceeded), + "warning", + ), formatUiKeyValue(ui, "Empty-response retries", String(runtimeMetrics.emptyResponseRetries), "warning"), formatUiKeyValue(ui, "Session affinity entries", String(sessionAffinityEntries), "muted"), formatUiKeyValue( diff --git a/lib/codex-manager/settings-hub.ts b/lib/codex-manager/settings-hub.ts index 99cbdae4a..fbc686f62 100644 --- a/lib/codex-manager/settings-hub.ts +++ b/lib/codex-manager/settings-hub.ts @@ -185,6 +185,7 @@ type BackendNumberSettingKey = | "proactiveRefreshBufferMs" | "parallelProbingMaxConcurrency" | "fastSessionMaxInputItems" + | "retryAllAccountsAbsoluteCeilingMs" | "networkErrorCooldownMs" | "serverErrorCooldownMs" | "fetchTimeoutMs" @@ -377,6 +378,15 @@ const BACKEND_NUMBER_OPTIONS: BackendNumberSettingOption[] = [ step: 2, unit: "count", }, + { + key: "retryAllAccountsAbsoluteCeilingMs", + label: "Retry-All Absolute Wait Ceiling", + description: "Total max wait for retry-all-on-rate-limit. Set 0 for unlimited.", + min: 0, + max: 24 * 60 * 60_000, + step: 30_000, + unit: "ms", + }, { key: "networkErrorCooldownMs", label: "Network Error Cooldown", @@ -486,6 +496,7 @@ const BACKEND_CATEGORY_OPTIONS: BackendCategoryOption[] = [ "preemptiveQuotaRemainingPercent5h", "preemptiveQuotaRemainingPercent7d", "preemptiveQuotaMaxDeferralMs", + "retryAllAccountsAbsoluteCeilingMs", ], }, { @@ -974,8 +985,15 @@ function buildBackendSettingsPreview( config.preemptiveQuotaRemainingPercent7d ?? BACKEND_DEFAULTS.preemptiveQuotaRemainingPercent7d ?? 5; + const retryAllAbsoluteCeilingMs = + config.retryAllAccountsAbsoluteCeilingMs ?? + BACKEND_DEFAULTS.retryAllAccountsAbsoluteCeilingMs ?? + 0; const fetchTimeout = config.fetchTimeoutMs ?? BACKEND_DEFAULTS.fetchTimeoutMs ?? 60_000; const stallTimeout = config.streamStallTimeoutMs ?? BACKEND_DEFAULTS.streamStallTimeoutMs ?? 45_000; + const retryAllAbsoluteCeilingOption = BACKEND_NUMBER_OPTION_BY_KEY.get( + "retryAllAccountsAbsoluteCeilingMs", + ); const fetchTimeoutOption = BACKEND_NUMBER_OPTION_BY_KEY.get("fetchTimeoutMs"); const stallTimeoutOption = BACKEND_NUMBER_OPTION_BY_KEY.get("streamStallTimeoutMs"); @@ -993,6 +1011,7 @@ function buildBackendSettingsPreview( const hint = [ `thresholds 5h<=${highlightIfFocused("preemptiveQuotaRemainingPercent5h", `${threshold5h}%`)}`, `7d<=${highlightIfFocused("preemptiveQuotaRemainingPercent7d", `${threshold7d}%`)}`, + `retry ceiling ${highlightIfFocused("retryAllAccountsAbsoluteCeilingMs", retryAllAbsoluteCeilingOption ? formatBackendNumberValue(retryAllAbsoluteCeilingOption, retryAllAbsoluteCeilingMs) : `${retryAllAbsoluteCeilingMs}ms`)}`, `timeouts ${highlightIfFocused("fetchTimeoutMs", fetchTimeoutOption ? formatBackendNumberValue(fetchTimeoutOption, fetchTimeout) : `${fetchTimeout}ms`)}/${highlightIfFocused("streamStallTimeoutMs", stallTimeoutOption ? formatBackendNumberValue(stallTimeoutOption, stallTimeout) : `${stallTimeout}ms`)}`, ].join(" | "); diff --git a/lib/config.ts b/lib/config.ts index f9e7ecf85..b16427ff3 100644 --- a/lib/config.ts +++ b/lib/config.ts @@ -125,6 +125,7 @@ export const DEFAULT_PLUGIN_CONFIG: PluginConfig = { retryAllAccountsRateLimited: true, retryAllAccountsMaxWaitMs: 0, retryAllAccountsMaxRetries: Infinity, + retryAllAccountsAbsoluteCeilingMs: 0, unsupportedCodexPolicy: "strict", fallbackOnUnsupportedCodexModel: false, fallbackToGpt52OnUnsupportedGpt53: true, @@ -591,6 +592,15 @@ export function getRetryAllAccountsMaxRetries(pluginConfig: PluginConfig): numbe ); } +export function getRetryAllAccountsAbsoluteCeilingMs(pluginConfig: PluginConfig): number { + return resolveNumberSetting( + "CODEX_AUTH_RETRY_ALL_ABSOLUTE_CEILING_MS", + pluginConfig.retryAllAccountsAbsoluteCeilingMs, + 0, + { min: 0 }, + ); +} + export function getUnsupportedCodexPolicy( pluginConfig: PluginConfig, ): UnsupportedCodexPolicy { diff --git a/lib/request/retry-governor.ts b/lib/request/retry-governor.ts new file mode 100644 index 000000000..890f421c2 --- /dev/null +++ b/lib/request/retry-governor.ts @@ -0,0 +1,72 @@ +export interface RetryAllAccountsRateLimitDecisionInput { + enabled: boolean; + accountCount: number; + waitMs: number; + maxWaitMs: number; + currentRetryCount: number; + maxRetries: number; + accumulatedWaitMs: number; + absoluteCeilingMs: number; +} + +export type RetryAllAccountsRateLimitDecisionReason = + | "allowed" + | "disabled" + | "no-accounts" + | "no-wait" + | "wait-exceeds-max" + | "retry-limit-reached" + | "absolute-ceiling-exceeded"; + +export interface RetryAllAccountsRateLimitDecision { + shouldRetry: boolean; + reason: RetryAllAccountsRateLimitDecisionReason; +} + +function clampNonNegative(value: number): number { + if (!Number.isFinite(value)) return 0; + return Math.max(0, Math.floor(value)); +} + +function normalizeRetryLimit(value: number): number { + if (!Number.isFinite(value)) return Number.POSITIVE_INFINITY; + return clampNonNegative(value); +} + +/** + * Decide whether "retry all accounts when rate-limited" should run for the current loop. + * + * This helper is pure and deterministic so retry behavior can be tested without + * exercising the full request pipeline. + */ +export function decideRetryAllAccountsRateLimited( + input: RetryAllAccountsRateLimitDecisionInput, +): RetryAllAccountsRateLimitDecision { + const accountCount = clampNonNegative(input.accountCount); + const waitMs = clampNonNegative(input.waitMs); + const maxWaitMs = clampNonNegative(input.maxWaitMs); + const currentRetryCount = clampNonNegative(input.currentRetryCount); + const maxRetries = normalizeRetryLimit(input.maxRetries); + const accumulatedWaitMs = clampNonNegative(input.accumulatedWaitMs); + const absoluteCeilingMs = clampNonNegative(input.absoluteCeilingMs); + + if (!input.enabled) { + return { shouldRetry: false, reason: "disabled" }; + } + if (accountCount === 0) { + return { shouldRetry: false, reason: "no-accounts" }; + } + if (waitMs === 0) { + return { shouldRetry: false, reason: "no-wait" }; + } + if (maxWaitMs > 0 && waitMs > maxWaitMs) { + return { shouldRetry: false, reason: "wait-exceeds-max" }; + } + if (currentRetryCount >= maxRetries) { + return { shouldRetry: false, reason: "retry-limit-reached" }; + } + if (absoluteCeilingMs > 0 && accumulatedWaitMs + waitMs > absoluteCeilingMs) { + return { shouldRetry: false, reason: "absolute-ceiling-exceeded" }; + } + return { shouldRetry: true, reason: "allowed" }; +} diff --git a/lib/schemas.ts b/lib/schemas.ts index 55028b6ed..40e7bfaff 100644 --- a/lib/schemas.ts +++ b/lib/schemas.ts @@ -21,6 +21,7 @@ export const PluginConfigSchema = z.object({ retryAllAccountsRateLimited: z.boolean().optional(), retryAllAccountsMaxWaitMs: z.number().min(0).optional(), retryAllAccountsMaxRetries: z.number().min(0).optional(), + retryAllAccountsAbsoluteCeilingMs: z.number().min(0).optional(), unsupportedCodexPolicy: z.enum(["strict", "fallback"]).optional(), fallbackOnUnsupportedCodexModel: z.boolean().optional(), fallbackToGpt52OnUnsupportedGpt53: z.boolean().optional(), diff --git a/test/codex-manager-cli.test.ts b/test/codex-manager-cli.test.ts index 27261cd27..21ddbe232 100644 --- a/test/codex-manager-cli.test.ts +++ b/test/codex-manager-cli.test.ts @@ -1358,6 +1358,7 @@ describe("codex manager cli commands", () => { { type: "open-category", key: "rotation-quota" }, { type: "toggle", key: "preemptiveQuotaEnabled" }, { type: "bump", key: "preemptiveQuotaRemainingPercent5h", direction: 1 }, + { type: "bump", key: "retryAllAccountsAbsoluteCeilingMs", direction: 1 }, { type: "back" }, { type: "save" }, { type: "back" }, @@ -1374,6 +1375,7 @@ describe("codex manager cli commands", () => { expect.objectContaining({ preemptiveQuotaEnabled: expect.any(Boolean), preemptiveQuotaRemainingPercent5h: expect.any(Number), + retryAllAccountsAbsoluteCeilingMs: expect.any(Number), }), ); }); diff --git a/test/index-retry.test.ts b/test/index-retry.test.ts index 3813edc48..d73ff1c78 100644 --- a/test/index-retry.test.ts +++ b/test/index-retry.test.ts @@ -146,6 +146,7 @@ describe("OpenAIAuthPlugin rate-limit retry", () => { "CODEX_AUTH_RETRY_ALL_RATE_LIMITED", "CODEX_AUTH_RETRY_ALL_MAX_WAIT_MS", "CODEX_AUTH_RETRY_ALL_MAX_RETRIES", + "CODEX_AUTH_RETRY_ALL_ABSOLUTE_CEILING_MS", "CODEX_AUTH_TOKEN_REFRESH_SKEW_MS", "CODEX_AUTH_RATE_LIMIT_TOAST_DEBOUNCE_MS", "CODEX_AUTH_PREWARM", @@ -160,6 +161,7 @@ describe("OpenAIAuthPlugin rate-limit retry", () => { process.env.CODEX_AUTH_RETRY_ALL_RATE_LIMITED = "1"; process.env.CODEX_AUTH_RETRY_ALL_MAX_WAIT_MS = "5000"; process.env.CODEX_AUTH_RETRY_ALL_MAX_RETRIES = "1"; + process.env.CODEX_AUTH_RETRY_ALL_ABSOLUTE_CEILING_MS = "0"; process.env.CODEX_AUTH_TOKEN_REFRESH_SKEW_MS = "0"; process.env.CODEX_AUTH_RATE_LIMIT_TOAST_DEBOUNCE_MS = "0"; process.env.CODEX_AUTH_PREWARM = "0"; @@ -213,5 +215,33 @@ describe("OpenAIAuthPlugin rate-limit retry", () => { expect(globalThis.fetch).toHaveBeenCalledTimes(1); expect(response.status).toBe(200); }); + + it("stops retrying when absolute retry wait ceiling would be exceeded", async () => { + process.env.CODEX_AUTH_RETRY_ALL_ABSOLUTE_CEILING_MS = "500"; + const { OpenAIAuthPlugin } = await import("../index.js"); + const client = { + tui: { showToast: vi.fn() }, + auth: { set: vi.fn() }, + } as any; + + const plugin = await OpenAIAuthPlugin({ client }); + const getAuth = async () => ({ + type: "oauth" as const, + access: "a", + refresh: "r", + expires: Date.now() + 60_000, + multiAccount: true, + }); + + const sdk = (await plugin.auth.loader(getAuth, { options: {}, models: {} })) as any; + const response = await sdk.fetch("https://example.com", {}); + + expect(globalThis.fetch).not.toHaveBeenCalled(); + expect(response.status).toBe(429); + + const metrics = await plugin.tool["codex-metrics"].execute(); + const plainMetrics = String(metrics).replace(/\u001b\[[0-9;]*m/g, ""); + expect(plainMetrics).toContain("Retry governor stops (absolute ceiling): 1"); + }); }); diff --git a/test/index.test.ts b/test/index.test.ts index d6d95497f..c6976b933 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -76,6 +76,7 @@ vi.mock("../lib/config.js", () => ({ getFastSessionMaxInputItems: () => 30, getRateLimitToastDebounceMs: () => 5000, getRetryAllAccountsMaxRetries: () => 3, + getRetryAllAccountsAbsoluteCeilingMs: () => 0, getRetryAllAccountsMaxWaitMs: () => 30000, getRetryAllAccountsRateLimited: () => true, getUnsupportedCodexPolicy: vi.fn(() => "fallback"), diff --git a/test/plugin-config.test.ts b/test/plugin-config.test.ts index 9caebf96b..4c557d92c 100644 --- a/test/plugin-config.test.ts +++ b/test/plugin-config.test.ts @@ -12,6 +12,7 @@ import { getUnsupportedCodexPolicy, getFallbackOnUnsupportedCodexModel, getTokenRefreshSkewMs, + getRetryAllAccountsAbsoluteCeilingMs, getRetryAllAccountsMaxRetries, getFallbackToGpt52OnUnsupportedGpt53, getUnsupportedCodexFallbackChain, @@ -63,6 +64,7 @@ describe('Plugin Configuration', () => { 'CODEX_AUTH_UNSUPPORTED_MODEL_POLICY', 'CODEX_AUTH_FALLBACK_UNSUPPORTED_MODEL', 'CODEX_AUTH_FALLBACK_GPT53_TO_GPT52', + 'CODEX_AUTH_RETRY_ALL_ABSOLUTE_CEILING_MS', 'CODEX_AUTH_PREEMPTIVE_QUOTA_ENABLED', 'CODEX_AUTH_PREEMPTIVE_QUOTA_5H_REMAINING_PCT', 'CODEX_AUTH_PREEMPTIVE_QUOTA_7D_REMAINING_PCT', @@ -106,6 +108,7 @@ describe('Plugin Configuration', () => { retryAllAccountsRateLimited: true, retryAllAccountsMaxWaitMs: 0, retryAllAccountsMaxRetries: Infinity, + retryAllAccountsAbsoluteCeilingMs: 0, unsupportedCodexPolicy: 'strict', fallbackOnUnsupportedCodexModel: false, fallbackToGpt52OnUnsupportedGpt53: true, @@ -164,6 +167,7 @@ describe('Plugin Configuration', () => { retryAllAccountsRateLimited: true, retryAllAccountsMaxWaitMs: 0, retryAllAccountsMaxRetries: Infinity, + retryAllAccountsAbsoluteCeilingMs: 0, unsupportedCodexPolicy: 'strict', fallbackOnUnsupportedCodexModel: false, fallbackToGpt52OnUnsupportedGpt53: true, @@ -419,6 +423,7 @@ describe('Plugin Configuration', () => { retryAllAccountsRateLimited: true, retryAllAccountsMaxWaitMs: 0, retryAllAccountsMaxRetries: Infinity, + retryAllAccountsAbsoluteCeilingMs: 0, unsupportedCodexPolicy: 'strict', fallbackOnUnsupportedCodexModel: false, fallbackToGpt52OnUnsupportedGpt53: true, @@ -483,6 +488,7 @@ describe('Plugin Configuration', () => { retryAllAccountsRateLimited: true, retryAllAccountsMaxWaitMs: 0, retryAllAccountsMaxRetries: Infinity, + retryAllAccountsAbsoluteCeilingMs: 0, unsupportedCodexPolicy: 'strict', fallbackOnUnsupportedCodexModel: false, fallbackToGpt52OnUnsupportedGpt53: true, @@ -541,6 +547,7 @@ describe('Plugin Configuration', () => { retryAllAccountsRateLimited: true, retryAllAccountsMaxWaitMs: 0, retryAllAccountsMaxRetries: Infinity, + retryAllAccountsAbsoluteCeilingMs: 0, unsupportedCodexPolicy: 'strict', fallbackOnUnsupportedCodexModel: false, fallbackToGpt52OnUnsupportedGpt53: true, @@ -939,6 +946,17 @@ describe('Plugin Configuration', () => { expect(result).toBe(5); }); + it('should default retry-all absolute ceiling to zero', () => { + delete process.env.CODEX_AUTH_RETRY_ALL_ABSOLUTE_CEILING_MS; + expect(getRetryAllAccountsAbsoluteCeilingMs({})).toBe(0); + }); + + it('should prioritize retry-all absolute ceiling env override', () => { + process.env.CODEX_AUTH_RETRY_ALL_ABSOLUTE_CEILING_MS = '12000'; + const config: PluginConfig = { retryAllAccountsAbsoluteCeilingMs: 5000 }; + expect(getRetryAllAccountsAbsoluteCeilingMs(config)).toBe(12000); + }); + it('should return env value without min constraint', () => { process.env.CODEX_AUTH_TOKEN_REFRESH_SKEW_MS = '30000'; const config: PluginConfig = { tokenRefreshSkewMs: 60000 }; diff --git a/test/retry-governor.test.ts b/test/retry-governor.test.ts new file mode 100644 index 000000000..3e7746338 --- /dev/null +++ b/test/retry-governor.test.ts @@ -0,0 +1,127 @@ +import { describe, expect, it } from "vitest"; +import { decideRetryAllAccountsRateLimited } from "../lib/request/retry-governor.js"; + +describe("decideRetryAllAccountsRateLimited", () => { + it("allows retry when all limits permit it", () => { + const result = decideRetryAllAccountsRateLimited({ + enabled: true, + accountCount: 2, + waitMs: 1_000, + maxWaitMs: 2_000, + currentRetryCount: 1, + maxRetries: 3, + accumulatedWaitMs: 2_000, + absoluteCeilingMs: 10_000, + }); + + expect(result).toEqual({ shouldRetry: true, reason: "allowed" }); + }); + + it("rejects retry when disabled", () => { + const result = decideRetryAllAccountsRateLimited({ + enabled: false, + accountCount: 2, + waitMs: 1_000, + maxWaitMs: 0, + currentRetryCount: 0, + maxRetries: Infinity, + accumulatedWaitMs: 0, + absoluteCeilingMs: 0, + }); + + expect(result).toEqual({ shouldRetry: false, reason: "disabled" }); + }); + + it("rejects retry when there are no accounts", () => { + const result = decideRetryAllAccountsRateLimited({ + enabled: true, + accountCount: 0, + waitMs: 1_000, + maxWaitMs: 0, + currentRetryCount: 0, + maxRetries: Infinity, + accumulatedWaitMs: 0, + absoluteCeilingMs: 0, + }); + + expect(result).toEqual({ shouldRetry: false, reason: "no-accounts" }); + }); + + it("rejects retry when wait time is non-positive", () => { + const result = decideRetryAllAccountsRateLimited({ + enabled: true, + accountCount: 2, + waitMs: 0, + maxWaitMs: 0, + currentRetryCount: 0, + maxRetries: Infinity, + accumulatedWaitMs: 0, + absoluteCeilingMs: 0, + }); + + expect(result).toEqual({ shouldRetry: false, reason: "no-wait" }); + }); + + it("rejects retry when wait exceeds max wait", () => { + const result = decideRetryAllAccountsRateLimited({ + enabled: true, + accountCount: 2, + waitMs: 1_500, + maxWaitMs: 1_000, + currentRetryCount: 0, + maxRetries: Infinity, + accumulatedWaitMs: 0, + absoluteCeilingMs: 0, + }); + + expect(result).toEqual({ shouldRetry: false, reason: "wait-exceeds-max" }); + }); + + it("rejects retry when max retries reached", () => { + const result = decideRetryAllAccountsRateLimited({ + enabled: true, + accountCount: 2, + waitMs: 1_000, + maxWaitMs: 0, + currentRetryCount: 2, + maxRetries: 2, + accumulatedWaitMs: 0, + absoluteCeilingMs: 0, + }); + + expect(result).toEqual({ shouldRetry: false, reason: "retry-limit-reached" }); + }); + + it("rejects retry when absolute ceiling would be exceeded", () => { + const result = decideRetryAllAccountsRateLimited({ + enabled: true, + accountCount: 2, + waitMs: 1_001, + maxWaitMs: 0, + currentRetryCount: 0, + maxRetries: Infinity, + accumulatedWaitMs: 1_000, + absoluteCeilingMs: 2_000, + }); + + expect(result).toEqual({ + shouldRetry: false, + reason: "absolute-ceiling-exceeded", + }); + }); + + it("treats zero absolute ceiling as unlimited", () => { + const result = decideRetryAllAccountsRateLimited({ + enabled: true, + accountCount: 2, + waitMs: 2_000, + maxWaitMs: 0, + currentRetryCount: 0, + maxRetries: Infinity, + accumulatedWaitMs: 100_000, + absoluteCeilingMs: 0, + }); + + expect(result).toEqual({ shouldRetry: true, reason: "allowed" }); + }); +}); diff --git a/test/schemas.test.ts b/test/schemas.test.ts index 16cd2f97d..7d3ee8c31 100644 --- a/test/schemas.test.ts +++ b/test/schemas.test.ts @@ -30,6 +30,7 @@ describe("PluginConfigSchema", () => { retryAllAccountsRateLimited: true, retryAllAccountsMaxWaitMs: 5000, retryAllAccountsMaxRetries: 3, + retryAllAccountsAbsoluteCeilingMs: 15000, unsupportedCodexPolicy: "strict", fallbackOnUnsupportedCodexModel: true, fallbackToGpt52OnUnsupportedGpt53: false, @@ -71,6 +72,7 @@ describe("PluginConfigSchema", () => { ["sessionAffinityMaxEntries", 7, 8], ["proactiveRefreshIntervalMs", 4999, 5000], ["proactiveRefreshBufferMs", 29_999, 30_000], + ["retryAllAccountsAbsoluteCeilingMs", -1, 0], ["preemptiveQuotaMaxDeferralMs", 999, 1000], ] as const)("enforces minimum for %s", (key, invalidValue, validValue) => { const invalidResult = PluginConfigSchema.safeParse({ [key]: invalidValue }); @@ -109,6 +111,7 @@ describe("PluginConfigSchema", () => { "sessionAffinityMaxEntries", "proactiveRefreshIntervalMs", "proactiveRefreshBufferMs", + "retryAllAccountsAbsoluteCeilingMs", "networkErrorCooldownMs", "serverErrorCooldownMs", "preemptiveQuotaRemainingPercent5h", diff --git a/test/settings-hub-utils.test.ts b/test/settings-hub-utils.test.ts index 24b48fbbb..9358e114d 100644 --- a/test/settings-hub-utils.test.ts +++ b/test/settings-hub-utils.test.ts @@ -64,6 +64,10 @@ describe("settings-hub utility coverage", () => { const api = await loadSettingsHubTestApi(); expect(api.clampBackendNumber("fetchTimeoutMs", 250)).toBe(1_000); expect(api.clampBackendNumber("fetchTimeoutMs", 999_999)).toBe(600_000); + expect(api.clampBackendNumber("retryAllAccountsAbsoluteCeilingMs", -1)).toBe(0); + expect(api.clampBackendNumber("retryAllAccountsAbsoluteCeilingMs", 999_999_999)).toBe( + 24 * 60 * 60_000, + ); expect(() => api.clampBackendNumber("unknown-setting", 5)).toThrow( "Unknown backend numeric setting key", ); From 35d658c032a0352c4ae473692a85877e242d9190 Mon Sep 17 00:00:00 2001 From: ndycode Date: Thu, 5 Mar 2026 17:47:59 +0800 Subject: [PATCH 2/8] fix(reliability): enforce absolute ceiling on jittered retry waits Bound jittered all-rate-limited sleeps by the configured absolute ceiling so actual wait time cannot overshoot the guardrail.\n\nAdds a deterministic regression test that forces +20% jitter and verifies retry execution still proceeds once the capped wait elapses.\n\nCo-authored-by: Codex --- index.ts | 15 +++++++++++++-- test/index-retry.test.ts | 29 +++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/index.ts b/index.ts index 55c8e52f6..ebf7d842a 100644 --- a/index.ts +++ b/index.ts @@ -2413,9 +2413,20 @@ while (attempted.size < Math.max(1, accountCount)) { if (retryDecision.shouldRetry) { const countdownMessage = `All ${count} account(s) rate-limited. Waiting`; - await sleepWithCountdown(addJitter(waitMs, 0.2), countdownMessage); + const jitteredWaitMs = addJitter(waitMs, 0.2); + const boundedWaitMs = + retryAllAccountsAbsoluteCeilingMs > 0 + ? Math.min( + jitteredWaitMs, + Math.max( + 0, + retryAllAccountsAbsoluteCeilingMs - accumulatedAllRateLimitedWaitMs, + ), + ) + : jitteredWaitMs; + await sleepWithCountdown(boundedWaitMs, countdownMessage); allRateLimitedRetries++; - accumulatedAllRateLimitedWaitMs += waitMs; + accumulatedAllRateLimitedWaitMs += boundedWaitMs; continue; } recordRetryGovernorStopReason(retryDecision.reason); diff --git a/test/index-retry.test.ts b/test/index-retry.test.ts index d73ff1c78..c2c6ee91e 100644 --- a/test/index-retry.test.ts +++ b/test/index-retry.test.ts @@ -243,5 +243,34 @@ describe("OpenAIAuthPlugin rate-limit retry", () => { const plainMetrics = String(metrics).replace(/\u001b\[[0-9;]*m/g, ""); expect(plainMetrics).toContain("Retry governor stops (absolute ceiling): 1"); }); + + it("caps jittered retry waits at the configured absolute ceiling", async () => { + process.env.CODEX_AUTH_RETRY_ALL_ABSOLUTE_CEILING_MS = "1100"; + vi.spyOn(Math, "random").mockReturnValue(1); + const { OpenAIAuthPlugin } = await import("../index.js"); + const client = { + tui: { showToast: vi.fn() }, + auth: { set: vi.fn() }, + } as any; + + const plugin = await OpenAIAuthPlugin({ client }); + const getAuth = async () => ({ + type: "oauth" as const, + access: "a", + refresh: "r", + expires: Date.now() + 60_000, + multiAccount: true, + }); + + const sdk = (await plugin.auth.loader(getAuth, { options: {}, models: {} })) as any; + const fetchPromise = sdk.fetch("https://example.com", {}); + expect(globalThis.fetch).not.toHaveBeenCalled(); + + await vi.advanceTimersByTimeAsync(1150); + expect(globalThis.fetch).toHaveBeenCalledTimes(1); + + const response = await fetchPromise; + expect(response.status).toBe(200); + }); }); From c9324566feacec742e3ccc69606903d63409d812 Mon Sep 17 00:00:00 2001 From: ndycode Date: Thu, 5 Mar 2026 19:18:15 +0800 Subject: [PATCH 3/8] fix(retry): align governor wait planning and config/docs bounds Align retry-governor inputs with planned jitter/bounded waits, suppress no-wait false-positive block logs, enforce 24h absolute-ceiling clamp in runtime config, and sync settings docs/UI semantics (0 = unlimited). Add regression coverage for jitter directions, equality boundary, env parsing cases, and settings-hub preview rendering. Co-authored-by: Codex --- docs/development/CONFIG_FIELDS.md | 4 +- docs/reference/settings.md | 2 + index.ts | 35 ++++++++------ lib/codex-manager/settings-hub.ts | 12 ++++- lib/config.ts | 2 +- test/codex-manager-cli.test.ts | 6 ++- test/index-retry.test.ts | 77 ++++++++++++++++++++++++++++--- test/plugin-config.test.ts | 17 +++++++ test/retry-governor.test.ts | 15 ++++++ test/settings-hub-utils.test.ts | 10 ++++ 10 files changed, 154 insertions(+), 26 deletions(-) diff --git a/docs/development/CONFIG_FIELDS.md b/docs/development/CONFIG_FIELDS.md index df82442d7..03f1a2673 100644 --- a/docs/development/CONFIG_FIELDS.md +++ b/docs/development/CONFIG_FIELDS.md @@ -62,7 +62,7 @@ Used only for host plugin mode through the host runtime config file. | `retryAllAccountsRateLimited` | `true` | | `retryAllAccountsMaxWaitMs` | `0` | | `retryAllAccountsMaxRetries` | `Infinity` | -| `retryAllAccountsAbsoluteCeilingMs` | `0` | +| `retryAllAccountsAbsoluteCeilingMs` | `0 ms (0–24h; 0 = unlimited)` | | `unsupportedCodexPolicy` | `strict` | | `fallbackOnUnsupportedCodexModel` | `false` | | `fallbackToGpt52OnUnsupportedGpt53` | `true` | @@ -194,7 +194,7 @@ Used only for host plugin mode through the host runtime config file. | `CODEX_TUI_V2` | Toggle TUI v2 | | `CODEX_TUI_COLOR_PROFILE` | TUI color profile | | `CODEX_TUI_GLYPHS` | TUI glyph mode | -| `CODEX_AUTH_RETRY_ALL_ABSOLUTE_CEILING_MS` | Absolute wait ceiling for retry-all-on-rate-limit loop | +| `CODEX_AUTH_RETRY_ALL_ABSOLUTE_CEILING_MS` | Absolute wait ceiling in ms for retry-all-on-rate-limit loop (`0–24h`, `0 = unlimited`) | | `CODEX_AUTH_FETCH_TIMEOUT_MS` | Request timeout override | | `CODEX_AUTH_STREAM_STALL_TIMEOUT_MS` | Stream stall timeout override | | `CODEX_MULTI_AUTH_SYNC_CODEX_CLI` | Toggle Codex CLI state sync | diff --git a/docs/reference/settings.md b/docs/reference/settings.md index 0accb9739..bb30b8d5a 100644 --- a/docs/reference/settings.md +++ b/docs/reference/settings.md @@ -87,6 +87,7 @@ Examples: - `retryAllAccountsMaxWaitMs` - `retryAllAccountsMaxRetries` - `retryAllAccountsAbsoluteCeilingMs` + Unit: milliseconds. Bounds: `0` to `24h`. `0` means unlimited. ### Refresh and Recovery @@ -128,6 +129,7 @@ Common operator overrides: - `CODEX_TUI_COLOR_PROFILE` - `CODEX_TUI_GLYPHS` - `CODEX_AUTH_RETRY_ALL_ABSOLUTE_CEILING_MS` + Rotation & Quota override for `retryAllAccountsAbsoluteCeilingMs` (ms, `0` to `24h`, `0` = unlimited). - `CODEX_AUTH_FETCH_TIMEOUT_MS` - `CODEX_AUTH_STREAM_STALL_TIMEOUT_MS` diff --git a/index.ts b/index.ts index ebf7d842a..e2ba33ef2 100644 --- a/index.ts +++ b/index.ts @@ -2400,10 +2400,21 @@ while (attempted.size < Math.max(1, accountCount)) { const waitMs = accountManager.getMinWaitTimeForFamily(modelFamily, model); const count = accountManager.getAccountCount(); + const jitteredWaitMs = waitMs > 0 ? addJitter(waitMs, 0.2) : 0; + const plannedWaitMs = + retryAllAccountsAbsoluteCeilingMs > 0 + ? Math.min( + jitteredWaitMs, + Math.max( + 0, + retryAllAccountsAbsoluteCeilingMs - accumulatedAllRateLimitedWaitMs, + ), + ) + : jitteredWaitMs; const retryDecision = decideRetryAllAccountsRateLimited({ enabled: retryAllAccountsRateLimited, accountCount: count, - waitMs, + waitMs: plannedWaitMs, maxWaitMs: retryAllAccountsMaxWaitMs, currentRetryCount: allRateLimitedRetries, maxRetries: retryAllAccountsMaxRetries, @@ -2413,28 +2424,22 @@ while (attempted.size < Math.max(1, accountCount)) { if (retryDecision.shouldRetry) { const countdownMessage = `All ${count} account(s) rate-limited. Waiting`; - const jitteredWaitMs = addJitter(waitMs, 0.2); - const boundedWaitMs = - retryAllAccountsAbsoluteCeilingMs > 0 - ? Math.min( - jitteredWaitMs, - Math.max( - 0, - retryAllAccountsAbsoluteCeilingMs - accumulatedAllRateLimitedWaitMs, - ), - ) - : jitteredWaitMs; - await sleepWithCountdown(boundedWaitMs, countdownMessage); + await sleepWithCountdown(plannedWaitMs, countdownMessage); allRateLimitedRetries++; - accumulatedAllRateLimitedWaitMs += boundedWaitMs; + accumulatedAllRateLimitedWaitMs += plannedWaitMs; continue; } recordRetryGovernorStopReason(retryDecision.reason); - if (retryDecision.reason !== "disabled" && retryDecision.reason !== "no-accounts") { + if ( + retryDecision.reason !== "disabled" && + retryDecision.reason !== "no-accounts" && + retryDecision.reason !== "no-wait" + ) { logDebug("Retry governor blocked all-rate-limited retry", { reason: retryDecision.reason, accountCount: count, waitMs, + plannedWaitMs, retryCount: allRateLimitedRetries, accumulatedWaitMs: accumulatedAllRateLimitedWaitMs, maxWaitMs: retryAllAccountsMaxWaitMs, diff --git a/lib/codex-manager/settings-hub.ts b/lib/codex-manager/settings-hub.ts index fbc686f62..77cb9f8b7 100644 --- a/lib/codex-manager/settings-hub.ts +++ b/lib/codex-manager/settings-hub.ts @@ -994,6 +994,11 @@ function buildBackendSettingsPreview( const retryAllAbsoluteCeilingOption = BACKEND_NUMBER_OPTION_BY_KEY.get( "retryAllAccountsAbsoluteCeilingMs", ); + const retryCeilingLabel = retryAllAbsoluteCeilingMs === 0 + ? "unlimited" + : retryAllAbsoluteCeilingOption + ? formatBackendNumberValue(retryAllAbsoluteCeilingOption, retryAllAbsoluteCeilingMs) + : `${retryAllAbsoluteCeilingMs}ms`; const fetchTimeoutOption = BACKEND_NUMBER_OPTION_BY_KEY.get("fetchTimeoutMs"); const stallTimeoutOption = BACKEND_NUMBER_OPTION_BY_KEY.get("streamStallTimeoutMs"); @@ -1011,7 +1016,7 @@ function buildBackendSettingsPreview( const hint = [ `thresholds 5h<=${highlightIfFocused("preemptiveQuotaRemainingPercent5h", `${threshold5h}%`)}`, `7d<=${highlightIfFocused("preemptiveQuotaRemainingPercent7d", `${threshold7d}%`)}`, - `retry ceiling ${highlightIfFocused("retryAllAccountsAbsoluteCeilingMs", retryAllAbsoluteCeilingOption ? formatBackendNumberValue(retryAllAbsoluteCeilingOption, retryAllAbsoluteCeilingMs) : `${retryAllAbsoluteCeilingMs}ms`)}`, + `retry ceiling ${highlightIfFocused("retryAllAccountsAbsoluteCeilingMs", retryCeilingLabel)}`, `timeouts ${highlightIfFocused("fetchTimeoutMs", fetchTimeoutOption ? formatBackendNumberValue(fetchTimeoutOption, fetchTimeout) : `${fetchTimeout}ms`)}/${highlightIfFocused("streamStallTimeoutMs", stallTimeoutOption ? formatBackendNumberValue(stallTimeoutOption, stallTimeout) : `${stallTimeout}ms`)}`, ].join(" | "); @@ -1088,6 +1093,10 @@ function clampBackendNumberForTests(settingKey: string, value: number): number { return clampBackendNumber(option, value); } +function buildBackendSettingsPreviewForTests(config: PluginConfig): { label: string; hint: string } { + return buildBackendSettingsPreview(config, getUiRuntimeOptions()); +} + async function withQueuedRetryForTests( pathKey: string, task: () => Promise, @@ -1112,6 +1121,7 @@ async function persistBackendConfigSelectionForTests( const __testOnly = { clampBackendNumber: clampBackendNumberForTests, + buildBackendSettingsPreview: buildBackendSettingsPreviewForTests, formatMenuLayoutMode, cloneDashboardSettings, withQueuedRetry: withQueuedRetryForTests, diff --git a/lib/config.ts b/lib/config.ts index b16427ff3..429e42bf8 100644 --- a/lib/config.ts +++ b/lib/config.ts @@ -597,7 +597,7 @@ export function getRetryAllAccountsAbsoluteCeilingMs(pluginConfig: PluginConfig) "CODEX_AUTH_RETRY_ALL_ABSOLUTE_CEILING_MS", pluginConfig.retryAllAccountsAbsoluteCeilingMs, 0, - { min: 0 }, + { min: 0, max: 24 * 60 * 60_000 }, ); } diff --git a/test/codex-manager-cli.test.ts b/test/codex-manager-cli.test.ts index 21ddbe232..5762fc5e8 100644 --- a/test/codex-manager-cli.test.ts +++ b/test/codex-manager-cli.test.ts @@ -1375,9 +1375,13 @@ describe("codex manager cli commands", () => { expect.objectContaining({ preemptiveQuotaEnabled: expect.any(Boolean), preemptiveQuotaRemainingPercent5h: expect.any(Number), - retryAllAccountsAbsoluteCeilingMs: expect.any(Number), + retryAllAccountsAbsoluteCeilingMs: 30_000, }), ); + const savedPluginConfig = savePluginConfigMock.mock.calls[0]?.[0] as + | { retryAllAccountsAbsoluteCeilingMs?: number } + | undefined; + expect(savedPluginConfig?.retryAllAccountsAbsoluteCeilingMs).toBe(30_000); }); it.each([ diff --git a/test/index-retry.test.ts b/test/index-retry.test.ts index c2c6ee91e..2732041d0 100644 --- a/test/index-retry.test.ts +++ b/test/index-retry.test.ts @@ -1,4 +1,5 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { stripVTControlCharacters } from "node:util"; process.env.CODEX_MULTI_AUTH_EXPOSE_ADMIN_TOOLS = "1"; @@ -216,7 +217,7 @@ describe("OpenAIAuthPlugin rate-limit retry", () => { expect(response.status).toBe(200); }); - it("stops retrying when absolute retry wait ceiling would be exceeded", async () => { + it("caps first retry wait to the configured absolute retry ceiling", async () => { process.env.CODEX_AUTH_RETRY_ALL_ABSOLUTE_CEILING_MS = "500"; const { OpenAIAuthPlugin } = await import("../index.js"); const client = { @@ -234,14 +235,19 @@ describe("OpenAIAuthPlugin rate-limit retry", () => { }); const sdk = (await plugin.auth.loader(getAuth, { options: {}, models: {} })) as any; - const response = await sdk.fetch("https://example.com", {}); - + const fetchPromise = sdk.fetch("https://example.com", {}); expect(globalThis.fetch).not.toHaveBeenCalled(); - expect(response.status).toBe(429); + await vi.advanceTimersByTimeAsync(499); + expect(globalThis.fetch).not.toHaveBeenCalled(); + await vi.advanceTimersByTimeAsync(1); + expect(globalThis.fetch).toHaveBeenCalledTimes(1); + + const response = await fetchPromise; + expect(response.status).toBe(200); const metrics = await plugin.tool["codex-metrics"].execute(); - const plainMetrics = String(metrics).replace(/\u001b\[[0-9;]*m/g, ""); - expect(plainMetrics).toContain("Retry governor stops (absolute ceiling): 1"); + const plainMetrics = stripVTControlCharacters(String(metrics)); + expect(plainMetrics).toContain("Retry governor stops (absolute ceiling): 0"); }); it("caps jittered retry waits at the configured absolute ceiling", async () => { @@ -272,5 +278,64 @@ describe("OpenAIAuthPlugin rate-limit retry", () => { const response = await fetchPromise; expect(response.status).toBe(200); }); + + it("consumes remaining ceiling budget under -20% jitter without premature stop", async () => { + process.env.CODEX_AUTH_RETRY_ALL_ABSOLUTE_CEILING_MS = "1600"; + process.env.CODEX_AUTH_RETRY_ALL_MAX_RETRIES = "2"; + vi.spyOn(Math, "random").mockReturnValue(0); + const { OpenAIAuthPlugin } = await import("../index.js"); + const client = { + tui: { showToast: vi.fn() }, + auth: { set: vi.fn() }, + } as any; + + const plugin = await OpenAIAuthPlugin({ client }); + const getAuth = async () => ({ + type: "oauth" as const, + access: "a", + refresh: "r", + expires: Date.now() + 60_000, + multiAccount: true, + }); + + const sdk = (await plugin.auth.loader(getAuth, { options: {}, models: {} })) as any; + const fetchPromise = sdk.fetch("https://example.com", {}); + expect(globalThis.fetch).not.toHaveBeenCalled(); + + await vi.advanceTimersByTimeAsync(1600); + expect(globalThis.fetch).toHaveBeenCalledTimes(1); + + const response = await fetchPromise; + expect(response.status).toBe(200); + }); + + it("does not allow +20% jitter to bypass max wait governor limits", async () => { + process.env.CODEX_AUTH_RETRY_ALL_MAX_WAIT_MS = "1000"; + process.env.CODEX_AUTH_RETRY_ALL_MAX_RETRIES = "2"; + vi.spyOn(Math, "random").mockReturnValue(1); + const { OpenAIAuthPlugin } = await import("../index.js"); + const client = { + tui: { showToast: vi.fn() }, + auth: { set: vi.fn() }, + } as any; + + const plugin = await OpenAIAuthPlugin({ client }); + const getAuth = async () => ({ + type: "oauth" as const, + access: "a", + refresh: "r", + expires: Date.now() + 60_000, + multiAccount: true, + }); + + const sdk = (await plugin.auth.loader(getAuth, { options: {}, models: {} })) as any; + const response = await sdk.fetch("https://example.com", {}); + expect(response.status).toBe(429); + expect(globalThis.fetch).not.toHaveBeenCalled(); + + const metrics = await plugin.tool["codex-metrics"].execute(); + const plainMetrics = stripVTControlCharacters(String(metrics)); + expect(plainMetrics).toContain("Retry governor stops (wait>max): 1"); + }); }); diff --git a/test/plugin-config.test.ts b/test/plugin-config.test.ts index 4c557d92c..eea4a2cf3 100644 --- a/test/plugin-config.test.ts +++ b/test/plugin-config.test.ts @@ -957,6 +957,23 @@ describe('Plugin Configuration', () => { expect(getRetryAllAccountsAbsoluteCeilingMs(config)).toBe(12000); }); + it('should clamp retry-all absolute ceiling to 24h upper bound', () => { + process.env.CODEX_AUTH_RETRY_ALL_ABSOLUTE_CEILING_MS = String(48 * 60 * 60_000); + expect(getRetryAllAccountsAbsoluteCeilingMs({ retryAllAccountsAbsoluteCeilingMs: 5000 })) + .toBe(24 * 60 * 60_000); + }); + + it('clamps negative retry-all absolute ceiling env override to zero', () => { + process.env.CODEX_AUTH_RETRY_ALL_ABSOLUTE_CEILING_MS = '-1'; + expect(getRetryAllAccountsAbsoluteCeilingMs({ retryAllAccountsAbsoluteCeilingMs: 5000 })).toBe(0); + }); + + it('falls back to config/default when retry-all absolute ceiling env override is invalid', () => { + process.env.CODEX_AUTH_RETRY_ALL_ABSOLUTE_CEILING_MS = 'not-a-number'; + expect(getRetryAllAccountsAbsoluteCeilingMs({ retryAllAccountsAbsoluteCeilingMs: 5000 })).toBe(5000); + expect(getRetryAllAccountsAbsoluteCeilingMs({})).toBe(0); + }); + it('should return env value without min constraint', () => { process.env.CODEX_AUTH_TOKEN_REFRESH_SKEW_MS = '30000'; const config: PluginConfig = { tokenRefreshSkewMs: 60000 }; diff --git a/test/retry-governor.test.ts b/test/retry-governor.test.ts index 3e7746338..83b31591f 100644 --- a/test/retry-governor.test.ts +++ b/test/retry-governor.test.ts @@ -110,6 +110,21 @@ describe("decideRetryAllAccountsRateLimited", () => { }); }); + it("allows retry when accumulated wait exactly matches the absolute ceiling", () => { + const result = decideRetryAllAccountsRateLimited({ + enabled: true, + accountCount: 2, + waitMs: 1_000, + maxWaitMs: 0, + currentRetryCount: 0, + maxRetries: Infinity, + accumulatedWaitMs: 1_000, + absoluteCeilingMs: 2_000, + }); + + expect(result).toEqual({ shouldRetry: true, reason: "allowed" }); + }); + it("treats zero absolute ceiling as unlimited", () => { const result = decideRetryAllAccountsRateLimited({ enabled: true, diff --git a/test/settings-hub-utils.test.ts b/test/settings-hub-utils.test.ts index 9358e114d..8e90b2324 100644 --- a/test/settings-hub-utils.test.ts +++ b/test/settings-hub-utils.test.ts @@ -7,6 +7,7 @@ import type { PluginConfig } from "../lib/types.js"; type SettingsHubTestApi = { clampBackendNumber: (settingKey: string, value: number) => number; + buildBackendSettingsPreview: (config: PluginConfig) => { label: string; hint: string }; formatMenuLayoutMode: (mode: "compact-details" | "expanded-rows") => string; cloneDashboardSettings: (settings: DashboardDisplaySettings) => DashboardDisplaySettings; withQueuedRetry: (pathKey: string, task: () => Promise) => Promise; @@ -73,6 +74,15 @@ describe("settings-hub utility coverage", () => { ); }); + it("renders retry-all absolute ceiling 0 as unlimited in preview", async () => { + const api = await loadSettingsHubTestApi(); + const preview = api.buildBackendSettingsPreview({ + retryAllAccountsAbsoluteCeilingMs: 0, + }); + expect(preview.hint).toContain("retry ceiling unlimited"); + expect(preview.hint).not.toContain("retry ceiling 0ms"); + }); + it("formats layout mode labels", async () => { const api = await loadSettingsHubTestApi(); expect(api.formatMenuLayoutMode("expanded-rows")).toBe("Expanded Rows"); From b942f6a36356d213145f120664cecd24ab40e43d Mon Sep 17 00:00:00 2001 From: ndycode Date: Thu, 5 Mar 2026 21:04:02 +0800 Subject: [PATCH 4/8] fix(retry): restore deterministic governor thresholds Evaluate retry-governor decisions against raw wait values so max-wait checks stay deterministic and absolute-ceiling telemetry remains reachable. Keep jitter only for sleep planning with a bounded wait, and add regression coverage for threshold and ceiling behavior. Co-authored-by: Codex --- index.ts | 41 ++++++++++++++++---------------- test/index-retry.test.ts | 51 +++++++++++++++++++++++++++++++--------- 2 files changed, 60 insertions(+), 32 deletions(-) diff --git a/index.ts b/index.ts index e2ba33ef2..ef0fcd3ad 100644 --- a/index.ts +++ b/index.ts @@ -2400,27 +2400,25 @@ while (attempted.size < Math.max(1, accountCount)) { const waitMs = accountManager.getMinWaitTimeForFamily(modelFamily, model); const count = accountManager.getAccountCount(); - const jitteredWaitMs = waitMs > 0 ? addJitter(waitMs, 0.2) : 0; - const plannedWaitMs = - retryAllAccountsAbsoluteCeilingMs > 0 - ? Math.min( - jitteredWaitMs, - Math.max( - 0, - retryAllAccountsAbsoluteCeilingMs - accumulatedAllRateLimitedWaitMs, - ), - ) - : jitteredWaitMs; - const retryDecision = decideRetryAllAccountsRateLimited({ - enabled: retryAllAccountsRateLimited, - accountCount: count, - waitMs: plannedWaitMs, - maxWaitMs: retryAllAccountsMaxWaitMs, - currentRetryCount: allRateLimitedRetries, - maxRetries: retryAllAccountsMaxRetries, - accumulatedWaitMs: accumulatedAllRateLimitedWaitMs, - absoluteCeilingMs: retryAllAccountsAbsoluteCeilingMs, - }); + const jitteredWaitMs = waitMs > 0 ? addJitter(waitMs, 0.2) : 0; + const remainingCeilingMs = + retryAllAccountsAbsoluteCeilingMs > 0 + ? Math.max( + 0, + retryAllAccountsAbsoluteCeilingMs - accumulatedAllRateLimitedWaitMs, + ) + : Number.POSITIVE_INFINITY; + const plannedWaitMs = Math.min(jitteredWaitMs, remainingCeilingMs); + const retryDecision = decideRetryAllAccountsRateLimited({ + enabled: retryAllAccountsRateLimited, + accountCount: count, + waitMs, + maxWaitMs: retryAllAccountsMaxWaitMs, + currentRetryCount: allRateLimitedRetries, + maxRetries: retryAllAccountsMaxRetries, + accumulatedWaitMs: accumulatedAllRateLimitedWaitMs, + absoluteCeilingMs: retryAllAccountsAbsoluteCeilingMs, + }); if (retryDecision.shouldRetry) { const countdownMessage = `All ${count} account(s) rate-limited. Waiting`; @@ -2439,6 +2437,7 @@ while (attempted.size < Math.max(1, accountCount)) { reason: retryDecision.reason, accountCount: count, waitMs, + jitteredWaitMs, plannedWaitMs, retryCount: allRateLimitedRetries, accumulatedWaitMs: accumulatedAllRateLimitedWaitMs, diff --git a/test/index-retry.test.ts b/test/index-retry.test.ts index 2732041d0..feaea05d4 100644 --- a/test/index-retry.test.ts +++ b/test/index-retry.test.ts @@ -217,7 +217,7 @@ describe("OpenAIAuthPlugin rate-limit retry", () => { expect(response.status).toBe(200); }); - it("caps first retry wait to the configured absolute retry ceiling", async () => { + it("stops immediately when absolute ceiling is below the raw retry wait", async () => { process.env.CODEX_AUTH_RETRY_ALL_ABSOLUTE_CEILING_MS = "500"; const { OpenAIAuthPlugin } = await import("../index.js"); const client = { @@ -235,19 +235,13 @@ describe("OpenAIAuthPlugin rate-limit retry", () => { }); const sdk = (await plugin.auth.loader(getAuth, { options: {}, models: {} })) as any; - const fetchPromise = sdk.fetch("https://example.com", {}); - expect(globalThis.fetch).not.toHaveBeenCalled(); - await vi.advanceTimersByTimeAsync(499); + const response = await sdk.fetch("https://example.com", {}); + expect(response.status).toBe(429); expect(globalThis.fetch).not.toHaveBeenCalled(); - await vi.advanceTimersByTimeAsync(1); - expect(globalThis.fetch).toHaveBeenCalledTimes(1); - - const response = await fetchPromise; - expect(response.status).toBe(200); const metrics = await plugin.tool["codex-metrics"].execute(); const plainMetrics = stripVTControlCharacters(String(metrics)); - expect(plainMetrics).toContain("Retry governor stops (absolute ceiling): 0"); + expect(plainMetrics).toContain("Retry governor stops (absolute ceiling): 1"); }); it("caps jittered retry waits at the configured absolute ceiling", async () => { @@ -309,7 +303,7 @@ describe("OpenAIAuthPlugin rate-limit retry", () => { expect(response.status).toBe(200); }); - it("does not allow +20% jitter to bypass max wait governor limits", async () => { + it("keeps max wait checks deterministic at the raw wait threshold", async () => { process.env.CODEX_AUTH_RETRY_ALL_MAX_WAIT_MS = "1000"; process.env.CODEX_AUTH_RETRY_ALL_MAX_RETRIES = "2"; vi.spyOn(Math, "random").mockReturnValue(1); @@ -328,6 +322,41 @@ describe("OpenAIAuthPlugin rate-limit retry", () => { multiAccount: true, }); + const sdk = (await plugin.auth.loader(getAuth, { options: {}, models: {} })) as any; + const fetchPromise = sdk.fetch("https://example.com", {}); + expect(globalThis.fetch).not.toHaveBeenCalled(); + await vi.advanceTimersByTimeAsync(1199); + expect(globalThis.fetch).not.toHaveBeenCalled(); + await vi.advanceTimersByTimeAsync(1); + expect(globalThis.fetch).toHaveBeenCalledTimes(1); + + const response = await fetchPromise; + expect(response.status).toBe(200); + + const metrics = await plugin.tool["codex-metrics"].execute(); + const plainMetrics = stripVTControlCharacters(String(metrics)); + expect(plainMetrics).toContain("Retry governor stops (wait>max): 0"); + }); + + it("blocks retries when raw wait exceeds max wait even under negative jitter", async () => { + process.env.CODEX_AUTH_RETRY_ALL_MAX_WAIT_MS = "999"; + process.env.CODEX_AUTH_RETRY_ALL_MAX_RETRIES = "2"; + vi.spyOn(Math, "random").mockReturnValue(0); + const { OpenAIAuthPlugin } = await import("../index.js"); + const client = { + tui: { showToast: vi.fn() }, + auth: { set: vi.fn() }, + } as any; + + const plugin = await OpenAIAuthPlugin({ client }); + const getAuth = async () => ({ + type: "oauth" as const, + access: "a", + refresh: "r", + expires: Date.now() + 60_000, + multiAccount: true, + }); + const sdk = (await plugin.auth.loader(getAuth, { options: {}, models: {} })) as any; const response = await sdk.fetch("https://example.com", {}); expect(response.status).toBe(429); From ae2cf9f8d9a70b339b9ca0d7b00ce67e5ee2a31b Mon Sep 17 00:00:00 2001 From: ndycode Date: Thu, 5 Mar 2026 21:09:21 +0800 Subject: [PATCH 5/8] test(retry): add concurrent request isolation regression Add a deterministic overlapping-request retry test to verify request-local retry budgets remain isolated and both requests can progress independently under the same plugin instance. Co-authored-by: Codex --- test/index-retry.test.ts | 40 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/test/index-retry.test.ts b/test/index-retry.test.ts index feaea05d4..6ecfd9eb4 100644 --- a/test/index-retry.test.ts +++ b/test/index-retry.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { stripVTControlCharacters } from "node:util"; process.env.CODEX_MULTI_AUTH_EXPOSE_ADMIN_TOOLS = "1"; +let mockInitialNullCalls = 1; vi.mock("@codex-ai/plugin/tool", () => { const makeSchema = () => ({ @@ -27,6 +28,7 @@ vi.mock("../lib/request/fetch-helpers.js", () => ({ refreshAndUpdateToken: async (auth: any) => auth, createCodexHeaders: () => new Headers(), handleErrorResponse: async (response: Response) => ({ response }), + getUnsupportedCodexModelInfo: () => ({ isUnsupported: false }), resolveUnsupportedCodexFallbackModel: () => undefined, shouldFallbackToGpt52OnUnsupportedGpt53: () => false, handleSuccessResponse: async (response: Response) => response, @@ -50,7 +52,7 @@ vi.mock("../lib/accounts.js", () => { getCurrentOrNextForFamily() { this.calls += 1; - if (this.calls === 1) return null; + if (this.calls <= mockInitialNullCalls) return null; return { index: 0, accountId: "account-1", email: "user@example.com" }; } @@ -166,6 +168,7 @@ describe("OpenAIAuthPlugin rate-limit retry", () => { process.env.CODEX_AUTH_TOKEN_REFRESH_SKEW_MS = "0"; process.env.CODEX_AUTH_RATE_LIMIT_TOAST_DEBOUNCE_MS = "0"; process.env.CODEX_AUTH_PREWARM = "0"; + mockInitialNullCalls = 1; vi.useFakeTimers(); originalFetch = globalThis.fetch; @@ -303,6 +306,41 @@ describe("OpenAIAuthPlugin rate-limit retry", () => { expect(response.status).toBe(200); }); + it("keeps retry budgets isolated across overlapping requests", async () => { + process.env.CODEX_AUTH_RETRY_ALL_MAX_RETRIES = "1"; + mockInitialNullCalls = 2; + vi.spyOn(Math, "random").mockReturnValue(0.5); + const { OpenAIAuthPlugin } = await import("../index.js"); + const client = { + tui: { showToast: vi.fn() }, + auth: { set: vi.fn() }, + } as any; + + const plugin = await OpenAIAuthPlugin({ client }); + const getAuth = async () => ({ + type: "oauth" as const, + access: "a", + refresh: "r", + expires: Date.now() + 60_000, + multiAccount: true, + }); + + const sdk = (await plugin.auth.loader(getAuth, { options: {}, models: {} })) as any; + const requestA = sdk.fetch("https://example.com/a", {}); + const requestB = sdk.fetch("https://example.com/b", {}); + + await vi.advanceTimersByTimeAsync(1000); + const [responseA, responseB] = await Promise.all([requestA, requestB]); + + expect(responseA.status).toBe(200); + expect(responseB.status).toBe(200); + expect(globalThis.fetch).toHaveBeenCalledTimes(2); + + const metrics = await plugin.tool["codex-metrics"].execute(); + const plainMetrics = stripVTControlCharacters(String(metrics)); + expect(plainMetrics).toContain("Retry governor stops (retry limit): 0"); + }); + it("keeps max wait checks deterministic at the raw wait threshold", async () => { process.env.CODEX_AUTH_RETRY_ALL_MAX_WAIT_MS = "1000"; process.env.CODEX_AUTH_RETRY_ALL_MAX_RETRIES = "2"; From 052f9356107c953c4dae983595128e98e7953393 Mon Sep 17 00:00:00 2001 From: ndycode Date: Thu, 5 Mar 2026 21:11:07 +0800 Subject: [PATCH 6/8] test(retry): cover retry-limit and overlapping request paths Extend retry integration tests to verify retry-limit metrics when retries are disabled and overlapping fetch requests keep retry budgets isolated. Co-authored-by: Codex --- test/index-retry.test.ts | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/test/index-retry.test.ts b/test/index-retry.test.ts index 6ecfd9eb4..d6cbab35a 100644 --- a/test/index-retry.test.ts +++ b/test/index-retry.test.ts @@ -247,6 +247,33 @@ describe("OpenAIAuthPlugin rate-limit retry", () => { expect(plainMetrics).toContain("Retry governor stops (absolute ceiling): 1"); }); + it("increments retry-limit stop metric when retries are disabled", async () => { + process.env.CODEX_AUTH_RETRY_ALL_MAX_RETRIES = "0"; + const { OpenAIAuthPlugin } = await import("../index.js"); + const client = { + tui: { showToast: vi.fn() }, + auth: { set: vi.fn() }, + } as any; + + const plugin = await OpenAIAuthPlugin({ client }); + const getAuth = async () => ({ + type: "oauth" as const, + access: "a", + refresh: "r", + expires: Date.now() + 60_000, + multiAccount: true, + }); + + const sdk = (await plugin.auth.loader(getAuth, { options: {}, models: {} })) as any; + const response = await sdk.fetch("https://example.com", {}); + expect(response.status).toBe(429); + expect(globalThis.fetch).not.toHaveBeenCalled(); + + const metrics = await plugin.tool["codex-metrics"].execute(); + const plainMetrics = stripVTControlCharacters(String(metrics)); + expect(plainMetrics).toContain("Retry governor stops (retry limit): 1"); + }); + it("caps jittered retry waits at the configured absolute ceiling", async () => { process.env.CODEX_AUTH_RETRY_ALL_ABSOLUTE_CEILING_MS = "1100"; vi.spyOn(Math, "random").mockReturnValue(1); From 11300d9c8a2c2e8e0f4ed7458fdc57a9888f964e Mon Sep 17 00:00:00 2001 From: ndycode Date: Thu, 5 Mar 2026 22:00:18 +0800 Subject: [PATCH 7/8] fix(pr40): harden retry ceiling coverage and logic Exercise second bounded wait in -20% jitter path and align retry governor absolute-ceiling checks with effective planned waits after the first retry. Co-authored-by: Codex --- index.ts | 1 + lib/request/retry-governor.ts | 5 ++++- test/index-retry.test.ts | 9 ++++++++- test/retry-governor.test.ts | 16 ++++++++++++++++ 4 files changed, 29 insertions(+), 2 deletions(-) diff --git a/index.ts b/index.ts index ef0fcd3ad..f457df466 100644 --- a/index.ts +++ b/index.ts @@ -2413,6 +2413,7 @@ while (attempted.size < Math.max(1, accountCount)) { enabled: retryAllAccountsRateLimited, accountCount: count, waitMs, + plannedWaitMs, maxWaitMs: retryAllAccountsMaxWaitMs, currentRetryCount: allRateLimitedRetries, maxRetries: retryAllAccountsMaxRetries, diff --git a/lib/request/retry-governor.ts b/lib/request/retry-governor.ts index 890f421c2..8968803e9 100644 --- a/lib/request/retry-governor.ts +++ b/lib/request/retry-governor.ts @@ -2,6 +2,7 @@ export interface RetryAllAccountsRateLimitDecisionInput { enabled: boolean; accountCount: number; waitMs: number; + plannedWaitMs?: number; maxWaitMs: number; currentRetryCount: number; maxRetries: number; @@ -44,11 +45,13 @@ export function decideRetryAllAccountsRateLimited( ): RetryAllAccountsRateLimitDecision { const accountCount = clampNonNegative(input.accountCount); const waitMs = clampNonNegative(input.waitMs); + const plannedWaitMs = clampNonNegative(input.plannedWaitMs ?? input.waitMs); const maxWaitMs = clampNonNegative(input.maxWaitMs); const currentRetryCount = clampNonNegative(input.currentRetryCount); const maxRetries = normalizeRetryLimit(input.maxRetries); const accumulatedWaitMs = clampNonNegative(input.accumulatedWaitMs); const absoluteCeilingMs = clampNonNegative(input.absoluteCeilingMs); + const ceilingCheckWaitMs = accumulatedWaitMs === 0 ? waitMs : plannedWaitMs; if (!input.enabled) { return { shouldRetry: false, reason: "disabled" }; @@ -65,7 +68,7 @@ export function decideRetryAllAccountsRateLimited( if (currentRetryCount >= maxRetries) { return { shouldRetry: false, reason: "retry-limit-reached" }; } - if (absoluteCeilingMs > 0 && accumulatedWaitMs + waitMs > absoluteCeilingMs) { + if (absoluteCeilingMs > 0 && accumulatedWaitMs + ceilingCheckWaitMs > absoluteCeilingMs) { return { shouldRetry: false, reason: "absolute-ceiling-exceeded" }; } return { shouldRetry: true, reason: "allowed" }; diff --git a/test/index-retry.test.ts b/test/index-retry.test.ts index d6cbab35a..4d9a10c2a 100644 --- a/test/index-retry.test.ts +++ b/test/index-retry.test.ts @@ -306,6 +306,7 @@ describe("OpenAIAuthPlugin rate-limit retry", () => { it("consumes remaining ceiling budget under -20% jitter without premature stop", async () => { process.env.CODEX_AUTH_RETRY_ALL_ABSOLUTE_CEILING_MS = "1600"; process.env.CODEX_AUTH_RETRY_ALL_MAX_RETRIES = "2"; + mockInitialNullCalls = 2; vi.spyOn(Math, "random").mockReturnValue(0); const { OpenAIAuthPlugin } = await import("../index.js"); const client = { @@ -326,11 +327,17 @@ describe("OpenAIAuthPlugin rate-limit retry", () => { const fetchPromise = sdk.fetch("https://example.com", {}); expect(globalThis.fetch).not.toHaveBeenCalled(); - await vi.advanceTimersByTimeAsync(1600); + await vi.advanceTimersByTimeAsync(1599); + expect(globalThis.fetch).not.toHaveBeenCalled(); + await vi.advanceTimersByTimeAsync(1); expect(globalThis.fetch).toHaveBeenCalledTimes(1); const response = await fetchPromise; expect(response.status).toBe(200); + + const metrics = await plugin.tool["codex-metrics"].execute(); + const plainMetrics = stripVTControlCharacters(String(metrics)); + expect(plainMetrics).toContain("Retry governor stops (absolute ceiling): 0"); }); it("keeps retry budgets isolated across overlapping requests", async () => { diff --git a/test/retry-governor.test.ts b/test/retry-governor.test.ts index 83b31591f..0c5ba5f21 100644 --- a/test/retry-governor.test.ts +++ b/test/retry-governor.test.ts @@ -125,6 +125,22 @@ describe("decideRetryAllAccountsRateLimited", () => { expect(result).toEqual({ shouldRetry: true, reason: "allowed" }); }); + it("uses planned wait for absolute ceiling checks when provided", () => { + const result = decideRetryAllAccountsRateLimited({ + enabled: true, + accountCount: 2, + waitMs: 1_000, + plannedWaitMs: 800, + maxWaitMs: 0, + currentRetryCount: 0, + maxRetries: Infinity, + accumulatedWaitMs: 800, + absoluteCeilingMs: 1_600, + }); + + expect(result).toEqual({ shouldRetry: true, reason: "allowed" }); + }); + it("treats zero absolute ceiling as unlimited", () => { const result = decideRetryAllAccountsRateLimited({ enabled: true, From f86407757d8239a7ba74231c10acb0816234cd78 Mon Sep 17 00:00:00 2001 From: ndycode Date: Thu, 5 Mar 2026 23:52:26 +0800 Subject: [PATCH 8/8] fix(retry): stop exhausted-ceiling zero-wait retries Add an explicit ceiling-exhausted guard and integration coverage to ensure exhausted absolute ceiling stops immediately instead of spinning into retry-limit handling. Co-authored-by: Codex --- lib/request/retry-governor.ts | 3 +++ test/index-retry.test.ts | 37 +++++++++++++++++++++++++++++++++++ test/retry-governor.test.ts | 23 ++++++++++++++++++++-- 3 files changed, 61 insertions(+), 2 deletions(-) diff --git a/lib/request/retry-governor.ts b/lib/request/retry-governor.ts index 8968803e9..08e885699 100644 --- a/lib/request/retry-governor.ts +++ b/lib/request/retry-governor.ts @@ -68,6 +68,9 @@ export function decideRetryAllAccountsRateLimited( if (currentRetryCount >= maxRetries) { return { shouldRetry: false, reason: "retry-limit-reached" }; } + if (absoluteCeilingMs > 0 && accumulatedWaitMs >= absoluteCeilingMs) { + return { shouldRetry: false, reason: "absolute-ceiling-exceeded" }; + } if (absoluteCeilingMs > 0 && accumulatedWaitMs + ceilingCheckWaitMs > absoluteCeilingMs) { return { shouldRetry: false, reason: "absolute-ceiling-exceeded" }; } diff --git a/test/index-retry.test.ts b/test/index-retry.test.ts index 4d9a10c2a..7e050033d 100644 --- a/test/index-retry.test.ts +++ b/test/index-retry.test.ts @@ -340,6 +340,43 @@ describe("OpenAIAuthPlugin rate-limit retry", () => { expect(plainMetrics).toContain("Retry governor stops (absolute ceiling): 0"); }); + it("stops once the absolute ceiling budget is exhausted instead of spinning zero-delay retries", async () => { + process.env.CODEX_AUTH_RETRY_ALL_ABSOLUTE_CEILING_MS = "1600"; + process.env.CODEX_AUTH_RETRY_ALL_MAX_RETRIES = "10"; + mockInitialNullCalls = 999; + vi.spyOn(Math, "random").mockReturnValue(0); + const { OpenAIAuthPlugin } = await import("../index.js"); + const client = { + tui: { showToast: vi.fn() }, + auth: { set: vi.fn() }, + } as any; + + const plugin = await OpenAIAuthPlugin({ client }); + const getAuth = async () => ({ + type: "oauth" as const, + access: "a", + refresh: "r", + expires: Date.now() + 60_000, + multiAccount: true, + }); + + const sdk = (await plugin.auth.loader(getAuth, { options: {}, models: {} })) as any; + const fetchPromise = sdk.fetch("https://example.com", {}); + expect(globalThis.fetch).not.toHaveBeenCalled(); + + await vi.advanceTimersByTimeAsync(1600); + await vi.advanceTimersByTimeAsync(0); + + const response = await fetchPromise; + expect(response.status).toBe(429); + expect(globalThis.fetch).not.toHaveBeenCalled(); + + const metrics = await plugin.tool["codex-metrics"].execute(); + const plainMetrics = stripVTControlCharacters(String(metrics)); + expect(plainMetrics).toContain("Retry governor stops (absolute ceiling): 1"); + expect(plainMetrics).toContain("Retry governor stops (retry limit): 0"); + }); + it("keeps retry budgets isolated across overlapping requests", async () => { process.env.CODEX_AUTH_RETRY_ALL_MAX_RETRIES = "1"; mockInitialNullCalls = 2; diff --git a/test/retry-governor.test.ts b/test/retry-governor.test.ts index 0c5ba5f21..7ac6ac799 100644 --- a/test/retry-governor.test.ts +++ b/test/retry-governor.test.ts @@ -110,16 +110,35 @@ describe("decideRetryAllAccountsRateLimited", () => { }); }); - it("allows retry when accumulated wait exactly matches the absolute ceiling", () => { + it("stops retry when accumulated wait exactly matches the absolute ceiling", () => { const result = decideRetryAllAccountsRateLimited({ enabled: true, accountCount: 2, waitMs: 1_000, + plannedWaitMs: 0, maxWaitMs: 0, currentRetryCount: 0, maxRetries: Infinity, accumulatedWaitMs: 1_000, - absoluteCeilingMs: 2_000, + absoluteCeilingMs: 1_000, + }); + + expect(result).toEqual({ + shouldRetry: false, + reason: "absolute-ceiling-exceeded", + }); + }); + + it("allows first retry that exactly consumes the absolute ceiling budget", () => { + const result = decideRetryAllAccountsRateLimited({ + enabled: true, + accountCount: 2, + waitMs: 1_000, + maxWaitMs: 0, + currentRetryCount: 0, + maxRetries: Infinity, + accumulatedWaitMs: 0, + absoluteCeilingMs: 1_000, }); expect(result).toEqual({ shouldRetry: true, reason: "allowed" });