Skip to content

fix(routing): reserve configured provider names before registry aliases - #3935

Closed
luvs01 wants to merge 1 commit into
lidge-jun:devfrom
luvs01:agent/provider-alias-ownership-20260908
Closed

fix(routing): reserve configured provider names before registry aliases#3935
luvs01 wants to merge 1 commit into
lidge-jun:devfrom
luvs01:agent/provider-alias-ownership-20260908

Conversation

@luvs01

@luvs01 luvs01 commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Summary

When a configured provider is named agy, the catalog could still advertise Google Antigravity models as agy/<model>. Selecting that name resolves to the configured provider, while the case variant AGY/<model> could activate Google's built-in alias instead.

Treat configured provider names as case-insensitive claims when selecting built-in aliases, both in catalog generation and the router. Google's catalog entries then retain google-antigravity/<model>. Exact canonical names, explicit aliases, and the existing fallback for unrecognized prefixes keep their current precedence.

Carry only the two source changes and regression from 5ad6b053274b73edf0d9b3f31544fa0e21e4a34c, preserving the original author. Extend the regression to agy and AgY owners and a case-variant request that exercises the router's fallback branch. Document the behavior in the existing provider configuration pages in all five locales.

Verification

Head 7ad74746ed260b441ddbceb7a46204085945aa3e, based on dev 942c028735d39b2ad410b1baa95670984e16576d.

  • Bun 1.4.0 on Windows: bun test tests/providers/provider-model-aliases.test.ts tests/lab/core-lab-boundary.test.ts --timeout 60000 passed 34 tests with 96 assertions.
  • Removing only the catalog ownership condition made both new cases fail because agy reappeared as Google's provider alias. Removing only the router ownership condition made both fail because AGY/<model> resolved through Google's explicit namespace. Each ablation restored the original bytes and verified SHA-256.
  • bun run typecheck, bun run privacy:scan, and git diff HEAD^ HEAD --check passed.
  • cd docs-site && bun run build built 425 pages. Text checks verified the new section and canonical example in the generated page for every locale, without opening previews.
  • The final rebase changed only three pre-existing dashboard files. The focused source/test, complete documentation tree, manifest and lockfile were unchanged; typecheck and privacy checks were rerun after the rebase.
  • Independent read-only review of the final eight-file change found no required correction or additional regression. It confirmed that canonical names remain case-sensitive and unrecognized prefixes retain their existing fallback.
  • bun run test:changed --timeout 60000 reached the wrapper's 900-second suite deadline and exited 124 without a completed selection summary; this is not a passing result. The focused checks above passed separately. Full cross-platform CI completed successfully with 26/26 jobs on this exact head. CodeRabbit completed review of this head with no actionable findings and there are no unresolved review threads. Readiness is supported by this exact-head cross-platform CI; the separate local 900-second limit remains recorded above.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

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.

Summary by CodeRabbit

  • Bug Fixes

    • Provider names that conflict with built-in short aliases now take precedence consistently in model routing and provider catalog displays.
    • Alias matching handles case variations correctly while preserving exact matching for canonical provider names.
    • Unrecognized provider prefixes continue to use the existing model-routing fallback behavior.
  • Documentation

    • Added guidance in multiple localized documentation versions explaining provider namespace aliases, conflicts, routing behavior, and case sensitivity.

@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Team

Run ID: 6653f731-3abc-4a84-b4a0-a168baf71645

📥 Commits

Reviewing files that changed from the base of the PR and between 942c028 and 7ad7474.

📒 Files selected for processing (8)
  • docs-site/src/content/docs/ja/reference/configuration/providers.md
  • docs-site/src/content/docs/ko/reference/configuration/providers.md
  • docs-site/src/content/docs/reference/configuration/providers.md
  • docs-site/src/content/docs/ru/reference/configuration/providers.md
  • docs-site/src/content/docs/zh-cn/reference/configuration/providers.md
  • src/providers/default-aliases.ts
  • src/router.ts
  • tests/providers/provider-model-aliases.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

The provider alias system now treats case-insensitive configured provider names as claims on conflicting built-in aliases. Catalog generation and model routing use the configured provider name, while canonical names remain case-sensitive. Tests and localized documentation cover the behavior.

Changes

Provider namespace alias handling

Layer / File(s) Summary
Provider name alias claims
src/providers/default-aliases.ts, src/router.ts
The alias claim checks now consider configured provider names, case-insensitively, in addition to explicit aliases. Conflicting registry aliases are suppressed during catalog generation and model routing.
Alias behavior validation and documentation
tests/providers/provider-model-aliases.test.ts, docs-site/src/content/docs/reference/configuration/providers.md, docs-site/src/content/docs/ja/reference/configuration/providers.md, docs-site/src/content/docs/ko/reference/configuration/providers.md, docs-site/src/content/docs/ru/reference/configuration/providers.md, docs-site/src/content/docs/zh-cn/reference/configuration/providers.md
Tests cover agy, AgY, canonical catalog names, routing, alias suppression, and AGY fallback. Documentation describes the same behavior in five locales.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 7ad74

Configured provider names now claim conflicting built-in aliases case-insensitively while preserving exact canonical-name matching and unrecognized-prefix fallback behavior. Catalog and routing behavior are covered by regression tests, with no current merge-blocking risk identified.

