merge: land remaining removeAccount retarget signal fix from PR #420

Merge the remaining HI-04 follow-up PR after the rollup landed.

This change was left out of the rollup merge and still carries a valid fix:
when removeAccount() silently retargets the active pointer onto a successor,
that successor now gets `lastSwitchReason = "rotation"` so downstream
consumers can tell the pool selected it.

Tests included on the branch cover:
- removed-current successor stamp
- no stamp when all remaining peers are disabled
- sole enabled successor stamp
- non-current removal does not stamp (via follow-up review fix)

Author: ndycode

lib/accounts.ts

@@ -1375,6 +1375,14 @@ export class AccountManager {
 			return true;
 		}
 
+		// Track successor accounts that we explicitly retarget onto because the
+		// removed account was the caller's "current" for a given family. We
+		// stamp each such successor with lastSwitchReason="rotation" so the
+		// retarget is auditable instead of silently carrying whatever stale
+		// reason the successor already had (HI-04). De-duplicated across
+		// families because lastSwitchReason is per-account, not per-family.
+		const retargetedSuccessors = new Set<number>();
+
 		for (const family of MODEL_FAMILIES) {
 			// Cursor: shift down if it was past the removed index, then normalize
 			// into [0, length). If the cursor was pointing AT the removed slot
@@ -1397,18 +1405,37 @@ export class AccountManager {
 			// AT the removed slot (or is now dangling off the end after
 			// splice), advance to the next enabled account instead of
 			// defaulting to -1. Fall back to -1 only when every remaining
-			// account is disabled or the pool is empty.
+			// account is disabled or the pool is empty. When we retarget off
+			// the removed slot onto a different account, record the successor
+			// so we can explicitly signal the retarget via lastSwitchReason.
 			let active = priorActive[family];
+			const activeWasRemoved = active === idx;
 			if (active > idx) {
 				active -= 1;
-			} else if (active === idx) {
+			} else if (activeWasRemoved) {
 				// Same numeric position now hosts the successor account.
 				active = this.findNextEnabled(Math.min(idx, this.accounts.length - 1));
 			}
 			if (active >= this.accounts.length) {
 				active = this.findNextEnabled(0);
 			}
 			this.currentAccountIndexByFamily[family] = active;
+			if (activeWasRemoved && active >= 0 && active < this.accounts.length) {
+				retargetedSuccessors.add(active);
+			}
+		}
+
+		// Stamp retarget signal on each successor account that replaced a
+		// removed "current" pointer. This mirrors the existing rotation
+		// convention used by setActiveIndex / markSwitched, so downstream
+		// callers observing lastSwitchReason see a clear audit trail that the
+		// pool re-chose this account rather than the user selecting it
+		// themselves.
+		for (const successorIdx of retargetedSuccessors) {
+			const successor = this.accounts[successorIdx];
+			if (successor) {
+				successor.lastSwitchReason = "rotation";
+			}
 		}
 
 		return true;

test/accounts.test.ts

@@ -3078,6 +3078,122 @@ describe("AccountManager", () => {
 			});
 		});
 
