fix(storage): consistent case-insensitive email identity + surface absorbed re-logins (#171) (#181)

ndycode · neil · claude · web-flow · commit 5c7e93663782 · 2026-06-19T21:44:50.000+08:00
* fix(storage): consistent case-insensitive email identity + surface absorbed re-logins (#171) Two edge cases surfaced by a deep release-readiness stress test in the #171 account-recovery layers, plus a third same-class divergence found in review. 1. Email case divergence (storage vs detectors): lib/storage/identity.ts compared emails case-sensitively (trim only) while the codex-doctor/ codex-health detectors lowercase. So `User@Example.com` (org) and `user@example.com` (disabled token re-login) escaped storage dedup yet were flagged as removable — the two layers disagreed on identity. Added `sameEmailIdentity` (trim+lowercase) used only at the email comparison site, and lowercased the dedup-by-email map key. accountId/org comparisons stay case-sensitive (#64/#85 preserved). 2. Surfacing blind spot: a user-disabled org account that absorbs a newer ENABLED token re-login correctly stays disabled (fail-closed) but was then skipped by every #171 diagnostic, so the user got no signal a fresh login landed on a disabled slot. New read-only findDisabledAccountsWithFreshCredential (disabled + unexpired access token), surfaced by codex-doctor (finding `disabled-account-fresh-credential`) and codex-health (text + `disabledWithFreshCredentialSlots` JSON), pointing the user to re-enable it. Never re-enables anything. 3. Third divergence (architect review): login-runner buildIdentityIndexes keyed byEmailNoOrg with trim-only while its lookup and getExactIdentityKey lowercase. Aligned to sanitizeEmail so a mixed-case re-login can't miss its existing no-org entry and append a duplicate. Tests: 5 unit tests for findDisabledAccountsWithFreshCredential; storage merge test for mixed-case email collapse; login-runner behavioral test (mixed-case re-login merges to 1 account; distinct emails stay 2). Mutation teeth-checks confirmed for bugs 1 and 2. Full suite green (2549 passed, 1 skipped), tsc + eslint + build clean, npm audit 0 vulnerabilities. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: add enabled-undefined + distinct-emails control cases (PR #181 review) Greptile P2s: - findDisabledAccountsWithFreshCredential: add an enabled:undefined case (no enabled field) asserting [] — teeth on the `enabled !== false` branch. - login-runner: commit the distinct-emails control (user@ + other@ stay 2 accounts) that the PR description claimed, proving the mixed-case merge test is non-vacuous. Both are email-only (no accountId) so they exercise the email-dedup path. Full suite green (2551 passed, 1 skipped), tsc clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: neil <neil@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,6 +12,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - `codex-doctor` now surfaces a finding when a disabled `accountIdSource: "token"` entry shadows an enabled, org-backed account that shares its email (a leftover a fresh re-login can mint instead of updating the org account). It is flagged with a `codex-remove` hint rather than auto-removed, because the only link between the two records is email and email-only merges must not collapse distinct multi-org accounts (#64). (#171)
 - `codex-doctor --fix` no longer fails silently when a stored credential is genuinely dead: a failed token refresh now reports `N account(s) need re-login` and points the user at `opencode auth login`, instead of leaving an all-dark pool unrepaired with no surfaced cause. (#171)
 - `codex-health` now surfaces the same recovery diagnostics as `codex-doctor` (read-only): it flags accounts blocked only by a stale cooldown/rate-limit (pointing at `codex-doctor --fix`) and disabled token-source duplicates (pointing at `codex-remove`), and includes `staleRecoverableSlots` / `disabledDuplicateSlots` in JSON output. This addresses the part of the issue that named `codex-health` explicitly. (#171)
+- A disabled `accountIdSource: "token"` duplicate (a re-login artifact) merging into the real org-source account by email no longer disables the canonical account. Storage dedup now lets the org account's own `enabled` state govern the merge, so a single-account pool can no longer end up dark and unrecoverable; fail-closed is preserved for genuinely user-disabled accounts. (#171)
+- Storage dedup now compares account emails case-insensitively, matching the `codex-doctor`/`codex-health` duplicate detectors. Previously `User@Example.com` (org) and `user@example.com` (token re-login) escaped dedup yet were still flagged as removable, so the two layers disagreed on identity. (#171)
+- `codex-doctor` and `codex-health` now surface a disabled account that holds a fresh login credential — the fingerprint of a recent re-login that landed on a deliberately-disabled slot — so the user is told to re-enable it if intended instead of getting no signal at all. (#171)
 - Caller-cancellation during a retry/backoff wait now surfaces as a proper `AbortError` carrying the caller's `signal.reason`, instead of an opaque `new Error("Aborted")` that dropped the cause. This aligns the retry-wait path with the fetch path and the `isAbortError` convention in `lib/codex-usage.ts`, improving diagnosability of the `Error: Aborted` symptom. (#176)
 - Fixed a flaky email-masking property test (#163) that intermittently failed CI: the assertion used a fragile `masked.includes(local)` substring check that false-positived when the local part repeats in the preserved domain (`abc@abc.com`) or contains the mask character itself (`a.*@a.aa` → `a.***`). It now asserts the exact masking contract (masked local = first ≤2 chars + `***`), with deterministic regression cases. No production code change.
 
diff --git a/lib/accounts/stale-state.ts b/lib/accounts/stale-state.ts
@@ -220,3 +220,48 @@ export function findStaleRecoverableAccounts(
 	return blocked;
 }
 
+/**
+ * Subset of a stored account used for the absorbed-credential diagnostic.
+ */
+export interface FreshCredentialScanAccount {
+	enabled?: boolean;
+	expiresAt?: number;
+	accessToken?: string;
+}
+
+/**
+ * Identify accounts that are DISABLED yet carry a fresh (unexpired) access
+ * token — the fingerprint of a user-disabled account that absorbed a newer
+ * enabled re-login during dedup (issue #171).
+ *
+ * The merge is intentionally fail-closed: a user-disabled record stays disabled
+ * even after it absorbs a fresh credential. That is correct, but it is silent —
+ * the absorbing record is org-source AND disabled, so it is skipped by every
+ * other #171 diagnostic and the user gets no hint that a fresh login they just
+ * performed landed on a deliberately-disabled slot. This read-only detector
+ * surfaces that case so tooling can suggest re-enabling the account if intended.
+ *
+ * Read-only: mutates nothing, and never re-enables (that stays a user decision).
+ *
+ * @returns the 0-based indexes of disabled accounts holding a fresh credential.
+ */
+export function findDisabledAccountsWithFreshCredential(
+	accounts: FreshCredentialScanAccount[],
+	now: number = nowMs(),
+): number[] {
+	const flagged: number[] = [];
+	for (let i = 0; i < accounts.length; i += 1) {
+		const account = accounts[i];
+		if (!account) continue;
+		if (account.enabled !== false) continue;
+		const hasToken =
+			typeof account.accessToken === "string" && account.accessToken.length > 0;
+		const fresh =
+			typeof account.expiresAt === "number" && account.expiresAt > now;
+		if (hasToken && fresh) {
+			flagged.push(i);
+		}
+	}
+	return flagged;
+}
+
diff --git a/lib/auth/login-runner.ts b/lib/auth/login-runner.ts
@@ -559,7 +559,11 @@ export async function persistAccountPool(
 				const organizationId = account.organizationId?.trim();
 				const accountId = account.accountId?.trim();
 				const refreshToken = account.refreshToken?.trim();
-				const email = account.email?.trim();
+				// Lowercase the email index key to match the lookup side
+				// (sanitizeEmail) and getExactIdentityKey; a trim-only key here let a
+				// mixed-case re-login miss its existing no-org entry and append a
+				// duplicate (#171 email-identity consistency).
+				const email = sanitizeEmail(account.email);
 
 				if (refreshToken) {
 					pushIndex(byRefreshTokenGlobal, refreshToken, i);
diff --git a/lib/storage/identity.ts b/lib/storage/identity.ts
@@ -189,6 +189,21 @@ function sameOptionalIdentity(left: string | undefined, right: string | undefine
   return !!normalizedLeft && !!normalizedRight && normalizedLeft === normalizedRight;
 }
 
+/**
+ * Email-specific identity comparison. Emails are case-insensitive, so this
+ * normalizes with `trim().toLowerCase()`. This MUST match the normalization in
+ * `lib/accounts/stale-state.ts` (findDisabledTokenSourceDuplicates) so the
+ * storage-merge layer and the codex-doctor/codex-health detectors agree on
+ * whether two records are the same account (issue #171): a divergence let
+ * `User@Example.com` (org) and `user@example.com` (disabled token re-login)
+ * escape dedup yet still get flagged as a removable duplicate.
+ */
+function sameEmailIdentity(left: string | undefined, right: string | undefined): boolean {
+  const normalizedLeft = normalizeWorkspaceIdentityPart(left)?.toLowerCase();
+  const normalizedRight = normalizeWorkspaceIdentityPart(right)?.toLowerCase();
+  return !!normalizedLeft && !!normalizedRight && normalizedLeft === normalizedRight;
+}
+
 function isLegacyRefreshTokenDuplicate<T extends AccountLike>(left: T, right: T): boolean {
   const refreshToken = normalizeWorkspaceIdentityPart(left.refreshToken);
   const leftOrganizationId = normalizeWorkspaceIdentityPart(left.organizationId);
@@ -202,7 +217,7 @@ function isLegacyRefreshTokenDuplicate<T extends AccountLike>(left: T, right: T)
   const rightTokenLike = !rightOrganizationId && right.accountIdSource === "token";
   if (
     ((leftOrgLike && rightTokenLike) || (rightOrgLike && leftTokenLike)) &&
-    (sameOptionalIdentity(left.email, right.email) ||
+    (sameEmailIdentity(left.email, right.email) ||
       (!!refreshToken && refreshToken === normalizeWorkspaceIdentityPart(right.refreshToken)))
   ) {
     return true;
@@ -337,7 +352,7 @@ export function deduplicateAccountsByEmail<T extends { organizationId?: string;
       continue;
     }
 
-    const email = account.email?.trim();
+    const email = account.email?.trim().toLowerCase();
     if (!email) {
       indicesToKeep.add(i);
       continue;
diff --git a/lib/tools/codex-doctor.ts b/lib/tools/codex-doctor.ts
@@ -14,6 +14,7 @@ import { queuedRefresh } from "../refresh-queue.js";
 import { MODEL_FAMILIES } from "../prompts/codex.js";
 import {
 	clearRefreshedAccountsStaleState,
+	findDisabledAccountsWithFreshCredential,
 	findDisabledTokenSourceDuplicates,
 } from "../accounts/stale-state.js";
 import { clearTuiQuotaSnapshot } from "../tui-quota-cache.js";
@@ -118,6 +119,21 @@ export function createCodexDoctorTool(ctx: ToolContext): ToolDefinition {
 						.join(", ")}).`,
 				});
 			}
+			// A user-disabled account that absorbed a fresh enabled re-login stays
+			// disabled (fail-closed) but is otherwise invisible to diagnostics (#171).
+			const disabledWithFreshCredential = storage
+				? findDisabledAccountsWithFreshCredential(storage.accounts)
+				: [];
+			if (disabledWithFreshCredential.length > 0) {
+				findings.push({
+					severity: "warning",
+					code: "disabled-account-fresh-credential",
+					summary: `${disabledWithFreshCredential.length} disabled account(s) hold a fresh login credential.`,
+					action: `A recent re-login landed on a disabled slot; re-enable it in oc-codex-multi-auth-accounts.json if intended (slots: ${disabledWithFreshCredential
+						.map((index) => index + 1)
+						.join(", ")}).`,
+				});
+			}
 			let routingVisibility: RoutingVisibilitySnapshot | null = null;
 			const appliedFixes: string[] = [];
 			const fixErrors: string[] = [];
diff --git a/lib/tools/codex-health.ts b/lib/tools/codex-health.ts
@@ -7,6 +7,7 @@ import { tool, type ToolDefinition } from "@opencode-ai/plugin/tool";
 import { loadAccounts } from "../storage.js";
 import { queuedRefresh } from "../refresh-queue.js";
 import {
+	findDisabledAccountsWithFreshCredential,
 	findDisabledTokenSourceDuplicates,
 	findStaleRecoverableAccounts,
 } from "../accounts/stale-state.js";
@@ -156,13 +157,22 @@ export function createCodexHealthTool(ctx: ToolContext): ToolDefinition {
 					`Duplicates: ${dupSlots.length} disabled duplicate entry(ies) shadow a real account (slots: ${dupSlots.join(", ")}). Remove with \`codex-remove\`.`,
 				);
 			}
+			const absorbedSlots = findDisabledAccountsWithFreshCredential(
+				storage.accounts,
+			).map((index) => index + 1);
+			if (absorbedSlots.length > 0) {
+				results.push(
+					`Disabled w/ fresh login: ${absorbedSlots.length} disabled account(s) hold a fresh credential (slots: ${absorbedSlots.join(", ")}) - a recent re-login landed on a disabled slot. Re-enable in oc-codex-multi-auth-accounts.json if intended.`,
+				);
+			}
 			if (outputFormat === "json") {
 				return renderJsonOutput({
 					totalAccounts: storage.accounts.length,
 					healthyCount,
 					unhealthyCount,
 					staleRecoverableSlots: staleSlots,
 					disabledDuplicateSlots: dupSlots,
+					disabledWithFreshCredentialSlots: absorbedSlots,
 					accounts: jsonAccounts,
 				});
 			}
diff --git a/test/login-runner.test.ts b/test/login-runner.test.ts
@@ -49,6 +49,63 @@ describe("login-runner persistAccountPool", () => {
 		await fs.rm(testDir, { recursive: true, force: true });
 	});
 
+	// Build a minimal JWT (header.payload.sig) whose payload carries an email
+	// claim, so extractAccountEmail/decodeJWT resolve a real address.
+	const jwtWithEmail = (email: string): string => {
+		const b64 = (o: unknown) =>
+			Buffer.from(JSON.stringify(o)).toString("base64url");
+		return `${b64({ alg: "none" })}.${b64({ email })}.sig`;
+	};
+
+	it("treats a mixed-case re-login as the same identity end-to-end (#171 email case)", async () => {
+		// Seed a stored email-only (no org/accountId) account with MixedCase email.
+		await persistAccountPool(
+			[
+				{
+					type: "success",
+					access: jwtWithEmail("User@Example.com"),
+					refresh: "refresh-old",
+					expires: Date.now() + 60_000,
+				},
+			],
+			false,
+		);
+		const first = await loadAccounts();
+		expect(first?.accounts).toHaveLength(1);
+
+		// Re-login: same person, lowercase email, fresh refresh token.
+		await persistAccountPool(
+			[
+				{
+					type: "success",
+					access: jwtWithEmail("user@example.com"),
+					refresh: "refresh-new",
+					expires: Date.now() + 60_000,
+				},
+			],
+			false,
+		);
+		const second = await loadAccounts();
+		// Case-insensitive email index => merged into the existing entry, not appended.
+		expect(second?.accounts).toHaveLength(1);
+		expect(second?.accounts[0]?.refreshToken).toBe("refresh-new");
+	});
+
+	it("keeps two genuinely distinct emails as separate accounts (control)", async () => {
+		await persistAccountPool(
+			[{ type: "success", access: jwtWithEmail("user@example.com"), refresh: "r-a", expires: Date.now() + 60_000 }],
+			false,
+		);
+		await persistAccountPool(
+			[{ type: "success", access: jwtWithEmail("other@example.com"), refresh: "r-b", expires: Date.now() + 60_000 }],
+			false,
+		);
+		const loaded = await loadAccounts();
+		// Distinct emails must NOT be merged — proves the merge test is non-vacuous.
+		expect(loaded?.accounts).toHaveLength(2);
+	});
+
+
 	it("serializes overlapping login persists without losing accounts", async () => {
 		const originalRename = fs.rename.bind(fs);
 		let firstRenameReleased = false;
diff --git a/test/stale-state.test.ts b/test/stale-state.test.ts
@@ -3,6 +3,7 @@ import {
 	clearRefreshedAccountStaleState,
 	clearRefreshedAccountsStaleState,
 	findDisabledTokenSourceDuplicates,
+	findDisabledAccountsWithFreshCredential,
 	findStaleRecoverableAccounts,
 	type StaleStateAccount,
 } from "../lib/accounts/stale-state.js";
@@ -168,3 +169,42 @@ describe("findStaleRecoverableAccounts", () => {
 	});
 });
 
+describe("findDisabledAccountsWithFreshCredential (issue #171)", () => {
+	const NOW = 1_700_000_000_000;
+	const FUTURE = NOW + 3_600_000;
+	const PAST = NOW - 3_600_000;
+
+	it("flags a disabled account that holds a fresh (unexpired) access token", () => {
+		const accounts = [{ enabled: false, accessToken: "tok", expiresAt: FUTURE }];
+		expect(findDisabledAccountsWithFreshCredential(accounts, NOW)).toEqual([0]);
+	});
+
+	it("ignores an enabled account (not the blind-spot case)", () => {
+		const accounts = [{ enabled: true, accessToken: "tok", expiresAt: FUTURE }];
+		expect(findDisabledAccountsWithFreshCredential(accounts, NOW)).toEqual([]);
+	});
+
+	it("ignores an account with no enabled field (undefined is not disabled)", () => {
+		const accounts = [{ accessToken: "tok", expiresAt: FUTURE }];
+		expect(findDisabledAccountsWithFreshCredential(accounts, NOW)).toEqual([]);
+	});
+
+	it("ignores a disabled account whose credential is expired", () => {
+		const accounts = [{ enabled: false, accessToken: "tok", expiresAt: PAST }];
+		expect(findDisabledAccountsWithFreshCredential(accounts, NOW)).toEqual([]);
+	});
+
+	it("ignores a disabled account with no access token", () => {
+		const accounts = [{ enabled: false, expiresAt: FUTURE }];
+		expect(findDisabledAccountsWithFreshCredential(accounts, NOW)).toEqual([]);
+	});
+
+	it("returns multiple flagged slots in order", () => {
+		const accounts = [
+			{ enabled: false, accessToken: "a", expiresAt: FUTURE },
+			{ enabled: true, accessToken: "b", expiresAt: FUTURE },
+			{ enabled: false, accessToken: "c", expiresAt: FUTURE },
+		];
+		expect(findDisabledAccountsWithFreshCredential(accounts, NOW)).toEqual([0, 2]);
+	});
+});
diff --git a/test/storage.test.ts b/test/storage.test.ts
@@ -318,6 +318,43 @@ describe("storage", () => {
 
     // Issue #171: a disabled token-source duplicate (a plugin re-login artifact)
     // must NOT disable the real org account it collapses into during dedup.
+    // Issue #171 (email case divergence): storage dedup must compare emails
+    // case-insensitively so it agrees with the codex-doctor/codex-health
+    // detectors (which lowercase). Otherwise User@Example.com (org) and
+    // user@example.com (token dup) escape dedup but get flagged as removable.
+    it("collapses an org + disabled token-dup that differ only by email case (#171)", async () => {
+      const now = Date.now();
+      await saveAccounts({
+        version: 3,
+        activeIndex: 0,
+        accounts: [
+          {
+            accountId: "org-AAA",
+            organizationId: "org-AAA",
+            accountIdSource: "org",
+            email: "User@Example.com",
+            refreshToken: "OLD-refresh",
+            addedAt: 100,
+            lastUsed: 200,
+          },
+          {
+            accountId: "uuid-fresh",
+            accountIdSource: "token",
+            email: "user@example.com",
+            refreshToken: "FRESH-refresh",
+            addedAt: 999,
+            lastUsed: 999,
+            enabled: false,
+          },
+        ],
+      });
+      const loaded = await loadAccounts();
+      // Case-insensitive email match => the two collapse into one org account.
+      expect(loaded?.accounts).toHaveLength(1);
+      expect(loaded!.accounts[0]!.organizationId).toBe("org-AAA");
+      expect(loaded!.accounts[0]!.enabled).not.toBe(false);
+    });
+
     it("keeps the org account enabled when a disabled token-source duplicate merges in (#171)", async () => {
       const now = Date.now();
       await saveAccounts({
