Skip to content

Commit 21dfa40

Browse files
committed
fix: fold review follow-ups into rebuilt main wave
1 parent c2dca9e commit 21dfa40

8 files changed

Lines changed: 117 additions & 17 deletions

File tree

lib/codex-manager/commands/report.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,10 @@ import {
1212
summarizeForecast,
1313
} from "../../forecast.js";
1414
import {
15+
type AccountIdentityMatch,
1516
applyRefreshedAccountPatch,
1617
persistRefreshedAccountPatch,
18+
type RefreshedAccountPatch,
1719
serializeForecastResults,
1820
} from "../forecast-report-shared.js";
1921
import {

lib/codex-manager/forecast-report-shared.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,8 @@ export function serializeForecastResults(
9696
index: number;
9797
label: string;
9898
isCurrent: boolean;
99+
selected: boolean;
100+
primaryReason?: string;
99101
availability: ForecastAccountResult["availability"];
100102
riskScore: number;
101103
riskLevel: ForecastAccountResult["riskLevel"];
@@ -116,6 +118,8 @@ export function serializeForecastResults(
116118
index: result.index,
117119
label: result.label,
118120
isCurrent: result.isCurrent,
121+
selected: false,
122+
primaryReason: result.reasons[0],
119123
availability: result.availability,
120124
riskScore: result.riskScore,
121125
riskLevel: result.riskLevel,

lib/config.ts

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1084,7 +1084,7 @@ export function getRateLimitDedupWindowMs(pluginConfig: PluginConfig): number {
10841084
return resolveNumberSetting(
10851085
"CODEX_AUTH_RATE_LIMIT_DEDUP_WINDOW_MS",
10861086
pluginConfig.rateLimitDedupWindowMs,
1087-
DEFAULT_PLUGIN_CONFIG.rateLimitDedupWindowMs,
1087+
DEFAULT_PLUGIN_CONFIG.rateLimitDedupWindowMs ?? 2_000,
10881088
{ min: 0 },
10891089
);
10901090
}
@@ -1093,7 +1093,7 @@ export function getRateLimitStateResetMs(pluginConfig: PluginConfig): number {
10931093
return resolveNumberSetting(
10941094
"CODEX_AUTH_RATE_LIMIT_STATE_RESET_MS",
10951095
pluginConfig.rateLimitStateResetMs,
1096-
DEFAULT_PLUGIN_CONFIG.rateLimitStateResetMs,
1096+
DEFAULT_PLUGIN_CONFIG.rateLimitStateResetMs ?? 120_000,
10971097
{ min: 1_000 },
10981098
);
10991099
}
@@ -1102,7 +1102,7 @@ export function getRateLimitMaxBackoffMs(pluginConfig: PluginConfig): number {
11021102
return resolveNumberSetting(
11031103
"CODEX_AUTH_RATE_LIMIT_MAX_BACKOFF_MS",
11041104
pluginConfig.rateLimitMaxBackoffMs,
1105-
DEFAULT_PLUGIN_CONFIG.rateLimitMaxBackoffMs,
1105+
DEFAULT_PLUGIN_CONFIG.rateLimitMaxBackoffMs ?? 60_000,
11061106
{ min: 1_000 },
11071107
);
11081108
}
@@ -1113,7 +1113,7 @@ export function getRateLimitShortRetryThresholdMs(
11131113
return resolveNumberSetting(
11141114
"CODEX_AUTH_RATE_LIMIT_SHORT_RETRY_THRESHOLD_MS",
11151115
pluginConfig.rateLimitShortRetryThresholdMs,
1116-
DEFAULT_PLUGIN_CONFIG.rateLimitShortRetryThresholdMs,
1116+
DEFAULT_PLUGIN_CONFIG.rateLimitShortRetryThresholdMs ?? 5_000,
11171117
{ min: 0 },
11181118
);
11191119
}
@@ -1842,11 +1842,20 @@ export function getPluginConfigExplainReport(): ConfigExplainReport {
18421842
const stored = resolveStoredPluginConfigRecord();
18431843
const storedRecord = stored.record ?? null;
18441844
const entries = CONFIG_EXPLAIN_ENTRIES.map((entry) => {
1845-
const value = entry.getValue(pluginConfig);
1845+
const rawValue = entry.getValue(pluginConfig);
1846+
const rawDefaultValue = DEFAULT_PLUGIN_CONFIG[entry.key];
1847+
const value =
1848+
entry.key === "retryAllAccountsMaxRetries" && rawValue === 0
1849+
? Number.POSITIVE_INFINITY
1850+
: rawValue;
1851+
const defaultValue =
1852+
entry.key === "retryAllAccountsMaxRetries" && rawDefaultValue === 0
1853+
? Number.POSITIVE_INFINITY
1854+
: rawDefaultValue;
18461855
return {
18471856
key: entry.key,
18481857
value: normalizeConfigExplainValue(value),
1849-
defaultValue: normalizeConfigExplainValue(DEFAULT_PLUGIN_CONFIG[entry.key]),
1858+
defaultValue: normalizeConfigExplainValue(defaultValue),
18501859
source: resolveConfigExplainSource(
18511860
entry,
18521861
pluginConfig,

lib/index.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,17 @@ export * from "./auth/auth.js";
88
export * from "./request/fetch-helpers.js";
99
export * from "./request/request-transformer.js";
1010
export * from "./request/response-handler.js";
11-
export * from "./request/rate-limit-backoff.js";
11+
export {
12+
MAX_SHORT_RETRY_ATTEMPTS,
13+
calculateBackoffMs,
14+
clearRateLimitBackoffState,
15+
configureRateLimitBackoff,
16+
getRateLimitBackoff,
17+
getRateLimitBackoffWithReason,
18+
getRateLimitShortRetryThresholdMs as getConfiguredRateLimitShortRetryThresholdMs,
19+
resetRateLimitBackoff,
20+
resetRateLimitBackoffConfig,
21+
} from "./request/rate-limit-backoff.js";
1222
export * from "./prompts/codex.js";
1323
export * from "./shutdown.js";
1424
export * from "./circuit-breaker.js";

lib/request/fetch-helpers.ts

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -982,23 +982,31 @@ function mapUsageLimit404WithBody(response: Response, bodyText: string): Respons
982982
if (!bodyText) return null;
983983

984984
let code = "";
985+
let type = "";
985986
try {
986-
const parsed = JSON.parse(bodyText) as { error?: { code?: string | number; type?: string } };
987-
code = (parsed?.error?.code ?? parsed?.error?.type ?? "").toString();
987+
const parsed = JSON.parse(bodyText) as {
988+
error?: { code?: string | number; type?: string | number };
989+
};
990+
code = (parsed?.error?.code ?? "").toString();
991+
type = (parsed?.error?.type ?? "").toString();
988992
} catch {
989993
code = "";
994+
type = "";
990995
}
991996

997+
const normalizedSignals = [code, type]
998+
.map((value) => value.toLowerCase())
999+
.filter((value) => value.length > 0);
1000+
9921001
// Check for entitlement errors first - these should NOT be treated as rate limits
993-
if (isEntitlementError(code, bodyText)) {
1002+
if (isEntitlementError(normalizedSignals.join(" "), bodyText)) {
9941003
return createEntitlementErrorResponse(bodyText);
9951004
}
9961005

9971006
// Only structured quota-limit codes should be remapped from 404 to 429.
9981007
// Free-text 404 bodies and generic rate_limit_* codes are too ambiguous and
9991008
// degrade downstream rate-limit reason classification to "unknown".
1000-
const normalizedCode = code.toLowerCase();
1001-
if (!normalizedCode.includes("usage_limit")) {
1009+
if (!normalizedSignals.some((value) => value.includes("usage_limit"))) {
10021010
return null;
10031011
}
10041012

lib/request/rate-limit-backoff.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,13 @@ export interface RateLimitBackoffResult {
1616
* - Reset backoff after a quiet period.
1717
*/
1818
const DEFAULT_RATE_LIMIT_DEDUP_WINDOW_MS =
19-
DEFAULT_PLUGIN_CONFIG.rateLimitDedupWindowMs;
19+
DEFAULT_PLUGIN_CONFIG.rateLimitDedupWindowMs ?? 2_000;
2020
const DEFAULT_RATE_LIMIT_STATE_RESET_MS =
21-
DEFAULT_PLUGIN_CONFIG.rateLimitStateResetMs;
22-
const DEFAULT_MAX_BACKOFF_MS = DEFAULT_PLUGIN_CONFIG.rateLimitMaxBackoffMs;
21+
DEFAULT_PLUGIN_CONFIG.rateLimitStateResetMs ?? 120_000;
22+
const DEFAULT_MAX_BACKOFF_MS =
23+
DEFAULT_PLUGIN_CONFIG.rateLimitMaxBackoffMs ?? 60_000;
2324
const DEFAULT_RATE_LIMIT_SHORT_RETRY_THRESHOLD_MS =
24-
DEFAULT_PLUGIN_CONFIG.rateLimitShortRetryThresholdMs;
25+
DEFAULT_PLUGIN_CONFIG.rateLimitShortRetryThresholdMs ?? 5_000;
2526
const RATE_LIMIT_BACKOFF_JITTER_FACTOR = 0.2;
2627

2728
interface RateLimitBackoffConfig {
@@ -281,8 +282,11 @@ export function getRateLimitBackoffWithReason(
281282
resolvedServerRetryAfterMs,
282283
resolvedStableAccountKey,
283284
);
285+
const normalizedBaseDelay = normalizeDelayMs(resolvedServerRetryAfterMs, 1000);
284286
const adjustedDelay = calculateBackoffMs(
285-
result.delayMs,
287+
result.attempt === 1 && !result.isDuplicate
288+
? normalizedBaseDelay
289+
: result.delayMs,
286290
result.attempt,
287291
resolvedReason,
288292
);

test/auto-update-checker.test.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,7 @@ describe("auto-update-checker", () => {
256256
const result = await checkForUpdates(true);
257257

258258
expect(result.currentVersion).toBe("0.0.0");
259+
expect(globalThis.fetch).toHaveBeenCalled();
259260
expect(logger.debug).toHaveBeenCalledWith(
260261
"Failed to read current package version",
261262
expect.objectContaining({ error: expect.any(String) }),
@@ -311,6 +312,37 @@ describe("auto-update-checker", () => {
311312
expect(globalThis.fetch).toHaveBeenCalled();
312313
});
313314

315+
it.each(["EBUSY", "EPERM"] as const)(
316+
"logs debug details when cache read fails on windows-style lock (%s)",
317+
async (code) => {
318+
vi.mocked(fs.existsSync).mockReturnValue(true);
319+
vi.mocked(fs.readFileSync).mockImplementation((path: unknown) => {
320+
if (String(path).includes("package.json")) {
321+
return JSON.stringify(mockPackageJson);
322+
}
323+
if (String(path).includes("update-check-cache.json")) {
324+
const error = new Error(`${code}: locked`) as NodeJS.ErrnoException;
325+
error.code = code;
326+
error.name = code;
327+
throw error;
328+
}
329+
throw new Error("File not found");
330+
});
331+
vi.mocked(globalThis.fetch).mockResolvedValue({
332+
ok: true,
333+
json: async () => ({ version: "5.0.0" }),
334+
} as Response);
335+
336+
await checkForUpdates();
337+
338+
expect(logger.debug).toHaveBeenCalledWith(
339+
"Failed to load update cache",
340+
expect.objectContaining({ error: expect.stringContaining(code) }),
341+
);
342+
expect(globalThis.fetch).toHaveBeenCalled();
343+
},
344+
);
345+
314346
it("handles cached null latestVersion without update", async () => {
315347
const cacheData = {
316348
lastCheck: Date.now() - 1000 * 60 * 60,

test/fetch-helpers.test.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -494,6 +494,37 @@ describe('Fetch Helpers Module', () => {
494494
expect(rateLimit?.retryAfterMs).toBeGreaterThan(0);
495495
});
496496

497+
it('maps usage-limit 404 errors to 429 when code and type disagree', async () => {
498+
const body = {
499+
error: {
500+
code: 'rate_limit_exceeded',
501+
type: 'usage_limit_reached',
502+
message: 'limit reached',
503+
},
504+
};
505+
const resp = new Response(JSON.stringify(body), { status: 404 });
506+
const { response: mapped, rateLimit } = await handleErrorResponse(resp);
507+
expect(mapped.status).toBe(429);
508+
expect(rateLimit?.retryAfterMs).toBeGreaterThan(0);
509+
});
510+
511+
it('maps entitlement-style 404 errors to 403 when only error.type carries the signal', async () => {
512+
const body = {
513+
error: {
514+
code: 'not_found',
515+
type: 'usage_not_included',
516+
message: 'Not included in your plan',
517+
},
518+
};
519+
const resp = new Response(JSON.stringify(body), { status: 404 });
520+
const { response: mapped, rateLimit } = await handleErrorResponse(resp);
521+
expect(mapped.status).toBe(403);
522+
const json = await mapped.json() as { error: { code?: string; message?: string } };
523+
expect(json.error.code).toBe('usage_not_included');
524+
expect(json.error.message).toContain('not included');
525+
expect(rateLimit).toBeUndefined();
526+
});
527+
497528
it('leaves non-usage 404 errors unchanged', async () => {
498529
const body = { error: { code: 'not_found', message: 'nope' } };
499530
const resp = new Response(JSON.stringify(body), { status: 404 });

0 commit comments

Comments
 (0)