fix(auth): do not unlink provider token on egress auth_required signal#800
Merged
Conversation
When maybeHandleAuthSignal receives an auth_required signal, it was unconditionally calling startAuthorizationPause with unlinkExistingProvider: true, deleting the stored OAuth token before sending a new auth link. This is wrong when the signal originates from an upstream 401 after token injection (scope/org/permission mismatch): the stored token is still valid, and deleting it creates a re-auth loop where every authorization attempt succeeds in the browser but Junior immediately asks again. The fix: remove unlinkExistingProvider from maybeHandleAuthSignal. OAuth completion already overwrites the stored token via userTokenStore.set(), so proactive deletion is unnecessary. If the token is genuinely revoked, re-auth will overwrite it on completion. Also add a logWarn for ResumeTurnBusyError in the OAuth callback waitUntil block — previously silently swallowed, making lock contention invisible in production. Fixes #799 Co-Authored-By: Claude Sonnet 4.5 <claude@anthropic.com> Co-Authored-By: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…n context Now that maybeHandleAuthSignal no longer passes unlinkExistingProvider, the option and its block in startAuthorizationPause are dead code. Remove the option, the unlink block, and the unlinkProvider import. Also move conversation context (conversationId, actorId, channelId) into the LogContext argument for oauth_callback_resume_busy so these fields are indexed properly in structured logging. Co-Authored-By: Claude Sonnet 4.5 <claude@anthropic.com> Co-Authored-By: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com>
Replace ternary-with-empty-object spread pattern with && spread in the
oauth_callback_resume_busy logWarn call. oxlint denies warnings, so
`...(cond ? {k:v} : {})` must be `...(cond && {k:v})`.
Co-Authored-By: Claude Sonnet 4.5 <claude@anthropic.com>
Co-Authored-By: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com>
After removing unlinkExistingProvider, the options object became a
single spread-only literal: { ...(scope ? {scope} : {}) }. oxlint
flags this as a useless new-object spread. Pass the options directly
as a conditional instead.
Co-Authored-By: Claude Sonnet 4.5 <claude@anthropic.com>
Co-Authored-By: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com>
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
1 participant
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.
maybeHandleAuthSignalwas unconditionally callingstartAuthorizationPausewithunlinkExistingProvider: true, deleting the stored OAuth token before sending a new auth link. When the signal originates from an upstream 401 after credential injection (scope/org/permission mismatch), the stored token is still valid — destroying it creates a re-auth loop where authorization succeeds in the browser but Junior immediately asks again.Token preservation on auth_required
Removed
unlinkExistingProvider: truefrommaybeHandleAuthSignal. The signal can represent either a pre-injection credential miss (missing/expired token) or a post-injection upstream 401. In the latter case the stored credential is fine. OAuth completion already overwrites the stored token viauserTokenStore.set(), so proactive deletion is unnecessary in both cases.ResumeTurnBusyError observability
Added
logWarnwhen the OAuth callbackwaitUntilresume is dropped due to a busy conversation lock. Previously silently swallowed, making lock contention invisible in production.Fixes #799
--
View Junior Session in Sentry