Skip to content

Commit 971bcea

Browse files
ndycodendycode
committed
fix: close remaining PR 387 follow-ups
1 parent 4f27fe5 commit 971bcea

6 files changed

Lines changed: 147 additions & 12 deletions

File tree

‎index.ts‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2723,22 +2723,30 @@ let sessionAffinityWriteVersion = 0;
27232723
continue;
27242724
}
27252725

2726+
const now = Date.now();
2727+
const poolCooldownUntil =
2728+
count > 0 && waitMs > 0
2729+
? armPoolExhaustionCooldown(waitMs, now)
2730+
: 0;
2731+
const effectiveWaitMs =
2732+
poolCooldownUntil > 0
2733+
? Math.max(0, poolCooldownUntil - now)
2734+
: waitMs;
27262735
const waitLabel =
2727-
waitMs > 0 ? formatWaitTime(waitMs) : "a bit";
2728-
if (count > 0 && waitMs > 0) {
2729-
armPoolExhaustionCooldown(waitMs);
2730-
}
2736+
effectiveWaitMs > 0
2737+
? formatWaitTime(effectiveWaitMs)
2738+
: "a bit";
27312739
const message =
27322740
count === 0
27332741
? "No Codex accounts configured. Run `codex login`."
2734-
: waitMs > 0
2742+
: effectiveWaitMs > 0
27352743
? `All ${count} account(s) are rate-limited. A short pool cooldown is now active for ${waitLabel}. Try again later or inspect \`codex auth status\`.`
27362744
: `All ${count} account(s) failed (server errors or auth issues). Check account health with \`codex auth report --json\`.`;
27372745
runtimeMetrics.failedRequests++;
27382746
runtimeMetrics.lastError = message;
27392747
syncRuntimeObservability(requestTraceId);
27402748
return new Response(JSON.stringify({ error: { message } }), {
2741-
status: waitMs > 0 ? 429 : 503,
2749+
status: effectiveWaitMs > 0 ? 429 : 503,
27422750
headers: {
27432751
"content-type": "application/json; charset=utf-8",
27442752
},

‎lib/runtime/runtime-observability.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,8 @@ export function mutateRuntimeObservabilitySnapshot(
198198
const nextSnapshot = structuredClone(snapshot);
199199
pendingWrite = (pendingWrite ?? Promise.resolve())
200200
.catch(() => undefined)
201-
.then(() => writeSnapshot(nextSnapshot));
201+
.then(() => writeSnapshot(nextSnapshot))
202+
.catch(() => undefined);
202203
}
203204

204205
export async function loadPersistedRuntimeObservabilitySnapshot(): Promise<RuntimeObservabilitySnapshot | null> {

‎test/index-retry.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -478,7 +478,7 @@ describe("OpenAIAuthPlugin rate-limit retry", () => {
478478
expect(globalThis.fetch).not.toHaveBeenCalled();
479479
expect(response.status).toBe(429);
480480
expect(payload.error.message).toContain("All 2 account(s) are rate-limited.");
481-
expect(payload.error.message).toContain("1000ms");
481+
expect(payload.error.message).toContain("15000ms");
482482
});
483483

484484
it("fast-fails after repeated cross-account 5xx errors arm the server-burst cooldown", async () => {

‎test/runtime-observability.test.ts‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,32 @@ describe("runtime observability snapshot versioning", () => {
9696
expect(unlinkMock).toHaveBeenCalled();
9797
});
9898

99+
it("contains permanent snapshot write failures without leaving pending writes rejected", async () => {
100+
process.env.VITEST = "";
101+
renameMock.mockImplementation(async () => {
102+
throw Object.assign(new Error("disk full"), { code: "EIO" });
103+
});
104+
105+
const mod = await import("../lib/runtime/runtime-observability.js");
106+
mod.mutateRuntimeObservabilitySnapshot((snapshot) => {
107+
snapshot.responsesRequests = 1;
108+
});
109+
110+
await vi.waitFor(() => {
111+
expect(renameMock).toHaveBeenCalled();
112+
});
113+
114+
renameMock.mockReset();
115+
renameMock.mockResolvedValue(undefined);
116+
mod.mutateRuntimeObservabilitySnapshot((snapshot) => {
117+
snapshot.responsesRequests = 2;
118+
});
119+
120+
await vi.waitFor(() => {
121+
expect(renameMock).toHaveBeenCalled();
122+
});
123+
});
124+
99125
it("seeds the first in-memory snapshot from disk before mutating", async () => {
100126
process.env.VITEST = "";
101127
readFileSyncMock.mockReturnValue(

‎test/runtime-services.test.ts‎

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -83,10 +83,10 @@ describe("runtime services helpers", () => {
8383
const resultPromise = ensureLiveAccountSyncState({
8484
enabled: true,
8585
targetPath: "/tmp/new",
86-
currentSync,
87-
currentPath: "/tmp/old",
88-
currentConfigKey: "old",
89-
createSync: vi.fn(),
86+
currentSync,
87+
currentPath: "/tmp/old",
88+
currentConfigKey: "old",
89+
createSync: vi.fn(),
9090
registerCleanup: vi.fn(),
9191
logWarn,
9292
pluginName: "plugin",
@@ -158,6 +158,26 @@ describe("runtime services helpers", () => {
158158
expect(result.liveAccountSyncConfigKey).toBe("50:500");
159159
});
160160

161+
it("keeps the existing watcher when configKey is undefined", async () => {
162+
const currentSync = { stop: vi.fn(), syncToPath: vi.fn().mockResolvedValue(undefined) };
163+
const result = await ensureLiveAccountSyncState({
164+
enabled: true,
165+
targetPath: "/tmp/a",
166+
currentSync,
167+
currentPath: "/tmp/a",
168+
currentConfigKey: "25:250",
169+
configKey: undefined,
170+
createSync: vi.fn(),
171+
registerCleanup: vi.fn(),
172+
logWarn: vi.fn(),
173+
pluginName: "plugin",
174+
});
175+
176+
expect(currentSync.stop).not.toHaveBeenCalled();
177+
expect(result.liveAccountSync).toBe(currentSync);
178+
expect(result.liveAccountSyncConfigKey).toBe("25:250");
179+
});
180+
161181
it("recreates refresh guardian when config changes and clears when disabled", () => {
162182
const oldGuardian = { stop: vi.fn(), start: vi.fn() };
163183
const createGuardian = vi.fn(() => ({ stop: vi.fn(), start: vi.fn() }));

‎test/storage-health-inspection.test.ts‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,25 @@ function sha256(value: string): string {
99
}
1010

1111
describe("inspectStorageHealth", () => {
12+
async function withStorageModule<T>(
13+
testName: string,
14+
runner: (args: { workDir: string; storagePath: string; resetMarkerPath: string; walPath: string; storageModule: typeof import("../lib/storage.js") }) => Promise<T>,
15+
): Promise<T> {
16+
const workDir = join(tmpdir(), `${testName}-${Date.now()}`);
17+
await fs.mkdir(workDir, { recursive: true });
18+
const storagePath = join(workDir, "accounts.json");
19+
const resetMarkerPath = `${storagePath}.reset-intent`;
20+
const walPath = `${storagePath}.wal`;
21+
const storageModule = await import("../lib/storage.js");
22+
storageModule.setStoragePathDirect(storagePath);
23+
try {
24+
return await runner({ workDir, storagePath, resetMarkerPath, walPath, storageModule });
25+
} finally {
26+
storageModule.setStoragePathDirect(null);
27+
await fs.rm(workDir, { recursive: true, force: true });
28+
}
29+
}
30+
1231
afterEach(() => {
1332
vi.resetModules();
1433
});
@@ -67,4 +86,65 @@ describe("inspectStorageHealth", () => {
6786
await fs.rm(workDir, { recursive: true, force: true });
6887
}
6988
});
89+
90+
it("reports healthy storage when primary storage is valid", async () => {
91+
await withStorageModule("storage-health-healthy", async ({ storagePath, storageModule }) => {
92+
await fs.writeFile(
93+
storagePath,
94+
JSON.stringify({
95+
version: 3,
96+
activeIndex: 0,
97+
activeIndexByFamily: { codex: 0 },
98+
accounts: [{ refreshToken: "r", addedAt: 1, lastUsed: 1 }],
99+
}),
100+
"utf-8",
101+
);
102+
const summary = await storageModule.inspectStorageHealth();
103+
expect(summary.state).toBe("healthy");
104+
});
105+
});
106+
107+
it("reports empty storage when primary storage is valid but has no accounts", async () => {
108+
await withStorageModule("storage-health-empty", async ({ storagePath, storageModule }) => {
109+
await fs.writeFile(
110+
storagePath,
111+
JSON.stringify({ version: 3, activeIndex: 0, activeIndexByFamily: { codex: 0 }, accounts: [] }),
112+
"utf-8",
113+
);
114+
const summary = await storageModule.inspectStorageHealth();
115+
expect(summary.state).toBe("empty");
116+
});
117+
});
118+
119+
it("reports intentional-reset when the reset marker exists", async () => {
120+
await withStorageModule("storage-health-reset", async ({ resetMarkerPath, storageModule }) => {
121+
await fs.writeFile(resetMarkerPath, "", "utf-8");
122+
const summary = await storageModule.inspectStorageHealth();
123+
expect(summary.state).toBe("intentional-reset");
124+
});
125+
});
126+
127+
it("reports corrupt storage when primary storage is malformed and WAL is unavailable", async () => {
128+
await withStorageModule("storage-health-corrupt-json", async ({ storagePath, storageModule }) => {
129+
await fs.writeFile(storagePath, "{ malformed-json", "utf-8");
130+
const summary = await storageModule.inspectStorageHealth();
131+
expect(summary.state).toBe("corrupt");
132+
});
133+
});
134+
135+
it("reports recoverable storage when invalid primary storage has a valid WAL", async () => {
136+
await withStorageModule("storage-health-recoverable-invalid", async ({ storagePath, walPath, storageModule }) => {
137+
await fs.writeFile(storagePath, "{ malformed-json", "utf-8");
138+
const content = JSON.stringify({
139+
version: 3,
140+
activeIndex: 0,
141+
activeIndexByFamily: { codex: 0 },
142+
accounts: [{ refreshToken: "refresh-token", addedAt: 1, lastUsed: 1 }],
143+
});
144+
await fs.writeFile(walPath, JSON.stringify({ version: 1, content, checksum: sha256(content) }), "utf-8");
145+
const summary = await storageModule.inspectStorageHealth();
146+
expect(summary.state).toBe("recoverable");
147+
expect(summary.recoverySource).toBe("wal");
148+
});
149+
});
70150
});

0 commit comments

Comments
 (0)