fix(auth): resolve kimi-coding-cn pool base_url for sk-kimi- keys - #69409
fix(auth): resolve kimi-coding-cn pool base_url for sk-kimi- keys#69409theone139344 wants to merge 1 commit into
Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for identifying the environment-pool/runtime mismatch. The implementation matches the existing runtime routing rule: current main only resolves kimi-coding in agent/credential_pool.py:2617, while hermes_cli/auth.py:6954 already resolves both Kimi provider IDs.
Problems
- The PR has no regression coverage for the changed pool path.
tests/hermes_cli/test_api_key_providers.py:916tests_resolve_kimi_base_url()in isolation, but it does not cover_seed_from_env("kimi-coding-cn", ...), where this defect occurs.
Suggested changes
- Add a focused test that seeds
KIMI_CN_API_KEY=sk-kimi-...through_seed_from_envand asserts the created entry usesKIMI_CODE_BASE_URL; retain a legacy-key assertion for the CN default endpoint.
Automated hermes-sweeper review.
| # kimi-coding-cn's ProviderConfig default is moonshot.cn; without resolve, | ||
| # pool persists the wrong base_url → Anthropic /v1/messages 404/401. | ||
| # (Carried local fix; upstream only special-cases kimi-coding.) | ||
| if provider in {"kimi-coding", "kimi-coding-cn"}: | ||
| base_url = _resolve_kimi_base_url(token, pconfig.inference_base_url, env_url) |
There was a problem hiding this comment.
Please add a regression test for this pool-specific branch: seed KIMI_CN_API_KEY=sk-kimi-... via _seed_from_env("kimi-coding-cn", entries) and assert the entry uses KIMI_CODE_BASE_URL. Existing helper tests do not exercise pool seeding.
SummarySeven PRs address or reference the Kimi endpoint-routing complex: #5566, #5669, and #7554 target initial environment-pool seeding; #18694 targets stale persisted entries; #69409 extends environment-pool seeding to kimi-coding-cn; and #10525/#10534 cover related CN runtime resolution. The evidence shows that persisted-pool normalization and CN environment-pool seeding are distinct remaining paths, while the original kimi-coding seeding fix is already represented on main by commit 4d1b988. Related pull requests
Duplicates#5566 and the kimi-coding portion of #5669 are functionally identical; #7554 repeats that same routing change while adding separate token-sanitization logic. #18694 and #69409 are complementary, not duplicates: they cover persisted-entry loading and environment-pool seeding respectively; #10534 is the merged runtime-resolution subset of #10525. Suggested consolidationKeep #69409 open with a salvage path: add the contributor-requested _seed_from_env("kimi-coding-cn", ...) regression tests for both KIMI_CODE_BASE_URL selection and legacy CN-default preservation. Keep #18694 open separately as the recorded best fix for #5908, but require it to preserve non-default stored URLs, integrate with current profile/global fallback semantics, and add pool-to-runtime coverage; leave #5566, #5669, #7554, and #10525 closed, with #10534 retained as the merged CN runtime reference. Cross-PR triage: Reviewed 7 pull requests and 2 issues in this complex. Diffs were read for 5 of 7 PRs (rest unavailable); Assessment working set: 12 kB of PR diffs, 15 kB of issue/PR text, 5 kB of discussion (11 comments), 10 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
_seed_from_env only called _resolve_kimi_base_url for kimi-coding, so kimi-coding-cn pool entries stayed on moonshot.cn and sk-kimi- keys got 401. Mirror the global+cn set used by resolve_api_key_provider_credentials.
82fe56a to
c9dc828
Compare
|
Rebased onto latest Why still needed: Local acceptance (live chat):
Please re-run CI. |
Summary
kimi-coding-cnwithsk-kimi-keys, resolve the correct Coding APIbase_urlso requests do not hit the wrong host.Test plan
kimi-coding-cn+sk-kimi-…resolvesapi.kimi.com/coding(or configured CN endpoint)kimi-codingpath unchanged