From e625525e241ddd80365d550ba8bd654324a960e3 Mon Sep 17 00:00:00 2001 From: Justin Blumencranz <96924014+j15z@users.noreply.github.com> Date: Fri, 7 Aug 2026 20:51:31 -0700 Subject: [PATCH 01/10] fix(mship): return the chat connect flow to the tab that started it Connecting an integration from a chat credential chip opened OAuth in a new tab and returned there, so the user landed on a second copy of the app while the conversation they started from sat stale behind it. The flow now runs in a popup and returns through a new self-closing page at /oauth/chat-complete, which publishes its verdict to the shared attempt record and closes. The chat tab picks that up over its storage listener and updates in place, so it never navigates. A blocked popup takes the same route in a new tab and still lands on the completion page, so both paths share one verdict source. That verdict is now the server's: reaching the completion page means Better Auth routed the flow to its success callback. The previous check diffed the workspace credential list, which reported failure whenever a user re-authorized an account they had already linked -- that path updates the account row and creates no new credential. The lock is stricter than the label. A failure to create the credential from its draft is swallowed server-side, so a flow can report success with nothing in the workspace; the row stays retryable unless the credential actually appears. Also: the popup is named per attempt so sibling rows cannot renavigate each other's window; a cross-origin connect URL keeps the anchor's noopener instead of taking the popup path; the focus verifier reads the attempt after its refetch so a verdict published mid-flight is not overwritten; and the verifier treats a popup parked on a terminal page (/oauth-error, the workspace error exit) as finished rather than waiting on it forever. --- .../chat-complete-handoff.test.tsx | 135 +++++++++++ .../chat-complete/chat-complete-handoff.tsx | 71 ++++++ apps/sim/app/oauth/chat-complete/page.tsx | 28 +++ .../special-tags/special-tags.test.tsx | 210 ++++++++++++++++++ .../special-tags/use-oauth-chip-connection.ts | 120 +++++++++- .../credentials/oauth-chat-attempt.test.ts | 62 ++++++ .../sim/lib/credentials/oauth-chat-attempt.ts | 77 ++++++- 7 files changed, 691 insertions(+), 12 deletions(-) create mode 100644 apps/sim/app/oauth/chat-complete/chat-complete-handoff.test.tsx create mode 100644 apps/sim/app/oauth/chat-complete/chat-complete-handoff.tsx create mode 100644 apps/sim/app/oauth/chat-complete/page.tsx diff --git a/apps/sim/app/oauth/chat-complete/chat-complete-handoff.test.tsx b/apps/sim/app/oauth/chat-complete/chat-complete-handoff.test.tsx new file mode 100644 index 00000000000..8b28bf1f473 --- /dev/null +++ b/apps/sim/app/oauth/chat-complete/chat-complete-handoff.test.tsx @@ -0,0 +1,135 @@ +/** + * @vitest-environment jsdom + */ +import { act } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { + createOAuthChatAttempt, + type OAuthChatAttempt, + readOAuthChatAttempt, +} from '@/lib/credentials/oauth-chat-attempt' +import { ChatCompleteHandoff } from '@/app/oauth/chat-complete/chat-complete-handoff' + +function renderAt(search: string): { root: Root } { + ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true + window.history.replaceState({}, '', `/oauth/chat-complete${search}`) + const root: Root = createRoot(document.createElement('div')) + act(() => root.render()) + return { root } +} + +describe('ChatCompleteHandoff', () => { + let attempt: OAuthChatAttempt + + beforeEach(() => { + vi.clearAllMocks() + window.localStorage.clear() + vi.spyOn(window, 'close').mockImplementation(() => {}) + attempt = createOAuthChatAttempt({ + workspaceId: 'workspace-1', + providerId: 'google-email', + baseProviderId: 'google', + displayName: 'Gmail', + controlId: 'message-1:0:0', + baselineCredentialIds: ['existing-gmail'], + }) + }) + + it('publishes success on arrival, without requiring a new credential to appear', () => { + // Re-authorizing an already-linked account updates the account row rather + // than creating one, so no new credential lands — reaching this page is + // still the server telling us the flow succeeded. + const { root } = renderAt(`?oauthAttempt=${attempt.id}`) + + expect(readOAuthChatAttempt(attempt.id)?.status).toBe('connected') + expect(window.close).toHaveBeenCalledOnce() + act(() => root.unmount()) + }) + + it('publishes failure when the provider returned an error', () => { + const { root } = renderAt(`?oauthAttempt=${attempt.id}&error=access_denied`) + + expect(readOAuthChatAttempt(attempt.id)?.status).toBe('failed') + act(() => root.unmount()) + }) + + it('leaves an unrelated attempt untouched when no attempt is named', () => { + const { root } = renderAt('') + + expect(readOAuthChatAttempt(attempt.id)?.status).toBe('pending') + expect(window.close).toHaveBeenCalledOnce() + act(() => root.unmount()) + }) + + it('still publishes the verdict when the attempt store rejects the write', () => { + const setItem = vi.spyOn(Storage.prototype, 'setItem').mockImplementation(() => { + throw new DOMException('quota', 'QuotaExceededError') + }) + + // The verdict is lost, but the window must still be released — an + // unguarded throw would strand the popup open on this page. + expect(() => renderAt(`?oauthAttempt=${attempt.id}`)).not.toThrow() + expect(window.close).toHaveBeenCalledOnce() + setItem.mockRestore() + }) + + describe('close-refused fallback', () => { + const realLocation = window.location + + afterEach(() => { + vi.useRealTimers() + Object.defineProperty(window, 'location', { configurable: true, value: realLocation }) + }) + + /** + * jsdom performs no navigation and forbids redefining `location.replace`, + * so the whole location is swapped for a stub carrying only what the + * handoff reads: the current href, the origin, and the redirect sink. + */ + function renderWithStubbedLocation(search: string): { calls: string[]; root: Root } { + const calls: string[] = [] + Object.defineProperty(window, 'location', { + configurable: true, + value: { + href: `https://sim.test/oauth/chat-complete${search}`, + origin: 'https://sim.test', + replace: (url: string) => calls.push(url), + }, + }) + ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true + const root: Root = createRoot(document.createElement('div')) + act(() => root.render()) + return { calls, root } + } + + it('redirects to a same-origin returnTo once the close is refused', () => { + vi.useFakeTimers() + const returnTo = 'https://sim.test/workspace/workspace-1/chat/chat-1' + + const { calls, root } = renderWithStubbedLocation( + `?oauthAttempt=${attempt.id}&returnTo=${encodeURIComponent(returnTo)}` + ) + act(() => { + vi.advanceTimersByTime(400) + }) + + expect(calls).toEqual([returnTo]) + act(() => root.unmount()) + }) + + it('refuses a cross-origin returnTo and falls back to the workspace', () => { + vi.useFakeTimers() + + const { calls, root } = renderWithStubbedLocation( + `?oauthAttempt=${attempt.id}&returnTo=${encodeURIComponent('https://evil.example/steal')}` + ) + act(() => { + vi.advanceTimersByTime(400) + }) + + expect(calls).toEqual(['/workspace']) + act(() => root.unmount()) + }) + }) +}) diff --git a/apps/sim/app/oauth/chat-complete/chat-complete-handoff.tsx b/apps/sim/app/oauth/chat-complete/chat-complete-handoff.tsx new file mode 100644 index 00000000000..9b6c3f4a08c --- /dev/null +++ b/apps/sim/app/oauth/chat-complete/chat-complete-handoff.tsx @@ -0,0 +1,71 @@ +'use client' + +import { useEffect, useRef } from 'react' +import { + OAUTH_CHAT_ATTEMPT_PARAM, + OAUTH_CHAT_RETURN_TO_PARAM, + setOAuthChatAttemptStatus, +} from '@/lib/credentials/oauth-chat-attempt' + +const CLOSE_FALLBACK_DELAY_MS = 400 + +/** + * The fallback redirect must never leave this origin — the target rides in a + * query param the user could have tampered with. + */ +function sanitizeReturnTo(raw: string | null): string | null { + if (!raw) return null + try { + const url = new URL(raw, window.location.origin) + return url.origin === window.location.origin ? url.toString() : null + } catch { + return null + } +} + +/** + * Behavior half of the chat OAuth return leg: publishes the verdict to the + * attempt record — which the chat tab's chip picks up over its storage + * listener — then closes the window. Renders nothing, so the page's frame is + * plain server-rendered markup that paints before this hydrates. + * + * Reaching this page IS the verdict. Better Auth routes a flow here only as + * its success `callbackURL`, sending failures to `onAPIError.errorURL` + * (`/oauth-error`) or back here with an `error` code, so the server has + * already decided by the time this runs. That is a strictly better signal than + * the credential diffing the generic-page return does: re-authorizing an + * already-linked account updates the account row instead of creating one, so + * no new credential appears and a diff would call a perfectly good connect a + * failure. + * + * A window the browser refuses to close redirects on to the chat surface + * instead. That is the popup-blocked path: the anchor's `target='_blank'` + * opens this leg in a new tab, which no script may close. That URL + * deliberately carries no attempt id — the verdict is already published, and + * the destination's return router would otherwise re-decide it by the very + * diff this page exists to avoid. + */ +export function ChatCompleteHandoff() { + const ranRef = useRef(false) + + useEffect(() => { + if (ranRef.current) return + ranRef.current = true + + const params = new URL(window.location.href).searchParams + const attemptId = params.get(OAUTH_CHAT_ATTEMPT_PARAM) + const returnTo = sanitizeReturnTo(params.get(OAUTH_CHAT_RETURN_TO_PARAM)) + + if (attemptId) { + setOAuthChatAttemptStatus(attemptId, params.has('error') ? 'failed' : 'connected') + } + + window.close() + const timer = window.setTimeout(() => { + window.location.replace(returnTo ?? '/workspace') + }, CLOSE_FALLBACK_DELAY_MS) + return () => window.clearTimeout(timer) + }, []) + + return null +} diff --git a/apps/sim/app/oauth/chat-complete/page.tsx b/apps/sim/app/oauth/chat-complete/page.tsx new file mode 100644 index 00000000000..3567713cbe5 --- /dev/null +++ b/apps/sim/app/oauth/chat-complete/page.tsx @@ -0,0 +1,28 @@ +import type { Metadata } from 'next' +import { DesktopHandoffShell } from '@/app/desktop/components/desktop-handoff-shell' +import { ChatCompleteHandoff } from '@/app/oauth/chat-complete/chat-complete-handoff' + +export const metadata: Metadata = { + title: 'Returning to Sim', + robots: { index: false }, +} + +/** + * Post-OAuth return leg for the chat credential chips. The chip rewrites the + * authorize URL's return param to land here, so the OAuth window finishes on + * this page — which signals the chat tab and closes — instead of loading a + * second copy of the app. + * + * Shown for a few hundred milliseconds in a popup, or briefly in the original + * tab when the popup was blocked, so it wears the same handoff frame as the + * other minimal-chrome gates (the 404, the desktop connect screens) rather + * than styling of its own. + */ +export default function ChatCompletePage() { + return ( + <> + + + + ) +} diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.test.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.test.tsx index 69cb9c3bda9..5a604b29884 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.test.tsx @@ -1,5 +1,6 @@ /** * @vitest-environment jsdom + * @vitest-environment-options { "url": "https://sim.test/workspace/workspace-1/chat/chat-1" } */ import { act } from 'react' import { createRoot, type Root } from 'react-dom/client' @@ -51,6 +52,7 @@ vi.mock('@/hooks/queries/environment', () => ({ }), })) +import { toast } from '@sim/emcn' import { createOAuthChatAttempt, setOAuthChatAttemptStatus, @@ -319,6 +321,214 @@ describe('CredentialDisplay link tag', () => { act(() => root.unmount()) }) + it('runs the connect in a popup so the chat tab never navigates', () => { + const popup = { focus: vi.fn() } + const openSpy = vi + .spyOn(window, 'open') + .mockReturnValue(popup as unknown as ReturnType) + const { container, root } = renderCredentialLink({ + type: 'link', + provider: 'google-email', + value: + 'https://sim.test/api/auth/oauth2/authorize?providerId=google-email&callbackURL=https%3A%2F%2Fsim.test%2Fworkspace%2Fworkspace-1%2Fchat%2Fchat-1', + }) + + const link = container.querySelector('a') + const defaultPrevented = !link?.dispatchEvent( + new MouseEvent('click', { bubbles: true, cancelable: true }) + ) + + expect(defaultPrevented).toBe(true) + expect(popup.focus).toHaveBeenCalledOnce() + const openedUrl = new URL(openSpy.mock.calls[0][0] as string) + const callbackUrl = new URL(openedUrl.searchParams.get('callbackURL') ?? '') + expect(callbackUrl.pathname).toBe('/oauth/chat-complete') + // Named per attempt: a shared name would let a sibling row renavigate this + // popup and strand this attempt with no return leg. + const attemptId = callbackUrl.searchParams.get('oauthAttempt') + expect(openSpy.mock.calls[0][1]).toBe(`sim-oauth-connect-${attemptId}`) + openSpy.mockRestore() + act(() => root.unmount()) + }) + + it('keeps a cross-origin connect URL on the anchor instead of a popup', () => { + const openSpy = vi.spyOn(window, 'open') + const { container, root } = renderCredentialLink({ + type: 'link', + provider: 'google-email', + value: + 'https://evil.example/api/auth/oauth2/authorize?providerId=google-email&callbackURL=https%3A%2F%2Fevil.example%2Fsink', + }) + + const link = container.querySelector('a') + link?.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true })) + + // The anchor carries rel='noopener noreferrer'; window.open would not. + expect(openSpy).not.toHaveBeenCalled() + expect(link?.getAttribute('rel')).toBe('noopener noreferrer') + openSpy.mockRestore() + act(() => root.unmount()) + }) + + it('announces the connection once the popup publishes its verdict', async () => { + const toastSuccess = vi.spyOn(toast, 'success').mockImplementation(() => '') + const popup = { focus: vi.fn(), closed: false } + const openSpy = vi + .spyOn(window, 'open') + .mockReturnValue(popup as unknown as ReturnType) + const { container, root } = renderCredentialLink({ + type: 'link', + provider: 'google-email', + value: + 'https://sim.test/api/auth/oauth2/authorize?providerId=google-email&callbackURL=https%3A%2F%2Fsim.test%2Fworkspace%2Fworkspace-1%2Fchat%2Fchat-1', + }) + + await act(async () => { + container + .querySelector('a') + ?.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true })) + }) + const attemptId = new URL( + new URL(openSpy.mock.calls[0][0] as string).searchParams.get('callbackURL') ?? '' + ).searchParams.get('oauthAttempt') as string + + expect(toastSuccess).not.toHaveBeenCalled() + await act(async () => { + setOAuthChatAttemptStatus(attemptId, 'connected') + }) + + expect(toastSuccess).toHaveBeenCalledWith('Gmail connected successfully.') + openSpy.mockRestore() + toastSuccess.mockRestore() + act(() => root.unmount()) + }) + + it('settles the row when the popup dies on a page that publishes no verdict', async () => { + // A denied consent lands on /oauth-error, which never writes a verdict and + // never self-closes. The row must not defer to it forever. + const popup = { + focus: vi.fn(), + closed: false, + location: { origin: 'https://sim.test', pathname: '/oauth-error' }, + } + const openSpy = vi + .spyOn(window, 'open') + .mockReturnValue(popup as unknown as ReturnType) + const { container, root } = renderCredentialLink({ + type: 'link', + provider: 'google-email', + value: + 'https://sim.test/api/auth/oauth2/authorize?providerId=google-email&callbackURL=https%3A%2F%2Fsim.test%2Fworkspace%2Fworkspace-1%2Fchat%2Fchat-1', + }) + + await act(async () => { + container + .querySelector('a') + ?.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true })) + }) + await act(async () => { + window.dispatchEvent(new Event('blur')) + window.dispatchEvent(new Event('focus')) + }) + + expect(container.textContent).toContain('Not connected — connect Gmail') + openSpy.mockRestore() + act(() => root.unmount()) + }) + + it('keeps the row retryable when the verdict lands but no credential appears', async () => { + // The credential-draft failure is swallowed server-side, so the flow can + // report success with nothing in the workspace. Label follows the verdict; + // the control must stay clickable so the user can try again. + const toastSuccess = vi.spyOn(toast, 'success').mockImplementation(() => '') + const popup = { focus: vi.fn(), closed: false } + const openSpy = vi + .spyOn(window, 'open') + .mockReturnValue(popup as unknown as ReturnType) + const { container, root } = renderCredentialLink({ + type: 'link', + provider: 'google-email', + value: + 'https://sim.test/api/auth/oauth2/authorize?providerId=google-email&callbackURL=https%3A%2F%2Fsim.test%2Fworkspace%2Fworkspace-1%2Fchat%2Fchat-1', + }) + + await act(async () => { + container + .querySelector('a') + ?.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true })) + }) + const attemptId = new URL( + new URL(openSpy.mock.calls[0][0] as string).searchParams.get('callbackURL') ?? '' + ).searchParams.get('oauthAttempt') as string + await act(async () => { + setOAuthChatAttemptStatus(attemptId, 'connected') + }) + + expect(container.textContent).toContain('Connected Gmail') + expect(container.querySelector('a')?.getAttribute('aria-disabled')).toBe('false') + openSpy.mockRestore() + toastSuccess.mockRestore() + act(() => root.unmount()) + }) + + it('keeps waiting when the tab is refocused while the connect popup is still open', async () => { + const popup = { + focus: vi.fn(), + closed: false, + // Still on the provider's pages: reading location across origins throws. + get location(): Location { + throw new DOMException('cross-origin', 'SecurityError') + }, + } + const openSpy = vi + .spyOn(window, 'open') + .mockReturnValue(popup as unknown as ReturnType) + const { container, root } = renderCredentialLink({ + type: 'link', + provider: 'google-email', + value: + 'https://sim.test/api/auth/oauth2/authorize?providerId=google-email&callbackURL=https%3A%2F%2Fsim.test%2Fworkspace%2Fworkspace-1%2Fchat%2Fchat-1', + }) + + await act(async () => { + container + .querySelector('a') + ?.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true })) + }) + await act(async () => { + window.dispatchEvent(new Event('blur')) + window.dispatchEvent(new Event('focus')) + }) + + expect(container.textContent).toContain('Waiting for Gmail connection') + openSpy.mockRestore() + act(() => root.unmount()) + }) + + it('navigates the tab through the same completion page when the popup is blocked', () => { + const openSpy = vi.spyOn(window, 'open').mockReturnValue(null) + const { container, root } = renderCredentialLink({ + type: 'link', + provider: 'google-email', + value: + 'https://sim.test/api/auth/oauth2/authorize?providerId=google-email&callbackURL=https%3A%2F%2Fsim.test%2Fworkspace%2Fworkspace-1%2Fchat%2Fchat-1', + }) + + const link = container.querySelector('a') + const defaultPrevented = !link?.dispatchEvent( + new MouseEvent('click', { bubbles: true, cancelable: true }) + ) + + expect(defaultPrevented).toBe(false) + const callbackUrl = new URL( + new URL(link?.getAttribute('href') ?? '').searchParams.get('callbackURL') ?? '' + ) + expect(callbackUrl.pathname).toBe('/oauth/chat-complete') + expect(callbackUrl.searchParams.get('oauthAttempt')).not.toBeNull() + openSpy.mockRestore() + act(() => root.unmount()) + }) + it('waits for the credential baseline before opening an OAuth link', () => { mockUseWorkspaceCredentials.mockReturnValue({ data: undefined, diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/use-oauth-chip-connection.ts b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/use-oauth-chip-connection.ts index 74c4b95ebb4..2997e626b6e 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/use-oauth-chip-connection.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/use-oauth-chip-connection.ts @@ -1,9 +1,11 @@ 'use client' import { useCallback, useEffect, useMemo, useRef, useState } from 'react' +import { toast } from '@sim/emcn' import { useParams } from 'next/navigation' import { addOAuthChatAttemptToAuthorizeUrl, + buildOAuthChatCompleteAuthorizeUrl, clearActiveDesktopOAuthChatAttempt, createOAuthChatAttempt, getOAuthCredentialBaseline, @@ -23,6 +25,47 @@ import type { OAuthProvider } from '@/lib/oauth/types' import { parseProvider } from '@/lib/oauth/utils' import { useWorkspaceCredentials } from '@/hooks/queries/credentials' +const OAUTH_POPUP_WINDOW_NAME = 'sim-oauth-connect' +/** Matches the MCP OAuth popup (`hooks/mcp/use-mcp-oauth-popup.ts`) so the two consent windows open alike. */ +const OAUTH_POPUP_FEATURES = 'width=560,height=720,resizable=yes,scrollbars=yes' + +/** + * Same-origin pages an OAuth flow can die on without ever reaching the return + * leg. Better Auth sends pre-state failures — a denied consent, most often — + * to its global error page rather than this flow's callback, and the + * custom-provider callbacks exit to the workspace root with an `error` param. + * Neither publishes a verdict, so a popup sitting on one is finished, not + * in flight. + */ +const OAUTH_POPUP_TERMINAL_PATHS = new Set(['/oauth-error', '/workspace']) + +/** + * Whether the popup still owns the flow — i.e. whether the row should keep + * deferring to it instead of deciding for itself. + * + * A provider page that sets COOP `same-origin` severs the handle, making + * `closed` throw. Treat that as closed: the flow then falls back to the + * ordinary focus verification rather than waiting on a window we cannot see. + */ +function isPopupStillOpen(popup: { window: Window } | null): boolean { + if (!popup) return false + let closed: boolean + try { + closed = popup.window.closed + } catch { + return false + } + if (closed) return false + try { + const { origin, pathname } = popup.window.location + if (origin === window.location.origin && OAUTH_POPUP_TERMINAL_PATHS.has(pathname)) return false + } catch { + // Reading `location` across origins throws, which is the signal we want: + // the popup is still on the provider's pages, so the flow is in flight. + } + return true +} + interface UseOAuthChipConnectionParams { /** Authorize URL streamed by the agent; provider and reconnect scope are read from it. */ connectUrl?: string @@ -43,9 +86,12 @@ export interface OAuthChipConnection { /** True when the row should read as connected, from any signal. */ connected: boolean /** - * True only when *this row's* attempt completed. Workspace-wide observation - * cannot be attributed to one row, so this — not {@link connected} — is what - * may lock the control. + * True only when *this row's* attempt completed AND the credential it was + * supposed to produce is actually present (a reconnect, which produces no + * new credential, is exempt). Workspace-wide observation cannot be + * attributed to one row, so this — not {@link connected} — is what may lock + * the control, and it is deliberately stricter than the label so a connect + * that reported success without landing stays retryable. */ connectedFromAttempt: boolean /** The workspace already holds a credential this row would connect. */ @@ -117,6 +163,7 @@ export function useOAuthChipConnection({ const [connectedFromWorkspaceChange, setConnectedFromWorkspaceChange] = useState(false) const onConnectedRef = useRef(onConnected) const oauthWindowWasAwayRef = useRef(false) + const popupRef = useRef<{ window: Window; attemptId: string } | null>(null) const workspaceCredentialBaselineRef = useRef<{ scope: string baseline: ReturnType @@ -131,8 +178,16 @@ export function useOAuthChipConnection({ credentialTarget, workspaceOAuthCredentials ) - const connectedFromAttempt = connectionStatus === 'connected' - const connected = connectedFromAttempt || connectedFromWorkspaceChange + const verdictConnected = connectionStatus === 'connected' + const connected = verdictConnected || connectedFromWorkspaceChange + // The label trusts the return leg's verdict; the lock does not. A failure to + // create the credential from its draft is swallowed server-side, so a flow + // can report success with nothing in the workspace to show for it — locking + // on the verdict alone would strand the row saying "Connected" with no way + // to retry. A reconnect legitimately produces no new credential, so it has + // nothing to corroborate against and keeps trusting the verdict. + const connectedFromAttempt = + verdictConnected && (reconnectCredentialId ? true : connectedFromWorkspaceChange) useEffect(() => { onConnectedRef.current = onConnected @@ -229,7 +284,6 @@ export function useOAuthChipConnection({ const verifyAfterReturn = async () => { if (!oauthWindowWasAwayRef.current || document.visibilityState !== 'visible') return oauthWindowWasAwayRef.current = false - const attempt = readRowAttempt() const result = await refetchWorkspaceOAuthCredentials() const credentials = result.data ?? [] @@ -246,7 +300,17 @@ export function useOAuthChipConnection({ ) } + // Read the attempt only after the await. A snapshot taken before it goes + // stale the moment the popup publishes its verdict mid-refetch, and the + // stale 'pending' would license the overwrite this check exists to stop — + // deterministically to 'failed' on the reconnect branch below. + const attempt = readRowAttempt() if (!attempt || attempt.status !== 'pending') return + // Clicking back to this tab while the popup is still on the provider's + // consent screen is not a verdict — the flow it owns has not returned + // yet, and settling here would flash "not connected" under a connect the + // user is midway through. Its own return leg publishes the result. + if (isPopupStillOpen(popupRef.current)) return // A reconnect is verified by the callback path, which has proof that the // OAuth flow returned. A plain focus event cannot distinguish it from an // unrelated edit to the same credential. @@ -280,6 +344,23 @@ export function useOAuthChipConnection({ if (connected) onConnectedRef.current?.() }, [connected]) + /** + * The popup publishes the verdict and closes, showing the user nothing they + * keep, so the launching tab surfaces the success toast. Failure + * deliberately stays a label (the retry text): a focus flip mid-flow can + * settle 'failed' prematurely while the popup is still open, and a toast on + * that would read as the flow failing behind the user's back. + */ + useEffect(() => { + const popup = popupRef.current + if (connectionStatus !== 'connected' || !popup) return + const attempt = readOAuthChatAttempt(popup.attemptId) + popupRef.current = null + if (attempt?.status === 'connected') { + toast.success(`${attempt.displayName} connected successfully.`) + } + }, [connectionStatus]) + /** * Desktop app: OAuth cannot run in an embedded window — not in the app * window (better-auth binds the flow's state to the initiating browser's @@ -327,7 +408,32 @@ export function useOAuthChipConnection({ return } - event.currentTarget.href = addOAuthChatAttemptToAuthorizeUrl(connectUrl, attempt.id) + // Web: run the whole flow in a popup so this tab never navigates — the + // return leg lands on the self-closing chat-complete page, whose verdict + // reaches this row over the storage listener. A blocked popup navigates + // to the same URL instead, so the flow keeps the completion page's + // server-backed verdict either way; only an authorize URL with no return + // param to rewrite falls all the way back to the plain tab return. + const completeUrl = buildOAuthChatCompleteAuthorizeUrl(connectUrl, attempt.id) + if (completeUrl) { + // Named per attempt: a card can render several credential rows, and a + // shared name would let the second click renavigate the first row's + // live popup, stranding that attempt with no return leg. + const popup = window.open( + completeUrl, + `${OAUTH_POPUP_WINDOW_NAME}-${attempt.id}`, + OAUTH_POPUP_FEATURES + ) + if (popup) { + event.preventDefault() + popup.focus?.() + popupRef.current = { window: popup, attemptId: attempt.id } + return + } + } + + event.currentTarget.href = + completeUrl ?? addOAuthChatAttemptToAuthorizeUrl(connectUrl, attempt.id) }, [ baseProviderId, diff --git a/apps/sim/lib/credentials/oauth-chat-attempt.test.ts b/apps/sim/lib/credentials/oauth-chat-attempt.test.ts index 3a20b017316..99ae6e78bb1 100644 --- a/apps/sim/lib/credentials/oauth-chat-attempt.test.ts +++ b/apps/sim/lib/credentials/oauth-chat-attempt.test.ts @@ -1,15 +1,19 @@ /** * @vitest-environment jsdom + * @vitest-environment-options { "url": "https://sim.test/workspace/workspace-1/chat/chat-1" } */ import { beforeEach, describe, expect, it } from 'vitest' import { addOAuthChatAttemptToAuthorizeUrl, + buildOAuthChatCompleteAuthorizeUrl, createOAuthChatAttempt, getOAuthCredentialBaseline, hasOAuthCredentialChanged, OAUTH_CHAT_ATTEMPT_EVENT, OAUTH_CHAT_ATTEMPT_PARAM, + OAUTH_CHAT_COMPLETE_PATH, + OAUTH_CHAT_RETURN_TO_PARAM, readLatestOAuthChatAttempt, readOAuthChatAttempt, resolveActiveDesktopOAuthChatAttempt, @@ -47,6 +51,64 @@ describe('OAuth chat attempts', () => { } ) + it('routes the return through the chat-complete page', () => { + const authorizeUrl = buildOAuthChatCompleteAuthorizeUrl( + 'https://sim.test/api/auth/oauth2/authorize?providerId=google-email&callbackURL=https%3A%2F%2Fsim.test%2Fworkspace%2Fworkspace-1%2Fchat%2Fchat-1', + 'attempt-1' + ) + + const callbackUrl = new URL(new URL(authorizeUrl ?? '').searchParams.get('callbackURL') ?? '') + expect(callbackUrl.pathname).toBe(OAUTH_CHAT_COMPLETE_PATH) + expect(callbackUrl.searchParams.get(OAUTH_CHAT_ATTEMPT_PARAM)).toBe('attempt-1') + // Anchored on the server-generated return URL's origin, not this tab's — + // a proxied deployment's two origins need not agree. + expect(callbackUrl.origin).toBe('https://sim.test') + }) + + it('refuses a cross-origin authorize URL so the attempt id never leaves the app', () => { + expect( + buildOAuthChatCompleteAuthorizeUrl( + 'https://evil.example/api/auth/oauth2/authorize?providerId=google-email&callbackURL=https%3A%2F%2Fevil.example%2Fsink', + 'attempt-1' + ) + ).toBeNull() + }) + + it('leaves the attempt off the close-fallback target so it is not re-decided', () => { + const authorizeUrl = buildOAuthChatCompleteAuthorizeUrl( + 'https://sim.test/api/auth/oauth2/authorize?providerId=google-email&callbackURL=https%3A%2F%2Fsim.test%2Fworkspace%2Fworkspace-1%2Fchat%2Fchat-1', + 'attempt-1' + ) + + const callbackUrl = new URL(new URL(authorizeUrl ?? '').searchParams.get('callbackURL') ?? '') + const returnTo = new URL(callbackUrl.searchParams.get(OAUTH_CHAT_RETURN_TO_PARAM) ?? '') + expect(returnTo.pathname).toBe('/workspace/workspace-1/chat/chat-1') + expect(returnTo.searchParams.get(OAUTH_CHAT_ATTEMPT_PARAM)).toBeNull() + }) + + it.each(['instagram', 'shopify', 'trello'])( + 'routes the %s return through the chat-complete page', + (provider) => { + const authorizeUrl = buildOAuthChatCompleteAuthorizeUrl( + `https://sim.test/api/auth/${provider}/authorize?returnUrl=${encodeURIComponent('https://sim.test/workspace/workspace-1/chat/chat-1')}`, + 'attempt-2' + ) + + const returnUrl = new URL(new URL(authorizeUrl ?? '').searchParams.get('returnUrl') ?? '') + expect(returnUrl.pathname).toBe(OAUTH_CHAT_COMPLETE_PATH) + expect(returnUrl.searchParams.get(OAUTH_CHAT_ATTEMPT_PARAM)).toBe('attempt-2') + } + ) + + it('declines a chat-complete URL when there is no return param to rewrite', () => { + expect( + buildOAuthChatCompleteAuthorizeUrl( + 'https://sim.test/api/auth/oauth2/authorize?providerId=google-email', + 'attempt-3' + ) + ).toBeNull() + }) + it('publishes and persists server-verified completion', () => { let publishedStatus: string | undefined window.addEventListener( diff --git a/apps/sim/lib/credentials/oauth-chat-attempt.ts b/apps/sim/lib/credentials/oauth-chat-attempt.ts index 65ad90a3a31..0a6fd6bed3e 100644 --- a/apps/sim/lib/credentials/oauth-chat-attempt.ts +++ b/apps/sim/lib/credentials/oauth-chat-attempt.ts @@ -4,6 +4,8 @@ import { generateShortId } from '@sim/utils/id' export const OAUTH_CHAT_ATTEMPT_PARAM = 'oauthAttempt' export const OAUTH_CHAT_ATTEMPT_EVENT = 'sim:oauth-chat-attempt' +export const OAUTH_CHAT_COMPLETE_PATH = '/oauth/chat-complete' +export const OAUTH_CHAT_RETURN_TO_PARAM = 'returnTo' const OAUTH_CHAT_ATTEMPT_KEY_PREFIX = 'sim.oauth-chat-attempt.' const OAUTH_CHAT_LATEST_KEY_PREFIX = 'sim.oauth-chat-latest.' @@ -143,8 +145,13 @@ function isOAuthChatAttempt(value: unknown): value is OAuthChatAttempt { function writeOAuthChatAttempt(attempt: OAuthChatAttempt): void { if (typeof window === 'undefined') return - window.localStorage.setItem(attemptStorageKey(attempt.id), JSON.stringify(attempt)) - window.localStorage.setItem(latestAttemptStorageKey(attempt), attempt.id) + // A blocked or full store must not throw into the caller: the chat-complete + // page writes the verdict before closing its window, so an unguarded throw + // would strand the popup open instead of losing only the verdict. + try { + window.localStorage.setItem(attemptStorageKey(attempt.id), JSON.stringify(attempt)) + window.localStorage.setItem(latestAttemptStorageKey(attempt), attempt.id) + } catch {} window.dispatchEvent( new CustomEvent(OAUTH_CHAT_ATTEMPT_EVENT, { detail: attempt }) ) @@ -244,11 +251,31 @@ function appendAttemptToReturnUrl(rawReturnUrl: string, attemptId: string): stri return returnUrl.toString() } -/** Adds the attempt id to the eventual same-origin OAuth return URL. */ -export function addOAuthChatAttemptToAuthorizeUrl(rawUrl: string, attemptId: string): string { +interface AuthorizeReturnTarget { + authorizeUrl: URL + /** Which param this provider's authorize route reads the return URL from. */ + returnParam: 'callbackURL' | 'returnUrl' + rawReturnUrl: string | null +} + +/** + * Locates the return URL an authorize link will come back through. Shared so + * both builders below agree on which param carries it — a provider added to + * one and missed in the other would break only the path its caller uses. + */ +function resolveAuthorizeReturnTarget(rawUrl: string): AuthorizeReturnTarget { const authorizeUrl = new URL(rawUrl, window.location.origin) const returnParam = authorizeUrl.searchParams.has('callbackURL') ? 'callbackURL' : 'returnUrl' - const rawReturnUrl = authorizeUrl.searchParams.get(returnParam) + return { + authorizeUrl, + returnParam, + rawReturnUrl: authorizeUrl.searchParams.get(returnParam), + } +} + +/** Adds the attempt id to the eventual same-origin OAuth return URL. */ +export function addOAuthChatAttemptToAuthorizeUrl(rawUrl: string, attemptId: string): string { + const { authorizeUrl, returnParam, rawReturnUrl } = resolveAuthorizeReturnTarget(rawUrl) if (rawReturnUrl) { authorizeUrl.searchParams.set(returnParam, appendAttemptToReturnUrl(rawReturnUrl, attemptId)) @@ -258,3 +285,43 @@ export function addOAuthChatAttemptToAuthorizeUrl(rawUrl: string, attemptId: str return authorizeUrl.toString() } + +/** + * Chat-flavored authorize URL: the return leg lands on the lightweight + * chat-complete page — which publishes the verdict and closes the window — + * instead of reloading the whole app in the OAuth window. + * + * The complete page's fallback redirect target is the plain return URL with no + * attempt id, so a window that cannot close lands on the chat surface without + * its return router re-deciding a verdict the completion page already + * published. + * + * Returns null when the authorize URL is not a same-origin Sim route, or + * carries no return param to rewrite; the caller falls back to + * {@link addOAuthChatAttemptToAuthorizeUrl} and its plain anchor navigation. + */ +export function buildOAuthChatCompleteAuthorizeUrl( + rawUrl: string, + attemptId: string +): string | null { + const { authorizeUrl, returnParam, rawReturnUrl } = resolveAuthorizeReturnTarget(rawUrl) + // The connect URL is streamed model output, checked only for a safe protocol. + // Refusing a foreign origin here keeps the attempt id out of a URL we do not + // control, and keeps the caller on its anchor — whose rel='noopener' the + // popup path would otherwise drop, handing a hostile page our window handle. + if (authorizeUrl.origin !== window.location.origin) return null + if (!rawReturnUrl) return null + + // Anchored on the return URL the server generated, not this tab's origin — + // both the authorize route and the custom-provider callbacks accept a return + // target only when it matches the deployment's configured base URL, which a + // proxied or aliased origin need not equal. + const completeUrl = new URL( + OAUTH_CHAT_COMPLETE_PATH, + new URL(rawReturnUrl, window.location.origin).origin + ) + completeUrl.searchParams.set(OAUTH_CHAT_ATTEMPT_PARAM, attemptId) + completeUrl.searchParams.set(OAUTH_CHAT_RETURN_TO_PARAM, rawReturnUrl) + authorizeUrl.searchParams.set(returnParam, completeUrl.toString()) + return authorizeUrl.toString() +} From 58f5e88eac4fef590fe21c31d7219e62388c0ec9 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 7 Aug 2026 22:14:36 -0700 Subject: [PATCH 02/10] fix(mship): settle the connect row from the popup, not from focus alone Addresses the review findings on the chat OAuth return leg. - Watch the popup on an interval. A provider interstitial bouncing to the workspace root, a denied consent on /oauth-error, or a closed window all end the flow without publishing a verdict or firing any event in this tab, so the row waited forever. The focus handler also no longer consumes the away flag when it defers to a live popup. - Focus an already-running popup on a repeat click instead of starting a rival attempt, which orphaned the first flow's verdict on an attempt id the row had stopped reading. - Settle from the refetched credentials on the popup success path, so the row's lock is corroborated and a connected row stops being clickable. Extracts the shared refetch-then-decide step into settleFromCredentials, used by the focus handler, the popup watcher, and the success path. --- .../special-tags/special-tags.test.tsx | 197 ++++++++++++++++++ .../special-tags/use-oauth-chip-connection.ts | 133 ++++++++---- 2 files changed, 286 insertions(+), 44 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.test.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.test.tsx index 5a604b29884..175e2691308 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.test.tsx @@ -505,6 +505,133 @@ describe('CredentialDisplay link tag', () => { act(() => root.unmount()) }) + it('settles the row from the popup watcher when the flow ends with no event in this tab', async () => { + // A provider interstitial (Trello parks on one for ~3s) bounces to the + // workspace root without ever reaching the completion page. Neither page + // publishes a verdict, and a user who never leaves this tab gets no focus + // event either — only the watcher can end the wait. + vi.useFakeTimers() + const popup = { + focus: vi.fn(), + closed: false, + location: { origin: 'https://sim.test', pathname: '/api/auth/trello/callback' }, + } + const openSpy = vi + .spyOn(window, 'open') + .mockReturnValue(popup as unknown as ReturnType) + const { container, root } = renderCredentialLink({ + type: 'link', + provider: 'google-email', + value: + 'https://sim.test/api/auth/oauth2/authorize?providerId=google-email&callbackURL=https%3A%2F%2Fsim.test%2Fworkspace%2Fworkspace-1%2Fchat%2Fchat-1', + }) + + await act(async () => { + container + .querySelector('a') + ?.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true })) + }) + await act(async () => { + await vi.advanceTimersByTimeAsync(2000) + }) + + // The interstitial is not terminal, so the row is still deferring. + expect(container.textContent).toContain('Waiting for Gmail connection') + + popup.location.pathname = '/workspace' + await act(async () => { + await vi.advanceTimersByTimeAsync(2000) + }) + + expect(container.textContent).toContain('Not connected — connect Gmail') + vi.useRealTimers() + openSpy.mockRestore() + act(() => root.unmount()) + }) + + it('focuses the live popup instead of starting a rival attempt on a repeat click', async () => { + const toastSuccess = vi.spyOn(toast, 'success').mockImplementation(() => '') + const popup = { + focus: vi.fn(), + closed: false, + // Still on the provider's pages: reading location across origins throws. + get location(): Location { + throw new DOMException('cross-origin', 'SecurityError') + }, + } + const openSpy = vi + .spyOn(window, 'open') + .mockReturnValue(popup as unknown as ReturnType) + const { container, root } = renderCredentialLink({ + type: 'link', + provider: 'google-email', + value: + 'https://sim.test/api/auth/oauth2/authorize?providerId=google-email&callbackURL=https%3A%2F%2Fsim.test%2Fworkspace%2Fworkspace-1%2Fchat%2Fchat-1', + }) + const link = container.querySelector('a') + + await act(async () => { + link?.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true })) + }) + const attemptId = new URL( + new URL(openSpy.mock.calls[0][0] as string).searchParams.get('callbackURL') ?? '' + ).searchParams.get('oauthAttempt') as string + await act(async () => { + link?.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true })) + }) + + // No second window, and no second attempt to strand the first one's verdict. + expect(openSpy).toHaveBeenCalledOnce() + expect(popup.focus).toHaveBeenCalledTimes(2) + + await act(async () => { + setOAuthChatAttemptStatus(attemptId, 'connected') + }) + + expect(container.textContent).toContain('Connected Gmail') + openSpy.mockRestore() + toastSuccess.mockRestore() + act(() => root.unmount()) + }) + + it('locks the row once the popup verdict is corroborated by a refetched credential', async () => { + const toastSuccess = vi.spyOn(toast, 'success').mockImplementation(() => '') + const popup = { focus: vi.fn(), closed: false } + const openSpy = vi + .spyOn(window, 'open') + .mockReturnValue(popup as unknown as ReturnType) + mockRefetchWorkspaceCredentials.mockResolvedValue({ + data: [{ id: 'new-gmail', providerId: 'google', updatedAt: '2026-08-07T10:05:00Z' }], + }) + const { container, root } = renderCredentialLink({ + type: 'link', + provider: 'google-email', + value: + 'https://sim.test/api/auth/oauth2/authorize?providerId=google-email&callbackURL=https%3A%2F%2Fsim.test%2Fworkspace%2Fworkspace-1%2Fchat%2Fchat-1', + }) + + await act(async () => { + container + .querySelector('a') + ?.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true })) + }) + const attemptId = new URL( + new URL(openSpy.mock.calls[0][0] as string).searchParams.get('callbackURL') ?? '' + ).searchParams.get('oauthAttempt') as string + await act(async () => { + setOAuthChatAttemptStatus(attemptId, 'connected') + }) + + // The popup flow never blurs this tab, so this refetch is the only one that + // can corroborate the verdict and let the control lock. + expect(mockRefetchWorkspaceCredentials).toHaveBeenCalled() + expect(container.textContent).toContain('Connected Gmail') + expect(container.querySelector('a')?.getAttribute('aria-disabled')).toBe('true') + openSpy.mockRestore() + toastSuccess.mockRestore() + act(() => root.unmount()) + }) + it('navigates the tab through the same completion page when the popup is blocked', () => { const openSpy = vi.spyOn(window, 'open').mockReturnValue(null) const { container, root } = renderCredentialLink({ @@ -816,6 +943,76 @@ describe('CredentialDisplay link tag', () => { expect(container.querySelector('input')).toBeNull() act(() => root.unmount()) }) + + it('keeps a typed secret while a sibling row runs its OAuth connect', async () => { + // The card's secret drafts live in component state until its Submit, so a + // connect that navigates this tab away discards whatever the user already + // typed into a sibling row. Running the flow in a popup is what makes the + // mixed card safe: this tab is never unloaded, so the draft outlives the + // connect — including the re-render its verdict and refetch trigger. + const toastSuccess = vi.spyOn(toast, 'success').mockImplementation(() => '') + const popup = { focus: vi.fn(), closed: false } + const openSpy = vi + .spyOn(window, 'open') + .mockReturnValue(popup as unknown as ReturnType) + const container = document.createElement('div') + const root: Root = createRoot(container) + const onOptionSelect = vi.fn() + const data: CredentialItemData[] = [ + { + type: 'link', + provider: 'google-email', + value: + 'https://sim.test/api/auth/oauth2/authorize?providerId=google-email&callbackURL=https%3A%2F%2Fsim.test%2Fworkspace%2Fworkspace-1%2Fchat%2Fchat-1', + }, + { type: 'secret_input', name: 'OPENAI_API_KEY' }, + ] + act(() => { + root.render( + + ) + }) + + const secretInput = container.querySelector('input') + act(() => { + if (!secretInput) return + const valueSetter = Object.getOwnPropertyDescriptor( + window.HTMLInputElement.prototype, + 'value' + )?.set + valueSetter?.call(secretInput, 'sk-test-key') + secretInput.dispatchEvent(new Event('input', { bubbles: true })) + }) + + await act(async () => { + container + .querySelector('a') + ?.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true })) + }) + const attemptId = new URL( + new URL(openSpy.mock.calls[0][0] as string).searchParams.get('callbackURL') ?? '' + ).searchParams.get('oauthAttempt') as string + await act(async () => { + setOAuthChatAttemptStatus(attemptId, 'connected') + }) + + // The field masks while unfocused, so assert on what the draft is actually + // for: submitting it. A mask of the right length is not proof it survived. + const submitButton = Array.from(container.querySelectorAll('button')).find( + (button) => button.textContent === 'Submit' + ) + await act(async () => { + submitButton?.click() + }) + + expect(mockUpsertWorkspaceEnvironment).toHaveBeenCalledWith({ + workspaceId: 'workspace-1', + variables: { OPENAI_API_KEY: 'sk-test-key' }, + }) + openSpy.mockRestore() + toastSuccess.mockRestore() + act(() => root.unmount()) + }) }) describe('parseSpecialTags sim_key placeholder', () => { diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/use-oauth-chip-connection.ts b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/use-oauth-chip-connection.ts index 2997e626b6e..ab06904773d 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/use-oauth-chip-connection.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/use-oauth-chip-connection.ts @@ -28,6 +28,12 @@ import { useWorkspaceCredentials } from '@/hooks/queries/credentials' const OAUTH_POPUP_WINDOW_NAME = 'sim-oauth-connect' /** Matches the MCP OAuth popup (`hooks/mcp/use-mcp-oauth-popup.ts`) so the two consent windows open alike. */ const OAUTH_POPUP_FEATURES = 'width=560,height=720,resizable=yes,scrollbars=yes' +/** + * How often to re-examine a live popup. Matches the MCP OAuth popup's own + * watcher: a closed or terminal window fires no event in the opener, so + * polling the handle is the only way to observe it. + */ +const OAUTH_POPUP_POLL_INTERVAL_MS = 400 /** * Same-origin pages an OAuth flow can die on without ever reaching the return @@ -266,6 +272,44 @@ export function useOAuthChipConnection({ }) }, [activeAttemptId, controlId, providerId, reconnectCredentialId, workspaceId]) + /** + * Refetches the workspace credentials and settles this row's pending attempt + * against them. Shared by the two endings the completion page cannot cover: a + * return to this tab, and a popup seen closed or parked on a terminal page. + * Idempotent — an attempt is only rewritten while still `pending`. + */ + const settleFromCredentials = useCallback(async () => { + const result = await refetchWorkspaceOAuthCredentials() + const credentials = result.data ?? [] + + // Also covers a credential connected from another tab, which this row never + // launched and query state alone would surface only on its next fetch. + const storedBaseline = workspaceCredentialBaselineRef.current + if (!reconnectCredentialId && storedBaseline?.scope === credentialScope) { + setConnectedFromWorkspaceChange( + hasOAuthCredentialChanged({ ...credentialTarget, ...storedBaseline.baseline }, credentials) + ) + } + + // Read after the await: a snapshot taken before it goes stale the moment the + // popup publishes its verdict mid-refetch, and that stale 'pending' would + // license the very overwrite the guard below exists to prevent. + const attempt = readRowAttempt() + if (!attempt || attempt.status !== 'pending') return + // Only the callback path can prove a reconnect returned — an unrelated edit + // to the credential is indistinguishable from one here. + const attemptConnected = reconnectCredentialId + ? false + : hasOAuthCredentialChanged(attempt, credentials) + setOAuthChatAttemptStatus(attempt.id, attemptConnected ? 'connected' : 'failed') + }, [ + credentialScope, + credentialTarget, + readRowAttempt, + reconnectCredentialId, + refetchWorkspaceOAuthCredentials, + ]) + useEffect(() => { const syncStatus = () => setConnectionStatus(readRowAttempt()?.status ?? null) window.addEventListener(OAUTH_CHAT_ATTEMPT_EVENT, syncStatus) @@ -281,47 +325,19 @@ export function useOAuthChipConnection({ const markAway = () => { oauthWindowWasAwayRef.current = true } - const verifyAfterReturn = async () => { + const verifyAfterReturn = () => { if (!oauthWindowWasAwayRef.current || document.visibilityState !== 'visible') return - oauthWindowWasAwayRef.current = false - const result = await refetchWorkspaceOAuthCredentials() - const credentials = result.data ?? [] - - // Refetching on every return closes the other-tab gap even when this row - // did not launch the connection. Query state normally drives the effect - // above; updating here as well makes the result immediate and deterministic. - const storedBaseline = workspaceCredentialBaselineRef.current - if (!reconnectCredentialId && storedBaseline?.scope === credentialScope) { - setConnectedFromWorkspaceChange( - hasOAuthCredentialChanged( - { ...credentialTarget, ...storedBaseline.baseline }, - credentials - ) - ) - } - - // Read the attempt only after the await. A snapshot taken before it goes - // stale the moment the popup publishes its verdict mid-refetch, and the - // stale 'pending' would license the overwrite this check exists to stop — - // deterministically to 'failed' on the reconnect branch below. - const attempt = readRowAttempt() - if (!attempt || attempt.status !== 'pending') return - // Clicking back to this tab while the popup is still on the provider's - // consent screen is not a verdict — the flow it owns has not returned - // yet, and settling here would flash "not connected" under a connect the - // user is midway through. Its own return leg publishes the result. + // A live popup still owns the flow; settling now would flash "not + // connected" mid-connect. The away flag deliberately survives this bail — + // consuming it would spend the only signal a later return has to work + // with, and no second focus event is guaranteed to arrive. if (isPopupStillOpen(popupRef.current)) return - // A reconnect is verified by the callback path, which has proof that the - // OAuth flow returned. A plain focus event cannot distinguish it from an - // unrelated edit to the same credential. - const attemptConnected = reconnectCredentialId - ? false - : hasOAuthCredentialChanged(attempt, credentials) - setOAuthChatAttemptStatus(attempt.id, attemptConnected ? 'connected' : 'failed') + oauthWindowWasAwayRef.current = false + void settleFromCredentials() } const handleVisibilityChange = () => { if (document.visibilityState === 'hidden') markAway() - else void verifyAfterReturn() + else verifyAfterReturn() } window.addEventListener('blur', markAway) @@ -332,13 +348,24 @@ export function useOAuthChipConnection({ window.removeEventListener('focus', verifyAfterReturn) document.removeEventListener('visibilitychange', handleVisibilityChange) } - }, [ - credentialScope, - credentialTarget, - readRowAttempt, - reconnectCredentialId, - refetchWorkspaceOAuthCredentials, - ]) + }, [settleFromCredentials]) + + /** + * Watches a live popup for the endings that publish no verdict — a provider + * interstitial exiting to the workspace root, a denied consent landing on the + * OAuth error page, or the user closing the window. None reach the completion + * page or fire an event here, and the user need never leave this tab for a + * focus event either, so polling is the only way the row learns it is over. + */ + useEffect(() => { + if (connectionStatus !== 'pending' || !popupRef.current) return + const poll = window.setInterval(() => { + if (isPopupStillOpen(popupRef.current)) return + popupRef.current = null + void settleFromCredentials() + }, OAUTH_POPUP_POLL_INTERVAL_MS) + return () => window.clearInterval(poll) + }, [connectionStatus, settleFromCredentials]) useEffect(() => { if (connected) onConnectedRef.current?.() @@ -356,10 +383,14 @@ export function useOAuthChipConnection({ if (connectionStatus !== 'connected' || !popup) return const attempt = readOAuthChatAttempt(popup.attemptId) popupRef.current = null + // The verdict settles the label, but the lock also waits on the credential + // being observable. A popup flow never blurs this tab, so nothing else + // refetches here and the row would read "Connected" yet stay clickable. + void settleFromCredentials() if (attempt?.status === 'connected') { toast.success(`${attempt.displayName} connected successfully.`) } - }, [connectionStatus]) + }, [connectionStatus, settleFromCredentials]) /** * Desktop app: OAuth cannot run in an embedded window — not in the app @@ -376,6 +407,20 @@ export function useOAuthChipConnection({ event.preventDefault() return } + // A second click would replace both the active attempt and the window + // handle, orphaning the first flow — its verdict lands on an attempt id + // the row no longer reads, so a successful connect waits forever. The + // live window is the flow; surface it instead of starting a rival one. + const livePopup = popupRef.current + if (isPopupStillOpen(livePopup)) { + event.preventDefault() + try { + livePopup?.window.focus?.() + } catch { + // A COOP-severed handle refuses focus, but still owns the flow. + } + return + } const attempt = createOAuthChatAttempt({ workspaceId, providerId, From d173a316c5c4220f37ccbf7c75d85f6e42ce0046 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 7 Aug 2026 22:28:08 -0700 Subject: [PATCH 03/10] fix(mship): never read a disowned popup handle as a finished flow A provider page with COOP same-origin disowns the popup, and the disowned handle reports closed for a consent screen still running. The watcher took that as an ending and published 'failed' against a live flow. - Replace the boolean with a three-state observation. Only a same-origin terminal page counts as 'ended'; a closed-or-disowned handle is 'unobservable' and publishes no verdict. Closing a popup hands focus back to this tab anyway, so the focus verification settles that case. - Stop the interval before settling. The refetch leaves the status pending for its duration, so a running interval could fire again and resolve an attempt a retry had since replaced. - Gate the success toast on a launched-attempt ref rather than the window handle, which the watcher clears before React applies the verdict. --- .../special-tags/special-tags.test.tsx | 33 ++++++++++ .../special-tags/use-oauth-chip-connection.ts | 61 ++++++++++++++----- 2 files changed, 78 insertions(+), 16 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.test.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.test.tsx index 175e2691308..91384a45290 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.test.tsx @@ -549,6 +549,39 @@ describe('CredentialDisplay link tag', () => { act(() => root.unmount()) }) + it('does not fail the row when a disowned popup handle reports closed', async () => { + // A provider page with COOP `same-origin` disowns the window, and the + // disowned handle reports `closed` for a consent screen that is still + // running. The watcher must not read that as an ending: publishing 'failed' + // here would fight a live flow, and the row would invite a rival retry. + vi.useFakeTimers() + const popup = { focus: vi.fn(), closed: true } + const openSpy = vi + .spyOn(window, 'open') + .mockReturnValue(popup as unknown as ReturnType) + const { container, root } = renderCredentialLink({ + type: 'link', + provider: 'google-email', + value: + 'https://sim.test/api/auth/oauth2/authorize?providerId=google-email&callbackURL=https%3A%2F%2Fsim.test%2Fworkspace%2Fworkspace-1%2Fchat%2Fchat-1', + }) + + await act(async () => { + container + .querySelector('a') + ?.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true })) + }) + await act(async () => { + await vi.advanceTimersByTimeAsync(4000) + }) + + expect(container.textContent).toContain('Waiting for Gmail connection') + expect(container.textContent).not.toContain('Not connected') + vi.useRealTimers() + openSpy.mockRestore() + act(() => root.unmount()) + }) + it('focuses the live popup instead of starting a rival attempt on a repeat click', async () => { const toastSuccess = vi.spyOn(toast, 'success').mockImplementation(() => '') const popup = { diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/use-oauth-chip-connection.ts b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/use-oauth-chip-connection.ts index ab06904773d..a0f7064c3f5 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/use-oauth-chip-connection.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/use-oauth-chip-connection.ts @@ -46,30 +46,43 @@ const OAUTH_POPUP_POLL_INTERVAL_MS = 400 const OAUTH_POPUP_TERMINAL_PATHS = new Set(['/oauth-error', '/workspace']) /** - * Whether the popup still owns the flow — i.e. whether the row should keep - * deferring to it instead of deciding for itself. + * What the opener can actually prove about a popup it launched. * - * A provider page that sets COOP `same-origin` severs the handle, making - * `closed` throw. Treat that as closed: the flow then falls back to the - * ordinary focus verification rather than waiting on a window we cannot see. + * `ended` is reserved for positive evidence — a same-origin page we can read + * that is known to publish no verdict. A handle that reports closed is only + * `unobservable`: a provider page with COOP `same-origin` disowns the window, + * and the disowned handle reports `closed` for a consent screen that is still + * running. Treating that as an ending would fail a live flow. */ -function isPopupStillOpen(popup: { window: Window } | null): boolean { - if (!popup) return false +type PopupObservation = 'live' | 'ended' | 'unobservable' + +function observePopup(popup: { window: Window } | null): PopupObservation { + if (!popup) return 'unobservable' let closed: boolean try { closed = popup.window.closed } catch { - return false + return 'unobservable' } - if (closed) return false + if (closed) return 'unobservable' try { const { origin, pathname } = popup.window.location - if (origin === window.location.origin && OAUTH_POPUP_TERMINAL_PATHS.has(pathname)) return false + if (origin === window.location.origin && OAUTH_POPUP_TERMINAL_PATHS.has(pathname)) { + return 'ended' + } } catch { // Reading `location` across origins throws, which is the signal we want: // the popup is still on the provider's pages, so the flow is in flight. } - return true + return 'live' +} + +/** + * Whether the popup still owns the flow — i.e. whether the row should keep + * deferring to it instead of deciding for itself. + */ +function isPopupStillOpen(popup: { window: Window } | null): boolean { + return observePopup(popup) === 'live' } interface UseOAuthChipConnectionParams { @@ -170,6 +183,8 @@ export function useOAuthChipConnection({ const onConnectedRef = useRef(onConnected) const oauthWindowWasAwayRef = useRef(false) const popupRef = useRef<{ window: Window; attemptId: string } | null>(null) + /** Attempt this row launched in a popup, outliving the window handle so the success toast survives the watcher clearing it. */ + const launchedAttemptIdRef = useRef(null) const workspaceCredentialBaselineRef = useRef<{ scope: string baseline: ReturnType @@ -360,9 +375,18 @@ export function useOAuthChipConnection({ useEffect(() => { if (connectionStatus !== 'pending' || !popupRef.current) return const poll = window.setInterval(() => { - if (isPopupStillOpen(popupRef.current)) return + const observation = observePopup(popupRef.current) + if (observation === 'live') return + // Stop before settling: the refetch leaves the status `pending` for its + // duration, so a running interval would keep firing and a later tick + // could resolve an attempt a retry had since replaced. + window.clearInterval(poll) popupRef.current = null - void settleFromCredentials() + // Only `ended` is proof the flow finished with nothing published. A + // closed-or-disowned handle is not, so this publishes no verdict for it — + // closing a popup hands focus back to this tab anyway, and the focus + // verification settles it with the same refetch. + if (observation === 'ended') void settleFromCredentials() }, OAUTH_POPUP_POLL_INTERVAL_MS) return () => window.clearInterval(poll) }, [connectionStatus, settleFromCredentials]) @@ -379,9 +403,13 @@ export function useOAuthChipConnection({ * that would read as the flow failing behind the user's back. */ useEffect(() => { - const popup = popupRef.current - if (connectionStatus !== 'connected' || !popup) return - const attempt = readOAuthChatAttempt(popup.attemptId) + const launchedAttemptId = launchedAttemptIdRef.current + if (connectionStatus !== 'connected' || !launchedAttemptId) return + const attempt = readOAuthChatAttempt(launchedAttemptId) + // Tracked separately from `popupRef`, which the watcher clears the moment + // the window looks finished — often before React has applied the + // storage-driven verdict, which would swallow the announcement. + launchedAttemptIdRef.current = null popupRef.current = null // The verdict settles the label, but the lock also waits on the credential // being observable. A popup flow never blurs this tab, so nothing else @@ -473,6 +501,7 @@ export function useOAuthChipConnection({ event.preventDefault() popup.focus?.() popupRef.current = { window: popup, attemptId: attempt.id } + launchedAttemptIdRef.current = attempt.id return } } From 10bd7b7afb9d35f85bb9b648a64d13ab84d2d73c Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 7 Aug 2026 22:32:27 -0700 Subject: [PATCH 04/10] fix(oauth): keep the chat connect return leg in its opener's browsing context MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit /oauth/chat-complete runs as a popup but fell into the strict COOP rule, so same-origin moved it into its own browsing-context group the moment it loaded — disowning it from the tab that opened it. That is the documented cause of a popup that is not reliably script-closable and whose opener sees window.closed report true for a live window. Matches it to its opener's same-origin-allow-popups instead, which is the directive the platform provides for exactly this case. --- apps/sim/next.config.ts | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/apps/sim/next.config.ts b/apps/sim/next.config.ts index 172bf16d220..1b8b15517d3 100644 --- a/apps/sim/next.config.ts +++ b/apps/sim/next.config.ts @@ -318,8 +318,12 @@ const nextConfig: NextConfig = { }, { // Exclude Vercel internal resources and static assets from strict COOP, Google Drive Picker - // and the /demo Cal.com booking embed to prevent 'refused to connect' / slow-load issues - source: '/((?!_next|_vercel|api|favicon.ico|w/.*|workspace/.*|api/tools/drive|demo).*)', + // and the /demo Cal.com booking embed to prevent 'refused to connect' / slow-load issues. + // `oauth/chat-complete` runs *as* a popup: `same-origin` moves a document into its own + // browsing-context group, which would disown it from the opener that launched it the moment + // it loads — leaving it not reliably script-closable and its opener unable to observe it. + source: + '/((?!_next|_vercel|api|favicon.ico|w/.*|workspace/.*|api/tools/drive|demo|oauth/chat-complete).*)', headers: [ { key: 'Cross-Origin-Opener-Policy', @@ -342,8 +346,11 @@ const nextConfig: NextConfig = { ], }, { - // For main app routes, Google Drive Picker, the /demo Cal.com embed, and Vercel resources - use permissive policies - source: '/(w/.*|workspace/.*|api/tools/drive|demo.*|_next/.*|_vercel/.*)', + // For main app routes, Google Drive Picker, the /demo Cal.com embed, the chat OAuth popup + // return leg, and Vercel resources - use permissive policies. The return leg matches its + // opener's value so the two stay in one browsing-context group. + source: + '/(w/.*|workspace/.*|api/tools/drive|demo.*|oauth/chat-complete|_next/.*|_vercel/.*)', headers: [ { key: 'Cross-Origin-Embedder-Policy', From 86b57e372fd07ab824eaa1b8443f54425d8423d4 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 7 Aug 2026 22:35:00 -0700 Subject: [PATCH 05/10] fix(oauth): keep every page an OAuth popup lands on observable to its opener The popup watcher settles on a same-origin terminal page, but both entries in OAUTH_POPUP_TERMINAL_PATHS were served strict same-origin COOP, which moves the popup into its own browsing-context group. The opener could then neither read its location nor trust window.closed, so the terminal-page branch could never fire in production and a flow exiting through one of those pages left the row waiting until the user happened to refocus the tab. Serves /oauth-error and the /workspace root the same same-origin-allow-popups their opener uses. The workspace root previously fell under the strict rule while every /workspace/... route already got the permissive one. --- apps/sim/next.config.ts | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/apps/sim/next.config.ts b/apps/sim/next.config.ts index 1b8b15517d3..3cfa4976ee0 100644 --- a/apps/sim/next.config.ts +++ b/apps/sim/next.config.ts @@ -319,11 +319,15 @@ const nextConfig: NextConfig = { { // Exclude Vercel internal resources and static assets from strict COOP, Google Drive Picker // and the /demo Cal.com booking embed to prevent 'refused to connect' / slow-load issues. - // `oauth/chat-complete` runs *as* a popup: `same-origin` moves a document into its own - // browsing-context group, which would disown it from the opener that launched it the moment - // it loads — leaving it not reliably script-closable and its opener unable to observe it. + // The OAuth popup pages are excluded because `same-origin` moves a document into its own + // browsing-context group, disowning it from the opener that launched it: the popup stops + // being reliably script-closable and its opener sees `closed` report true for a live window. + // That covers the return leg (`oauth/chat-complete`) and both pages a flow can exit on + // without reaching it — `oauth-error` and the `workspace` root, which the custom-provider + // callbacks bounce to. Bare `workspace` also matches `/workspace` itself, which previously + // fell here while every `/workspace/...` route got the permissive policy below. source: - '/((?!_next|_vercel|api|favicon.ico|w/.*|workspace/.*|api/tools/drive|demo|oauth/chat-complete).*)', + '/((?!_next|_vercel|api|favicon.ico|w/.*|workspace|api/tools/drive|demo|oauth-error|oauth/chat-complete).*)', headers: [ { key: 'Cross-Origin-Opener-Policy', @@ -346,11 +350,11 @@ const nextConfig: NextConfig = { ], }, { - // For main app routes, Google Drive Picker, the /demo Cal.com embed, the chat OAuth popup - // return leg, and Vercel resources - use permissive policies. The return leg matches its - // opener's value so the two stay in one browsing-context group. + // For main app routes, Google Drive Picker, the /demo Cal.com embed, the pages an OAuth + // popup can land on, and Vercel resources - use permissive policies. The popup pages match + // their opener's value so the two stay in one browsing-context group. source: - '/(w/.*|workspace/.*|api/tools/drive|demo.*|oauth/chat-complete|_next/.*|_vercel/.*)', + '/(w/.*|workspace.*|api/tools/drive|demo.*|oauth-error|oauth/chat-complete|_next/.*|_vercel/.*)', headers: [ { key: 'Cross-Origin-Embedder-Policy', From 656c7badf174eefce2da51f993b932f12b2e4a8c Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 7 Aug 2026 22:38:40 -0700 Subject: [PATCH 06/10] test(mship): cover the announcement surviving an early popup release The success toast is gated on the launched-attempt ref rather than the window handle; nothing pinned that. Adds the regression test, and trims the comment duplication the fix left behind. --- .../chat-complete/chat-complete-handoff.tsx | 27 +++++------- .../special-tags/special-tags.test.tsx | 41 +++++++++++++++++++ .../special-tags/use-oauth-chip-connection.ts | 3 -- 3 files changed, 52 insertions(+), 19 deletions(-) diff --git a/apps/sim/app/oauth/chat-complete/chat-complete-handoff.tsx b/apps/sim/app/oauth/chat-complete/chat-complete-handoff.tsx index 9b6c3f4a08c..258bf17d1e8 100644 --- a/apps/sim/app/oauth/chat-complete/chat-complete-handoff.tsx +++ b/apps/sim/app/oauth/chat-complete/chat-complete-handoff.tsx @@ -26,24 +26,19 @@ function sanitizeReturnTo(raw: string | null): string | null { /** * Behavior half of the chat OAuth return leg: publishes the verdict to the * attempt record — which the chat tab's chip picks up over its storage - * listener — then closes the window. Renders nothing, so the page's frame is - * plain server-rendered markup that paints before this hydrates. + * listener — then closes the window. Renders nothing, so the page's frame + * paints as server markup before this hydrates. * - * Reaching this page IS the verdict. Better Auth routes a flow here only as - * its success `callbackURL`, sending failures to `onAPIError.errorURL` - * (`/oauth-error`) or back here with an `error` code, so the server has - * already decided by the time this runs. That is a strictly better signal than - * the credential diffing the generic-page return does: re-authorizing an - * already-linked account updates the account row instead of creating one, so - * no new credential appears and a diff would call a perfectly good connect a - * failure. + * Reaching this page IS the verdict: Better Auth routes a flow here only as its + * success `callbackURL`, sending failures to `/oauth-error` or back here with + * an `error` code. That beats diffing the credential list, which calls a + * re-authorized account a failure — that path updates the existing account row + * and creates nothing for a diff to find. * - * A window the browser refuses to close redirects on to the chat surface - * instead. That is the popup-blocked path: the anchor's `target='_blank'` - * opens this leg in a new tab, which no script may close. That URL - * deliberately carries no attempt id — the verdict is already published, and - * the destination's return router would otherwise re-decide it by the very - * diff this page exists to avoid. + * A window the browser refuses to close redirects to the chat instead (the + * popup-blocked path opens this leg in a tab, which no script may close). That + * URL carries no attempt id on purpose: the verdict is already published, and + * the destination would otherwise re-decide it by the very diff above. */ export function ChatCompleteHandoff() { const ranRef = useRef(false) diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.test.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.test.tsx index 91384a45290..d45ba6e845a 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.test.tsx @@ -582,6 +582,47 @@ describe('CredentialDisplay link tag', () => { act(() => root.unmount()) }) + it('still announces the connection when the watcher released the popup first', async () => { + // The watcher drops the window handle as soon as it stops being observable, + // which routinely happens before React applies the storage-driven verdict. + // The announcement has to survive that, so it cannot be gated on the handle. + vi.useFakeTimers() + const toastSuccess = vi.spyOn(toast, 'success').mockImplementation(() => '') + const popup = { focus: vi.fn(), closed: false } + const openSpy = vi + .spyOn(window, 'open') + .mockReturnValue(popup as unknown as ReturnType) + const { container, root } = renderCredentialLink({ + type: 'link', + provider: 'google-email', + value: + 'https://sim.test/api/auth/oauth2/authorize?providerId=google-email&callbackURL=https%3A%2F%2Fsim.test%2Fworkspace%2Fworkspace-1%2Fchat%2Fchat-1', + }) + + await act(async () => { + container + .querySelector('a') + ?.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true })) + }) + const attemptId = new URL( + new URL(openSpy.mock.calls[0][0] as string).searchParams.get('callbackURL') ?? '' + ).searchParams.get('oauthAttempt') as string + + popup.closed = true + await act(async () => { + await vi.advanceTimersByTimeAsync(2000) + }) + await act(async () => { + setOAuthChatAttemptStatus(attemptId, 'connected') + }) + + expect(toastSuccess).toHaveBeenCalledWith('Gmail connected successfully.') + vi.useRealTimers() + openSpy.mockRestore() + toastSuccess.mockRestore() + act(() => root.unmount()) + }) + it('focuses the live popup instead of starting a rival attempt on a repeat click', async () => { const toastSuccess = vi.spyOn(toast, 'success').mockImplementation(() => '') const popup = { diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/use-oauth-chip-connection.ts b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/use-oauth-chip-connection.ts index a0f7064c3f5..c7655411b85 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/use-oauth-chip-connection.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/use-oauth-chip-connection.ts @@ -406,9 +406,6 @@ export function useOAuthChipConnection({ const launchedAttemptId = launchedAttemptIdRef.current if (connectionStatus !== 'connected' || !launchedAttemptId) return const attempt = readOAuthChatAttempt(launchedAttemptId) - // Tracked separately from `popupRef`, which the watcher clears the moment - // the window looks finished — often before React has applied the - // storage-driven verdict, which would swallow the announcement. launchedAttemptIdRef.current = null popupRef.current = null // The verdict settles the label, but the lock also waits on the credential From 5b6bfde821967c7e2489cc6b08d0a70327a6180f Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 7 Aug 2026 22:59:56 -0700 Subject: [PATCH 07/10] fix(mship): bound the wait on a popup whose outcome became unobservable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A closed handle and a COOP-disowned one are indistinguishable, so the watcher published no verdict for either and relied on the focus verification to settle it. That recovers the normal case — closing a popup hands focus back — but not one where the opener was never blurred, leaving the row waiting indefinitely. Arms the same safety timeout the MCP OAuth popup uses for the same reason: past it, the row decides from the credential list rather than waiting on a verdict that is never going to arrive. Cleared as soon as a real verdict lands. --- .../special-tags/special-tags.test.tsx | 9 ++++++ .../special-tags/use-oauth-chip-connection.ts | 31 +++++++++++++++---- 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.test.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.test.tsx index d45ba6e845a..05c428c4a8d 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.test.tsx @@ -577,6 +577,15 @@ describe('CredentialDisplay link tag', () => { expect(container.textContent).toContain('Waiting for Gmail connection') expect(container.textContent).not.toContain('Not connected') + + // Indistinguishable from a popup the user simply closed, so the wait is + // bounded rather than indefinite: past the safety timeout the row decides + // from the credential list instead of waiting on a verdict that never came. + await act(async () => { + await vi.advanceTimersByTimeAsync(10 * 60 * 1000) + }) + + expect(container.textContent).toContain('Not connected — connect Gmail') vi.useRealTimers() openSpy.mockRestore() act(() => root.unmount()) diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/use-oauth-chip-connection.ts b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/use-oauth-chip-connection.ts index c7655411b85..621c0ac3253 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/use-oauth-chip-connection.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/use-oauth-chip-connection.ts @@ -34,6 +34,13 @@ const OAUTH_POPUP_FEATURES = 'width=560,height=720,resizable=yes,scrollbars=yes' * polling the handle is the only way to observe it. */ const OAUTH_POPUP_POLL_INTERVAL_MS = 400 +/** + * How long to keep waiting on a popup whose outcome became unobservable before + * deciding from the credential list anyway. Matches the MCP OAuth popup's + * safety timeout — long enough not to cut a slow consent short, short enough + * that an abandoned flow does not leave the row waiting forever. + */ +const OAUTH_POPUP_UNOBSERVABLE_TIMEOUT_MS = 10 * 60 * 1000 /** * Same-origin pages an OAuth flow can die on without ever reaching the return @@ -374,6 +381,7 @@ export function useOAuthChipConnection({ */ useEffect(() => { if (connectionStatus !== 'pending' || !popupRef.current) return + let unobservableTimer: number | undefined const poll = window.setInterval(() => { const observation = observePopup(popupRef.current) if (observation === 'live') return @@ -382,13 +390,24 @@ export function useOAuthChipConnection({ // could resolve an attempt a retry had since replaced. window.clearInterval(poll) popupRef.current = null - // Only `ended` is proof the flow finished with nothing published. A - // closed-or-disowned handle is not, so this publishes no verdict for it — - // closing a popup hands focus back to this tab anyway, and the focus - // verification settles it with the same refetch. - if (observation === 'ended') void settleFromCredentials() + // Only `ended` is proof the flow finished with nothing published. + if (observation === 'ended') { + void settleFromCredentials() + return + } + // A closed handle and a disowned one are indistinguishable, so neither + // deciding now nor waiting forever is right. Closing a popup normally + // hands focus back and the focus verification settles it; this bounds the + // case where that never arrives — the opener was never blurred, or the + // flow completed somewhere this tab cannot see. + unobservableTimer = window.setTimeout(() => { + void settleFromCredentials() + }, OAUTH_POPUP_UNOBSERVABLE_TIMEOUT_MS) }, OAUTH_POPUP_POLL_INTERVAL_MS) - return () => window.clearInterval(poll) + return () => { + window.clearInterval(poll) + if (unobservableTimer !== undefined) window.clearTimeout(unobservableTimer) + } }, [connectionStatus, settleFromCredentials]) useEffect(() => { From f0c87ede86a2f6343afcdabff7eb5e903a772f73 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 7 Aug 2026 23:13:46 -0700 Subject: [PATCH 08/10] 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. --- .../special-tags/special-tags.test.tsx | 42 ++++++++++++++++ .../special-tags/use-oauth-chip-connection.ts | 50 ++++++++++++------- 2 files changed, 74 insertions(+), 18 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.test.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.test.tsx index 05c428c4a8d..9f7177d444c 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.test.tsx @@ -591,6 +591,48 @@ describe('CredentialDisplay link tag', () => { act(() => root.unmount()) }) + it('keeps the unobservable deadline across a remount', async () => { + // The transcript virtualizes, so a row can scroll away mid-connect and come + // back with no window handle and no blur behind it. The deadline is derived + // from the attempt's requestedAt rather than held in the effect, so the + // remounted row inherits the remaining time instead of waiting forever. + vi.useFakeTimers() + const popup = { focus: vi.fn(), closed: false } + const openSpy = vi + .spyOn(window, 'open') + .mockReturnValue(popup as unknown as ReturnType) + const data: CredentialItemData = { + type: 'link', + provider: 'google-email', + value: + 'https://sim.test/api/auth/oauth2/authorize?providerId=google-email&callbackURL=https%3A%2F%2Fsim.test%2Fworkspace%2Fworkspace-1%2Fchat%2Fchat-1', + } + const first = renderCredentialLink(data) + + await act(async () => { + first.container + .querySelector('a') + ?.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true })) + }) + popup.closed = true + await act(async () => { + await vi.advanceTimersByTimeAsync(60 * 1000) + }) + act(() => first.root.unmount()) + + const second = renderCredentialLink(data) + expect(second.container.textContent).toContain('Waiting for Gmail connection') + + await act(async () => { + await vi.advanceTimersByTimeAsync(10 * 60 * 1000) + }) + + expect(second.container.textContent).toContain('Not connected — connect Gmail') + vi.useRealTimers() + openSpy.mockRestore() + act(() => second.root.unmount()) + }) + it('still announces the connection when the watcher released the popup first', async () => { // The watcher drops the window handle as soon as it stops being observable, // which routinely happens before React applies the storage-driven verdict. diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/use-oauth-chip-connection.ts b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/use-oauth-chip-connection.ts index 621c0ac3253..195659fe44f 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/use-oauth-chip-connection.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/use-oauth-chip-connection.ts @@ -381,7 +381,6 @@ export function useOAuthChipConnection({ */ useEffect(() => { if (connectionStatus !== 'pending' || !popupRef.current) return - let unobservableTimer: number | undefined const poll = window.setInterval(() => { const observation = observePopup(popupRef.current) if (observation === 'live') return @@ -390,26 +389,41 @@ export function useOAuthChipConnection({ // could resolve an attempt a retry had since replaced. window.clearInterval(poll) popupRef.current = null - // Only `ended` is proof the flow finished with nothing published. - if (observation === 'ended') { - void settleFromCredentials() - return - } - // A closed handle and a disowned one are indistinguishable, so neither - // deciding now nor waiting forever is right. Closing a popup normally - // hands focus back and the focus verification settles it; this bounds the - // case where that never arrives — the opener was never blurred, or the - // flow completed somewhere this tab cannot see. - unobservableTimer = window.setTimeout(() => { - void settleFromCredentials() - }, OAUTH_POPUP_UNOBSERVABLE_TIMEOUT_MS) + // Only `ended` is proof the flow finished with nothing published. A + // closed-or-disowned handle is not — the deadline below bounds that. + if (observation === 'ended') void settleFromCredentials() }, OAUTH_POPUP_POLL_INTERVAL_MS) - return () => { - window.clearInterval(poll) - if (unobservableTimer !== undefined) window.clearTimeout(unobservableTimer) - } + return () => window.clearInterval(poll) }, [connectionStatus, settleFromCredentials]) + /** + * Backstop deadline for a pending attempt whose outcome never becomes + * observable — a closed-or-disowned popup this tab cannot read, with no blur + * for the focus verification to work from. + * + * Bounds every pending attempt rather than only one this mount launched: the + * transcript virtualizes, so a row can remount onto an attempt restored from + * storage with no window handle at all. The deadline comes from the attempt's + * own `requestedAt`, so remounting re-arms with the time remaining instead of + * restarting the clock or dropping the bound entirely. + */ + useEffect(() => { + if (connectionStatus !== 'pending') return + const attempt = readRowAttempt() + if (!attempt) return + const remaining = Math.max( + 0, + OAUTH_POPUP_UNOBSERVABLE_TIMEOUT_MS - (Date.now() - attempt.requestedAt) + ) + const deadline = window.setTimeout(() => { + // A popup still demonstrably running owns the flow; the watcher settles + // it the moment it ends, so there is nothing to bound here. + if (isPopupStillOpen(popupRef.current)) return + void settleFromCredentials() + }, remaining) + return () => window.clearTimeout(deadline) + }, [connectionStatus, readRowAttempt, settleFromCredentials]) + useEffect(() => { if (connected) onConnectedRef.current?.() }, [connected]) From 8d3c51905c5eabac94b3db71b7ee5a43eb6644b3 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 7 Aug 2026 23:24:26 -0700 Subject: [PATCH 09/10] fix(mship): bind a settle to its own attempt and keep the deadline armed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../special-tags/special-tags.test.tsx | 47 +++++++++++++++++++ .../special-tags/use-oauth-chip-connection.ts | 29 ++++++++---- 2 files changed, 67 insertions(+), 9 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.test.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.test.tsx index 9f7177d444c..f487ce2f2e6 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.test.tsx @@ -591,6 +591,53 @@ describe('CredentialDisplay link tag', () => { act(() => root.unmount()) }) + it('holds the deadline open while the popup is live, then settles once it is not', async () => { + // A consent screen can outlive the safety timeout. Expiring against a live + // window would spend the bound early and leave nothing to catch the popup + // dying unobservably later, so the deadline waits instead of firing. + vi.useFakeTimers() + const popup = { + focus: vi.fn(), + closed: false, + // Still on the provider's pages: reading location across origins throws. + get location(): Location { + throw new DOMException('cross-origin', 'SecurityError') + }, + } + const openSpy = vi + .spyOn(window, 'open') + .mockReturnValue(popup as unknown as ReturnType) + const { container, root } = renderCredentialLink({ + type: 'link', + provider: 'google-email', + value: + 'https://sim.test/api/auth/oauth2/authorize?providerId=google-email&callbackURL=https%3A%2F%2Fsim.test%2Fworkspace%2Fworkspace-1%2Fchat%2Fchat-1', + }) + + await act(async () => { + container + .querySelector('a') + ?.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true })) + }) + await act(async () => { + await vi.advanceTimersByTimeAsync(11 * 60 * 1000) + }) + + expect(container.textContent).toContain('Waiting for Gmail connection') + + // Dies unobservably well after the original deadline — the watcher releases + // the handle without a verdict, so only a still-armed deadline can settle. + popup.closed = true + await act(async () => { + await vi.advanceTimersByTimeAsync(2000) + }) + + expect(container.textContent).toContain('Not connected — connect Gmail') + vi.useRealTimers() + openSpy.mockRestore() + act(() => root.unmount()) + }) + it('keeps the unobservable deadline across a remount', async () => { // The transcript virtualizes, so a row can scroll away mid-connect and come // back with no window handle and no blur behind it. The deadline is derived diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/use-oauth-chip-connection.ts b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/use-oauth-chip-connection.ts index 195659fe44f..c2b72b6e9b6 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/use-oauth-chip-connection.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/use-oauth-chip-connection.ts @@ -301,6 +301,11 @@ export function useOAuthChipConnection({ * Idempotent — an attempt is only rewritten while still `pending`. */ const settleFromCredentials = useCallback(async () => { + // Identity is captured up front so the verdict below can only land on the + // attempt this settle was started for. A retry during the refetch installs + // a replacement, and resolving that one from a run it never triggered would + // fail a connect whose popup is still going. + const targetAttemptId = readRowAttempt()?.id const result = await refetchWorkspaceOAuthCredentials() const credentials = result.data ?? [] @@ -313,11 +318,11 @@ export function useOAuthChipConnection({ ) } - // Read after the await: a snapshot taken before it goes stale the moment the - // popup publishes its verdict mid-refetch, and that stale 'pending' would - // license the very overwrite the guard below exists to prevent. + // Status is re-read after the await: the snapshot above goes stale the + // moment the popup publishes its verdict mid-refetch, and that stale + // 'pending' would license the very overwrite this guard exists to prevent. const attempt = readRowAttempt() - if (!attempt || attempt.status !== 'pending') return + if (!attempt || attempt.id !== targetAttemptId || attempt.status !== 'pending') return // Only the callback path can prove a reconnect returned — an unrelated edit // to the credential is indistinguishable from one here. const attemptConnected = reconnectCredentialId @@ -415,12 +420,18 @@ export function useOAuthChipConnection({ 0, OAUTH_POPUP_UNOBSERVABLE_TIMEOUT_MS - (Date.now() - attempt.requestedAt) ) - const deadline = window.setTimeout(() => { - // A popup still demonstrably running owns the flow; the watcher settles - // it the moment it ends, so there is nothing to bound here. - if (isPopupStillOpen(popupRef.current)) return + let deadline: number + const settleUnlessPopupOwnsIt = () => { + // A popup still demonstrably running owns the flow, so the deadline waits + // rather than expiring: giving up here would spend the bound on a live + // window that can still die unobservably long afterwards. + if (isPopupStillOpen(popupRef.current)) { + deadline = window.setTimeout(settleUnlessPopupOwnsIt, OAUTH_POPUP_POLL_INTERVAL_MS) + return + } void settleFromCredentials() - }, remaining) + } + deadline = window.setTimeout(settleUnlessPopupOwnsIt, remaining) return () => window.clearTimeout(deadline) }, [connectionStatus, readRowAttempt, settleFromCredentials]) From f41e66c131e26eb6d8bdca4e21eca5094b53031a Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 7 Aug 2026 23:32:08 -0700 Subject: [PATCH 10/10] chore(mship): tighten the comments on the OAuth popup flow Trims the COOP rationale in next.config.ts to the point, and condenses the longest blocks in the connect hook without dropping the reasoning a reader needs to keep the invariants. --- .../special-tags/use-oauth-chip-connection.ts | 87 +++++++------------ apps/sim/next.config.ts | 16 ++-- 2 files changed, 38 insertions(+), 65 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/use-oauth-chip-connection.ts b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/use-oauth-chip-connection.ts index c2b72b6e9b6..f8933527f8c 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/use-oauth-chip-connection.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/use-oauth-chip-connection.ts @@ -34,32 +34,22 @@ const OAUTH_POPUP_FEATURES = 'width=560,height=720,resizable=yes,scrollbars=yes' * polling the handle is the only way to observe it. */ const OAUTH_POPUP_POLL_INTERVAL_MS = 400 -/** - * How long to keep waiting on a popup whose outcome became unobservable before - * deciding from the credential list anyway. Matches the MCP OAuth popup's - * safety timeout — long enough not to cut a slow consent short, short enough - * that an abandoned flow does not leave the row waiting forever. - */ +/** Matches the MCP OAuth popup's safety timeout: bounds a flow whose outcome never becomes observable. */ const OAUTH_POPUP_UNOBSERVABLE_TIMEOUT_MS = 10 * 60 * 1000 /** - * Same-origin pages an OAuth flow can die on without ever reaching the return - * leg. Better Auth sends pre-state failures — a denied consent, most often — - * to its global error page rather than this flow's callback, and the - * custom-provider callbacks exit to the workspace root with an `error` param. - * Neither publishes a verdict, so a popup sitting on one is finished, not - * in flight. + * Same-origin pages an OAuth flow can die on without reaching the return leg — + * Better Auth sends pre-state failures (usually a denied consent) to its global + * error page, and the custom-provider callbacks exit to the workspace root. + * Neither publishes a verdict, so a popup sitting on one is finished. */ const OAUTH_POPUP_TERMINAL_PATHS = new Set(['/oauth-error', '/workspace']) /** - * What the opener can actually prove about a popup it launched. - * - * `ended` is reserved for positive evidence — a same-origin page we can read - * that is known to publish no verdict. A handle that reports closed is only - * `unobservable`: a provider page with COOP `same-origin` disowns the window, - * and the disowned handle reports `closed` for a consent screen that is still - * running. Treating that as an ending would fail a live flow. + * What the opener can actually prove about a popup it launched. `ended` needs + * positive evidence: a same-origin page we can read that publishes no verdict. + * A handle reporting closed is only `unobservable` — COOP disowns the window + * and the disowned handle reports closed for a consent screen still running. */ type PopupObservation = 'live' | 'ended' | 'unobservable' @@ -208,12 +198,10 @@ export function useOAuthChipConnection({ ) const verdictConnected = connectionStatus === 'connected' const connected = verdictConnected || connectedFromWorkspaceChange - // The label trusts the return leg's verdict; the lock does not. A failure to - // create the credential from its draft is swallowed server-side, so a flow - // can report success with nothing in the workspace to show for it — locking - // on the verdict alone would strand the row saying "Connected" with no way - // to retry. A reconnect legitimately produces no new credential, so it has - // nothing to corroborate against and keeps trusting the verdict. + // The label trusts the verdict; the lock does not. A swallowed credential-draft + // failure lets a flow report success with nothing in the workspace, and locking + // on that would strand the row saying "Connected" with no way to retry. A + // reconnect produces no new credential, so it has nothing to corroborate against. const connectedFromAttempt = verdictConnected && (reconnectCredentialId ? true : connectedFromWorkspaceChange) @@ -222,15 +210,13 @@ export function useOAuthChipConnection({ }, [onConnected]) /** - * A credential for this row can also appear without the row launching it — - * the integrations page in another tab, or a desktop flow that never comes - * back through the return URL. Diffing the workspace list against the - * baseline captured for this scope surfaces that. + * A credential can appear without this row launching it — the integrations + * page in another tab, or a desktop flow that never returns through the URL. + * Diffing the workspace list against this scope's baseline surfaces that. * - * This signal is workspace-wide, so it cannot be attributed to one row: - * sibling chips for the same provider all see the same change. It therefore - * only ever *shows* the row as satisfied — {@link connectedFromAttempt} is - * what locks it, so a second same-provider row stays clickable. + * Workspace-wide, so it cannot be attributed to one row: it only ever *shows* + * the row satisfied. {@link connectedFromAttempt} is what locks it, so a + * sibling chip for the same provider stays clickable. */ useEffect(() => { if (!isFetched) return @@ -268,12 +254,10 @@ export function useOAuthChipConnection({ ]) /** - * This row's attempt: the one named by the return URL when we came back from - * the provider, else the last one stored for this exact row. The stored - * lookup is what survives a reload — and what covers the transcript - * rendering only after the return hook has already stripped the URL param. - * Both are scoped to the row, so a sibling chip for the same provider can - * never claim this one's result. + * This row's attempt: the one named by the return URL, else the last one + * stored for this exact row. The stored lookup survives a reload, and covers + * a transcript rendered after the return hook stripped the URL param. Both + * are row-scoped, so a sibling chip can never claim this one's result. */ const readRowAttempt = useCallback((): OAuthChatAttempt | null => { const active = activeAttemptId ? readOAuthChatAttempt(activeAttemptId) : null @@ -378,11 +362,9 @@ export function useOAuthChipConnection({ }, [settleFromCredentials]) /** - * Watches a live popup for the endings that publish no verdict — a provider - * interstitial exiting to the workspace root, a denied consent landing on the - * OAuth error page, or the user closing the window. None reach the completion - * page or fire an event here, and the user need never leave this tab for a - * focus event either, so polling is the only way the row learns it is over. + * Watches a live popup for the endings that publish no verdict. None fire an + * event here, and the user need never leave this tab for a focus event + * either, so polling is the only way the row learns the flow is over. */ useEffect(() => { if (connectionStatus !== 'pending' || !popupRef.current) return @@ -390,8 +372,7 @@ export function useOAuthChipConnection({ const observation = observePopup(popupRef.current) if (observation === 'live') return // Stop before settling: the refetch leaves the status `pending` for its - // duration, so a running interval would keep firing and a later tick - // could resolve an attempt a retry had since replaced. + // duration, so a later tick could resolve an attempt a retry replaced. window.clearInterval(poll) popupRef.current = null // Only `ended` is proof the flow finished with nothing published. A @@ -402,15 +383,11 @@ export function useOAuthChipConnection({ }, [connectionStatus, settleFromCredentials]) /** - * Backstop deadline for a pending attempt whose outcome never becomes - * observable — a closed-or-disowned popup this tab cannot read, with no blur - * for the focus verification to work from. - * - * Bounds every pending attempt rather than only one this mount launched: the - * transcript virtualizes, so a row can remount onto an attempt restored from - * storage with no window handle at all. The deadline comes from the attempt's - * own `requestedAt`, so remounting re-arms with the time remaining instead of - * restarting the clock or dropping the bound entirely. + * Backstop for a pending attempt whose outcome never becomes observable. + * Bounds every pending attempt, not just one this mount launched — the + * transcript virtualizes, so a row can remount with no window handle at all. + * Dated from the attempt's `requestedAt`, so a remount inherits the time + * remaining rather than restarting the clock. */ useEffect(() => { if (connectionStatus !== 'pending') return diff --git a/apps/sim/next.config.ts b/apps/sim/next.config.ts index 3cfa4976ee0..8fdc0915632 100644 --- a/apps/sim/next.config.ts +++ b/apps/sim/next.config.ts @@ -319,13 +319,9 @@ const nextConfig: NextConfig = { { // Exclude Vercel internal resources and static assets from strict COOP, Google Drive Picker // and the /demo Cal.com booking embed to prevent 'refused to connect' / slow-load issues. - // The OAuth popup pages are excluded because `same-origin` moves a document into its own - // browsing-context group, disowning it from the opener that launched it: the popup stops - // being reliably script-closable and its opener sees `closed` report true for a live window. - // That covers the return leg (`oauth/chat-complete`) and both pages a flow can exit on - // without reaching it — `oauth-error` and the `workspace` root, which the custom-provider - // callbacks bounce to. Bare `workspace` also matches `/workspace` itself, which previously - // fell here while every `/workspace/...` route got the permissive policy below. + // The pages an OAuth popup can land on are excluded too: `same-origin` would disown the + // popup from its opener, leaving it not reliably script-closable and reporting `closed` + // for a live window. source: '/((?!_next|_vercel|api|favicon.ico|w/.*|workspace|api/tools/drive|demo|oauth-error|oauth/chat-complete).*)', headers: [ @@ -350,9 +346,9 @@ const nextConfig: NextConfig = { ], }, { - // For main app routes, Google Drive Picker, the /demo Cal.com embed, the pages an OAuth - // popup can land on, and Vercel resources - use permissive policies. The popup pages match - // their opener's value so the two stay in one browsing-context group. + // For main app routes, Google Drive Picker, the /demo Cal.com embed, the OAuth popup pages, + // and Vercel resources - use permissive policies. The popup pages match their opener's + // value so the two stay in one browsing-context group. source: '/(w/.*|workspace.*|api/tools/drive|demo.*|oauth-error|oauth/chat-complete|_next/.*|_vercel/.*)', headers: [