fix(pin): keep a manual pin pointing at its account across account removal by ndycode · Pull Request #638 · ndycode/codex-multi-auth

ndycode · 2026-07-23T19:50:06Z

HIGH — a manual pin silently routes to the wrong account after a removal

chooseAccount honors pinnedAccountIndex by position. But the two account-removal fixups remap activeIndex/activeIndexByFamily and never touch pinnedAccountIndex, and the loader only range-checks the pin (not identity). So once accounts shift, a stale in-range pin points at a different account — and an out-of-range pin returns null → the pinned-path suppresses stale-state recovery → the pool wedges with a 503.

The dangerous path is automatic and silent: the runtime account-check removes accounts whose refresh tokens were revoked, with no user action.

Scenario A (silent mis-route): pool [A,B,C,D], user pins index 2 (C). A's token is revoked → auto-removed → pool [B,C,D], disk pin still 2. On the next proxy load the pin (2 < 3, still "valid") routes every request to D — an account the user never pinned. No error surfaces, and it persists across reloads because the loader keeps a stale in-range pin.

Scenario B (wedge): pool [A,B,C], pin=2 (C, last). Remove B → [A,C], disk pin still 2 → 2 >= count → chooseAccount returns null; because the pin is set, stale-state recovery is suppressed → 503 "pinned account unavailable" until the user manually unpins.

Fix

Both removal sites now follow the pinned account by identity via a new reconcilePinnedAccountIndex(pinnedAccount, nextAccounts) helper — capture the pinned account before the change, then re-resolve its new index afterward (or clear the pin when the account is gone, never leave it dangling):

lib/codex-manager/login-menu-actions.ts — interactive delete (single splice); the in-memory menu view is also kept in sync.
lib/runtime/account-check.ts — the automatic revoked-token removal (multi-account filter). This path already invalidates the account-manager cache, so the reconciled pin is picked up on reload.

MEDIUM — proxy hot-path pin reader accepted pins every other reader rejects

readStorageMetaFromDisk (lib/runtime/rotation-storage-meta.ts) truncated the pin with only a Number.isFinite check, omitting the Number.isInteger / >= 0 validation that readPinAndGenFromDisk, the loader, and the zod schema all enforce — and it's the value the proxy actually routes on. A negative or non-integer disk pin slipped straight into chooseAccount and wedged the pool. It now rejects non-integer/negative pins to null, matching the other readers.

Tests

test/login-menu-actions.test.ts: deleting a lower-indexed account shifts the pin to keep pointing at the same account; deleting the pinned account clears the pin.
test/runtime-account-check.test.ts: the silent deep-probe auto-removal re-resolves the pin by identity.
test/issue-474-pin-safety.test.ts: readStorageMetaFromDisk rejects negative/non-integer pins; unit coverage for reconcilePinnedAccountIndex (including the multi-account-removal case).

Full suite green: 5,192 passed, 3 skipped, 0 failed; typecheck + lint clean.

Not included (residual, lower confidence)

