fix(identity): address review feedback on dedupe determinism

Author: sdip15fa

index.ts

@@ -633,19 +633,6 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => {
 						byRefreshTokenGlobal: Map<string, number[]>;
 					};
 
-					const pickNewestFromIndices = (indices: number[]): number | undefined => {
-						if (indices.length === 0) return undefined;
-						const first = indices[0];
-						if (typeof first !== "number") return undefined;
-						let newestIndex = first;
-						for (let i = 1; i < indices.length; i += 1) {
-							const candidate = indices[i];
-							if (typeof candidate !== "number") continue;
-							newestIndex = pickNewestAccountIndex(newestIndex, candidate);
-						}
-						return newestIndex;
-					};
-
 					const resolveNoOrgRefreshMatch = (
 						indexes: IdentityIndexes,
 						refreshToken: string,
@@ -654,25 +641,28 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => {
 						const candidateId = candidateAccountId?.trim() || undefined;
 						const matches = indexes.byRefreshTokenNoOrg.get(refreshToken);
 						if (!matches || matches.length === 0) return undefined;
+						let newestNoAccountId: number | undefined;
+						let newestExactAccountId: number | undefined;
 
-						const withNoAccountId = matches.filter((index) => {
-							const existing = accounts[index];
-							return !normalizeStoredAccountId(existing);
-						});
-
-						if (!candidateId) {
-							return pickNewestFromIndices(withNoAccountId);
-						}
-
-						const exactMatches = matches.filter((index) => {
+						for (const index of matches) {
 							const existing = accounts[index];
-							return normalizeStoredAccountId(existing) === candidateId;
-						});
-						if (exactMatches.length > 0) {
-							return pickNewestFromIndices(exactMatches);
+							const existingAccountId = normalizeStoredAccountId(existing);
+							if (!existingAccountId) {
+								newestNoAccountId =
+									typeof newestNoAccountId === "number"
+										? pickNewestAccountIndex(newestNoAccountId, index)
+										: index;
+								continue;
+							}
+							if (candidateId && existingAccountId === candidateId) {
+								newestExactAccountId =
+									typeof newestExactAccountId === "number"
+										? pickNewestAccountIndex(newestExactAccountId, index)
+										: index;
+							}
 						}
 
-						return pickNewestFromIndices(withNoAccountId);
+						return newestExactAccountId ?? newestNoAccountId;
 					};
 
 					const resolveUniqueOrgScopedMatch = (
@@ -900,6 +890,11 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => {
 						const target = accounts[preferredOrgIndex];
 						const source = accounts[fallbackIndex];
 						if (!target || !source) return true;
+						const targetAccountId = normalizeStoredAccountId(target);
+						const sourceAccountId = normalizeStoredAccountId(source);
+						if (!targetAccountId && sourceAccountId) {
+							return false;
+						}
 						if (hasDistinctNonEmptyAccountIds(target, source)) {
 							return false;
 						}

lib/auth/token-utils.ts

@@ -246,17 +246,19 @@ function extractCanonicalOrganizationIds(
 	const auth = payload[JWT_CLAIM_PATH];
 	if (!isRecord(auth)) return [];
 
-	const organizationIds = extractOrganizationIdsByIndex(auth.organizations);
-	if (organizationIds.length === 0) return organizationIds;
-
 	// Authoritative source: idToken['https://api.openai.com/auth'].organizations[0].id
 	// Only fall back to broader field extraction when this exact path is missing.
 	const primaryOrganizationId = extractPrimaryAuthClaimOrganizationId(payload);
+
 	if (primaryOrganizationId) {
+		const organizationIds = extractOrganizationIdsByIndex(auth.organizations);
+		if (organizationIds.length === 0) return [primaryOrganizationId];
 		organizationIds[0] = primaryOrganizationId;
+		return organizationIds;
 	}
 
-	return organizationIds;
+	// Fallback: broader field extraction
+	return extractOrganizationIdsByIndex(auth.organizations);
 }
 
 function resolveOrganizationOverridesForKey(

lib/storage.ts

@@ -205,6 +205,12 @@ type AccountLike = {
   lastUsed?: number;
 };
 
+type RefreshDedupeEntry = {
+  byOrg: Map<string, number>;
+  preferredOrgIndex?: number;
+  fallbackIndices: number[];
+};
+
 async function ensureGitignore(storagePath: string): Promise<void> {
   if (!currentStoragePath) return;
 
@@ -426,11 +432,7 @@ function deduplicateAccountsByRefreshToken<T extends AccountLike>(accounts: T[])
     return true;
   };
 
-  const refreshMap = new Map<string, {
-    byOrg: Map<string, number>;
-    preferredOrgIndex?: number;
-    fallbackIndices: number[];
-  }>();
+  const refreshMap = new Map<string, RefreshDedupeEntry>();
 
   const pickPreferredOrgIndex = (
     existingIndex: number | undefined,
@@ -441,37 +443,31 @@ function deduplicateAccountsByRefreshToken<T extends AccountLike>(accounts: T[])
   };
 
   const collapseFallbackIntoPreferredOrg = (
-    entry: {
-      byOrg: Map<string, number>;
-      preferredOrgIndex?: number;
-      fallbackIndices: number[];
-    },
+    entry: RefreshDedupeEntry,
   ): void => {
     if (entry.preferredOrgIndex === undefined || entry.fallbackIndices.length === 0) {
       return;
     }
 
     const preferredOrgIndex = entry.preferredOrgIndex;
-    const target = working[preferredOrgIndex];
-    if (!target) return;
-
     const remainingFallbacks: number[] = [];
     for (const fallbackIndex of entry.fallbackIndices) {
       if (fallbackIndex === preferredOrgIndex) continue;
       const source = working[fallbackIndex];
       if (!source) continue;
 
-      if (!canMergeByRefreshToken(target, source)) {
+      const currentPreferred = working[preferredOrgIndex];
+      if (!currentPreferred) {
         remainingFallbacks.push(fallbackIndex);
         continue;
       }
 
-      const mergeTarget = working[preferredOrgIndex];
-      if (!mergeTarget) {
+      if (!canMergeByRefreshToken(currentPreferred, source)) {
         remainingFallbacks.push(fallbackIndex);
         continue;
       }
-      working[preferredOrgIndex] = mergeAccountRecords(mergeTarget, source);
+
+      working[preferredOrgIndex] = mergeAccountRecords(currentPreferred, source);
       indicesToRemove.add(fallbackIndex);
     }
     entry.fallbackIndices = remainingFallbacks;

test/accounts.test.ts

@@ -433,7 +433,7 @@ describe("AccountManager", () => {
 
   describe("removeAccount", () => {
     // Note: Tests in this block cover in-memory manager behavior.
-    // Canonical persistence dedupes no-org entries; org-different entries are preserved.
+    // No-org duplicates collapse only when accountId is same/missing; distinct accountId entries are preserved.
     it("removes an account and updates indices", () => {
       const now = Date.now();
       const stored = {
@@ -509,7 +509,7 @@ describe("AccountManager", () => {
     });
 
     it("removes only targeted workspace when email/token are shared (manager-level, no orgId)", () => {
-      // Note: In-memory manager can hold multiple entries; canonical dedupe would collapse no-org duplicates
+      // Note: In-memory manager can hold multiple entries; canonical dedupe would collapse no-org duplicates only when accountId is same/missing
       const now = Date.now();
       const stored = {
         version: 3 as const,
@@ -936,7 +936,7 @@ describe("AccountManager", () => {
 
   describe("setActiveIndex", () => {
     // Note: Tests in this block cover in-memory manager behavior.
-    // Canonical persistence dedupes no-org entries; org-different entries are preserved.
+    // No-org duplicates collapse only when accountId is same/missing; distinct accountId entries are preserved.
     it("sets active index and returns account", () => {
       const now = Date.now();
       const stored = {
@@ -973,7 +973,7 @@ describe("AccountManager", () => {
     });
 
     it("switches between distinct workspace accounts sharing email and token (manager-level, no orgId)", () => {
-      // Note: In-memory manager can hold multiple entries; canonical dedupe would collapse no-org duplicates
+      // Note: In-memory manager can hold multiple entries; canonical dedupe would collapse no-org duplicates only when accountId is same/missing
       const now = Date.now();
       const stored = {
         version: 3 as const,