Serialize device auth to reuse concurrent logins#197
Merged
Conversation
ericmj
approved these changes
Jun 15, 2026
device_auth was the only auth-resolution path in hex_cli_auth not wrapped in global:trans. Concurrent callers that all needed to authenticate (e.g. parallel commands with no credentials) each kicked off their own device auth flow, prompting the user and persisting a token multiple times instead of sharing a single login. Wrap maybe_authenticate_and_retry in a single global lock (the device auth token is always persisted to the global OAuth scope, so the lock is not keyed by repo). Inside the lock, re-resolve auth first and reuse a token that differs from the one we arrived with: a waiting caller picks up the login a prior winner just persisted instead of prompting again. The "differs from current" check covers both entry reasons — no_credentials (no key in config) and token_refresh_failed (the rejected key in config) — and avoids an infinite retry loop when the server keeps rejecting a token that still looks valid by expiry. Add a regression test that drives concurrent callers through a barrier and blocks the winner inside should_authenticate, asserting exactly one caller is prompted and the persisted login is reused by the rest. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
30ac561 to
cd1d29f
Compare
Member
|
@maennchen Let me know if this is ready to merge |
Member
Author
|
@ericmj Ready. But I'll open more for other stuff surfacing over in |
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
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
device_authwas the only auth-resolution path inhex_cli_authnot wrapped inglobal:trans. Concurrent callers that all needed to authenticate (e.g. parallel commands run with no credentials) each kicked off their own device auth flow — prompting the user and persisting a token multiple times instead of sharing a single login.Fix
maybe_authenticate_and_retryin a single global lock. The device auth token is always persisted to the global OAuth scope, so the lock is intentionally not keyed by repo.no_credentials— no key in config, so any resolved token differs → reuse.token_refresh_failed— the rejected key is in config, so the winner re-runs device auth while a later waiter reuses the freshly persisted token.Test
Adds a regression test that drives concurrent callers through a barrier and blocks the winner inside
should_authenticate, asserting exactly one caller is prompted and the persisted login is reused by the rest. It fails against the unfixed code (multiple concurrent prompts) and passes with the fix. Full suite: 33/33.