feat(model-client): reuse one OAuth login across branch-specific token URLs#2774
Open
abstraktor wants to merge 1 commit into
Open
feat(model-client): reuse one OAuth login across branch-specific token URLs#2774abstraktor wants to merge 1 commit into
abstraktor wants to merge 1 commit into
Conversation
Contributor
Test Results 259 files 259 suites 46m 36s ⏱️ Results for commit 9769206. ♻️ This comment has been updated with latest results. |
Contributor
JVM coverage report
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
abstraktor
force-pushed
the
017-hybrid-modelix-tokens
branch
3 times, most recently
from
b3031af to
1f6d57a
Compare
…30–T033) - Remove `tokenUrl` from `TokenCacheKey` so all branch-specific token endpoints keyed to the same issuer+clientId share one credential entry. - Move `cachedTokens` to a `ModelixAuthClient` companion object so the shared refresh token is visible across all `ModelixAuthClient` instances (one per `ModelClientV2` builder) — one interactive login per process; separate processes log in independently (credentials in memory only). - Store the raw refresh token; each branch sends its own `grant_type=refresh_token` to its own tokenUrl, obtaining a branch-scoped access token without a second PKCE login. Cache those access tokens per token URL and reuse them until a 60s pre-expiry skew margin to avoid re-minting (and re-issue reactively on a 401). - Single-flight all refresh/PKCE per (issuer, clientId) via `authMutex`, always chaining to the latest rotated refresh token. - On `invalid_grant`, discard the stored refresh token and the access tokens minted from it, then fall back to interactive PKCE. - Replace the removed `getTokens()` with Java-friendly, non-suspending accessors `getRefreshToken(config)` / `getAccessToken(config)` that read the process-wide cache; make the suspend auth helpers internal. - Stop logging the access token; inject the IO dispatcher, extract port constants, and use error() so detekt stays clean. - Tests (`CredentialCachingTest`): cache-key (T030), two-branch one-PKCE (T032), invalid_grant fallback (T033), plus concurrent single-flight, single-use refresh-token rotation, and cross-client-isolation. Tests are isolated by unique per-test cache keys (random ports), not by clearing the shared cache — a global clear would race with parallel tests (e.g. OAuthTest). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
abstraktor
force-pushed
the
017-hybrid-modelix-tokens
branch
from
1f6d57a to
9769206
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
On the JVM,
ModelixAuthClientcaches OAuth credentials perModelClientV2instance and keys the cache by the full token URL. When a deployment issues branch-specific token URLs (same issuer + client ID, but a different per-branch token endpoint), each branch forks the credential cache and triggers a separate interactive PKCE login. Opening several repositories/branches in one session therefore prompts the user to log in repeatedly.Change
(authorizationUrl, clientId)— droptokenUrlfromTokenCacheKey. The per-branch token URL still selects which token is minted, but it no longer forks the refresh-credential cache.ModelixAuthClientinstances within a process: one interactive login per session. Separate processes still log in independently (in-memory only, nothing persisted).refresh_tokengrant against each branch's token URL, so every branch obtains its own scoped access token from the single login.invalid_grant, discard the stored refresh token and fall back to interactive PKCE.Tests
model-client/src/jvmTest/.../client2/CredentialCachingTest.kt:ModelClientV2clients on different branch token URLs require exactly one PKCE login (the second reuses the refresh token);invalid_grantrefresh response triggers a fresh PKCE login.Existing
RefreshTokenTest,OAuthTest,TokenEndpointTest, andModelixAuthClientTestcontinue to pass.