From 1a792b343dde30e67c4c5b77598ae579354b9925 Mon Sep 17 00:00:00 2001 From: ndycode Date: Mon, 8 Jun 2026 04:10:06 +0800 Subject: [PATCH] fix: correctness bugs in flagged-storage, recovery, and docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found via a parallel multi-agent review sweep of the lib/ tree. Only high-confidence defects with observable wrong behavior were fixed; several reviewer findings were verified as false positives or by-design and deliberately left alone (see PR notes). - flagged-storage: the lastSwitchReason/cooldownReason type guards in normalizeFlaggedStorage dropped two valid enum members. "manual" (lastSwitchReason) and "server-error" (cooldownReason) are part of the type, the Zod schema, and are produced at runtime, but the guards omitted them — so on any load/normalize/save round-trip those values were silently coerced to undefined, erasing switch/cooldown provenance from flagged records. Added both to the guards. (+2 regression tests) - recovery/findEmptyMessageByIndex: add the `role === "assistant"` guard that its sibling findMessageByIndexNeedingThinking already has, so the index-offset fallback can't select an unrelated user message to repair. (Currently export-only with no live caller; hardening to prevent misuse.) - rotation: HybridSelectionConfig.freshnessWeight JSDoc said "default: 0.1" while DEFAULT_HYBRID_SELECTION_CONFIG uses 2.0; corrected the doc. - README: fix the "Current prerelease" link, which still pointed at v2.3.0-beta.0 after the v2.3.0-beta.1 release (the doc exists). This is what test/documentation.test.ts (release-history) asserts. - Added test/fs-retry.test.ts covering shouldRetryFileOperation across the full FILE_RETRY_CODES set and rejection of non-transient / code-less errors. Verified: build, typecheck, lint, full vitest suite (4433 passed, 0 failed, 3 skipped). npm audit unaffected. --- README.md | 2 +- lib/recovery/storage.ts | 2 +- lib/rotation.ts | 2 +- lib/storage/flagged-storage.ts | 4 +++- test/flagged-storage.test.ts | 36 ++++++++++++++++++++++++++++++++++ test/fs-retry.test.ts | 33 +++++++++++++++++++++++++++++++ 6 files changed, 75 insertions(+), 4 deletions(-) create mode 100644 test/fs-retry.test.ts diff --git a/README.md b/README.md index c602e199..78abe177 100644 --- a/README.md +++ b/README.md @@ -383,7 +383,7 @@ codex-multi-auth doctor --json ## Release Notes -- Current prerelease: [docs/releases/v2.3.0-beta.0.md](docs/releases/v2.3.0-beta.0.md) — install via `npm i -g codex-multi-auth@beta` +- Current prerelease: [docs/releases/v2.3.0-beta.1.md](docs/releases/v2.3.0-beta.1.md) — install via `npm i -g codex-multi-auth@beta` - Current stable: [docs/releases/v2.2.2.md](docs/releases/v2.2.2.md) — install via `npm i -g codex-multi-auth` - Previous stable: [docs/releases/v2.2.1.md](docs/releases/v2.2.1.md) - Previous stable: [docs/releases/v2.2.0.md](docs/releases/v2.2.0.md) diff --git a/lib/recovery/storage.ts b/lib/recovery/storage.ts index 4e04a3f4..68ff20f7 100644 --- a/lib/recovery/storage.ts +++ b/lib/recovery/storage.ts @@ -672,7 +672,7 @@ export function findEmptyMessageByIndex( if (idx < 0 || idx >= messages.length) continue; const targetMsg = messages[idx]; - if (!targetMsg) continue; + if (!targetMsg || targetMsg.role !== "assistant") continue; if (!messageHasContent(targetMsg.id)) { return targetMsg.id; diff --git a/lib/rotation.ts b/lib/rotation.ts index ca41b075..50aae032 100644 --- a/lib/rotation.ts +++ b/lib/rotation.ts @@ -385,7 +385,7 @@ export interface HybridSelectionConfig { healthWeight: number; /** Weight for token count (default: 5) */ tokenWeight: number; - /** Weight for freshness/last used (default: 0.1) */ + /** Weight for freshness/last used (default: 2) */ freshnessWeight: number; } diff --git a/lib/storage/flagged-storage.ts b/lib/storage/flagged-storage.ts index f5e55c26..ccdb128f 100644 --- a/lib/storage/flagged-storage.ts +++ b/lib/storage/flagged-storage.ts @@ -46,12 +46,14 @@ export function normalizeFlaggedStorage( value === "initial" || value === "rotation" || value === "best" || - value === "restore"; + value === "restore" || + value === "manual"; const isCooldownReason = ( value: unknown, ): value is AccountMetadataV3["cooldownReason"] => value === "auth-failure" || value === "network-error" || + value === "server-error" || value === "rate-limit"; let rateLimitResetTimes: diff --git a/test/flagged-storage.test.ts b/test/flagged-storage.test.ts index 51fa4262..73458cf0 100644 --- a/test/flagged-storage.test.ts +++ b/test/flagged-storage.test.ts @@ -28,4 +28,40 @@ describe("flagged storage helper", () => { expect(result.accounts[0]?.refreshToken).toBe("token-1"); expect(result.accounts[0]?.lastError).toBe("oops"); }); + + it("preserves the 'manual' last switch reason on round-trip", () => { + const result = normalizeFlaggedStorage( + { + version: 1, + accounts: [ + { + refreshToken: "token-1", + flaggedAt: 10, + lastSwitchReason: "manual", + }, + ], + }, + { isRecord, now: () => 99 }, + ); + + expect(result.accounts[0]?.lastSwitchReason).toBe("manual"); + }); + + it("preserves the 'server-error' cooldown reason on round-trip", () => { + const result = normalizeFlaggedStorage( + { + version: 1, + accounts: [ + { + refreshToken: "token-1", + flaggedAt: 10, + cooldownReason: "server-error", + }, + ], + }, + { isRecord, now: () => 99 }, + ); + + expect(result.accounts[0]?.cooldownReason).toBe("server-error"); + }); }); diff --git a/test/fs-retry.test.ts b/test/fs-retry.test.ts new file mode 100644 index 00000000..7c089378 --- /dev/null +++ b/test/fs-retry.test.ts @@ -0,0 +1,33 @@ +import { describe, expect, it } from "vitest"; +import { + FILE_RETRY_CODES, + shouldRetryFileOperation, +} from "../lib/fs-retry.js"; + +function errnoError(code: string): NodeJS.ErrnoException { + const error = new Error(code) as NodeJS.ErrnoException; + error.code = code; + return error; +} + +describe("shouldRetryFileOperation", () => { + it("treats every transient lock code as retryable", () => { + for (const code of ["EBUSY", "EPERM", "EAGAIN", "ENOTEMPTY", "EACCES"]) { + expect(shouldRetryFileOperation(errnoError(code)), code).toBe(true); + expect(FILE_RETRY_CODES.has(code), code).toBe(true); + } + }); + + it("does not retry non-transient errors", () => { + for (const code of ["ENOENT", "EISDIR", "EINVAL", "EROFS"]) { + expect(shouldRetryFileOperation(errnoError(code)), code).toBe(false); + } + }); + + it("returns false for non-errors and errors without a code", () => { + expect(shouldRetryFileOperation(undefined)).toBe(false); + expect(shouldRetryFileOperation(null)).toBe(false); + expect(shouldRetryFileOperation("EACCES")).toBe(false); + expect(shouldRetryFileOperation(new Error("no code"))).toBe(false); + }); +});