diff --git a/lib/accounts.ts b/lib/accounts.ts index 9d4d96ed..480d01d3 100644 --- a/lib/accounts.ts +++ b/lib/accounts.ts @@ -1316,16 +1316,38 @@ export class AccountManager { // Snapshot family pointers before splice so we can distinguish "was // pointing at the removed account" from "was pointing past it". - const priorCursor: Record = {} as Record; - const priorActive: Record = {} as Record; + const priorCursor: Record = {} as Record< + ModelFamily, + number + >; + const priorActive: Record = {} as Record< + ModelFamily, + number + >; for (const family of MODEL_FAMILIES) { priorCursor[family] = this.cursorByFamily[family]; priorActive[family] = this.currentAccountIndexByFamily[family]; } this.accounts.splice(idx, 1); + // Clear numeric-keyed tracker state in the shifted range. After reindex, + // any refresh-only account that moved from N to N-1 must not inherit the + // stale health/token entries that used to belong to the old numeric slot. + getHealthTracker().clearNumericKeysAtOrAbove(idx); + getTokenTracker().clearNumericKeysAtOrAbove(idx); this.accounts.forEach((acc, index) => { acc.index = index; + // Invalidate the cached runtime tracker key when it was keyed by + // numeric index (fallback path in getRuntimeAccountIdentityKey). + // After the splice+reindex above, a remaining account that was at + // index N (e.g. 3) may now live at index N-1 (e.g. 2); if we keep + // the previously cached numeric key, rotation/health/token state + // queries would consult the stale position and mismatch the + // current one. Identity-based string keys remain stable because + // accountId/email are not affected by array position changes. + if (typeof acc._runtimeTrackerKey === "number") { + acc._runtimeTrackerKey = undefined; + } }); if (this.accounts.length === 0) { diff --git a/test/accounts.test.ts b/test/accounts.test.ts index 740ba89f..47f0b0df 100644 --- a/test/accounts.test.ts +++ b/test/accounts.test.ts @@ -2888,6 +2888,121 @@ describe("AccountManager", () => { expect(otherAccount).not.toBeNull(); expect(otherAccount?.enabled).not.toBe(false); }); + + it("invalidates numeric runtime tracker key after removeAccount reindex (HI-01)", () => { + // Regression: _runtimeTrackerKey was cached to the numeric + // account.index when no accountId/email was present. After + // removeAccount spliced the array and reindexed surviving + // accounts, the cached numeric key still pointed at the + // old (stale) position, so getRuntimeTrackerKey returned the + // wrong key and rotation/health/token tracker lookups + // mismatched the current index. Identity-based string keys + // must continue to survive a reindex unchanged. + const now = Date.now(); + const stored = { + version: 3 as const, + activeIndex: 0, + accounts: [ + { refreshToken: "token-a", addedAt: now, lastUsed: now }, + { + refreshToken: "token-b", + email: "remove-me@example.com", + addedAt: now, + lastUsed: now, + }, + { refreshToken: "token-c", addedAt: now, lastUsed: now }, + { + refreshToken: "token-d", + addedAt: now, + lastUsed: now, + }, + ], + }; + + const manager = new AccountManager(undefined, stored); + + const refreshOnlyBefore = manager.getAccountByIndex(2)!; + const refreshOnlyAfterShift = manager.getAccountByIndex(3)!; + + // Prime the cache: getRuntimeTrackerKey writes + // _runtimeTrackerKey on first access. The refresh-only + // account caches its numeric index (2); the identity + // account caches its string identity key. + const refreshKeyBefore = getRuntimeTrackerKey(refreshOnlyBefore); + const refreshOnlyAfterShiftKeyBefore = getRuntimeTrackerKey( + refreshOnlyAfterShift, + ); + expect(refreshKeyBefore).toBe(2); + expect(refreshOnlyAfterShiftKeyBefore).toBe(3); + expect(refreshOnlyBefore._runtimeTrackerKey).toBe(2); + expect(refreshOnlyAfterShift._runtimeTrackerKey).toBe(3); + + // Record observable tracker state keyed by the current + // tracker key so we can prove the same account is + // consulted under its new key after the reindex. Use the + // health tracker: that is what recordFailure mutates via + // getRuntimeTrackerKey. + const healthTracker = getHealthTracker(); + const tokenTracker = getTokenTracker(); + const initialRefreshScore = healthTracker.getScore( + refreshKeyBefore, + "codex", + ); + manager.recordFailure(refreshOnlyBefore, "codex"); + tokenTracker.tryConsume(refreshKeyBefore, "codex"); + const postFailureScore = healthTracker.getScore( + refreshKeyBefore, + "codex", + ); + expect(postFailureScore).toBeLessThan(initialRefreshScore); + expect(tokenTracker.getTokens(refreshKeyBefore, "codex")).toBeLessThan( + 50, + ); + + // Remove the identity-bearing account at index 1. The + // refresh-only account that was at index 2 now lives at + // index 1 after the splice + reindex. + const victim = manager.getAccountByIndex(1)!; + expect(manager.removeAccount(victim)).toBe(true); + + const refreshOnlyAfter = manager.getAccountByIndex(1); + expect(refreshOnlyAfter).not.toBeNull(); + expect(refreshOnlyAfter?.refreshToken).toBe("token-c"); + expect(refreshOnlyAfter?.index).toBe(1); + + // getRuntimeTrackerKey must now report the NEW numeric + // index (1), not the stale cached value (2). Before the + // fix, the stale cache caused this assertion to fail with 2. + const refreshKeyAfter = getRuntimeTrackerKey(refreshOnlyAfter!); + expect(refreshKeyAfter).toBe(1); + expect(refreshOnlyAfter?._runtimeTrackerKey).toBe(1); + + // The health score recorded under the pre-reindex key (2) + // must NOT bleed into the post-reindex key (1). Querying + // the new key should return the default (pristine) score. + // This proves the runtime tracker consults the refreshed + // key after reindex. + expect(healthTracker.getScore(refreshKeyAfter, "codex")).toBe( + initialRefreshScore, + ); + + // The next refresh-only survivor shifts from numeric key 3 to 2. + // The stale numeric state for old key 2 must NOT bleed onto it. + const refreshOnlyShifted = manager.getAccountByIndex(2); + expect(refreshOnlyShifted).not.toBeNull(); + expect(refreshOnlyShifted?.refreshToken).toBe("token-d"); + expect(refreshOnlyShifted?.index).toBe(2); + const refreshOnlyShiftedKeyAfter = getRuntimeTrackerKey( + refreshOnlyShifted!, + ); + expect(refreshOnlyShiftedKeyAfter).toBe(2); + expect(healthTracker.getScore(refreshOnlyShiftedKeyAfter, "codex")).toBe( + initialRefreshScore, + ); + expect(tokenTracker.getTokens(refreshOnlyShiftedKeyAfter, "codex")).toBe( + 50, + ); + }); }); describe("flushPendingSave", () => {