Skip to content

fix(client/auth): discard stored client registrations whose secret has expired - #3264

Open
claude[bot] wants to merge 9 commits into
mainfrom
fix/oauth-discard-expired-client-registration
Open

fix(client/auth): discard stored client registrations whose secret has expired#3264
claude[bot] wants to merge 9 commits into
mainfrom
fix/oauth-discard-expired-client-registration

fix(client/auth): discard stored client registrations with an expired…

4fac3c7
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Aug 7, 2026 in 8m 29s

Code review found 2 potential issues

Found 3 candidates, confirmed 2. See review comments for details.

Details

Severity Count
🔴 Important 0
🟡 Nit 2
🟣 Pre-existing 0
Severity File:Line Issue
🟡 Nit src/mcp/client/auth/oauth2.py:578-583 docs/client/oauth-clients.md not updated for the new stored-registration discard behavior
🟡 Nit src/mcp/client/auth/oauth2.py:577-581 Mid-session secret lapse not re-checked; long-lived client can still get permanently stuck

Annotations

Check warning on line 583 in src/mcp/client/auth/oauth2.py

See this annotation in the file changed.

@claude claude / Claude Code Review

docs/client/oauth-clients.md not updated for the new stored-registration discard behavior

This PR changes user-visible behavior (a stored registration with a lapsed `client_secret_expires_at` is now discarded on load, minting a fresh `client_id` on the next 401 flow) but doesn't update `docs/client/oauth-clients.md`, which now contains two stale statements: "Stored `client_info` still wins over both." and "The provider registers dynamically the first time it finds no stored `client_info`." A small edit to that existing page mentioning the expired-secret discard would satisfy the AGEN

Check warning on line 581 in src/mcp/client/auth/oauth2.py

See this annotation in the file changed.

@claude claude / Claude Code Review

Mid-session secret lapse not re-checked; long-lived client can still get permanently stuck

The new `stored_registration_expired()` check only runs in `_initialize()`, which executes once per provider instance — so if the secret lapses while the process is running (the long-lived-client scenario from #3256), the stale in-memory `context.client_info` is never re-validated: with no refresh token, the 401 flow skips re-registration, runs a full interactive authorization, then fails at token exchange with `invalid_client`, and repeats identically on every request until process restart. Con