fix-refresh-guardian-commit-failures

Author: ndycode

lib/accounts.ts

@@ -779,7 +779,7 @@ export class AccountManager {
 		account.refreshToken = auth.refresh;
 		account.access = auth.access;
 		account.expires = auth.expires;
-		const tokenAccountId = extractAccountId(auth.access);
+		const tokenAccountId = extractAccountId(auth.access)?.trim() || undefined;
 		if (
 			tokenAccountId &&
 			(shouldUpdateAccountIdFromToken(account.accountIdSource, account.accountId))
@@ -836,6 +836,8 @@ export class AccountManager {
 		const nextEmail = sanitizeEmail(extractAccountEmail(auth.access));
 		try {
 			return await withAccountStorageTransaction(async (_current, persist) => {
+				// Snapshot the live in-memory pool under the storage lock so refresh
+				// persistence merges against the latest account state.
 				const nextStorage = structuredClone(
 					this.buildStorageSnapshot(),
 				) as AccountStorageV3;

lib/refresh-guardian.ts

@@ -1,6 +1,7 @@
 import { createLogger } from "./logger.js";
 import { refreshExpiringAccounts } from "./proactive-refresh.js";
 import type { AccountManager } from "./accounts.js";
+import { CodexAuthError } from "./errors.js";
 import type { CooldownReason } from "./storage.js";
 import type { TokenResult } from "./types.js";
 
@@ -148,12 +149,19 @@ export class RefreshGuardian {
 						sourceIndex: sourceAccount.index,
 						error: error instanceof Error ? error.message : String(error),
 					});
-					const account = manager.getAccountByIdentity(sourceAccount, refreshedAuth);
+					const account =
+						manager.getAccountByIdentity(sourceAccount, refreshedAuth) ??
+						manager.getAccountByIdentity(sourceAccount);
+					const cooldownReason: CooldownReason =
+						error instanceof CodexAuthError && !error.retryable
+							? "auth-failure"
+							: "network-error";
 					if (account) {
-						manager.markAccountCoolingDown(account, this.bufferMs, "network-error");
+						manager.markAccountCoolingDown(account, this.bufferMs, cooldownReason);
 					}
 					this.stats.failed += 1;
-					this.stats.networkFailed += 1;
+					if (cooldownReason === "auth-failure") this.stats.authFailed += 1;
+					else this.stats.networkFailed += 1;
 					return !!account;
 				}
 				this.stats.refreshed += 1;

test/refresh-guardian.test.ts

@@ -1,11 +1,13 @@
 import { describe, expect, it, beforeEach, afterEach, vi } from "vitest";
-import type { AccountManager, ManagedAccount, OAuthAuthDetails } from "../lib/accounts.js";
+import type { AccountManager, ManagedAccount } from "../lib/accounts.js";
 import {
   extractAccountEmail,
   extractAccountId,
   sanitizeEmail,
 } from "../lib/auth/token-utils.js";
+import { CodexAuthError } from "../lib/errors.js";
 import { findMatchingAccountIndex } from "../lib/storage.js";
+import type { OAuthAuthDetails } from "../lib/types.js";
 
 const refreshExpiringAccountsMock = vi.fn();
 const applyRefreshResultMock = vi.fn();
@@ -872,4 +874,48 @@ describe("refresh-guardian", () => {
     expect(stats.networkFailed).toBe(1);
     expect(stats.rateLimited).toBe(1);
   });
+
+  it("treats non-retryable commit failures as auth cooldowns", async () => {
+    const accountA = createManagedAccount(0);
+    const manager = createManagerMock([accountA]);
+    const commitRefreshedAuthMock = manager
+      .commitRefreshedAuth as ReturnType<typeof vi.fn>;
+    commitRefreshedAuthMock.mockRejectedValueOnce(
+      new CodexAuthError("refresh persistence failed", { retryable: false }),
+    );
+
+    const { RefreshGuardian } = await import("../lib/refresh-guardian.js");
+    const guardian = new RefreshGuardian(() => manager, {
+      bufferMs: 60_000,
+      intervalMs: 5_000,
+    });
+
+    refreshExpiringAccountsMock.mockResolvedValue(
+      new Map([
+        [
+          0,
+          {
+            refreshed: true,
+            reason: "success",
+            tokenResult: {
+              type: "success",
+              access: "access-0",
+              refresh: "refresh-0-new",
+              expires: Date.now() + 3_600_000,
+            },
+          },
+        ],
+      ]),
+    );
+
+    await guardian.tick();
+
+    expect(
+      manager.markAccountCoolingDown as ReturnType<typeof vi.fn>,
+    ).toHaveBeenCalledWith(accountA, 60_000, "auth-failure");
+    const stats = guardian.getStats();
+    expect(stats.failed).toBe(1);
+    expect(stats.authFailed).toBe(1);
+    expect(stats.networkFailed).toBe(0);
+  });
 });