Skip to content

Commit f0c87ed

Browse files
committed
fix(mship): survive a remount while an attempt is still pending
The unobservable deadline lived in the watcher effect's closure, so it was armed only by the mount that launched the popup. The transcript virtualizes: a row scrolled away mid-connect came back with no window handle and no blur behind it, and nothing re-armed the bound. Derives the deadline from the attempt's own requestedAt and arms it for any pending attempt, so a remount inherits the time remaining rather than restarting the clock or losing it. A demonstrably live popup still owns the flow and is left to the watcher.
1 parent 5b6bfde commit f0c87ed

2 files changed

Lines changed: 74 additions & 18 deletions

File tree

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

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

594+
it('keeps the unobservable deadline across a remount', async () => {
595+
// The transcript virtualizes, so a row can scroll away mid-connect and come
596+
// back with no window handle and no blur behind it. The deadline is derived
597+
// from the attempt's requestedAt rather than held in the effect, so the
598+
// remounted row inherits the remaining time instead of waiting forever.
599+
vi.useFakeTimers()
600+
const popup = { focus: vi.fn(), closed: false }
601+
const openSpy = vi
602+
.spyOn(window, 'open')
603+
.mockReturnValue(popup as unknown as ReturnType<typeof window.open>)
604+
const data: CredentialItemData = {
605+
type: 'link',
606+
provider: 'google-email',
607+
value:
608+
'https://sim.test/api/auth/oauth2/authorize?providerId=google-email&callbackURL=https%3A%2F%2Fsim.test%2Fworkspace%2Fworkspace-1%2Fchat%2Fchat-1',
609+
}
610+
const first = renderCredentialLink(data)
611+
612+
await act(async () => {
613+
first.container
614+
.querySelector('a')
615+
?.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true }))
616+
})
617+
popup.closed = true
618+
await act(async () => {
619+
await vi.advanceTimersByTimeAsync(60 * 1000)
620+
})
621+
act(() => first.root.unmount())
622+
623+
const second = renderCredentialLink(data)
624+
expect(second.container.textContent).toContain('Waiting for Gmail connection')
625+
626+
await act(async () => {
627+
await vi.advanceTimersByTimeAsync(10 * 60 * 1000)
628+
})
629+
630+
expect(second.container.textContent).toContain('Not connected — connect Gmail')
631+
vi.useRealTimers()
632+
openSpy.mockRestore()
633+
act(() => second.root.unmount())
634+
})
635+
594636
it('still announces the connection when the watcher released the popup first', async () => {
595637
// The watcher drops the window handle as soon as it stops being observable,
596638
// which routinely happens before React applies the storage-driven verdict.

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

Lines changed: 32 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -381,7 +381,6 @@ export function useOAuthChipConnection({
381381
*/
382382
useEffect(() => {
383383
if (connectionStatus !== 'pending' || !popupRef.current) return
384-
let unobservableTimer: number | undefined
385384
const poll = window.setInterval(() => {
386385
const observation = observePopup(popupRef.current)
387386
if (observation === 'live') return
@@ -390,26 +389,41 @@ export function useOAuthChipConnection({
390389
// could resolve an attempt a retry had since replaced.
391390
window.clearInterval(poll)
392391
popupRef.current = null
393-
// Only `ended` is proof the flow finished with nothing published.
394-
if (observation === 'ended') {
395-
void settleFromCredentials()
396-
return
397-
}
398-
// A closed handle and a disowned one are indistinguishable, so neither
399-
// deciding now nor waiting forever is right. Closing a popup normally
400-
// hands focus back and the focus verification settles it; this bounds the
401-
// case where that never arrives — the opener was never blurred, or the
402-
// flow completed somewhere this tab cannot see.
403-
unobservableTimer = window.setTimeout(() => {
404-
void settleFromCredentials()
405-
}, OAUTH_POPUP_UNOBSERVABLE_TIMEOUT_MS)
392+
// Only `ended` is proof the flow finished with nothing published. A
393+
// closed-or-disowned handle is not — the deadline below bounds that.
394+
if (observation === 'ended') void settleFromCredentials()
406395
}, OAUTH_POPUP_POLL_INTERVAL_MS)
407-
return () => {
408-
window.clearInterval(poll)
409-
if (unobservableTimer !== undefined) window.clearTimeout(unobservableTimer)
410-
}
396+
return () => window.clearInterval(poll)
411397
}, [connectionStatus, settleFromCredentials])
412398

399+
/**
400+
* Backstop deadline for a pending attempt whose outcome never becomes
401+
* observable — a closed-or-disowned popup this tab cannot read, with no blur
402+
* for the focus verification to work from.
403+
*
404+
* Bounds every pending attempt rather than only one this mount launched: the
405+
* transcript virtualizes, so a row can remount onto an attempt restored from
406+
* storage with no window handle at all. The deadline comes from the attempt's
407+
* own `requestedAt`, so remounting re-arms with the time remaining instead of
408+
* restarting the clock or dropping the bound entirely.
409+
*/
410+
useEffect(() => {
411+
if (connectionStatus !== 'pending') return
412+
const attempt = readRowAttempt()
413+
if (!attempt) return
414+
const remaining = Math.max(
415+
0,
416+
OAUTH_POPUP_UNOBSERVABLE_TIMEOUT_MS - (Date.now() - attempt.requestedAt)
417+
)
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
422+
void settleFromCredentials()
423+
}, remaining)
424+
return () => window.clearTimeout(deadline)
425+
}, [connectionStatus, readRowAttempt, settleFromCredentials])
426+
413427
useEffect(() => {
414428
if (connected) onConnectedRef.current?.()
415429
}, [connected])

0 commit comments

Comments
 (0)