Fix session expiration handling and keychain fallback logic#88
Merged
Conversation
Two fixes: 1. keychain.ts: When OS keychain getPassword() returns null (entry not found) without throwing, also check the file-based fallback. This fixes a cross-process issue where the CLI stores credentials in the file (because keychain setPassword throws) but the bridge reads from the OS keychain (getPassword succeeds with null) and never checks the file. Fixes proxy bearer token authentication returning 400 instead of 401. 2. bridge/index.ts: Make handlePossibleExpiration async and await the session status update before sending the error response to the CLI. Previously the status update was fire-and-forget, causing a race where the CLI would check sessions.json before the bridge had written the 'expired' status, resulting in 'crashed' instead. Schedule shutdown via setImmediate so the error response is sent first. https://claude.ai/code/session_01S5dYbXc9Numzts5NtQRTxc
setImmediate fires in the very next event loop iteration, which may not give enough time for the socket.write() in sendError to flush to the client before shutdown tears down the socket. A 100ms setTimeout provides a safer margin. https://claude.ai/code/session_01S5dYbXc9Numzts5NtQRTxc
…tion The previous approach detected keychain availability lazily per-operation, which could cause split-brain: writes go to file fallback while reads succeed from keychain (returning null), leading to stale data forever. Now we probe with a test write+read+delete on first use. If both work, keychain is used for everything; otherwise file fallback is used. This also removes the band-aid fileGet fallback in keychainGet. https://claude.ai/code/session_01S5dYbXc9Numzts5NtQRTxc
Ensures getPassword returns the exact value we set, and uses a unique key per probe to avoid conflicts with concurrent mcpc instances. https://claude.ai/code/session_01S5dYbXc9Numzts5NtQRTxc
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 improves session expiration handling in the bridge process and adds a keychain fallback mechanism to handle cases where the OS keychain is unavailable for writes but file-based storage is used.
Key Changes
Bridge Process Session Expiration Handling
handlePossibleExpirationasync: Changed from synchronous to asynchronous to properly await session status updatesmarkSessionStatusAndExitwhich chains multiple async operations, now directly callsupdateSessionto synchronously update the session status, making it immediately visible to the CLIsetImmediateto schedule shutdown after the error response is sent, ensuring the CLI receives the response before the process exitsforwardMcpRequest, now callshandlePossibleExpirationbeforesendErrorso session status is updated before the error response is sent to the CLIKeychain Fallback Logic
keychainGet: When the OS keychain returns null but is available, now also checks the file-based fallback storageImplementation Details
updateSession) rather than through a chained async operation, ensuring the status is persisted and visible to the CLI immediatelysetImmediateto allow the error response to be sent before the process terminateshttps://claude.ai/code/session_01S5dYbXc9Numzts5NtQRTxc