Improve Kimi K2 credential errors#1917
Conversation
|
Codex review: needs maintainer review before merge. Reviewed July 9, 2026, 1:54 PM ET / 17:54 UTC. Summary Reproducibility: yes. from source inspection: current main sends the raw API key after only checking the trimmed value for emptiness and lacks a dedicated 401 invalid-credentials path. The PR adds injected-transport tests for the missing-key, trimmed-key, 401, and 403 paths without requiring live credential probes. Review metrics: 2 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review detailsBest possible solution: Merge the focused fetcher/test/docs change after steipete accepts the Kimi K2 status-code semantics and required checks pass. Do we have a high-confidence way to reproduce the issue? Yes, from source inspection: current main sends the raw API key after only checking the trimmed value for emptiness and lacks a dedicated 401 invalid-credentials path. The PR adds injected-transport tests for the missing-key, trimmed-key, 401, and 403 paths without requiring live credential probes. Is this the best way to solve the issue? Yes, if maintainers accept the status-code mapping; the patch keeps the fix in the shared API-token seam and Kimi K2 fetcher with focused regression coverage. A stricter provider-specific classifier is the safer alternative only if maintainers want more than 401 versus body-preserving non-200 handling. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 3b039d15e125. Label changesLabel justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (13 earlier review cycles; latest 8 shown)
|
5954ec4 to
8cc104d
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8cc104d8f0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| case 401, 403: | ||
| throw KimiK2UsageError.invalidCredentials |
There was a problem hiding this comment.
Keep 403 credit responses out of credential failures
For a valid key with no credits or insufficient permissions, the Kimrel/Kimi K2 docs list HTTP 403 as an “insufficient credits or permissions” response and also document GET /api/user/credits as the usage endpoint (https://kimrel.com/api-docs). This branch now turns that quota/permission state into “API key is invalid or expired” and discards the response body that previously reached apiError, so affected users will be told to replace credentials instead of topping up or fixing permissions; keep 403 in the API-error path or classify it from the error body.
Useful? React with 👍 / 👎.
8cc104d to
f682a0a
Compare
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
f682a0a to
8428895
Compare
Co-authored-by: JoeVenner <ylafrimi@gmail.com>
…keys Map only HTTP 401 to invalidCredentials. Kimrel returns 403 for insufficient credits or missing permissions, so keep 403 (and other non-200 responses) on the body-preserving apiError path so users with a valid key see the real actionable error instead of a misleading 'invalid or expired' message. Split the status-mapping test into 401 invalidCredentials and 403 body-preserving cases.
Co-authored-by: JoeVenner <ylafrimi@gmail.com>
Co-authored-by: JoeVenner <ylafrimi@gmail.com>
Co-authored-by: JoeVenner <ylafrimi@gmail.com>
39ca87c to
5b58bdc
Compare
Co-authored-by: joeVenner <ylafrimi@gmail.com>
Summary
Missing Kimi K2 API key.instead of the generic no-strategy error.Verification
Exact candidate:
60c8663fb607f9544b4093f56620970123c85dc3swift test --filter 'APITokenFetchStrategyTests|KimiK2UsageFetcherTests': 17 tests passed.make check: passed; SwiftFormat and SwiftLint report zero violations.make test: all 50/50 local shards passed.autoreview --mode local: clean; no accepted/actionable findings.CodexGitCommit=60c8663f; Developer ID signature, deep/strict verification, and Gatekeeper assessment passed.Missing Kimi K2 API key.;Kimi K2 API key is invalid or revoked.;CodexBar Debugstatus item.Public Model Identifier Gate: PASS (model-free diff).
Dependency freshness: PASS (no dependency changes).
Remaining live-proof blocker
A fresh key was created from the authenticated Kimrel API Keys page and stored through the scoped QA credential workflow. The correctly shaped key is currently rejected with HTTP 401 by both the documented legacy credit endpoint and the current Kimrel credit endpoint; the current chat endpoint reaches the service but returns HTTP 500. Because the required authenticated HTTP 200 credit response could not be obtained, this PR is intentionally not being merged yet.