feat(ENG-294): make polling the default OAuth completion mechanism#155
Merged
adefreitas merged 4 commits intoMay 12, 2026
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Replace the previous postMessage/BroadcastChannel/localStorage signalling between the OAuth popup and the hub with the polling endpoint as the sole completion signal. Removes ~200 LOC of cross-window communication paths, keeps a popup-closed watcher (gated by COOP detection) for fast cancel on non-COOP browsers, and adds a 5-min poll timeout that surfaces a warning banner at the top of the picker. Race between the popup-close watcher and a just-completed poll is resolved by re-checking pollingIntervalRef after every await, so whichever path observes the terminal status first wins and the other bails without double-cancelling or double-firing onSuccess. Dev sandbox (Vite default tab, Vite Suspense MRE, Next.js) now shows the resolved API/app URLs above the manual connect-session-token input so it's obvious which environment the token belongs to.
adefreitas
force-pushed
the
ENG-294/polling-default-remove-window-communication
branch
from
979da09 to
bcdba57
Compare
![aikido-pr-checks[bot]](https://avatars.githubusercontent.com/in/898896?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the integration picker OAuth completion flow to rely solely on polling /hub/connection_attempts/{id}, removing prior popup→hub signaling mechanisms and adding UX/behavior for timeouts and early popup closes.
Changes:
- Replace popup signaling (postMessage/BroadcastChannel/localStorage) with polling as the single OAuth completion mechanism.
- Add a 5-minute polling timeout that cancels the connection attempt and surfaces a warning banner in the picker.
- Improve dev sandboxes by displaying the active api/app environment URLs above the token input.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/modules/integration-picker/IntegrationPicker.tsx | Displays a timeout warning banner when polling exceeds the timeout. |
| src/modules/integration-picker/hooks/useIntegrationPicker.ts | Implements polling-only OAuth completion, timeout cancellation, and popup-close watcher logic. |
| dev/vite/SuspenseMRE.tsx | Prints api/app environment URLs in the Vite Suspense MRE sandbox UI. |
| dev/vite/main.tsx | Prints api/app environment URLs in the Vite sandbox wrapper UI. |
| dev/nextjs/app/HubWrapper.tsx | Prints api/app environment URLs in the Next.js sandbox wrapper UI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- handle cancelConnectionAttempt rejections with debug-logged .catch (4 sites) - build OAuth popup URL with URLSearchParams instead of manual concatenation - log final poll failure in the popup watcher on debug
The /connect/oauth2 endpoint matches the token value in its raw form, so percent-encoding the base64 padding `=` as `%3D` (which URLSearchParams does automatically) causes the backend to reject the token. The popup opens, closes immediately, and the watcher resets the picker to idle. Reverting to the literal template-string concatenation. The .catch fixes on cancelConnectionAttempt and the watcher's final poll stay.
…dow.open The synchronous coopDetectedRef check after window.open always saw about:blank — the destination URL's COOP header hadn't been applied yet, so the flag was always false. ~1s later the popup loads api.stackone.com which sets COOP, Chrome severs the parent's reference, and popup.closed starts returning true falsely. The watcher then fires the cancel branch mid-OAuth and resets the picker to its idle/edit state. Move the COOP check to the watcher's first tick: by then the destination URL has loaded and any COOP header is in effect, so popup.closed === true strongly implies isolation rather than a real user-close (the user can't realistically close a freshly-opened popup in <1s anyway). Trade-off: a user who closes the popup within the first second on a non-COOP destination will now be classified as COOP and won't get the snappy cancel — they'll hit the 5-min poll timeout banner instead. Worth it to keep the live OAuth flow from being killed under COOP, which is the much more common case in practice.
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
postMessage/BroadcastChannel/localStoragesignalling. PollingGET /hub/connection_attempts/{id}is now the only OAuth completion signal.connection_attempt_idon the OAuth redirect; hard-fail with a user-visible error ifcreateConnectionAttemptdoesn't return one.<Alert type="warning" />banner ("Connection timed out. Please try again.") at the top of the picker.authenticatedit fireshandleSuccess, otherwise it cancels the attempt and resets state. COOP-affected environments still fall through to the 5-min banner.pollingIntervalRefafter everyawait pollConnectionAttempt(...), so whichever path observes the terminal status first wins and the other bails — no double-cancel, no double-fire ofonSuccess.EventTypeto justAccountConnected(the other members were no longer referenced anywhere in the repo and weren't re-exported fromsrc/index.ts).Environment — api: <apiUrl> · app: <appUrl>above the manual connect-session-token input so it's obvious which environment the pasted token belongs to.Net diff: 5 files changed, +121/−212.
Test plan
authenticatedand the success view renders.popup.closed === trueright afterwindow.open(or run in an environment that enforces it), close the popup early, confirm polling keeps running, then wait 5 min and confirm the warning banner renders.parent.postMessage({ type: 'AccountConnected', account: { id, provider } })on success.:3001, Next.js on:3002) and verify the new "Environment — api/app" line shows the right URLs.