fix: harden guardian identity reconciliation

Author: ndycode

lib/refresh-guardian.ts

@@ -25,28 +25,78 @@ export interface RefreshGuardianStats {
 
 const DEFAULT_INTERVAL_MS = 60_000;
 
+function findMatchingLiveAccountIndexes(
+	liveAccounts: ManagedAccount[],
+	predicate: (candidate: ManagedAccount) => boolean,
+): number[] {
+	const matches: number[] = [];
+	for (const [index, candidate] of liveAccounts.entries()) {
+		if (predicate(candidate)) {
+			matches.push(index);
+		}
+	}
+	return matches;
+}
+
 function resolveLiveAccountIndex(
 	liveAccounts: ManagedAccount[],
 	sourceAccount: ManagedAccount,
 ): number {
 	if (sourceAccount.accountId) {
-		const byAccountId = liveAccounts.findIndex(
+		const accountIdMatches = findMatchingLiveAccountIndexes(
+			liveAccounts,
 			(candidate) => candidate.accountId === sourceAccount.accountId,
 		);
-		if (byAccountId >= 0) return byAccountId;
+		const resolvedIndex = accountIdMatches[0];
+		if (resolvedIndex !== undefined) {
+			log.debug("Resolved refreshed account by accountId", {
+				sourceIndex: sourceAccount.index,
+				resolvedIndex,
+				matchCount: accountIdMatches.length,
+			});
+			if (accountIdMatches.length > 1) {
+				log.warn("Duplicate live accountId matches during refresh reconciliation", {
+					sourceIndex: sourceAccount.index,
+					resolvedIndex,
+					matchCount: accountIdMatches.length,
+				});
+			}
+			return resolvedIndex;
+		}
 	}
 
 	const sourceEmail = sanitizeEmail(sourceAccount.email);
 	if (sourceEmail) {
-		const byEmail = liveAccounts.findIndex(
+		const emailMatches = findMatchingLiveAccountIndexes(
+			liveAccounts,
 			(candidate) => sanitizeEmail(candidate.email) === sourceEmail,
 		);
-		if (byEmail >= 0) return byEmail;
+		const resolvedIndex = emailMatches[0];
+		if (resolvedIndex !== undefined) {
+			log.debug("Resolved refreshed account by email", {
+				sourceIndex: sourceAccount.index,
+				resolvedIndex,
+				matchCount: emailMatches.length,
+			});
+			if (emailMatches.length > 1) {
+				log.warn("Duplicate live email matches during refresh reconciliation", {
+					sourceIndex: sourceAccount.index,
+					resolvedIndex,
+					matchCount: emailMatches.length,
+				});
+			}
+			return resolvedIndex;
+		}
 	}
 
-	return liveAccounts.findIndex(
+	const byToken = liveAccounts.findIndex(
 		(candidate) => candidate.refreshToken === sourceAccount.refreshToken,
 	);
+	log.debug("Resolved refreshed account by refresh token fallback", {
+		sourceIndex: sourceAccount.index,
+		resolvedIndex: byToken,
+	});
+	return byToken;
 }
 
 export class RefreshGuardian {
@@ -141,11 +191,11 @@ export class RefreshGuardian {
 			for (const candidate of snapshot) {
 				snapshotByIndex.set(candidate.index, candidate);
 			}
+			const liveAccounts = manager.getAccountsSnapshot();
 
 			for (const [accountIndex, result] of refreshResults.entries()) {
 				const sourceAccount = snapshotByIndex.get(accountIndex);
 				if (!sourceAccount) continue;
-				const liveAccounts = manager.getAccountsSnapshot();
 				const resolvedIndex = resolveLiveAccountIndex(liveAccounts, sourceAccount);
 				const account = resolvedIndex >= 0 ? manager.getAccountByIndex(resolvedIndex) : null;
 				if (!account) continue;

test/refresh-guardian.test.ts

@@ -216,7 +216,7 @@ describe("refresh-guardian", () => {
     expect(tickSpy).toHaveBeenCalledTimes(1);
   });
 
-  it("resolves refreshed account using stable refresh token when indices shift", async () => {
+  it("resolves refreshed account by accountId when indices shift", async () => {
     const originalA = createManagedAccount(0);
     const originalB = createManagedAccount(1);
     const liveB = { ...originalB, index: 0 };
@@ -336,8 +336,73 @@ describe("refresh-guardian", () => {
     ).toHaveBeenCalledWith(liveA);
   });
 
-  it("falls back to normalized email when accountId is unavailable", async () => {
-    const originalA = { ...createManagedAccount(0), accountId: undefined, email: " User0@Example.com " };
+  it("falls back to refreshToken when accountId is unavailable and email is invalid", async () => {
+    const originalA = {
+      ...createManagedAccount(0),
+      accountId: undefined,
+      email: "invalid-no-at",
+    };
+    const originalB = createManagedAccount(1);
+    const liveA = {
+      ...originalA,
+      index: 1,
+    };
+    const liveB = { ...originalB, index: 0 };
+    const snapshots = [
+      [originalA, originalB],
+      [liveB, liveA],
+    ];
+    let readCount = 0;
+    const manager = {
+      getAccountsSnapshot: vi.fn(
+        () => snapshots[Math.min(readCount++, snapshots.length - 1)],
+      ),
+      getAccountByIndex: vi.fn(
+        (index: number) =>
+          [liveB, liveA].find((account) => account.index === index) ?? null,
+      ),
+      clearAuthFailures: vi.fn(),
+      markAccountCoolingDown: vi.fn(),
+      setAccountEnabled: vi.fn(),
+      saveToDiskDebounced: vi.fn(),
+    } as unknown as AccountManager;
+    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-token",
+              refresh: "refresh-token",
+              expires: Date.now() + 3_600_000,
+            },
+          },
+        ],
+      ]),
+    );
+
+    await guardian.tick();
+
+    expect(applyRefreshResultMock).toHaveBeenCalledWith(
+      liveA,
+      expect.objectContaining({ type: "success" }),
+    );
+    expect(
+      manager.clearAuthFailures as ReturnType<typeof vi.fn>,
+    ).toHaveBeenCalledWith(liveA);
+  });
+
+  it("treats empty string accountId the same as undefined by using normalized email", async () => {
+    const originalA = { ...createManagedAccount(0), accountId: "", email: " User0@Example.com " };
     const originalB = createManagedAccount(1);
     const liveA = {
       ...originalA,
@@ -642,6 +707,9 @@ describe("refresh-guardian", () => {
     expect(
       manager.saveToDiskDebounced as ReturnType<typeof vi.fn>,
     ).toHaveBeenCalledTimes(1);
+    expect(
+      manager.getAccountsSnapshot as ReturnType<typeof vi.fn>,
+    ).toHaveBeenCalledTimes(2);
 
     const stats = guardian.getStats();
     expect(stats.runs).toBe(1);