Skip to content

fix(accounts): honor autoSwitchThreshold=0 as disabled - #5091

Merged
lidge-jun merged 2 commits into
lidge-jun:devfrom
luvs01:fix/autoswitch-threshold-zero
Sep 19, 2026
Merged

lidge-jun merged 2 commits into
lidge-jun:devfrom
luvs01:fix/autoswitch-threshold-zero

Conversation

@luvs01

@luvs01 luvs01 commented Sep 18, 2026 •

Copy link
Copy Markdown
Collaborator

Motivation

A generic OAuth provider's autoSwitchThreshold uses 0 as the stored "off" value, but isOverAutoSwitchThreshold had no non-positive guard, so 100 - headroom >= 0 was always true. With fill-first routing, a threshold explicitly stored as 0 (disabled) therefore read as exceeded on every request and proactively rotated requests — and billing — onto another pooled account.

Description

  • Add a non-positive guard in isOverAutoSwitchThreshold so threshold <= 0 always returns false, disabling proactive usage-based switching while leaving reactive recovery (429/401 driven) unchanged (src/oauth/generic-account-failover.ts).
  • Update CLI status logic to treat a stored zero threshold as off when the pool is live and to render a clearer human reason for the off state (src/cli/account-extended.ts).
  • Add focused regression tests asserting fill-first holds the active account when autoSwitchThreshold is 0 and that the CLI status JSON reports a zero threshold as disabled (tests/oauth/generic-oauth-failover.test.ts, tests/cli/cli-account-pool-verbs.test.ts).
  • Update structure docs to record the invariant (structure/config.md, structure/providers/xai-grok.md, structure/transports/inventory.md).

Tests

  • bun test tests/oauth/generic-oauth-failover.test.ts — 45 pass, 0 fail.
  • bun test tests/cli/cli-account-pool-verbs.test.ts — 30 pass, 0 fail.
  • bun run typecheck — clean.
  • bun run structure:check — passed.

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

  • All CI tests are green on my local testing.

  • I pushed my PR to the latest dev commit.

  • I resolved all correct Codex and CodeRabbit findings.

  • My PR is ready for review.

@coderabbitai

coderabbitai Bot commented Sep 18, 2026 •

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 50b2dd65-1c2b-43f1-800a-9df30c4b6aed

📥 Commits

Reviewing files that changed from the base of the PR and between d1745ee and d8351c7.

📒 Files selected for processing (6)
  • src/cli/account-extended.ts
  • src/oauth/generic-account-failover.ts
  • structure/providers/xai-grok.md
  • structure/transports/inventory.md
  • tests/cli/cli-account-pool-verbs.test.ts
  • tests/oauth/generic-oauth-failover.test.ts
 _______________________________________________
< Dyson of code review: I suck up all the bugs. >
 -----------------------------------------------
  \
   \   \
        \ /\
        ( )
      .( o ).
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

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.

@github-actions github-actions Bot added the intake: hygiene-blocked Deterministic PR hygiene checks failed label Sep 18, 2026
@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Deterministic hygiene checks failed.

  • unsponsored_surface — This changes an authentication, workflow, release-automation, or dependency surface. MAINTAINERS.md requires security review for these; ask a maintainer to apply maintainer-sponsored once they have reviewed it. Paths: src/oauth/generic-account-failover.ts.

@github-actions github-actions Bot added the bug Something isn't working label Sep 18, 2026
@github-actions

github-actions Bot commented Sep 18, 2026 •

Copy link
Copy Markdown
Contributor

⏳ DRAFT

  • hygiene: unsponsored_surface.

What to do

  • Fix unsponsored_surface — This changes an authentication, workflow, release-automation, or dependency surface. MAINTAINERS.md requires security review for these; ask a maintainer to apply maintainer-sponsored once they have reviewed it. Paths: src/oauth/generic-account-failover.ts.

Review readiness checklist

  • ✅ All CI tests are green on my local testing.
  • ✅ I pushed my PR to the latest dev commit.
  • ✅ I resolved all correct Codex and CodeRabbit findings.
  • ✅ My PR is ready for review.