Suggested reviewers: lidge-j

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 3 files. (5 skipped: 5 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
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 Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: configured provider names take precedence over registry aliases during routing. It matches the routing changes, regression tests, and documen…
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 3 files. (5 skipped: 5 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@luvs01

luvs01 commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 66 / 80

이 PR은 설정한 공급자 이름이 레지스트리 기본 축약명과 대소문자만 다르게 겹칠 때, 카탈로그와 라우터가 그 축약명을 다른 공급자(예: Google Antigravity의 agy)에 계속 붙여 주던 구멍을 막습니다. 지금 dev HEAD는 942c02873(팁 #3931 GUI 서브에이전트 roster/fallback 분리, package 2.48.0 open-dev after #3925)입니다. HEAD의 src/providers/default-aliases.ts effectiveProviderAliassrc/router.ts Pass 2는 “다른 공급자의 alias 필드가 그 축약명을 가져갔는지”만 봤습니다. 그래서 공급자 키 자체를 agy/AgY로 만든 경우에는 레지스트리 축약이 살아남아, 카탈로그에는 agy/<model>이 Google 쪽으로 보이고, 선택하면 설정 공급자로 가거나 대소문자 변형(AGY/...)은 레지스트리 쪽으로 새는 식의 불일치가 났습니다. 이 브랜치는 소유 판정에 “설정된 공급자 이름(대소문자 무시)”을 같은 조건으로 넣고, 문서(en/ko/ja/ru/zh-cn providers.md)와 tests/providers/provider-model-aliases.test.tsagy/AgY 소유·AGY/ 폴백 회귀를 추가합니다. 원본 커밋 5ad6b0532의 저자 크레딧을 유지한 carry이고, 베이스는 현재 tip과 같습니다. types/config 분할과 무관하고 close-don't-rebase 대상이 아닙니다.

동작 순서는 문서와 코드가 같이 말합니다. 정식(canonical) 공급자 이름은 여전히 대소문자 완전 일치가 필요하고, Pass 1의 명시적 provider.alias가 이깁니다. Pass 2 레지스트리 축약은 “다른 키/별칭이 그 이름을 이미 가져간 경우”에만 끕니다. 인식되지 않는 접두사는 기존 default-provider 폴백을 탑니다. 회귀는 Google 모델의 providerAlias가 null이고 표시 이름이 google-antigravity/...로 남는지, agy/...는 설정 공급자로 가는지, AGY/...는 레지스트리 축약을 다시 켜지 않고 기본 공급자 폴백으로 가는지까지 한 번에 확인합니다. 범위는 aliases·router·테스트·5로케일 문서뿐이라 릴리스 트레인(#3932/#3933 main 쪽 로스터 보정)과도 겹치지 않습니다. 다만 아직 draft이고 hygiene/label/resolve-pr 쪽 체크가 이 head에서 다시 도는 중이라, 제품 코드는 작아도 “ready” 전에 CI 초록이 맞습니다.

라인 src/providers/default-aliases.ts effectiveProviderAlias - claimedByOther에 name.toLowerCase() === lower를 넣어, 공급자 키 자체가 레지스트리 축약을 가져간 경우에도 축약을 끈다. 예전엔 p.alias만 봤다.
라인 src/router.ts Pass 2 registryMatches - 카탈로그와 같은 소유 규칙을 라우터에도 맞춘다. Pass 1(명시 alias)·canonical exact match·unknown prefix 폴백 순서는 그대로다.
경로 tests/providers/provider-model-aliases.test.ts - agy/AgY 각각에 대해 Google 표시명·라우팅·AGY/ 폴백(routeReason default-provider)을 고정한다. 회귀 핵심이다.
경로 docs-site .../configuration/providers.md (5로케일) - 운영자가 읽는 계약 문장을 코드와 같이 올린다. 동작 설명용이지 새 설정 키는 없다.
경로 PR draft / CI - MERGEABLE이지만 draft+게이트 재실행 중. Windows에서 aliases·lab-boundary만 돌린 작성자 증거는 있고, 호스트 Cross-platform 전체는 이 exact head 기준으로 더 봐야 한다.

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

  • 공급자 키로 축약을 ‘가져가는’ 정책을 대소문자 무시로 둘지(이번 PR), 아니면 키도 exact-case만 소유로 인정할지
  • AGY/...를 기본 공급자 폴백(모델 id에 슬래시 포함)으로 두는 기존 계약을 유지할지, 대소문자만 다른 요청을 설정 공급자로 접을지
  • draft를 언제 ready로 올릴지(작성자 로컬 초점 테스트 vs 전체 Cross-platform)

너의 추천
의도·범위가 분명한 #3531/#3601 후속 버그 픽스다. draft 유지한 채 이 head CI가 초록이면 ready 후 dev 머지. 키 소유를 대소문자 무시로 두는 현재 선택이 문서·테스트와 일치하니, 바꾸려면 테스트부터 다시 써야 한다. types/config 분할과 무관하니 close-don't-rebase 대상이 아니다.

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

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

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

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

✅ READY

  • all PR quality gates passed; the review readiness checklist is complete.

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.

This pull request is already Ready for Review.
The review-ready label marks this PR as ready; review automation runs independently.
Maintainers: @lidge-jun @Ingwannu

Hygiene

Deterministic PR hygiene checks passed.

@lidge-jun

Copy link
Copy Markdown
Owner

Carried without source changes to current dev via #3966, landed bbea77a. The original patch-id and runtime/test blobs match; intervening documentation is preserved. luvs01 remains the source author and a landed Co-authored-by. Exact-head CI34169304167 passed19 jobs with2explicit skips, including agy/AgY and warm-cache regressions. Exact-head remote Bun1.4.0 docs build produced425pages without deployment. Full landed tree matches the expected merge result. Closing this original as carried, not directly merged.

@lidge-jun lidge-jun closed this Sep 7, 2026
@lidge-jun lidge-jun added the landed-via-maintainer Original PR closed after landing via a maintainer merge train label Sep 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working landed-via-maintainer Original PR closed after landing via a maintainer merge train review-ready

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants