fix(accounts): normalize active pointer when the active account is disabled

[ndycode]

Summary

Normalizes the active-account pointer so it always references a routable (enabled) account. Previously getActiveIndexForFamily() only bounds-clamped the stored index, and setAccountEnabled(idx, false) set the enabled flag without repairing any active-index pointer that referenced the just-disabled slot. UI / automation / routing could therefore hold a stale pointer to a disabled account after a user toggled it off in the CLI dashboard.

Problem

Audit (docs/audits/MASTER_AUDIT.md §5 HIGH AUDIT-H10 / dim-D-routing.md D-05) demonstrated the dangling-pointer case:

1. User has accounts [0, 1, 2], active pointer = 1.
2. User disables account 1 via the settings-hub or codex auth command.
3. account.enabled = false but activeIndexByFamily.codex = 1 remains.
4. Next getActiveIndexForFamily("codex") returns 1 — a disabled account.

Consumers either returned a disabled ManagedAccount or forced an implicit skip inside the fetch loop — neither is easy to observe or explain.

Change

lib/accounts.ts getActiveIndexForFamily()

- if (index < 0 || index >= this.accounts.length) {
-   return this.accounts.length > 0 ? 0 : -1;
- }
- return index;
+ if (this.accounts.length === 0) return -1;
+ const clamped = index < 0 || index >= this.accounts.length ? 0 : index;
+ // "Active" must mean ROUTABLE, not merely in-range.
+ if (this.accounts[clamped]?.enabled !== false) return clamped;
+ for (let step = 1; step < this.accounts.length; step += 1) {
+   const candidate = (clamped + step) % this.accounts.length;
+   if (this.accounts[candidate]?.enabled !== false) return candidate;
+ }
+ return clamped;  // all disabled — let caller surface via unavailable-pool path

lib/accounts.ts setAccountEnabled(index, false)

Walks forward from the just-disabled slot to the next enabled account for each family whose activeIndexByFamily pointer matched index. No-op when enabled = true.

Test

Added a regression test in test/accounts.test.ts that exercises the disable-active-account flow and asserts the pointer advances to the next enabled slot:

it("normalizes active pointer to next enabled slot when active account is disabled (AUDIT-H10)", () => {
  // Set active = 1, disable account 1, expect active pointer to become 2
});

Verification

• Full suite: 225/225 files, 3419/3419 tests pass (+1 regression test)
• npm run typecheck exit 0
• npm run lint exit 0
• No new dependencies
• No new public API

Audit reference

• docs/audits/MASTER_AUDIT.md §5 HIGH (post-Oracle adjustment) → AUDIT-H10
• docs/audits/evidence/dim-D-routing.md D-05
• docs/audits/evidence/oracle-verdicts.md §1.1 (Oracle demoted H10 to MEDIUM; this PR still closes it at the implementation level)

Scope guarantees

• ✅ One concern — active-pointer normalization on disable + read
• ✅ All-disabled case preserves existing semantics (pointer stays clamped, callers surface via unavailable-pool path)
• ✅ No changes to activeIndex (the global pointer) — only activeIndexByFamily
• ✅ Full test suite green

Follow-up

Phase 1 continues with PR-G (release hygiene — AUDIT-H7 pack:check + AUDIT-H8 AGENTS.md regen + AUDIT-M31 tmp-file leakage). Tracked in .sisyphus/plans/phase1-implementation.md.

note: greptile review for oc-chatgpt-multi-auth. cite files like lib/foo.ts:123. confirm regression tests + windows concurrency/token redaction coverage.

Greptile Summary

fixes the dangling active-pointer bug (AUDIT-H10 / D-05) by patching two sites: getActiveIndexForFamily now defensively walks forward past disabled slots on every read, and setAccountEnabled(idx, false) eagerly normalizes both currentAccountIndexByFamily and cursorByFamily in lockstep so the stored pointer is repaired immediately rather than deferred to the next read.

the PR description's change snippet shows return clamped; for the all-disabled case, but the actual code (and all four new regression tests) correctly return -1 — the description is just a stale draft excerpt and doesn't affect correctness.

Confidence Score: 5/5

safe to merge — fix is correct, all-disabled edge case returns -1 as expected, no P0/P1 findings

all remaining findings are P2 — the only gap is that cursorByFamily normalization isn't directly exercised by a getCurrentOrNextForFamily call post-disable, but the skip-disabled loop in rotation masks any incorrectness there, and the four core AUDIT-H10 scenarios are well covered

test/accounts.test.ts — add a getCurrentOrNextForFamily assertion to cover the cursorByFamily normalization path

Important Files Changed

Filename	Overview
lib/accounts.ts	getActiveIndexForFamily now walks past disabled slots (defensive read) and setAccountEnabled eagerly repairs both currentAccountIndexByFamily and cursorByFamily; logic is sound and PR description's "return clamped" snippet is outdated — code correctly returns -1 for the all-disabled case
test/accounts.test.ts	four new regression tests cover AUDIT-H10 scenarios well; cursorByFamily normalization path in setAccountEnabled has no direct test exercising getCurrentOrNextForFamily post-disable

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["setAccountEnabled(idx, false)"] --> B["account.enabled = false"]
    B --> C{"currentAccountIndexByFamily matches idx?"}
    C -- yes --> D["findNextEnabled(idx)"]
    D --> E{found?}
    E -- yes --> F["update currentAccountIndexByFamily"]
    E -- no --> G["leave stale - all disabled"]
    C -- no --> H["skip"]
    F --> I{"cursorByFamily matches idx?"}
    H --> I
    G --> I
    I -- yes --> J["findNextEnabled(idx) again"]
    J --> K{found?}
    K -- yes --> L["update cursorByFamily"]
    K -- no --> M["leave stale"]
    I -- no --> N["skip"]

    P["getActiveIndexForFamily(family)"] --> Q{accounts empty?}
    Q -- yes --> R["return -1"]
    Q -- no --> S["clamp index to valid range"]
    S --> T{"accounts-at-clamped enabled?"}
    T -- yes --> U["return clamped"]
    T -- no --> V["walk forward N-1 steps"]
    V --> W{enabled candidate found?}
    W -- yes --> X["return candidate"]
    W -- no --> Y["return -1 sentinel"]

Loading

Prompt To Fix All With AI

This is a comment left during a code review.
Path: test/accounts.test.ts
Line: 3622-3660

Comment:
**`cursorByFamily` normalization not exercised by tests**

the new tests only call `getActiveIndexForFamily` after `setAccountEnabled`, which reads from `currentAccountIndexByFamily`. the companion normalization of `cursorByFamily` (also updated in lockstep in `setAccountEnabled`) is never directly verified.

a follow-on call to `getCurrentOrNextForFamily` after disabling the active account would confirm the cursor doesn't restart from the disabled slot — without it, the `cursorByFamily` branch in `setAccountEnabled` (lines 1289-1294 of `accounts.ts`) is dead to coverage. given that `getCurrentOrNextForFamily` already skips disabled accounts anyway, a stale cursor is masked; but the stated goal of the PR is to keep both pointers in sync, and that invariant isn't tested.

```ts
// add inside the "normalizes active pointer" test, after the first expect:
expect(manager.getCurrentOrNextForFamily("codex")?.refreshToken).toBe("token-2"); // cursor must start at 2, not 1
```

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (2): Last reviewed commit: "fix(accounts): harden pointer normalizat..." | Re-trigger Greptile}

[coderabbitai]

Warning

Rate limit exceeded

@ndycode has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 54 minutes and 18 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 54 minutes and 18 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 0cca00d9-ab37-4ba7-981f-b8666e041998

📥 Commits

Reviewing files that changed from the base of the PR and between 1f6da97 and d7af34d.

📒 Files selected for processing (2)

• lib/accounts.ts
• test/accounts.test.ts

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/active-pointer-normalization

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch fix/active-pointer-normalization

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[chatgpt-codex-connector]

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.