✅ 4/4 boxes ticked.

Automatic draft conversion failed (token cannot change draft status). Please convert this pull request to a draft manually. The required enforce-target check will keep failing until every issue above is resolved.

@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 56 / 80

계정을 자동으로 바꾸지 말라고 0을 저장했는데, 그 0이 "다 썼으니 바꿔"로 읽히던 버그를 고친 PR이에요.

여러 계정을 모아 쓰는 generic OAuth 풀의 fill-first는 지금 계정을 쓰다가, 사용량이 기준을 넘으면 다음 계정으로 넘겨요. 기준 숫자 autoSwitchThreshold에서 0은 끄기예요. Codex 풀과 Anthropic 풀은 이미 0이면 안 넘겨요. generic 풀의 isOverAutoSwitchThreshold만 그 확인이 없었어요. 식은 100 - 남은 비율 >= 기준인데, 기준이 0이면 남은 비율이 0에서 100 사이인 한 항상 참이에요. 끄기로 둔 계정이 요청마다 다른 계정으로 넘어가고, 그 계정에 과금이 붙었어요.

고친 곳은 한 줄이에요. 기준이 0 이하면 넘지 않았다고 보고 지금 계정을 유지해요. 429나 401이 나서 복구하는 길은 그대로예요. 상태 명령도 풀이 켜져 있어도 저장된 0을 꺼짐으로 보여 줘요. 테스트 두 개와 문서가 이 규칙을 적어요. 베이스는 dev예요. 같은 구멍을 막는 다른 열린 PR은 없어요.

structure/config.md - 설정 그룹 표 바로 밑에 CLI 상태 문단이 들어갔어요. 이번에 고친 규칙은 src/oauth/generic-account-failover.ts의 비교이고, 같은 문장은 이미 structure/transports/inventory.md에 있어요. "명령이 0을 저장한다"는 이번 diff 전에 있던 동작이에요. 이 문단은 빼는 편이 맞아요.

tests/cli/cli-account-pool-verbs.test.ts - JSON의 enabled: false만 봐요. 화면에 찍는 usage-based switching disabled는 테스트가 없어요. 동작은 맞고, 문구만 비어 있어요.

메인테이너의 판단이 필요한 지점

아직 draft예요. src/oauth/generic-account-failover.ts를 건드려 hygiene가 unsponsored_surface로 실패해요. 코드가 틀린 게 아니라 인증 쪽 파일이라, 메인테이너가 보고 maintainer-sponsored를 붙여야 게이트가 풀려요. enforce-target 실패도 그 이유예요.

너의 추천

가드 한 줄은 Codex, Anthropic과 같은 뜻이고, 사용량 100% 테스트가 예전 버그를 잡아요. 문서 문단만 정리하고 스폰서 라벨을 단 뒤 머지하면 돼요. 닫을 중복 PR은 없어요.

이 댓글은 grok-bot이 작성했습니다

@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 62 / 80

이 PR은 로그인 계정을 여러 개 묶어 쓸 때, "사용량이 이 퍼센트면 다음 계정으로 넘겨라"를 0으로 저장하면 그 기능이 꺼지게 고칩니다.

범용 풀(xAI, Antigravity 같은 OAuth 계정)이 fill-first이면, 지금 계정의 사용량이 저장해 둔 숫자를 넘을 때 다음 계정으로 바꿉니다. 끄는 값은 0입니다. CLI에서 끄면 0이 저장됩니다. Codex 풀과 Anthropic 풀은 이미 0이면 사용량 때문에 옮기지 않습니다. 범용 풀만 그 확인이 없었습니다.

남은 사용량을 숫자로 알고 있으면 예전 식은 100 - 남은양 >= 0이었습니다. 남은 양이 0에서 100 사이면 이 식은 항상 맞습니다. 거의 안 쓴 계정도, 다 쓴 계정도 "이미 넘었다"가 됩니다. 끄라고 넣어 둔 0이 요청마다 다른 계정으로 일을 넘겼고, 요금도 그 계정에 붙었습니다. 서버가 429나 401로 거절해서 옮기는 길은 이 숫자와 따로입니다. 이번 수정은 그 길을 그대로 둡니다.

고친 코드는 isOverAutoSwitchThreshold 맨 앞 한 줄입니다. 숫자가 0 이하면 바로 거짓이라, 사용량으로는 안 옮깁니다. 고르는 쪽은 저장값이 0부터 100 사이의 정수일 때만 그 숫자를 쓰고, 아니면 기본 80을 씁니다. 그래서 실제로 이 가드에 걸리는 저장값은 0입니다. CLI 상태는 풀이 살아 있고 저장값이 0이면 켜짐이 아니라 꺼짐으로 보여 줍니다. 사람용 한 줄에는 usage-based switching disabled라고 적습니다. 테스트는 사용량 100%여도 지금 계정을 유지하는지, JSON에 enabled: false가 나오는지 봅니다. 문서 세 곳도 같은 규칙을 적습니다.

베이스는 dev입니다. types.ts와 config.ts는 안 바꿉니다. 이 버그를 같이 고치려는 다른 열린 PR은 없습니다. 본문 체크는 네 칸이 모두 켜져 있지만 PR은 아직 드래프트입니다. 위생 검사가 src/oauth/generic-account-failover.ts를 인증 쪽으로 보고, maintainer-sponsored 라벨이 없기 때문입니다. 본문에 적힌 테스트 숫자는 이 리뷰에서 다시 실행하지 않았습니다.

structure/config.md - 새 문단이 설정 묶음 표 바로 아래, 환경 변수 문단 앞에 있습니다. 키 이름 oauthAccountFailover.autoSwitchThreshold는 없습니다. 동작 설명은 structure/transports/inventory.md에 이미 있습니다. 표만 읽는 사람은 어느 설정인지 알기 어렵습니다. 동작이 틀린 것은 아닙니다.

메인테이너의 판단이 필요한 지점

  • 위생 관문이 이 파일을 인증 표면으로 봅니다. 바뀐 것은 "0이면 사용량으로 안 넘긴다"는 한 줄입니다. maintainer-sponsored를 달고 드래프트를 풀지는 메인테이너 몫입니다.
  • structure/config.md 문단을 그 자리에 둘지. 없어도 코드가 따르는 규칙은 inventory에 있습니다.

너의 추천

코드는 이대로 둬도 됩니다. 0을 꺼짐으로 읽는 쪽이 Codex, Anthropic과 같고, 테스트가 그 경우를 고정합니다. types.ts/config.ts 분할과 무관하니 이 PR을 닫지 마세요. 중복으로 닫을 PR도 없습니다.

드래프트를 풀기 전에 maintainer-sponsored만 달면 위생 실패가 사라집니다. 문서 문단은 옮겨도 되고, 머지를 막을 이유는 아닙니다.

이 댓글은 grok-bot이 작성했습니다

luvs01 and others added 2 commits September 19, 2026 21:33
Also drop the config.md paragraph that duplicated the transports inventory rule without naming the oauthAccountFailover.autoSwitchThreshold key.
@lidge-jun
lidge-jun force-pushed the fix/autoswitch-threshold-zero branch from 43dc193 to d8351c7 Compare September 19, 2026 12:40
@luvs01

luvs01 commented Sep 19, 2026

Copy link
Copy Markdown
Collaborator Author

Author note: the hygiene/enforce-target gates fail with unsponsored_surface — the change touches the account/failover surface, so it needs the maintainer-sponsored label before it can leave draft. The two review findings (stale doc paragraph, missing non-JSON output assertion) are addressed at head 43dc193.

@lidge-jun
lidge-jun marked this pull request as ready for review September 19, 2026 17:26
@lidge-jun
lidge-jun merged commit 5d0122a into lidge-jun:dev Sep 19, 2026
6 of 10 checks passed
@luvs01
luvs01 deleted the fix/autoswitch-threshold-zero branch September 19, 2026 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working intake: hygiene-blocked Deterministic PR hygiene checks failed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants