Skip to content

Commit 8d3c519

Browse files
committed
fix(mship): bind a settle to its own attempt and keep the deadline armed
Two races the previous rounds left behind. A settle read the attempt only after its refetch, so a retry landing during that window was resolved by a run it never triggered — failing a replacement whose popup was still going. The attempt id is now captured before the await and the verdict only lands if it still matches; the status is still re-read after, so a verdict published mid-refetch is not overwritten. The safety deadline was one-shot. A consent screen that outlived it consumed the timeout while still live, leaving nothing to catch the popup dying unobservably afterwards. It now re-checks at the poll interval instead of expiring against a live window.
1 parent f0c87ed commit 8d3c519

2 files changed

Lines changed: 67 additions & 9 deletions

File tree

apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.test.tsx

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -591,6 +591,53 @@ describe('CredentialDisplay link tag', () => {
591591
act(() => root.unmount())
592592
})
593593

594+
it('holds the deadline open while the popup is live, then settles once it is not', async () => {
595+
// A consent screen can outlive the safety timeout. Expiring against a live
596+
// window would spend the bound early and leave nothing to catch the popup
597+
// dying unobservably later, so the deadline waits instead of firing.
598+
vi.useFakeTimers()
599+
const popup = {
600+
focus: vi.fn(),
601+
closed: false,
602+
// Still on the provider's pages: reading location across origins throws.
603+
get location(): Location {
604+
throw new DOMException('cross-origin', 'SecurityError')
605+
},
606+
}
607+
const openSpy = vi
608+
.spyOn(window, 'open')
609+
.mockReturnValue(popup as unknown as ReturnType<typeof window.open>)
610+
const { container, root } = renderCredentialLink({
611+
type: 'link',
612+
provider: 'google-email',
613+
value:
614+
'https://sim.test/api/auth/oauth2/authorize?providerId=google-email&callbackURL=https%3A%2F%2Fsim.test%2Fworkspace%2Fworkspace-1%2Fchat%2Fchat-1',
615+
})
616+
617+
await act(async () => {
618+
container
619+
.querySelector('a')
620+
?.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true }))
621+
})
622+
await act(async () => {
623+
await vi.advanceTimersByTimeAsync(11 * 60 * 1000)
624+
})
625+
626+
expect(container.textContent).toContain('Waiting for Gmail connection')
627+
628+
// Dies unobservably well after the original deadline — the watcher releases
629+
// the handle without a verdict, so only a still-armed deadline can settle.
630+
popup.closed = true
631+
await act(async () => {
632+
await vi.advanceTimersByTimeAsync(2000)
633+
})
634+
635+
expect(container.textContent).toContain('Not connected — connect Gmail')
636+
vi.useRealTimers()
637+
openSpy.mockRestore()
638+
act(() => root.unmount())
639+
})
640+
594641
it('keeps the unobservable deadline across a remount', async () => {
595642
// The transcript virtualizes, so a row can scroll away mid-connect and come
596643
// back with no window handle and no blur behind it. The deadline is derived

apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/use-oauth-chip-connection.ts

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,11 @@ export function useOAuthChipConnection({
301301
* Idempotent — an attempt is only rewritten while still `pending`.
302302
*/
303303
const settleFromCredentials = useCallback(async () => {
304+
// Identity is captured up front so the verdict below can only land on the
305+
// attempt this settle was started for. A retry during the refetch installs
306+
// a replacement, and resolving that one from a run it never triggered would
307+
// fail a connect whose popup is still going.
308+
const targetAttemptId = readRowAttempt()?.id
304309
const result = await refetchWorkspaceOAuthCredentials()
305310
const credentials = result.data ?? []
306311

@@ -313,11 +318,11 @@ export function useOAuthChipConnection({
313318
)
314319
}
315320

316-
// Read after the await: a snapshot taken before it goes stale the moment the
317-
// popup publishes its verdict mid-refetch, and that stale 'pending' would
318-
// license the very overwrite the guard below exists to prevent.
321+
// Status is re-read after the await: the snapshot above goes stale the
322+
// moment the popup publishes its verdict mid-refetch, and that stale
323+
// 'pending' would license the very overwrite this guard exists to prevent.
319324
const attempt = readRowAttempt()
320-
if (!attempt || attempt.status !== 'pending') return
325+
if (!attempt || attempt.id !== targetAttemptId || attempt.status !== 'pending') return
321326
// Only the callback path can prove a reconnect returned — an unrelated edit
322327
// to the credential is indistinguishable from one here.
323328
const attemptConnected = reconnectCredentialId
@@ -415,12 +420,18 @@ export function useOAuthChipConnection({
415420
0,
416421
OAUTH_POPUP_UNOBSERVABLE_TIMEOUT_MS - (Date.now() - attempt.requestedAt)
417422
)
418-
const deadline = window.setTimeout(() => {
419-
// A popup still demonstrably running owns the flow; the watcher settles
420-
// it the moment it ends, so there is nothing to bound here.
421-
if (isPopupStillOpen(popupRef.current)) return
423+
let deadline: number
424+
const settleUnlessPopupOwnsIt = () => {
425+
// A popup still demonstrably running owns the flow, so the deadline waits
426+
// rather than expiring: giving up here would spend the bound on a live
427+
// window that can still die unobservably long afterwards.
428+
if (isPopupStillOpen(popupRef.current)) {
429+
deadline = window.setTimeout(settleUnlessPopupOwnsIt, OAUTH_POPUP_POLL_INTERVAL_MS)
430+
return
431+
}
422432
void settleFromCredentials()
423-
}, remaining)
433+
}
434+
deadline = window.setTimeout(settleUnlessPopupOwnsIt, remaining)
424435
return () => window.clearTimeout(deadline)
425436
}, [connectionStatus, readRowAttempt, settleFromCredentials])
426437

0 commit comments

Comments
 (0)