+		describe("removed-current retarget signal (HI-04)", () => {
+			// PR #413 hardened removeAccount() to avoid the pointer-dangle
+			// default-to-minus-one behavior, but it silently retargeted the
+			// current pointer onto a different account when the removed
+			// account WAS the current one. HI-04 flagged that silent
+			// retarget: callers observing getCurrentAccountForFamily()
+			// receive an account they never selected with no audit trail.
+			//
+			// The fix stamps lastSwitchReason="rotation" on the successor
+			// whenever removeAccount() retargets off the removed slot. This
+			// mirrors the convention already used by setActiveIndex() and
+			// markSwitched() for pool-driven selection events.
+
+			it("stamps lastSwitchReason='rotation' on successor when current is removed", () => {
+				const now = Date.now();
+				const stored = {
+					version: 3 as const,
+					activeIndex: 1,
+					activeIndexByFamily: { codex: 1 },
+					accounts: [
+						{ refreshToken: "token-1", addedAt: now, lastUsed: now, lastSwitchReason: "initial" as const },
+						{ refreshToken: "token-2", addedAt: now, lastUsed: now, lastSwitchReason: "initial" as const },
+						{ refreshToken: "token-3", addedAt: now, lastUsed: now, lastSwitchReason: "initial" as const },
+					],
+				};
+
+				const manager = new AccountManager(undefined, stored as never);
+				const active = manager.getCurrentAccountForFamily("codex");
+				expect(active?.refreshToken).toBe("token-2");
+
+				// Remove the currently active (middle) account. The successor
+				// (token-3) shifts into index 1 and becomes the new current.
+				manager.removeAccount(active!);
+
+				const after = manager.getCurrentAccountForFamily("codex");
+				expect(after?.refreshToken).toBe("token-3");
+				// Successor must be explicitly stamped with the rotation
+				// reason so callers can tell the pool retargeted them rather
+				// than assuming they still hold the account they originally
+				// selected.
+				expect(after?.lastSwitchReason).toBe("rotation");
+
+				// The untouched account (token-1) retains its prior reason,
+				// proving we only stamp the successor the retarget landed on.
+				const untouched = manager.getAccountByIndex(0);
+				expect(untouched?.refreshToken).toBe("token-1");
+				expect(untouched?.lastSwitchReason).toBe("initial");
+			});
+
+			it("does not stamp a successor when every remaining account is disabled", () => {
+				const now = Date.now();
+				const stored = {
+					version: 3 as const,
+					activeIndex: 2,
+					activeIndexByFamily: { codex: 2 },
+					accounts: [
+						{ refreshToken: "token-1", addedAt: now, lastUsed: now, enabled: false, lastSwitchReason: "initial" as const },
+						{ refreshToken: "token-2", addedAt: now, lastUsed: now, enabled: false, lastSwitchReason: "initial" as const },
+						{ refreshToken: "token-3", addedAt: now, lastUsed: now, enabled: true, lastSwitchReason: "initial" as const },
+					],
+				};
+
+				const manager = new AccountManager(undefined, stored as never);
+				const active = manager.getCurrentAccountForFamily("codex");
+				expect(active?.refreshToken).toBe("token-3");
+
+				// Remove the last enabled account; remaining pool is entirely
+				// disabled. Pointer must fall to -1 (no routable account).
+				manager.removeAccount(active!);
+
+				expect(manager.getCurrentAccountForFamily("codex")).toBeNull();
+				expect(manager.getActiveIndexForFamily("codex")).toBe(-1);
+
+				// Neither surviving (disabled) account may be silently
+				// promoted by having rotation stamped on them. Their stored
+				// reason must stay "initial".
+				for (let i = 0; i < manager.getAccountCount(); i++) {
+					const acc = manager.getAccountByIndex(i);
+					expect(acc?.lastSwitchReason).toBe("initial");
+				}
+			});
+
+			it("stamps the sole enabled successor when all other peers are disabled", () => {
+				const now = Date.now();
+				const stored = {
+					version: 3 as const,
+					activeIndex: 0,
+					activeIndexByFamily: { codex: 0 },
+					accounts: [
+						{ refreshToken: "token-1", addedAt: now, lastUsed: now, enabled: true, lastSwitchReason: "initial" as const },
+						{ refreshToken: "token-2", addedAt: now, lastUsed: now, enabled: false, lastSwitchReason: "initial" as const },
+						{ refreshToken: "token-3", addedAt: now, lastUsed: now, enabled: true, lastSwitchReason: "initial" as const },
+					],
+				};
+
+				const manager = new AccountManager(undefined, stored as never);
+				const active = manager.getCurrentAccountForFamily("codex");
+				expect(active?.refreshToken).toBe("token-1");
+
+				// Remove the currently active account (token-1). token-2 is
+				// disabled, so findNextEnabled must skip it and land on
+				// token-3 (the single remaining enabled peer).
+				manager.removeAccount(active!);
+
+				const after = manager.getCurrentAccountForFamily("codex");
+				expect(after?.refreshToken).toBe("token-3");
+				expect(after?.lastSwitchReason).toBe("rotation");
+
+				// The disabled peer that was skipped during retarget must
+				// keep its original reason — we only stamp the account the
+				// pointer actually lands on.
+				const disabledPeer = manager.getAccountByIndex(0);
+				expect(disabledPeer?.refreshToken).toBe("token-2");
+				expect(disabledPeer?.lastSwitchReason).toBe("initial");
+			});
+		});
 	describe("flushPendingSave", () => {
 		it("flushes pending debounced save", async () => {
 			const { saveAccounts } = await import("../lib/storage.js");