fix(accounts): signal removed-current instead of silent retarget (HI-04)

PR #413 hardened removeAccount pointer normalization so the current
pointer no longer defaults to -1 when the active account is removed
while other accounts remain in the pool. It did so by retargeting the
pointer onto the successor account at the post-splice slot, which
silently substitutes a different account for the caller's "current"
without any audit trail.

HI-04 (from .sisyphus/notepads/deep-audit/reports/accounts-rotation.json)
flagged this as a correctness issue: getCurrentAccountForFamily()
returns an account the caller never selected, with no lastSwitchReason
indicating that the pool chose it. This masks pool-driven retargets in
logs, dashboards, and session-affinity consumers that key on
lastSwitchReason to distinguish user-selected from pool-selected
identities.

Fix: when removeAccount retargets the active pointer off the removed
slot onto a new successor (priorActive[family] === idx), stamp
lastSwitchReason="rotation" on that successor. This mirrors the
existing convention already used by setActiveIndex() and markSwitched()
for pool-driven selection events, so downstream observers see a
consistent retarget signal instead of the successor's stale prior
reason.

Successors are deduped across families (lastSwitchReason is per-account,
not per-family) via a Set, and the signal is only applied when we
actually retargeted off the removed slot. If the pool collapses to no
routable account (every remaining peer disabled), no successor is
stamped and the pointer falls to -1 — matching the "no routable
account" contract PR #413 established.

Tests (test/accounts.test.ts "removed-current retarget signal (HI-04)"):
- removing the currently active (middle) account stamps rotation on
  the successor and leaves untouched peers with their prior reason
- removing the last enabled account when every remaining peer is
  disabled yields pointer=-1 and does NOT stamp any disabled peer
- removing the currently active account with exactly one other enabled
  peer (plus a disabled peer that findNextEnabled must skip) lands on
  the enabled successor and stamps rotation only on it

Source: .sisyphus/notepads/deep-audit/reports/accounts-rotation.json HI-04

Author: ndycode

lib/accounts.ts

@@ -1316,8 +1316,14 @@ 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<ModelFamily, number> = {} as Record<ModelFamily, number>;
-		const priorActive: Record<ModelFamily, number> = {} as Record<ModelFamily, number>;
+		const priorCursor: Record<ModelFamily, number> = {} as Record<
+			ModelFamily,
+			number
+		>;
+		const priorActive: Record<ModelFamily, number> = {} as Record<
+			ModelFamily,
+			number
+		>;
 		for (const family of MODEL_FAMILIES) {
 			priorCursor[family] = this.cursorByFamily[family];
 			priorActive[family] = this.currentAccountIndexByFamily[family];
@@ -1336,6 +1342,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
@@ -1358,18 +1372,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

@@ -2890,6 +2890,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");