Fix unauthorized sessions to persist without auto-retry#190
Open
Fix unauthorized sessions to persist without auto-retry#190
Conversation
Unauthorized sessions were being flipped back to 'connecting' by consolidateSessions() on every `mcpc` invocation, even when the only credentials were a static bearer token supplied via --header. These retries cannot succeed because the token never changes, so the displayed status kept toggling between 'connecting' and 'unauthorized' and the real state was only surfaced once the user actually hit the session. Scope the auto-retry to sessions with an OAuth profile, where another session may have refreshed the shared tokens in the keychain. Also include the bridge log path in the server auth error so the user has a pointer for debugging, matching how the expired-session error already surfaces the log path. https://claude.ai/code/session_011CjxmErafRX4ypgb2CQkea
Covers the two guarantees added in the previous commit: - A bearer-only session that fails the initial connect returns exit code 4 and an auth error whose message includes the bridge log path (`check logs at .../bridge-<session>.log`). - The failed session stays `unauthorized` across `mcpc` invocations, even after `lastConnectionAttemptAt` is aged past the 10s auto-retry cooldown. Before the fix, consolidateSessions() would flip the status back to `connecting` whenever the cooldown window had elapsed. The cooldown assertion fakes the post-failure bridge state (old `lastConnectionAttemptAt`, pid cleared) by editing sessions.json directly — otherwise we'd need to wait 10+ seconds for the bridge to exit on its own, which would slow the suite down. Verified both ways: test passes with the fix, reverting only src/lib/sessions.ts causes assertion 3 to fail with exactly the regression message (`expected status to stay 'unauthorized' after cooldown, got: 'connecting'`). https://claude.ai/code/session_011CjxmErafRX4ypgb2CQkea
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
3 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.
Summary
This PR fixes how
mcpchandles sessions that fail with authentication errors. Previously, sessions using static bearer tokens (via--header "Authorization: ...") would incorrectly flip betweenunauthorizedandconnectingstatus on everymcpcinvocation after the auto-retry cooldown elapsed. Now they properly stayunauthorizedsince retrying the same rejected token cannot succeed without user intervention.Additionally, authentication errors now include the path to the bridge log file to help users investigate failures.
Key Changes
consolidateSessions()to only auto-retryunauthorizedsessions that have an OAuth profile. Sessions without a profile (static bearer tokens) cannot self-heal and are excluded from automatic retry, preventing the status flip issuecreateServerAuthError()to accept and display alogPathparameter, pointing users to the bridge log file for debuggingclassifyAndThrowSessionError()andensureBridgeReady()to pass the bridge log file path when creating auth errorsconnectSession()command to include the log path in auth error messagesImplementation Details
The core fix distinguishes between two types of unauthorized sessions:
This prevents the confusing behavior where a session would appear to be retrying indefinitely without user action, while still supporting the legitimate use case of token refresh across sessions.
https://claude.ai/code/session_011CjxmErafRX4ypgb2CQkea