The removal fixups do not bump affinityGeneration, so a concurrently running proxy's sticky session affinity isn't invalidated for the reshuffled pool (and restore-from-backup persists the backup's pin/gen verbatim). That interacts with the #474 compare-and-set and is a separate, lower-severity concern — deferred to avoid reintroducing the clobber bug that mechanism prevents. The pin correctness (the HIGH) is fully fixed here.

Context: found by a coverage-gap audit of the AccountManager selection path — the existing pin tests only ever add accounts, never delete below a pin.

🤖 Generated with Claude Code

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 position-based pin drift — both removal sites (interactive delete in login-menu-actions.ts and auto-removal in account-check.ts) now capture the pinned account by identity before mutation and call reconcilePinnedAccountIndex after, so the pin follows its account or is cleared cleanly. a secondary fix aligns the proxy hot-path validator in readStorageMetaFromDisk with the other readers, rejecting negative/fractional disk pins instead of truncating them into chooseAccount.

new reconcilePinnedAccountIndex helper in storage.ts wraps findMatchingAccountIndex; returns undefined (not a stale slot) when the pinned account is gone.
replaceManageActionStorage now syncs pinnedAccountIndex to the in-memory view so the cli menu and disk stay in sync after a delete.
test coverage closes both scenario a (shift) and scenario b (clear) for interactive delete, deep-probe auto-removal, the hot-path validator, and the reconcile helper directly.

Confidence Score: 5/5

safe to merge — the identity-capture + reconcile pattern is correct at both removal sites, pin is never left dangling, and the hot-path validator change is strictly more conservative

both removal sites correctly capture the pinned account before the list mutation and re-resolve by identity after; the reconcile helper delegates to the well-tested findMatchingAccountIndex with a clean undefined-clear path; the validator tightening in readStorageMetaFromDisk removes a pre-existing gap with no correctness regression on normal integer pins; tests cover shift, clear, and no-pin paths end-to-end including the previously flagged scenario b

no files require special attention; the acknowledged affinityGeneration/concurrency gap is pre-existing and correctly deferred

Important Files Changed

Filename	Overview
lib/storage.ts	adds `reconcilePinnedAccountIndex` — a clean thin wrapper over `findMatchingAccountIndex` that follows a captured account by identity and returns `undefined` (clear) when the account is absent; type-safe, correctly thin
lib/codex-manager/login-menu-actions.ts	captures pinned account by identity before the splice inside `withAccountStorageTransaction`, reconciles after, and syncs the reconciled pin to the in-memory view via `replaceManageActionStorage`; ordering is correct
lib/runtime/account-check.ts	captures pinned account before the multi-account filter and reconciles after; `storageChanged = true` ensures `saveAccounts` is called so the pin hits disk; pre-existing concurrency/affinity-generation concern is acknowledged and deferred
lib/runtime/rotation-storage-meta.ts	replaces the `Math.trunc`+`isFinite` path with `Number.isInteger`+`>= 0`, matching `readPinAndGenFromDisk`; negative/fractional disk pins now return `null` instead of reaching `chooseAccount`
test/login-menu-actions.test.ts	adds shift and clear path tests for interactive delete; asserts both the persisted value and the in-memory `storage.pinnedAccountIndex` after `replaceManageActionStorage`
test/runtime-account-check.test.ts	adds two deep-probe tests: pin shift (lower account revoked) and pin clear (pinned account revoked); addresses the previously flagged missing scenario B coverage
test/issue-474-pin-safety.test.ts	adds unit tests for `readStorageMetaFromDisk` pin validation (negative/fractional rejection) and `reconcilePinnedAccountIndex` (shift, clear, no-op when pin undefined)

Sequence Diagram

sequenceDiagram
    participant Caller
    participant RemovalSite as login-menu-actions / account-check
    participant Storage as storage.ts
    participant Disk as openai-codex-accounts.json
    participant Proxy as rotation-storage-meta (hot path)

    Caller->>RemovalSite: delete account / auto-remove revoked token
    RemovalSite->>Storage: "read pinnedAccount = accounts[pinnedAccountIndex]"
    note over RemovalSite: capture by identity BEFORE mutation
    RemovalSite->>Storage: splice / filter accounts
    RemovalSite->>Storage: reconcilePinnedAccountIndex(pinnedAccount, nextAccounts)
    Storage-->>RemovalSite: new index (shifted) OR undefined (cleared)
    RemovalSite->>Disk: saveAccounts(workingStorage)
    RemovalSite->>RemovalSite: replaceManageActionStorage (in-memory sync)
    Proxy->>Disk: readStorageMetaFromDisk on next request
    note over Proxy: isInteger + >= 0 guard (fixed) negative/fractional to null
    Proxy-->>Caller: routes to correct account (or no pin)

_{Reviews (2): Last reviewed commit: "test(pin): cover the pin-clear path when..." | Re-trigger Greptile}

…moval Account removal fixups remapped activeIndex/activeIndexByFamily but never touched pinnedAccountIndex, and the loader only range-checks the pin (not identity). Because chooseAccount honors the pin by POSITION, a stale in-range pin silently routed EVERY request to the wrong account, and an out-of-range pin wedged the pool with a 503. The dangerous path is automatic: runtime account-check removes accounts whose refresh tokens were revoked with no user action, leaving the pin dangling. Both removal sites (interactive delete in login-menu-actions and the auto-removal filter in runtime account-check) now follow the pinned account by IDENTITY via reconcilePinnedAccountIndex — re-resolving its new index, or clearing the pin when the account is gone — and the in-memory menu view is kept in sync. Also tighten the proxy hot-path pin reader (readStorageMetaFromDisk): it accepted negative/non-integer pins that every other reader (readPinAndGenFromDisk, the loader, the zod schema) rejects, and it is the value the proxy actually routes on — a malformed disk pin reached chooseAccount and wedged the pool. It now rejects non-integer/negative pins to null, matching the other readers. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XfmQjR1jh1pb6n4YPiHDsJ

chatgpt-codex-connector · 2026-07-23T19:50:11Z

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.

coderabbitai · 2026-07-23T19:50:26Z

📝 Walkthrough

Walkthrough

the diff adds identity-based pinned-account reconciliation for login deletion and runtime account filtering, strict persisted-index validation, and regression tests. windows-specific edge cases and concurrency behavior are not covered by the added tests.

Changes

pinned account safety

Layer / File(s)	Summary
pin contract and validation `lib/storage.ts:1041`, `lib/runtime/rotation-storage-meta.ts:134`, `test/issue-474-pin-safety.test.ts:500`	`reconcilePinnedAccountIndex` resolves pins by account identity, while persisted pins require non-negative integers. tests cover matching, clearing, and validation.
login deletion reconciliation `lib/codex-manager/login-menu-actions.ts:256`, `lib/codex-manager/login-menu-actions.ts:339`, `test/login-menu-actions.test.ts:268`	account deletion re-resolves the pin after removal and synchronizes it into menu storage. tests cover shifted and deleted pins.
runtime filter reconciliation `lib/runtime/account-check.ts:327`, `test/runtime-account-check.test.ts:459`	runtime filtering re-resolves or clears the pin after invalid accounts are removed, with persisted ordering assertions.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

ndycode/codex-multi-auth#475: adds the persisted pinned-account routing and affinity behavior used by this lifecycle handling.

Suggested labels: bug

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	the title matches the change, but the summary is 74 characters and exceeds the 72-character limit.	shorten the summary to 72 characters or fewer while keeping the conventional-commit format.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description check	✅ Passed	the description covers the fix, validation, and risk notes, but it misses the template's exact headings and docs/governance checklist.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/pin-integrity-on-removal

✨ Simplify code

Create PR with simplified code
Commit simplified code in branch fix/pin-integrity-on-removal

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

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

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/runtime-account-check.test.ts`:
- Around line 460-508: The existing regression test covers index shifting after
removing a lower account but not removal of the pinned account itself. Add a
test alongside the current case using the runtime account-check flow and
fixtures where the pinned account has a flaggable refresh failure, then assert
the `saveAccounts` payload persists `pinnedAccountIndex` as undefined after the
account is filtered out.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: be621651-268c-4cd0-a558-66c842e98347

📥 Commits

Reviewing files that changed from the base of the PR and between a35f7c8 and fa63d4d.

📒 Files selected for processing (7)

lib/codex-manager/login-menu-actions.ts
lib/runtime/account-check.ts
lib/runtime/rotation-storage-meta.ts
lib/storage.ts
test/issue-474-pin-safety.test.ts
test/login-menu-actions.test.ts
test/runtime-account-check.test.ts

📜 Review details

⏰ Context from checks skipped due to timeout. (1)

GitHub Check: Greptile Review

🧰 Additional context used

📓 Path-based instructions (15)

**/*.{ts,js}

📄 CodeRabbit inference engine (AGENTS.md)

Use ESM only ("type": "module"), Node >= 18.17

Files:

lib/storage.ts
test/login-menu-actions.test.ts
test/runtime-account-check.test.ts
lib/runtime/rotation-storage-meta.ts
lib/runtime/account-check.ts
lib/codex-manager/login-menu-actions.ts
test/issue-474-pin-safety.test.ts

**/*.ts

📄 CodeRabbit inference engine (AGENTS.md)

Do not use as any, @ts-ignore, or @ts-expect-error in TypeScript files

Files:

lib/storage.ts
test/login-menu-actions.test.ts
test/runtime-account-check.test.ts
lib/runtime/rotation-storage-meta.ts
lib/runtime/account-check.ts
lib/codex-manager/login-menu-actions.ts
test/issue-474-pin-safety.test.ts

{lib,scripts}/**/*.{ts,js}

📄 CodeRabbit inference engine (AGENTS.md)

Windows filesystem safety: retry transient EBUSY/EPERM/ENOTEMPTY cleanup and write failures where tests cover Windows locks

Files:

lib/storage.ts
lib/runtime/rotation-storage-meta.ts
lib/runtime/account-check.ts
lib/codex-manager/login-menu-actions.ts

**/*.{js,ts,mjs,cjs}

📄 CodeRabbit inference engine (README.md)

**/*.{js,ts,mjs,cjs}: Keep npm installation scripts side-effect-free; postinstall may print a short notice but must not modify runtime state or perform setup, especially in CI or non-interactive installs.
Never run npm install or update commands automatically; only display a manual upgrade notice when appropriate.
Do not publish or take ownership of a global codex binary; preserve the official OpenAI installation as the owner of the codex command.
Keep runtime rotation and local bridge services loopback-only, and protect local bridge access with hashed client tokens.
Keep OAuth credentials and account state local; do not send them to external services as part of normal account management.
Treat Responses background mode as opt-in: requests with background: true must use stateful store=true, while default stateless routing uses store=false.
Use bounded outbound request budgets, avoid whole-pool replay when every account is rate-limited, and enter cooldown after repeated cross-account 5xx bursts.
Make experimental synchronization and backup flows non-destructive by default: preview before applying sync, preserve destination-only accounts, and fail safely on backup filename collisions.

Files:

lib/storage.ts
test/login-menu-actions.test.ts
test/runtime-account-check.test.ts
lib/runtime/rotation-storage-meta.ts
lib/runtime/account-check.ts
lib/codex-manager/login-menu-actions.ts
test/issue-474-pin-safety.test.ts

lib/**/*.ts

📄 CodeRabbit inference engine (lib/AGENTS.md)

lib/**/*.ts: Route all public exports through lib/index.ts or documented package subpaths.
Keep module dependencies acyclic and preserve the layering types/constants → storage → accounts → runtime → manager/CLI; lower layers must not import higher layers.
Preserve runtime rotation pass-through semantics except for intentionally changed auth or provider headers.
Deduplicate emails using normalizeEmailKey(), which trims and lowercases the email.
Use classes for state requiring multiple independent instances or dependency injection, including AccountManager, CircuitBreaker, SessionAffinityStore, and the CodexError hierarchy. Reserve module-level state for genuinely process-global concerns and provide a test reset helper for such state.
Never import from dist/ in source tests or library code.
Never suppress type errors.
Never patch official Codex application binaries for desktop routing.
Never use bare recursive cleanup in Windows-sensitive paths without retry handling.

Files:

lib/storage.ts
lib/runtime/rotation-storage-meta.ts
lib/runtime/account-check.ts
lib/codex-manager/login-menu-actions.ts

lib/{storage/**/*.ts,storage.ts,runtime-paths.ts}

📄 CodeRabbit inference engine (lib/AGENTS.md)

lib/{storage/**/*.ts,storage.ts,runtime-paths.ts}: Resolve project storage identity with resolveProjectStorageIdentityRoot; never derive project pools directly from raw worktree paths.
Never key project storage directly by worktree path.

Files:

lib/storage.ts

lib/**

⚙️ CodeRabbit configuration file

focus on auth rotation, windows filesystem IO, and concurrency. verify every change cites affected tests (vitest) and that new queues handle EBUSY/429 scenarios. check for logging that leaks tokens or emails.

Files:

lib/storage.ts
lib/runtime/rotation-storage-meta.ts
lib/runtime/account-check.ts
lib/codex-manager/login-menu-actions.ts

test/**/*.test.ts

📄 CodeRabbit inference engine (test/AGENTS.md)

test/**/*.test.ts: Write Vitest test suites with globals enabled (describe, it, expect)
Maintain 80%+ coverage threshold across statements, branches, functions, and lines
Use removeWithRetry() for Windows filesystem cleanup instead of bare fs.rm to handle EBUSY, EPERM, and ENOTEMPTY errors
Do not rely on dist/ in tests; use source files instead
Do not skip tests without justification
Relax lint rules for test files as configured in eslint.config.js

Files:

test/login-menu-actions.test.ts
test/runtime-account-check.test.ts
test/issue-474-pin-safety.test.ts

{scripts/**/*.js,test/**/*.ts}

📄 CodeRabbit inference engine (AGENTS.md)

Do not use bare recursive delete logic in Windows-sensitive scripts/tests without retry handling

Files:

test/login-menu-actions.test.ts
test/runtime-account-check.test.ts
test/issue-474-pin-safety.test.ts

test/**

⚙️ CodeRabbit configuration file

tests must stay deterministic and use vitest. demand regression cases that reproduce concurrency bugs, token refresh races, and windows filesystem behavior. reject changes that mock real secrets or skip assertions.

Files:

test/login-menu-actions.test.ts
test/runtime-account-check.test.ts
test/issue-474-pin-safety.test.ts

lib/runtime/**/*.ts

📄 CodeRabbit inference engine (AGENTS.md)

Do not expose account emails or tokens in runtime proxy client response headers or logs

Files:

lib/runtime/rotation-storage-meta.ts
lib/runtime/account-check.ts

{lib/runtime/**/*.ts,lib/policy/**/*.ts}

📄 CodeRabbit inference engine (AGENTS.md)

Runtime rotation is default-on through codexRuntimeRotationProxy; users can opt out with codex-multi-auth rotation disable or CODEX_MULTI_AUTH_RUNTIME_ROTATION_PROXY=0

Files:

lib/runtime/rotation-storage-meta.ts
lib/runtime/account-check.ts

lib/{storage,runtime}/**/*.ts

📄 CodeRabbit inference engine (AGENTS.md)

Local project-owned state defaults to ~/.codex/multi-auth; official Codex state remains under ~/.codex

Files:

lib/runtime/rotation-storage-meta.ts
lib/runtime/account-check.ts

lib/{runtime-rotation-proxy.ts,runtime/**/*.ts}

📄 CodeRabbit inference engine (lib/AGENTS.md)

Runtime rotation must fail open to normal official Codex forwarding when startup helpers are unavailable.

Files:

lib/runtime/rotation-storage-meta.ts
lib/runtime/account-check.ts

{scripts/*.js,lib/codex-manager/**/*.ts}

📄 CodeRabbit inference engine (AGENTS.md)

Canonical package name is codex-multi-auth; canonical command family is codex-multi-auth ...

Files:

lib/codex-manager/login-menu-actions.ts

🧠 Learnings (2)

📚 Learning: 2026-06-04T06:14:18.093Z

Learnt from: ndycode
Repo: ndycode/codex-multi-auth PR: 510
File: test/scheduling-strategy-config.test.ts:1-1
Timestamp: 2026-06-04T06:14:18.093Z
Learning: In ndycode/codex-multi-auth, do not flag explicit imports from "vitest" (e.g., describe, it, expect, beforeEach/afterEach, etc.) in test files as issues—even if the Vitest config sets `globals: true`. The repo’s established convention is to keep these imports for consistency with neighboring tests; removing them would make files outliers.

Applied to files:

test/login-menu-actions.test.ts
test/runtime-account-check.test.ts
test/issue-474-pin-safety.test.ts

📚 Learning: 2026-06-04T06:14:24.975Z

Learnt from: ndycode
Repo: ndycode/codex-multi-auth PR: 510
File: test/runtime-rotation-proxy.test.ts:2478-2491
Timestamp: 2026-06-04T06:14:24.975Z
Learning: In ndycode/codex-multi-auth test files (e.g. `test/*.test.ts`), when creating V3 storage fixtures for accounts, it’s an intentional convention to use `as never` for deliberately minimal stored-account objects that only include `refreshToken`, `addedAt`, and `lastUsed`. Do not treat `as never` here as a type-safety problem: optional/other fields are expected to be populated by the runtime during execution, and the cast is used solely to keep the fixture minimal and consistent across existing tests.

Applied to files:

test/login-menu-actions.test.ts
test/runtime-account-check.test.ts
test/issue-474-pin-safety.test.ts

🔇 Additional comments (6)

lib/storage.ts (1)

1041-1061: LGTM!

lib/runtime/rotation-storage-meta.ts (1)

134-143: LGTM!

test/issue-474-pin-safety.test.ts (1)

18-18: LGTM!

Also applies to: 501-560

lib/codex-manager/login-menu-actions.ts (1)

10-10: LGTM!

Also applies to: 256-258, 339-352

test/login-menu-actions.test.ts (1)

269-309: LGTM!

lib/runtime/account-check.ts (1)

5-9: LGTM!

Also applies to: 327-341

…emoved The existing account-check regression only covered a LOWER account being revoked (pin shifts down to keep following its account). This adds the other runtime outcome: the pinned account is itself the one whose refresh token is revoked, so it is filtered out and the pin must be CLEARED rather than left dangling at a stale slot that now belongs to a different account. Verified non-vacuous: swapping reconcilePinnedAccountIndex for a naive index clamp fails this test with 'expected 1 to be undefined'. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XfmQjR1jh1pb6n4YPiHDsJ

Four independent bugs, each verified against current code before being fixed and each shipped with a regression test that fails without the fix. 1. Token and cost budget caps were inert (HIGH) Nothing ever wrote token counts into the usage ledger: every `usageRecorder.record()` call omitted them, so `summarizeUsageLedger` totalled zero and `evaluateBudgetGuard` compared `0 >= limit` for both `maxTokens` and `maxCostUsd`. Those caps could never fire — `budget set --cost 50` allowed unlimited spend and only `--requests` enforced anything. Add `lib/usage/usage-extraction.ts` and scan the forwarded body for the upstream `usage` object: line-by-line for SSE (only the current partial line is retained, so a multi-gigabyte stream is never buffered) and a capped accumulate-then-parse for JSON bodies. `forwardStreamingResponse` takes an optional chunk observer to feed it. The mapping is not a straight copy. `estimateUsageCostUsd` treats `outputTokens` and `reasoningTokens` as disjoint buckets and sums them, while OpenAI reports `output_tokens` INCLUSIVE of `output_tokens_details.reasoning_tokens`, so the reasoning share is subtracted out before recording or every reasoning request is billed twice. `input_tokens_details.cached_tokens` is left raw because the pricing function already subtracts it. Scope: the runtime rotation proxy only. The plugin-host path in index.ts still records zero tokens — its ledger row is written in a `finally` that runs when the handler returns the Response, which for a streaming request is before the client has drained the body and therefore before the upstream usage event exists. Deferring that write needs a stream-end hook and a fallback for clients that disconnect early, which is its own change. 2. Session affinity survived account removal and backup restore (MED) Session affinity maps a session to an account INDEX, and `affinityGeneration` is what invalidates it. Only `switch`, `best` and `unpin` bumped it. Deleting an account from the login menu, the AUTOMATIC removal of revoked-token accounts in the runtime account check, and restoring a backup all reshuffle indexes without bumping — so a live proxy kept gluing in-flight sessions to whatever slid into the old slot. Same failure class as the pin-by-position bug fixed in #638; reconciling the pin was never enough, because the affinity map is separate state. Add `bumpStorageAffinityGeneration` (max of the in-memory and on-disk counters, so a concurrent CLI process's increment is not lost) and call it from all three paths. 3. Two identity-less accounts shared one account-policy entry (MED) `getAccountPolicyKey` hashed the literal "unknown" when an account had neither an accountId nor an email, so every such account collapsed onto one key: pausing, draining or tagging one applied to all of them. Fall back to the refresh token, namespaced before hashing so it cannot collide with an email or accountId of the same text, mirroring `getAccountIdentityKey`'s `allowRefreshFallback`. Only the digest is persisted, and the key stays index-independent. `RuntimePolicyAccount` gains `refreshToken` so the runtime derives the same key the CLI writes; both existing call sites already pass full account objects, so nothing else changes. 4. withStreamingFailover ignored consumer backpressure (MED) `pump()` runs as a free-running loop from `start()` and enqueued every chunk as fast as upstream delivered it, never consulting `controller.desiredSize` — so a slow client buffered the entire response in memory. Verified: a 50-chunk source was drained completely for a consumer that had read exactly one chunk. Park the loop on a demand signal released by `pull()`, and release it on cancel so a parked pump unwinds instead of holding the upstream reader. The sibling forwarder in stream-failover-runtime.ts already had this via `waitForDrain`. Verification: full suite 5514 passed / 2 failed, both pre-existing native `codex` PATH-discovery cases in test/codex-bin-wrapper.test.ts that reproduce with these changes stashed. Typecheck and ESLint clean. The backpressure and usage-recording tests were mutation-checked: each fails when its fix is reverted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FRwXjYGfeTs8qZD4nS7MZk

greptile-apps Bot reviewed Jul 23, 2026

View reviewed changes

Comment thread test/runtime-account-check.test.ts

Comment thread lib/runtime/account-check.ts

coderabbitai Bot requested changes Jul 23, 2026

View reviewed changes

Comment thread test/runtime-account-check.test.ts

coderabbitai Bot approved these changes Jul 23, 2026

View reviewed changes

ndycode merged commit f73b7f7 into main Jul 23, 2026
2 checks passed

ndycode mentioned this pull request Aug 25, 2026

fix: repair five latent runtime defects (inert budget caps on both paths, stale session affinity, policy key collision, stream backpressure) #678

Merged

5 tasks

Navigation Menu

fix(pin): keep a manual pin pointing at its account across account removal - #638

fix(pin): keep a manual pin pointing at its account across account removal#638
ndycode merged 2 commits into
mainfrom
fix/pin-integrity-on-removal

ndycode commented Jul 23, 2026 •

edited by greptile-apps Bot

Loading

Uh oh!

chatgpt-codex-connector Bot commented Jul 23, 2026

Uh oh!

coderabbitai Bot commented Jul 23, 2026 •

edited

Loading

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

ndycode commented Jul 23, 2026 • edited by greptile-apps Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

HIGH — a manual pin silently routes to the wrong account after a removal

Fix

MEDIUM — proxy hot-path pin reader accepted pins every other reader rejects

Tests

Not included (residual, lower confidence)

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

Confidence Score: 5/5

Important Files Changed

Sequence Diagram

Uh oh!

chatgpt-codex-connector Bot commented Jul 23, 2026

Uh oh!

coderabbitai Bot commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

ndycode commented Jul 23, 2026 •

edited by greptile-apps Bot

Loading

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

coderabbitai Bot commented Jul 23, 2026 •

edited

Loading