Skip to content

fix(providers): rebase key failover on persisted state (carry of #3529) - #3546

Merged
lidge-jun merged 5 commits into
devfrom
codex/260905-carry-3529
Sep 4, 2026
Merged

fix(providers): rebase key failover on persisted state (carry of #3529)#3546
lidge-jun merged 5 commits into
devfrom
codex/260905-carry-3529

Conversation

@lidge-jun

Copy link
Copy Markdown
Owner

Summary

rotateKeyOn429 rebuilt the routed provider from the in-memory config, so a 429 rotation could resurrect a key the user had already removed from disk. This carry rebuilds from the committed provider row in src/providers/key-failover.ts, reapplies registry metadata, and preserves only the runtime fetch plus generated OpenCode session affinity. The author's final commit 92b4eda26 updates the JSDoc and structure/04_transports-and-sidecars.md that the CHANGES_REQUESTED review on 8b0327f4b asked for. core-lab-boundary stays green with the new src/router import.

Supersedes #3529 (maintainer carry: original is a contributor draft whose readiness checklist would reset on any push; carry = PR head merged with origin/dev).

Stack (wp1 merge train, independent layers — each targets dev):

# PR Layer Review focus
1 this carry of #3529 this PR's diff only

Unit: devlog/_plan/260905_open_work_closeout/ (010, 011).

Verification

  • bun run typecheck — exit 0 on the carry head.
  • bun test tests/adapters/key-failover.test.ts tests/lab/core-lab-boundary.test.ts — 33 pass / 0 fail; bun test tests/adapters/openai/openai-chat-native-policy.test.ts tests/providers/openrouter-provider-routing.test.ts tests/server/terminal-guard-server.test.ts tests/server/server-combo-failover-e2e.test.ts — 145 pass / 0 fail (three failover tests are RED on dev before the change).
  • Exact-head CI on this branch is the merge gate.

Checklist

  • Targets dev
  • Focused regression test present and green
  • Original author credited via Co-authored-by trailer

Co-authored-by: yansigit 44089734+yansigit@users.noreply.github.com

@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner September 4, 2026 22:42
@lidge-jun lidge-jun added the bug Something isn't working label Sep 4, 2026
@coderabbitai

coderabbitai Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

Next included review available in 12 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Team

Run ID: 8178699d-be31-4e2c-b121-8b58fc3a724c

📥 Commits

Reviewing files that changed from the base of the PR and between 980a9fb and 7c922af.

📒 Files selected for processing (7)
  • src/providers/key-failover.ts
  • structure/04_transports-and-sidecars.md
  • tests/adapters/key-failover.test.ts
  • tests/adapters/openai/openai-chat-native-policy.test.ts
  • tests/providers/openrouter-provider-routing.test.ts
  • tests/server/server-combo-failover-e2e.test.ts
  • tests/server/terminal-guard-server.test.ts

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 commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@lidge-jun

Copy link
Copy Markdown
Owner Author

리뷰 · 우선순위 74 / 80

설명

이 PR은 API 키 풀에서 429가 났을 때, “어느 설정 스냅샷을 믿고 키를 돌릴지”를 고치는 유지자 캐리입니다. 원본은 기여자 PR #3529(yansigit)이고, closeout 문서 devlog/_plan/260905_open_work_closeout/001_lane_bug_prs_a.md006_dispositions.md는 이미 #3529를 LAND_AS_IS로 적어 두었습니다. 원본은 DRAFT 체크리스트·CHANGES_REQUESTED 리뷰 때문에 머지 열차에 올리기 애매했고, 이 #3546은 최종 커밋 92b4eda26까지 포함한 헤드를 현재 origin/dev 위에 다시 올린 캐리입니다.

지금 dev HEAD는 d6b457462(#3538로 260905 open-work closeout 로드맵 유닛만 열림)입니다. HEAD의 src/providers/key-failover.ts rotateKeyOn429는 요청이 들고 있는 메모리 config.providers[name]을 바로 고친 뒤 saveConfigPreservingClaudeCode로 통째 저장합니다. 같은 시각에 관리 API·다른 요청이 디스크에서 키를 빼 버렸거나 이미 다른 키로 돌려 놓았다면, 지운 키를 다시 살리거나 건강한 키를 한 번 더 돌릴 수 있습니다. 이 버그는 카탈로그·가격·릴레이와는 무관하지만, 키 풀을 쓰는 운영에서 실제 사고로 이어질 수 있는 축입니다.

고치는 방법은 두 갈래입니다. 첫째, rotateKeyOn429mutatePersistedConfig 콜백 안에서만 디스크 fresh provider의 풀·활성 키를 보고 다음 키를 고릅니다. 저장이 성공(또는 unchanged로 채택)한 뒤에야 쿨다운을 올리고, 메모리 config.providers도 커밋된 행으로 맞춥니다. persist가 unavailable이면 쿨다운도 안 올리고 null을 돌려, “디스크는 안 바뀌었는데 메모리만 키가 식은” 상태를 막습니다. 둘째, rotateProviderTransportOn429는 예전처럼 { ...routedProvider, apiKey }만 하던 것을 버리고, 커밋된 행을 routedProviderConfig로 다시 정규화한 뒤, 런타임에만 있는 fetch와 OpenCode 세션 헤더(OPENCODE_GO_SESSION_HEADER / x-opencode-session)만 되살립니다. 중간 커밋에서 “persisted가 request-time을 이긴다”는 방향이 한 번 흔들렸고, 8b0327f4b·92b4eda26이 그 계약을 테스트·JSDoc·structure/04_transports-and-sidecars.md에 고정했습니다.

회귀 테스트는 tests/adapters/key-failover.test.ts에 동시성·리베이스·persist 실패·삭제된 optional 필드 부활 금지 케이스를 넣고, 페일오버를 타는 openai-chat-native-policy / openrouter-provider-routing / server-combo-failover-e2e / terminal-guard-server에는 saveConfig·clearKeyCooldowns·임시 OPENCODEX_HOME을 맞춰 두었습니다. 본문대로 페일오버 관련 케이스는 지금 dev에서 빨갛고, 이 헤드에서는 초록입니다. core-lab-boundarysrc/router import를 허용한 채 통과한다고 적혀 있습니다. Co-authored-by로 원 작성자가 남아 있습니다.

wp1 Stack A merge train의 독립 LAND_AS_IS 레이어로 보이며, 형제 캐리 #3544(LaTeX)·#3545(instructions)와도 파일 충돌이 없습니다. 이 글을 쓰는 시점에 hygiene/gates/changes/label/test 1–2·keyring ubuntu·windows·npm-global 일부는 통과했고, test 3–4·macos·keyring macos 등은 아직 pending입니다. 머지 게이트는 exact-head CI 초록을 기다리는 게 맞습니다.

라인 - src/providers/key-failover.ts rotateKeyOn429 - 메모리 provider를 직접 고치고 saveConfigPreservingClaudeCode로 덮어쓰던 경로가, 디스크에서 이미 지운 키를 다시 살릴 수 있다 (이 PR이 고치는 핵심).
경로 rotateProviderTransportOn429 - 최종 헤드는 커밋 행만 routedProviderConfig에 넣고 fetch/세션 헤더만 보존한다. 요청 중 지운 headers·레지스트리 전용 필드가 부활하지 않는 테스트가 그 계약을 고정한다.
경로 쿨다운 시점 - 저장 성공(또는 unchanged 채택) 뒤에야 keyCooldowns를 올린다. unavailable 때 쿨다운을 안 올리는 테스트가 이 계약을 고정한다.
경로 src/router import - lab 경계상 providers → router는 허용 방향이고, closeout 001도 사이클 없음을 이미 확인했다.
경로 원본 #3529 - 아직 OPEN이다. 캐리가 먼저 들어가면 leftover 정리 규칙대로 닫아야 한다.

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

  • exact-head CI(특히 남은 test 3–4/macos)가 초록이 된 뒤 바로 머지할지, wp1 Stack A 형제(#3544/#3545 등)와 순서를 맞출지
  • 머지 후 원본 #3529Landed via #3546 at <commit> + landed-via-maintainer로 닫을지 (서있는 leftover 규칙과 동일)
  • CHANGES_REQUESTED였던 원본 스레드를 캐리 헤드 기준으로 dismiss할지, 원본을 닫을 때 함께 정리할지

너의 추천
exact-head CI가 초록이면 #3546을 머지한다. 머지 직후 #3529에 landed 댓글·라벨을 달고 completed/superseded로 닫는다. 런타임 보존 범위(fetch+세션 헤더만)는 테스트·문서가 이미 맞춰 있으니, 이 PR에서 더 막을 이유는 없다.

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

@Ingwannu Ingwannu left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security-boundary review completed at exact head 7c922afaf4ade3b4d0083282cfd82a3799922ee3.

The retry now mutates the schema-valid committed provider row under mutatePersistedConfig, publishes cooldown state only after a successful/unchanged persistence decision, adopts a concurrent healthy rotation instead of rotating twice, and never recreates a missing or invalid config. rotateProviderTransportOn429 routes that committed row again, so deleted fields stay deleted while registry metadata is restored; only the explicit runtime-only fetch executor and generated OpenCode session-affinity header survive. The regression matrix covers removed keys/fields, concurrent pool edits, stale handlers, unavailable persistence, and the affected response/chat/combo paths. The structure document now states the same precedence.

Cross-platform CI run 33926640849 completed successfully on this exact SHA, including all Linux shards, both macOS shards, keyring/npm jobs, gates, and storage/API-usage checks. No unresolved review thread remains. Approved.

@lidge-jun

Copy link
Copy Markdown
Owner Author

Maintainer admin merge (ruleset bypass recorded per MAINTAINERS.md): carry of #3529, exact-head CI green on 7c922af (24 pass / 2 skipped / 0 fail; core-lab-boundary green; the docs blocker from the CHANGES_REQUESTED review on 8b0327f is addressed by the author's 92b4eda). wp1 of the 260905 open-work closeout.

@lidge-jun
lidge-jun merged commit 583d6a9 into dev Sep 4, 2026
29 of 30 checks passed
@lidge-jun
lidge-jun deleted the codex/260905-carry-3529 branch September 4, 2026 23:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants