Fix Parallel OAuth callback and browser handling - #48
Open
anirudhmehra wants to merge 3 commits into
Open
Conversation
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.
Summary
server.address()per request/login parallelopens one authorization tab instead of twoProblems
Callback crash during shutdown
Node documents that
server.address()returnsnullafterserver.close(). An already-active keep-alive connection can still dispatch a follow-up request, such as/favicon.ico, while the callback listener is closing. The request handler then dereferencesaddress.port, causing an uncaughtTypeErrorand terminating the host process before OAuth credentials are persisted.The callback regression test uses a real socket to keep the callback request active, begins listener shutdown, then sends
/favicon.icoon the same connection. It reproduces the originalnull.portstack without the fix.Callback shutdown can block credential storage
server.close()waits for active HTTP connections before invoking its callback. Chrome can keep the loopback callback connection active after the token exchange has completed, leaving Pi's login dialog open indefinitely and preventing the fetched API key from reaching credential storage. Navigating to the authorization URL again happened to close the old connection, which made login appear to require a second click.After calling
server.close()to stop new connections, the listener now calls Node's nativeserver.closeAllConnections(). The regression test keeps the callback request active and verifies login completes without any second browser action or client disconnect.Duplicate authorization tabs in Pi
The shared OAuth helper opened the authorization URL directly, then Pi opened the notified
auth_urlagain inLoginDialogComponent.showAuth(). The Pi adapter now passesopenBrowser: false, making Pi the sole browser owner. OpenCode remains unchanged because it explicitly needs the helper to open its URL.The shared callback helper is bundled into both the Pi extension and OpenCode plugin; the browser-opening change is Pi-specific.
Verification
pnpm test:cipnpm lintpnpm format:checkpnpm typecheckpnpm build