Skip to content

kimi: adopt peer-rotated credential on refresh 401 instead of clearing the login - #101

Open
flaviomartil wants to merge 1 commit into
raine:mainfrom
flaviomartil:fix/kimi-refresh-peer-rotation
Open

kimi: adopt peer-rotated credential on refresh 401 instead of clearing the login#101
flaviomartil wants to merge 1 commit into
raine:mainfrom
flaviomartil:fix/kimi-refresh-peer-rotation

Conversation

@flaviomartil

Copy link
Copy Markdown
Contributor

Problem

Kimi's OAuth refresh token rotates on every refresh. When another process that shares the same credential (most commonly the Kimi CLI itself, whose credential users seed into kimi/auth.json) refreshes first, the proxy's copy of the refresh token is invalidated. The next proxy refresh then fails with 401/403.

The current handling for 401/403 in KimiAuthManager::refresh_now clears the in-memory cache and deletes the stored credential (clear_auth()), forcing a manual re-login, even though the file on disk already holds a valid, newer credential written by the peer process.

Fix

On 401/403 from the refresh endpoint, reload the credential from disk once:

  • if the stored refresh token differs from the one that was just rejected, a peer rotated it: retry the refresh once with the newer token;
  • only clear the stored auth when the disk holds no newer credential, or when the newer credential is also rejected.

This mirrors the peer-rotation adoption that xaionaro-go/kimi-oauth-proxy implements against the same OAuth backend.

Tests

Two new unit tests in kimi/auth/manager.rs, using the existing test_http mock server and the CCP_KIMI_OAUTH_HOST override:

  • refresh_adopts_peer_rotated_credential_on_unauthorized: stale token gets 401, disk holds a newer credential, refresh succeeds with the peer token and persists the rotated result;
  • refresh_clears_auth_only_when_disk_has_no_newer_credential: 401 with an unchanged file still clears the login, preserving the existing re-authentication behavior.

cargo test --lib providers::kimi::auth: 7 passed.

The Kimi refresh token rotates on every refresh. When a peer process
sharing the credential (for example the Kimi CLI) refreshes first, the
proxy's copy is stale and the server rejects it with 401/403. The
previous handling cleared the stored login outright, forcing a manual
re-login even though the file on disk already held a valid, newer
credential.

Reload the credential from disk on 401/403 and retry once with the
newer refresh token before clearing. Auth is only discarded when the
disk holds no newer credential, or when the newer credential is also
rejected.
Copilot AI lite review requested due to automatic review settings August 5, 2026 11:57

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR improves the Kimi OAuth refresh flow in the proxy to handle refresh-token rotation across multiple processes sharing the same credential file, avoiding unnecessary forced re-logins when a peer has already rotated the token.

Changes:

  • Introduces a structured refresh error path to distinguish unauthorized refresh failures from other errors.
  • On 401/403 refresh responses, reloads credentials from disk once and retries refresh if a newer refresh token is detected.
  • Adds two unit tests covering peer-rotation adoption and the fallback behavior where auth is cleared only when no newer credential exists.
Suppressed comments (1)

src/providers/kimi/auth/manager.rs:309

  • std::env::set_var / remove_var are safe APIs; the unsafe block in Drop is unnecessary and may trip clippy::unnecessary_unsafe.
    impl Drop for EnvGuard {
        fn drop(&mut self) {
            unsafe {
                match self.previous.take() {
                    Some(value) => std::env::set_var(self.key, value),
                    None => std::env::remove_var(self.key),
                }
            }
        }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +291 to +299
impl EnvGuard {
fn set(key: &'static str, value: &str) -> Self {
let previous = std::env::var_os(key);
unsafe {
std::env::set_var(key, value);
}
Self { key, previous }
}
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This crate is on edition 2024 (see Cargo.toml), where std::env::set_var is an unsafe fn — the unsafe block is required and matches the existing EnvGuard pattern in src/config.rs tests. cargo clippy --lib --tests is clean.

@raine

raine commented Aug 5, 2026

Copy link
Copy Markdown
Owner

Could you clarify the real-world scenario that requires this change? Claude Code Proxy stores its own Kimi credentials separately from the Kimi CLI, so I would not expect the CLI to rotate the proxy refresh token unless the credential files are explicitly symlinked or synchronized.

Did you encounter this with shared credential files, multiple proxy instances, or concurrent proxy requests? A concrete reproduction would help me understand which case the fix needs to cover.

@flaviomartil

Copy link
Copy Markdown
Contributor Author

Verified on my machine: the credential files are indeed separate (no symlinks) — the proxy uses ~/.config/claude-code-proxy/kimi/auth.json, the Kimi CLI uses ~/.kimi/credentials/kimi-code.json. The CLI was not the peer.

The scenario is your option (b): multiple proxy instances sharing the proxy's own credential file. I run the proxy 24/7 as a Homebrew service on 127.0.0.1:18765. On Aug 2, while working on #99, I also ran dev/test builds from my fork that resolved the same real config/state dirs — proxy.log shows ~25 server listening lines on ephemeral ports (with test request IDs like http-body-test) between 17:54Z and 17:57Z, all logging to the real state dir. At 17:57:49Z the service instance started returning 401 "Not authenticated" and kept failing until I re-logged in the next day.

Since Kimi rotates the refresh token on every refresh (~15 min access tokens), whichever instance refreshed last invalidated the other's in-memory token; the stale instance's next refresh got 401 and the old code cleared the shared auth.json. Repro: run one instance as a service plus a second instance on the same config dir, wait for both to refresh — the one holding the stale token gets logged out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants