From 331e4bc29b5bf0693e68253f38e98b7c0e6bccb3 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Wed, 16 Sep 2026 14:56:34 -0400 Subject: [PATCH] fix(cloud-auth): keep Sign in clickable during a pending browser wait MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closing the cloud sign-in tab used to leave every Sign in button disabled as "Signing in…" until the 5-minute loopback timeout. A second click now starts another wait, the first tab can still complete, and the first successful callback wins. STA-7610 --- .../orca-profiles/profile-cloud-pkce.test.ts | 33 ++++++++++ .../src/components/UnexpectedSignoutCard.tsx | 7 +- .../artifacts/ArtifactPublishButton.test.tsx | 2 - .../artifacts/ArtifactPublishButton.tsx | 21 ++---- .../artifacts/ArtifactsPage.test.tsx | 2 +- .../components/artifacts/ArtifactsPage.tsx | 2 - .../artifacts/ArtifactsPageStates.tsx | 18 ++--- .../settings/ArtifactsSettingsPane.test.tsx | 14 +--- .../settings/ArtifactsSettingsPane.tsx | 11 ++-- .../src/components/settings/DevToolsPane.tsx | 17 +---- .../MobilePairingConnectionOptions.test.tsx | 3 - .../MobilePairingConnectionOptions.tsx | 4 -- .../settings/OrcaAccountSettingsPane.test.tsx | 36 ++++++---- .../settings/OrcaAccountSettingsPane.tsx | 16 ++--- .../settings/ShareSkillsSettingsPane.test.tsx | 2 - .../settings/ShareSkillsSettingsPane.tsx | 11 ++-- .../slices/orca-profiles-auth-actions.test.ts | 66 +++++++++++++++++-- .../slices/orca-profiles-auth-actions.ts | 31 +++++---- .../src/store/slices/orca-profiles.ts | 2 - 19 files changed, 165 insertions(+), 133 deletions(-) diff --git a/src/main/orca-profiles/profile-cloud-pkce.test.ts b/src/main/orca-profiles/profile-cloud-pkce.test.ts index 81b706f82bc..f5c0a802da1 100644 --- a/src/main/orca-profiles/profile-cloud-pkce.test.ts +++ b/src/main/orca-profiles/profile-cloud-pkce.test.ts @@ -148,4 +148,37 @@ describe('Orca cloud PKCE flow', () => { await readHttp(callbackUrl(redirectUri, { code: 'real-code', state })) await expect(flow).resolves.toMatchObject({ code: 'real-code', nonce }) }) + + it('keeps the first loopback alive when a second sign-in starts', async () => { + const first = beginOrcaCloudPkceFlow(config, 'local-default') + await vi.waitFor(() => expect(openExternalMock).toHaveBeenCalledTimes(1)) + const firstUrl = new URL(String(openExternalMock.mock.calls[0]?.[0])) + const firstRedirectUri = firstUrl.searchParams.get('redirect_uri') + const firstState = firstUrl.searchParams.get('state') + if (!firstRedirectUri || !firstState) { + throw new Error('Expected the first PKCE flow to create redirect_uri and state') + } + + const second = beginOrcaCloudPkceFlow(config, 'local-default') + await vi.waitFor(() => expect(openExternalMock).toHaveBeenCalledTimes(2)) + const secondUrl = new URL(String(openExternalMock.mock.calls[1]?.[0])) + const secondRedirectUri = secondUrl.searchParams.get('redirect_uri') + const secondState = secondUrl.searchParams.get('state') + if (!secondRedirectUri || !secondState) { + throw new Error('Expected the second PKCE flow to create redirect_uri and state') + } + expect(secondRedirectUri).not.toBe(firstRedirectUri) + + const firstResponse = await readHttp( + callbackUrl(firstRedirectUri, { code: 'first-code', state: firstState }) + ) + expect(firstResponse.statusCode).toBe(200) + await expect(first).resolves.toMatchObject({ code: 'first-code', state: firstState }) + + const secondResponse = await readHttp( + callbackUrl(secondRedirectUri, { code: 'second-code', state: secondState }) + ) + expect(secondResponse.statusCode).toBe(200) + await expect(second).resolves.toMatchObject({ code: 'second-code', state: secondState }) + }) }) diff --git a/src/renderer/src/components/UnexpectedSignoutCard.tsx b/src/renderer/src/components/UnexpectedSignoutCard.tsx index b11c8dfb2ed..dbfb106eb23 100644 --- a/src/renderer/src/components/UnexpectedSignoutCard.tsx +++ b/src/renderer/src/components/UnexpectedSignoutCard.tsx @@ -48,7 +48,6 @@ export function UnexpectedSignoutCard(): React.JSX.Element | null { const persistedDismissedVersion = useAppStore((s) => s.dismissedUnexpectedSignoutVersion) const dismissedVersions = useAppStore((s) => s.unexpectedSignoutDismissedVersions) const dismissForVersion = useAppStore((s) => s.dismissUnexpectedSignoutCard) - const connecting = useAppStore((s) => s.orcaProfileConnecting) const connect = useAppStore((s) => s.connectCurrentOrcaProfile) const [appVersion, setAppVersion] = useState(null) const [authRefreshReady, setAuthRefreshReady] = useState(false) @@ -236,12 +235,10 @@ export function UnexpectedSignoutCard(): React.JSX.Element | null { variant="default" size="sm" className="flex-1" - disabled={!canConnect || connecting} + disabled={!canConnect} onClick={() => void connect()} > - {connecting - ? translate('auto.components.UnexpectedSignoutCard.7e1a9c4d2f', 'Signing in…') - : translate('auto.components.UnexpectedSignoutCard.c5b3e8a17d', 'Sign in to Orca')} + {translate('auto.components.UnexpectedSignoutCard.c5b3e8a17d', 'Sign in to Orca')} diff --git a/src/renderer/src/components/artifacts/ArtifactPublishButton.test.tsx b/src/renderer/src/components/artifacts/ArtifactPublishButton.test.tsx index 6867ced65c5..77f64b0217c 100644 --- a/src/renderer/src/components/artifacts/ArtifactPublishButton.test.tsx +++ b/src/renderer/src/components/artifacts/ArtifactPublishButton.test.tsx @@ -17,7 +17,6 @@ const mocks = vi.hoisted(() => ({ openPopover: null as ((open: boolean) => void) | null, state: { orcaProfileAuthStatus: { configured: true, state: 'connected' } as Record, - orcaProfileConnecting: false, settings: { artifactSharingEnabled: true } } })) @@ -83,7 +82,6 @@ describe('ArtifactPublishButton', () => { mocks.copyLink.mockResolvedValue(true) mocks.openPopover = null mocks.state.orcaProfileAuthStatus = { configured: true, state: 'connected' } - mocks.state.orcaProfileConnecting = false mocks.state.settings = { artifactSharingEnabled: true } }) diff --git a/src/renderer/src/components/artifacts/ArtifactPublishButton.tsx b/src/renderer/src/components/artifacts/ArtifactPublishButton.tsx index f07d9f41619..bf6fb236003 100644 --- a/src/renderer/src/components/artifacts/ArtifactPublishButton.tsx +++ b/src/renderer/src/components/artifacts/ArtifactPublishButton.tsx @@ -35,7 +35,6 @@ export function ArtifactPublishButton({ const lookupSequence = useRef(0) const popoverContentRef = useRef(null) const authStatus = useAppStore((state) => state.orcaProfileAuthStatus) - const connecting = useAppStore((state) => state.orcaProfileConnecting) const connect = useAppStore((state) => state.connectCurrentOrcaProfile) const openSettingsPage = useAppStore((state) => state.openSettingsPage) const openSettingsTarget = useAppStore((state) => state.openSettingsTarget) @@ -56,7 +55,7 @@ export function ArtifactPublishButton({ const checkingLink = signedIn && currentLookup?.status !== 'loaded' && currentLookup?.status !== 'error' const publishedLink = currentLookup?.status === 'loaded' ? currentLookup.shareUrl : null - const busy = publishing || connecting + const busy = publishing const blocked = disabled || busy useEffect(() => { @@ -184,23 +183,15 @@ export function ArtifactPublishButton({ type="button" variant="outline" size="xs" - disabled={connecting || authStatus?.configured !== true} + disabled={authStatus?.configured !== true} onClick={() => void connect()} > - {connecting + {authStatus?.state === 'reconnect-required' ? translate( - 'auto.components.artifacts.ArtifactPublishButton.signingIn', - 'Signing in…' + 'auto.components.artifacts.ArtifactPublishButton.signInAgain', + 'Sign in again' ) - : authStatus?.state === 'reconnect-required' - ? translate( - 'auto.components.artifacts.ArtifactPublishButton.signInAgain', - 'Sign in again' - ) - : translate( - 'auto.components.artifacts.ArtifactPublishButton.signIn', - 'Sign in' - )} + : translate('auto.components.artifacts.ArtifactPublishButton.signIn', 'Sign in')} ) : null} diff --git a/src/renderer/src/components/artifacts/ArtifactsPage.test.tsx b/src/renderer/src/components/artifacts/ArtifactsPage.test.tsx index d6db83e62ff..6624b07da33 100644 --- a/src/renderer/src/components/artifacts/ArtifactsPage.test.tsx +++ b/src/renderer/src/components/artifacts/ArtifactsPage.test.tsx @@ -63,7 +63,7 @@ function storeState(): Record { closeArtifactsPage: mocks.closePage, connectCurrentOrcaProfile: mocks.connect, orcaProfileAuthStatus: mocks.authStatus, - orcaProfileConnecting: false, + refreshCurrentOrcaProfileAuth: mocks.refreshAuth, settings: mocks.settings, updateSettings: mocks.updateSettings, diff --git a/src/renderer/src/components/artifacts/ArtifactsPage.tsx b/src/renderer/src/components/artifacts/ArtifactsPage.tsx index b325f3c6923..3c12e06fe4f 100644 --- a/src/renderer/src/components/artifacts/ArtifactsPage.tsx +++ b/src/renderer/src/components/artifacts/ArtifactsPage.tsx @@ -20,7 +20,6 @@ const LOCAL_RUNTIME = { kind: 'local' } as const export default function ArtifactsPage(): React.JSX.Element { const closePage = useAppStore((state) => state.closeArtifactsPage) const authStatus = useAppStore((state) => state.orcaProfileAuthStatus) - const connecting = useAppStore((state) => state.orcaProfileConnecting) const connect = useAppStore((state) => state.connectCurrentOrcaProfile) const refreshAuth = useAppStore((state) => state.refreshCurrentOrcaProfileAuth) const openSettingsPage = useAppStore((state) => state.openSettingsPage) @@ -192,7 +191,6 @@ export default function ArtifactsPage(): React.JSX.Element { ) : null} {!signedIn ? ( void connect()} diff --git a/src/renderer/src/components/artifacts/ArtifactsPageStates.tsx b/src/renderer/src/components/artifacts/ArtifactsPageStates.tsx index bf7c97a133d..5974005b2ac 100644 --- a/src/renderer/src/components/artifacts/ArtifactsPageStates.tsx +++ b/src/renderer/src/components/artifacts/ArtifactsPageStates.tsx @@ -22,13 +22,11 @@ export function ArtifactsPageErrorBanner({ } export function ArtifactsPageAuthState({ - connecting, needsReconnect, configured, onConnect, onOpenAccountSettings }: { - connecting: boolean needsReconnect: boolean configured: boolean onConnect: () => void @@ -62,15 +60,13 @@ export function ArtifactsPageAuthState({

{configured ? ( - ) : (
diff --git a/src/renderer/src/components/settings/ArtifactsSettingsPane.test.tsx b/src/renderer/src/components/settings/ArtifactsSettingsPane.test.tsx index ff4a3c1aafa..0e74eb57fad 100644 --- a/src/renderer/src/components/settings/ArtifactsSettingsPane.test.tsx +++ b/src/renderer/src/components/settings/ArtifactsSettingsPane.test.tsx @@ -15,7 +15,6 @@ const mocks = vi.hoisted(() => ({ configured: true, state: 'connected' } as Record | null, - orcaProfileConnecting: false, isWebClient: false } })) @@ -46,7 +45,6 @@ describe('ArtifactsSettingsPane', () => { mocks.fetchAuthStatus.mockReset() mocks.openArtifactsPage.mockReset() mocks.state.orcaProfileAuthStatus = { configured: true, state: 'connected' } - mocks.state.orcaProfileConnecting = false mocks.state.isWebClient = false }) @@ -92,19 +90,11 @@ describe('ArtifactsSettingsPane', () => { expect(mocks.connect).toHaveBeenCalledOnce() }) - it('shows reconnect and connecting states', () => { + it('keeps sign in clickable while reconnect is required', () => { mocks.state.orcaProfileAuthStatus = { configured: true, state: 'reconnect-required' } - const { rerender } = render( - - ) + render() expect(screen.getByRole('button', { name: 'Sign in again' })).toBeEnabled() - - mocks.state.orcaProfileConnecting = true - rerender( - - ) - expect(screen.getByRole('button', { name: 'Signing in…' })).toBeDisabled() }) it('loads missing account status and disables sign in until configured', () => { diff --git a/src/renderer/src/components/settings/ArtifactsSettingsPane.tsx b/src/renderer/src/components/settings/ArtifactsSettingsPane.tsx index 7e8b87029d4..a90906a0759 100644 --- a/src/renderer/src/components/settings/ArtifactsSettingsPane.tsx +++ b/src/renderer/src/components/settings/ArtifactsSettingsPane.tsx @@ -18,7 +18,6 @@ export function ArtifactsSettingsPane({ }): React.JSX.Element { const openArtifactsPage = useAppStore((state) => state.openArtifactsPage) const authStatus = useAppStore((state) => state.orcaProfileAuthStatus) - const connecting = useAppStore((state) => state.orcaProfileConnecting) const connect = useAppStore((state) => state.connectCurrentOrcaProfile) const signedIn = authStatus?.state === 'connected' // Why: the capability lives in the desktop host's store and is deliberately absent from the @@ -128,14 +127,12 @@ export function ArtifactsSettingsPane({ ) : null} diff --git a/src/renderer/src/components/settings/DevToolsPane.tsx b/src/renderer/src/components/settings/DevToolsPane.tsx index 7042e67758a..c2631c5423f 100644 --- a/src/renderer/src/components/settings/DevToolsPane.tsx +++ b/src/renderer/src/components/settings/DevToolsPane.tsx @@ -139,7 +139,6 @@ function showDeleteFailureToast(): void { // progress; this surfaces it (and its status) in dev when the env vars are set. function OrcaCloudDevSubsection(): React.JSX.Element { const authStatus = useAppStore((s) => s.orcaProfileAuthStatus) - const connecting = useAppStore((s) => s.orcaProfileConnecting) const connect = useAppStore((s) => s.connectCurrentOrcaProfile) const signOut = useAppStore((s) => s.signOutCurrentOrcaProfile) const refresh = useAppStore((s) => s.fetchOrcaProfileAuthStatus) @@ -170,23 +169,11 @@ function OrcaCloudDevSubsection(): React.JSX.Element {

{connected ? ( - ) : ( - ) : ( - )}
diff --git a/src/renderer/src/components/settings/ShareSkillsSettingsPane.test.tsx b/src/renderer/src/components/settings/ShareSkillsSettingsPane.test.tsx index 32770122074..06052b4caf3 100644 --- a/src/renderer/src/components/settings/ShareSkillsSettingsPane.test.tsx +++ b/src/renderer/src/components/settings/ShareSkillsSettingsPane.test.tsx @@ -16,7 +16,6 @@ const mocks = vi.hoisted(() => ({ string, unknown > | null, - orcaProfileConnecting: false, isWebClient: false, settings: { showSkillsButton: false, agentSkillSharingEnabled: false } } @@ -50,7 +49,6 @@ describe('ShareSkillsSettingsPane', () => { mocks.openSkillsPage.mockReset() mocks.updateSettings.mockReset() mocks.state.orcaProfileAuthStatus = { configured: true, state: 'connected' } - mocks.state.orcaProfileConnecting = false mocks.state.isWebClient = false Object.defineProperty(window, 'api', { configurable: true, diff --git a/src/renderer/src/components/settings/ShareSkillsSettingsPane.tsx b/src/renderer/src/components/settings/ShareSkillsSettingsPane.tsx index ddbf35734c9..54ce5edd14a 100644 --- a/src/renderer/src/components/settings/ShareSkillsSettingsPane.tsx +++ b/src/renderer/src/components/settings/ShareSkillsSettingsPane.tsx @@ -14,7 +14,6 @@ export function ShareSkillsSettingsPane(): React.JSX.Element { const settings = useAppStore((state) => state.settings) const updateSettings = useAppStore((state) => state.updateSettings) const authStatus = useAppStore((state) => state.orcaProfileAuthStatus) - const connecting = useAppStore((state) => state.orcaProfileConnecting) const connect = useAppStore((state) => state.connectCurrentOrcaProfile) const signedIn = authStatus?.state === 'connected' const isWebClient = isWebClientLocation() @@ -135,14 +134,12 @@ export function ShareSkillsSettingsPane(): React.JSX.Element { ) : null} diff --git a/src/renderer/src/store/slices/orca-profiles-auth-actions.test.ts b/src/renderer/src/store/slices/orca-profiles-auth-actions.test.ts index 7f4f16abf5c..698cd9de05b 100644 --- a/src/renderer/src/store/slices/orca-profiles-auth-actions.test.ts +++ b/src/renderer/src/store/slices/orca-profiles-auth-actions.test.ts @@ -1,5 +1,4 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' -import { createTestStore } from './store-test-helpers' import type { ConnectCurrentOrcaProfileResult, CreateCloudLinkedOrcaProfileResult, @@ -9,6 +8,21 @@ import type { SelectOrcaProfileOrgResult, SignOutCurrentOrcaProfileResult } from '../../../../shared/orca-profiles' +import { createTestStore } from './store-test-helpers' + +const { toastErrorMock, toastSuccessMock } = vi.hoisted(() => ({ + toastErrorMock: vi.fn(), + toastSuccessMock: vi.fn() +})) + +vi.mock('sonner', () => ({ + toast: { + error: toastErrorMock, + info: vi.fn(), + success: toastSuccessMock, + warning: vi.fn() + } +})) const listState: OrcaProfileListState = { activeProfileId: 'local-default', @@ -73,6 +87,8 @@ const orcaProfilesApi = { describe('orca profile auth actions slice', () => { beforeEach(() => { vi.resetAllMocks() + toastErrorMock.mockReset() + toastSuccessMock.mockReset() orcaProfilesApi.authStatus.mockResolvedValue(localAuthStatus) vi.stubGlobal('window', { api: { @@ -98,13 +114,51 @@ describe('orca profile auth actions slice', () => { orcaProfilesApi.connectCurrent.mockResolvedValue(result) const store = createTestStore() - const pending = store.getState().connectCurrentOrcaProfile() - - expect(store.getState().orcaProfileConnecting).toBe(true) - await expect(pending).resolves.toEqual(result) - expect(store.getState().orcaProfileConnecting).toBe(false) + await expect(store.getState().connectCurrentOrcaProfile()).resolves.toEqual(result) expect(store.getState().orcaProfileAuthStatus).toEqual(connectedAuthStatus) expect(store.getState().orcaProfiles).toEqual(connectedProfiles) + expect(toastSuccessMock).toHaveBeenCalledOnce() + }) + + it('starts a second sign-in while the first browser wait is still open', async () => { + const connectedProfiles = [ + { + ...listState.profiles[0], + kind: 'cloud-linked' as const, + cloud: connectedAuthStatus.cloud + } + ] + const connected: ConnectCurrentOrcaProfileResult = { + status: 'connected', + auth: connectedAuthStatus, + activeProfileId: 'local-default', + profiles: connectedProfiles + } + const cancelled: ConnectCurrentOrcaProfileResult = { + status: 'cancelled', + auth: connectedAuthStatus + } + let finishFirst!: (value: ConnectCurrentOrcaProfileResult) => void + orcaProfilesApi.connectCurrent + .mockReturnValueOnce( + new Promise((resolve) => { + finishFirst = resolve + }) + ) + .mockResolvedValueOnce(connected) + const store = createTestStore() + + const first = store.getState().connectCurrentOrcaProfile() + const second = store.getState().connectCurrentOrcaProfile() + + expect(orcaProfilesApi.connectCurrent).toHaveBeenCalledTimes(2) + await expect(second).resolves.toEqual(connected) + expect(toastSuccessMock).toHaveBeenCalledOnce() + finishFirst(cancelled) + await expect(first).resolves.toEqual(cancelled) + expect(toastErrorMock).not.toHaveBeenCalled() + expect(toastSuccessMock).toHaveBeenCalledOnce() + expect(store.getState().orcaProfileAuthStatus).toEqual(connectedAuthStatus) }) it('refreshes current profile auth and stores fresh capability flags', async () => { diff --git a/src/renderer/src/store/slices/orca-profiles-auth-actions.ts b/src/renderer/src/store/slices/orca-profiles-auth-actions.ts index c8212962821..0a1474b9148 100644 --- a/src/renderer/src/store/slices/orca-profiles-auth-actions.ts +++ b/src/renderer/src/store/slices/orca-profiles-auth-actions.ts @@ -70,14 +70,12 @@ export const createOrcaProfilesAuthActions: StateCreator< }, connectCurrentOrcaProfile: async () => { - if (get().orcaProfileConnecting) { - return null - } - set({ orcaProfileConnecting: true }) try { + // Why: a pending browser callback must not block retry. Another click + // starts a second PKCE wait; the earlier tab can still complete. + const alreadyConnected = get().orcaProfileAuthStatus?.state === 'connected' const result = await window.api.orcaProfiles.connectCurrent() set({ - orcaProfileConnecting: false, orcaProfileAuthStatus: result.auth, ...(result.status === 'connected' ? { @@ -96,24 +94,29 @@ export const createOrcaProfilesAuthActions: StateCreator< description: result.auth.setupMessage } ) - } else if (result.status === 'failed') { + } else if ( + result.status === 'failed' && + !alreadyConnected && + result.auth.state !== 'connected' + ) { toast.error( translate('auto.store.slices.orca.profiles.33290e88ed', 'Failed to connect profile'), { description: result.error } ) - } else if (result.status === 'connected') { + } else if (result.status === 'connected' && !alreadyConnected) { toast.success(translate('auto.store.slices.orca.profiles.9fcb07a796', 'Profile connected')) } return result } catch (err) { console.error('Failed to connect Orca profile:', err) - set({ orcaProfileConnecting: false }) - toast.error( - translate('auto.store.slices.orca.profiles.33290e88ed', 'Failed to connect profile'), - { - description: err instanceof Error ? err.message : String(err) - } - ) + if (get().orcaProfileAuthStatus?.state !== 'connected') { + toast.error( + translate('auto.store.slices.orca.profiles.33290e88ed', 'Failed to connect profile'), + { + description: err instanceof Error ? err.message : String(err) + } + ) + } return null } }, diff --git a/src/renderer/src/store/slices/orca-profiles.ts b/src/renderer/src/store/slices/orca-profiles.ts index d885e96b7e3..e406938d0ac 100644 --- a/src/renderer/src/store/slices/orca-profiles.ts +++ b/src/renderer/src/store/slices/orca-profiles.ts @@ -21,7 +21,6 @@ export type OrcaProfilesSlice = OrcaProfilesAuthActions & { orcaProfilesMultiProfileUi: boolean orcaProfilesLoading: boolean orcaProfileSwitching: boolean - orcaProfileConnecting: boolean fetchOrcaProfiles: () => Promise fetchOrcaProfileAuthStatus: () => Promise createLocalOrcaProfile: (name?: string) => Promise @@ -42,7 +41,6 @@ export const createOrcaProfilesSlice: StateCreator { set({ orcaProfilesLoading: true })