Skip to content

fix(chat-provider): preserve refreshed OAuth token on connection recovery#2004

Open
ayokaa wants to merge 2 commits intoMoonshotAI:mainfrom
ayokaa:fix/stale-api-key-on-retry
Open

fix(chat-provider): preserve refreshed OAuth token on connection recovery#2004
ayokaa wants to merge 2 commits intoMoonshotAI:mainfrom
ayokaa:fix/stale-api-key-on-retry

Conversation

@ayokaa
Copy link
Copy Markdown

@ayokaa ayokaa commented Apr 22, 2026

Fixes #1971

Root Cause

When OAuth refreshes the access token, _apply_access_token() updates client.api_key but not self._api_key. If a transient network error triggers on_retryable_error() to rebuild the OpenAI client, it uses the stale self._api_key — a token that the server has already invalidated upon issuing the new one — causing 401 errors.

T+0:00    Login → token-A → self._api_key = "token-A", client.api_key = "token-A"
T+7:30    OAuth refresh → token-B, token-A invalidated by server
          → client.api_key = "token-B" ✅
          → self._api_key = "token-A" ❌ (never updated)
T+7:31    Network error → on_retryable_error() rebuilds client with self._api_key
          → New client uses token-A (already invalidated) → 401

Fix

Read api_key from the current live client instead of the cached self._api_key in on_retryable_error():

def on_retryable_error(self, error: BaseException) -> bool:
    old_client = self.client
    current_api_key = old_client.api_key  # read from live client
    self.client = create_openai_client(
        api_key=current_api_key,           # use latest token
        base_url=self._base_url,
        client_kwargs=self._client_kwargs,
    )
    self._api_key = current_api_key        # keep cache in sync
    close_replaced_openai_client(old_client, client_kwargs=self._client_kwargs)
    return True

Why TUN mode triggers this

TUN mode causes occasional network route changes that interrupt established HTTP connections, triggering APIConnectionErroron_retryable_error(). Without TUN mode, connections are stable and on_retryable_error() is never called, so the bug remains latent. After this fix, TUN mode usage no longer produces 401 errors.

Testing

  • 4 new tests in test_kimi_stale_api_key.py:
    • Token preservation after refresh + retry
    • Correct token at construction
    • Idempotent across multiple refresh/retry cycles
    • End-to-end: captures actual HTTP Authorization header via respx mock, confirms rebuilt client sends the refreshed token
  • 2793 existing tests pass, no regression

Checklist


Open in Devin Review

…very

When OAuth refreshes the access token, `_apply_access_token()` updates
`client.api_key` but not `self._api_key`. If a transient network error
triggers `on_retryable_error()` to rebuild the OpenAI client, it uses
the stale `self._api_key` — a token that the server has already
invalidated upon issuing the new one — causing 401 errors.

Fix: read `api_key` from the current live client instead of the cached
`self._api_key` in `on_retryable_error()`, and keep `self._api_key` in
sync.

Fixes MoonshotAI#1971
Copilot AI review requested due to automatic review settings April 22, 2026 12:07
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes a connection-recovery authentication bug in the Kosong Kimi chat provider where OAuth token refresh updates the live OpenAI client’s api_key, but a retry-triggered client rebuild previously reused a stale cached key, leading to intermittent 401s (notably in TUN mode).

Changes:

  • Update Kimi.on_retryable_error() to rebuild the client using old_client.api_key and sync self._api_key.
  • Add regression tests covering refresh+retry cycles and an end-to-end Authorization header assertion via respx.
  • Add changelog entries (root + package + docs).

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
packages/kosong/src/kosong/chat_provider/kimi.py Rebuilds the OpenAI client using the currently-active api_key to avoid stale-token 401s after refresh.
packages/kosong/tests/test_kimi_stale_api_key.py Adds regression + E2E tests to confirm the rebuilt client sends the refreshed Bearer token.
CHANGELOG.md Documents the user-visible fix in the root Unreleased changelog.
packages/kosong/CHANGELOG.md Records the provider-specific fix in the Kosong package changelog.
docs/en/release-notes/changelog.md Updates the English docs changelog entry (but this file is generated).
docs/zh/release-notes/changelog.md Adds the corresponding Chinese release note entry.

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


## Unreleased

- Kosong: Fix stale API key after OAuth token refresh in Kimi provider — `on_retryable_error` now reads the current `api_key` from the live client instead of the cached `_api_key`, so that OAuth token refreshes applied via `client.api_key` are preserved when the client is rebuilt after a retryable error
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

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

docs/en/release-notes/changelog.md is auto-generated from the root CHANGELOG.md via docs/scripts/sync-changelog.mjs (see docs/AGENTS.md). To avoid the doc page drifting, please regenerate it with npm run sync / make gen-docs rather than editing this file directly (and ensure the committed output matches the root changelog).

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@copilot apply changes based on this feedback

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 2 additional findings.

Open in Devin Review

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.

401 Authentication Error when TUN Mode Enabled

2 participants