Skip to content

Commit 457fdc2

Browse files
waleedlatif1claude
andcommitted
fix(desktop): keep sign-in handoffs current across cancel, migration, and sign-out
Retire in-flight launches on cancel, newer continue, and account teardown; move a provisional chat's handoff with its browser scope; skip reopening the tab when a bring-back's handoff was superseded; pin the platform in the continue-sign-in tests so they pass on Linux CI. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 1aa3421 commit 457fdc2

7 files changed

Lines changed: 191 additions & 32 deletions

File tree

‎apps/desktop/src/main/browser-import/index.test.ts‎

Lines changed: 87 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { beforeEach, describe, expect, it, vi } from 'vitest'
1+
import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'
22
import { BROWSER_SOURCES, type BrowserSource } from '@/main/browser-import/browser-sources'
33
import type { BrowserProfile } from '@/main/browser-import/types'
44
import { emptySkipCounts } from '@/main/browser-import/types'
@@ -35,9 +35,33 @@ vi.mock('@/main/browser-import/chromium-crypto', async (importOriginal) => ({
3535
const { openInBrowserProfile } = await import('@/main/browser-import/browser-launch')
3636
const { listAllBrowserProfiles } = await import('@/main/browser-import/chromium-profiles')
3737
const { readBrowserCookies } = await import('@/main/browser-import/chromium-cookies')
38-
const { bringSignInBack, cancelSignIn, continueSignIn, getPendingSignIn } = await import(
39-
'@/main/browser-import'
40-
)
38+
const {
39+
bringSignInBack,
40+
cancelSignIn,
41+
clearSignIns,
42+
continueSignIn,
43+
getPendingSignIn,
44+
migrateSignIn,
45+
} = await import('@/main/browser-import')
46+
47+
/** The importer is macOS-only; pin the platform so the suite means the same on every runner. */
48+
const realPlatform = Object.getOwnPropertyDescriptor(process, 'platform')
49+
beforeAll(() => Object.defineProperty(process, 'platform', { value: 'darwin' }))
50+
afterAll(() => {
51+
if (realPlatform) Object.defineProperty(process, 'platform', realPlatform)
52+
})
53+
54+
/** Holds the next browser launch open until the test releases it. */
55+
function holdLaunch(): () => void {
56+
let release: (() => void) | undefined
57+
vi.mocked(openInBrowserProfile).mockImplementationOnce(
58+
() =>
59+
new Promise<void>((resolve) => {
60+
release = resolve
61+
})
62+
)
63+
return () => release?.()
64+
}
4165

4266
const ARC = BROWSER_SOURCES.find(({ id }) => id === 'arc') as BrowserSource
4367
const PROFILE: BrowserProfile = {
@@ -71,12 +95,10 @@ function readsCookies(count: number) {
7195
}
7296

7397
beforeEach(() => {
74-
vi.clearAllMocks()
7598
vi.mocked(listAllBrowserProfiles).mockResolvedValue([PROFILE])
7699
vi.mocked(openInBrowserProfile).mockResolvedValue()
77100
readsCookies(2)
78-
cancelSignIn('chat-a')
79-
cancelSignIn('chat-b')
101+
clearSignIns()
80102
})
81103

82104
describe('continueSignIn', () => {
@@ -132,6 +154,7 @@ describe('bringSignInBack', () => {
132154
await expect(bringSignInBack('chat-a')).resolves.toEqual({
133155
result: { cookiesImported: 2, cookiesSkipped: 0 },
134156
handoff: expect.objectContaining({ tabId: 'tab-1', url: SIGN_IN_URL }),
157+
current: true,
135158
})
136159
expect(readBrowserCookies).toHaveBeenCalledWith('/arc/Profile 2/Cookies', expect.any(Buffer), {
137160
scope: { domain: 'amazon.com', exact: false },
@@ -167,7 +190,10 @@ describe('bringSignInBack', () => {
167190
await vi.waitFor(() => expect(finishRead).toBeTypeOf('function'))
168191
await continueSignIn('chat-a', request('https://accounts.google.com/signin'))
169192
finishRead?.()
170-
await expect(broughtBack).resolves.toMatchObject({ result: { cookiesImported: 2 } })
193+
await expect(broughtBack).resolves.toMatchObject({
194+
result: { cookiesImported: 2 },
195+
current: false,
196+
})
171197

172198
expect(getPendingSignIn('chat-a')).toEqual({ site: 'google.com', browserLabel: 'Arc' })
173199
})
@@ -189,3 +215,56 @@ describe('bringSignInBack', () => {
189215
expect(readBrowserCookies).not.toHaveBeenCalled()
190216
})
191217
})
218+
219+
describe('a handoff retired or moved while its browser launches', () => {
220+
it('records nothing once a cancel lands mid-launch', async () => {
221+
const release = holdLaunch()
222+
const continuing = continueSignIn('chat-a', request())
223+
await vi.waitFor(() => expect(openInBrowserProfile).toHaveBeenCalled())
224+
cancelSignIn('chat-a')
225+
release()
226+
227+
await expect(continuing).resolves.toEqual({})
228+
expect(getPendingSignIn('chat-a')).toBeNull()
229+
})
230+
231+
it('lets a newer continue win over an older launch that finishes later', async () => {
232+
const release = holdLaunch()
233+
const older = continueSignIn('chat-a', request())
234+
await vi.waitFor(() => expect(openInBrowserProfile).toHaveBeenCalled())
235+
await continueSignIn('chat-a', request('https://accounts.google.com/signin'))
236+
release()
237+
238+
await expect(older).resolves.toEqual({})
239+
expect(getPendingSignIn('chat-a')).toEqual({ site: 'google.com', browserLabel: 'Arc' })
240+
})
241+
242+
it('follows a provisional chat to its saved id, mid-launch or after', async () => {
243+
const release = holdLaunch()
244+
const continuing = continueSignIn('pending:new-chat', request())
245+
await vi.waitFor(() => expect(openInBrowserProfile).toHaveBeenCalled())
246+
migrateSignIn('pending:new-chat', 'chat-a')
247+
release()
248+
await continuing
249+
250+
expect(getPendingSignIn('chat-a')).toEqual({ site: 'amazon.com', browserLabel: 'Arc' })
251+
expect(getPendingSignIn('pending:new-chat')).toBeNull()
252+
migrateSignIn('chat-a', 'chat-b')
253+
await expect(bringSignInBack('chat-b')).resolves.toMatchObject({
254+
result: { cookiesImported: 2 },
255+
})
256+
})
257+
258+
it('forgets every handoff on account teardown, including one still launching', async () => {
259+
await continueSignIn('chat-a', request())
260+
const release = holdLaunch()
261+
const continuing = continueSignIn('chat-b', request())
262+
await vi.waitFor(() => expect(openInBrowserProfile).toHaveBeenCalledTimes(2))
263+
clearSignIns()
264+
release()
265+
266+
await expect(continuing).resolves.toEqual({})
267+
expect(getPendingSignIn('chat-a')).toBeNull()
268+
expect(getPendingSignIn('chat-b')).toBeNull()
269+
})
270+
})

‎apps/desktop/src/main/browser-import/index.ts‎

Lines changed: 64 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -89,66 +89,110 @@ interface SignInHandoff {
8989
scope: SiteScope
9090
}
9191

92-
/** At most one per chat: continuing another page replaces the previous handoff. */
93-
const handoffsByScope = new Map<string, SignInHandoff>()
92+
/**
93+
* A chat's handoff, created before the browser launches so a cancel, a newer
94+
* continue, sign-out, or the chat's id migrating during the launch can retire
95+
* or move it. A launch records its handoff only while its slot is still current.
96+
*/
97+
interface HandoffSlot {
98+
handoff?: SignInHandoff
99+
}
100+
101+
/** At most one per chat: continuing another page replaces the previous slot. */
102+
const slotsByScope = new Map<string, HandoffSlot>()
94103

95104
function pendingSignIn({ profile, scope }: SignInHandoff): BrowserPendingSignIn {
96105
return { site: scope.domain, browserLabel: profile.source.label }
97106
}
98107

108+
function isCurrentSlot(slot: HandoffSlot): boolean {
109+
for (const current of slotsByScope.values()) if (current === slot) return true
110+
return false
111+
}
112+
99113
/**
100114
* Opens a built-in browser page in one of the user's own browser profiles and
101115
* remembers it for the chat, so {@link bringSignInBack} imports the site the
102116
* user left from and reopens that page, whatever the tab shows by then.
103117
*
104118
* Only an https page goes, and only to a profile the importer can read back
105-
* from: a sign-in that can never return would strand the user.
119+
* from: a sign-in that can never return would strand the user. A handoff
120+
* retired while its browser launched resolves with neither `pending` nor
121+
* `error`.
106122
*/
107123
export async function continueSignIn(
108124
scopeId: string,
109125
request: { profileId: string; tabId: string; url: string }
110126
): Promise<BrowserContinueSignInResult> {
111127
const scope = isSafeExternalUrl(request.url) ? siteScopeFor(new URL(request.url).hostname) : null
112128
if (!scope) return { error: 'unsupported-page' }
129+
const slot: HandoffSlot = {}
130+
slotsByScope.set(scopeId, slot)
113131
const profiles = await listAllBrowserProfiles().catch(() => [])
114132
const profile = profiles.find(({ id }) => id === request.profileId)
115-
if (!profile?.cookiesPath) return { error: 'chrome-not-found' }
133+
if (!profile?.cookiesPath) return retire(slot, { error: 'chrome-not-found' })
116134
try {
117135
await openInBrowserProfile(profile.source.bundleId, profile.directory, request.url)
118136
} catch {
119-
return { error: 'launch-failed' }
137+
return retire(slot, { error: 'launch-failed' })
138+
}
139+
if (!isCurrentSlot(slot)) return {}
140+
slot.handoff = { profile, tabId: request.tabId, url: request.url, scope }
141+
return { pending: pendingSignIn(slot.handoff) }
142+
}
143+
144+
function retire(
145+
slot: HandoffSlot,
146+
result: BrowserContinueSignInResult
147+
): BrowserContinueSignInResult {
148+
for (const [scopeId, current] of slotsByScope) {
149+
if (current === slot) slotsByScope.delete(scopeId)
120150
}
121-
const handoff = { profile, tabId: request.tabId, url: request.url, scope }
122-
handoffsByScope.set(scopeId, handoff)
123-
return { pending: pendingSignIn(handoff) }
151+
return result
124152
}
125153

126154
export function getPendingSignIn(scopeId: string): BrowserPendingSignIn | null {
127-
const handoff = handoffsByScope.get(scopeId)
155+
const handoff = slotsByScope.get(scopeId)?.handoff
128156
return handoff ? pendingSignIn(handoff) : null
129157
}
130158

131159
export function cancelSignIn(scopeId: string): void {
132-
handoffsByScope.delete(scopeId)
160+
slotsByScope.delete(scopeId)
161+
}
162+
163+
/** Follows a provisional chat to its saved id, including a launch still in flight. */
164+
export function migrateSignIn(fromScopeId: string, toScopeId: string): void {
165+
const slot = slotsByScope.get(fromScopeId)
166+
if (!slot) return
167+
slotsByScope.delete(fromScopeId)
168+
slotsByScope.set(toScopeId, slot)
169+
}
170+
171+
/** Account teardown: forgets every handoff and retires launches still in flight. */
172+
export function clearSignIns(): void {
173+
slotsByScope.clear()
133174
}
134175

135176
/**
136177
* Imports the chat's handed-off site from the profile it was continued in. The
137178
* handoff stays pending until something comes back, so the user can finish
138-
* signing in and try again.
179+
* signing in and try again. `current` is false when a cancel, a newer
180+
* continue, or teardown retired the handoff while it was being imported.
139181
*/
140-
export async function bringSignInBack(
141-
scopeId: string
142-
): Promise<{ result: BrowserImportResult; handoff?: { tabId: string; url: string } }> {
143-
const handoff = handoffsByScope.get(scopeId)
144-
if (!handoff) return { result: cookieFailure('unknown') }
182+
export async function bringSignInBack(scopeId: string): Promise<{
183+
result: BrowserImportResult
184+
handoff?: { tabId: string; url: string }
185+
current?: boolean
186+
}> {
187+
const slot = slotsByScope.get(scopeId)
188+
const handoff = slot?.handoff
189+
if (!slot || !handoff) return { result: cookieFailure('unknown') }
145190
const result = await runCookieImport(
146191
handoff.profile.id,
147192
{ ...deps(), listProfiles: async () => [handoff.profile] },
148193
handoff.scope
149194
)
150-
if (result.cookiesImported > 0 && handoffsByScope.get(scopeId) === handoff) {
151-
handoffsByScope.delete(scopeId)
152-
}
153-
return { result, handoff }
195+
const current = slotsByScope.get(scopeId) === slot && slot.handoff === handoff
196+
if (current && result.cookiesImported > 0) slotsByScope.delete(scopeId)
197+
return { result, handoff, current }
154198
}

‎apps/desktop/src/main/index.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ import {
5353
setBrowserAppearanceTheme as setAgentBrowserTheme,
5454
setPanelFocused as setBrowserAgentPanelFocused,
5555
} from '@/main/browser-agent/session'
56+
import { clearSignIns } from '@/main/browser-import'
5657
import { attachClientInfo } from '@/main/client-info'
5758
import { NativeComputerUseClient } from '@/main/computer-use/native-client'
5859
import { ComputerUseService } from '@/main/computer-use/service'
@@ -390,6 +391,7 @@ function main(): void {
390391
clearHandoffState: async () => {
391392
const stores = [
392393
{ label: 'sign-in handoff state', clear: () => handoff.clear() },
394+
{ label: 'browser sign-in handoffs', clear: clearSignIns },
393395
{ label: 'recent tasks', clear: () => tray?.clearRecentChats() },
394396
{
395397
label: 'renderer session state',
@@ -895,6 +897,7 @@ function main(): void {
895897
...(kind === 'account' && origin
896898
? [
897899
{ label: 'sign-in handoff state', clear: () => handoff.clear() },
900+
{ label: 'browser sign-in handoffs', clear: clearSignIns },
898901
{ label: 'background executor', clear: () => desktopExecutor.signOut() },
899902
{
900903
label: 'terminal sessions',

‎apps/desktop/src/main/ipc.test.ts‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,11 @@ vi.mock('@/main/browser-import', () => ({
2626
bringSignInBack: vi.fn(async () => ({
2727
result: { cookiesImported: 2, cookiesSkipped: 0 },
2828
handoff: { tabId: 'tab-1', url: 'https://signin.example.com/login' },
29+
current: true,
2930
})),
3031
cancelSignIn: vi.fn(),
3132
getPendingSignIn: vi.fn(() => null),
33+
migrateSignIn: vi.fn(),
3234
}))
3335

3436
vi.mock('@/main/browser-search/suggestions', () => ({
@@ -149,6 +151,7 @@ import {
149151
importChromeCookies,
150152
importChromePasswords,
151153
listChromeImportProfiles,
154+
migrateSignIn,
152155
} from '@/main/browser-import'
153156
import { getSearchSuggestions } from '@/main/browser-search/suggestions'
154157
import { ComputerUseService } from '@/main/computer-use/service'
@@ -1149,11 +1152,23 @@ describe('registerIpcHandlers', () => {
11491152
})
11501153

11511154
expect(migrate).not.toHaveBeenCalled()
1155+
expect(migrateSignIn).not.toHaveBeenCalled()
11521156
expect(deps.scopeEvents.activateBrowser).toHaveBeenCalledTimes(1)
11531157

11541158
migrate.mockRestore()
11551159
})
11561160

1161+
it('moves a pending sign-in with the browser scope when a new chat gets its id', async () => {
1162+
const migrate = vi.spyOn(browserDriver, 'migrateBrowserScope').mockReturnValue(true)
1163+
const { invoke } = collectHandlers()
1164+
1165+
await invoke.get('browser-agent:activate-scope')?.(appEvent, 'pending:signin')
1166+
await invoke.get('browser-agent:migrate-scope')?.(appEvent, 'pending:signin', 'chat-saved')
1167+
1168+
expect(migrateSignIn).toHaveBeenCalledWith('pending:signin', 'chat-saved')
1169+
migrate.mockRestore()
1170+
})
1171+
11571172
it('requires a live user gesture before importing Chrome cookies', async () => {
11581173
const { invoke } = collectHandlers()
11591174
const handler = invoke.get('browser-import:cookies')
@@ -1305,10 +1320,23 @@ describe('registerIpcHandlers', () => {
13051320
focus.mockRestore()
13061321
})
13071322

1323+
it('leaves the tabs alone when a newer continue retired the handoff mid-import', async () => {
1324+
vi.mocked(bringSignInBack).mockResolvedValueOnce({
1325+
result: { cookiesImported: 2, cookiesSkipped: 0 },
1326+
handoff: { tabId: 'tab-1', url: 'https://signin.example.com/login' },
1327+
current: false,
1328+
})
1329+
1330+
await bringBack(activeAppEvent, 'chat-signin')
1331+
1332+
expect(panelAction).not.toHaveBeenCalled()
1333+
})
1334+
13081335
it('leaves the tabs alone when nothing came back', async () => {
13091336
vi.mocked(bringSignInBack).mockResolvedValueOnce({
13101337
result: { cookiesImported: 0, cookiesSkipped: 0 },
13111338
handoff: { tabId: 'tab-1', url: 'https://signin.example.com/login' },
1339+
current: true,
13121340
})
13131341

13141342
await bringBack(activeAppEvent, 'chat-signin')

‎apps/desktop/src/main/ipc.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ import {
8888
importChromeData,
8989
importChromePasswords,
9090
listChromeImportProfiles,
91+
migrateSignIn,
9192
} from '@/main/browser-import'
9293
import { getSearchSuggestions } from '@/main/browser-search/suggestions'
9394
import { listSites } from '@/main/browser-sites'
@@ -1181,6 +1182,7 @@ export function registerIpcHandlers(deps: IpcDeps): void {
11811182
return { tabs: [], activeTabId: null }
11821183
}
11831184
consumePendingScope(browserPendingScopesBySender, contents, from)
1185+
migrateSignIn(from, to)
11841186
if (browserScopeBySender.get(contents) === from) {
11851187
browserScopeBySender.set(contents, to)
11861188
deps.scopeEvents.activateBrowser(contents, to)
@@ -1548,8 +1550,8 @@ export function registerIpcHandlers(deps: IpcDeps): void {
15481550
handler: async (sender, rawScope) => {
15491551
const scopeId = activeRendererScope(browserScopeBySender, sender as WebContents, rawScope)
15501552
if (!scopeId) return { cookiesImported: 0, cookiesSkipped: 0, error: 'unknown' }
1551-
const { result, handoff } = await bringSignInBack(scopeId)
1552-
if (!handoff || result.cookiesImported === 0) return result
1553+
const { result, handoff, current } = await bringSignInBack(scopeId)
1554+
if (!handoff || !current || result.cookiesImported === 0) return result
15531555
const tabOpen = withBrowserScope(scopeId, () =>
15541556
peekTabsState().tabs.some(({ tabId }) => tabId === handoff.tabId)
15551557
)

‎apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/use-continue-sign-in.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ export function useContinueSignIn({ scopeId, pageUrl }: UseContinueSignInProps)
6767
async (profile: BrowserImportProfile) => {
6868
try {
6969
const result = await startHandoff(profile.id)
70-
if (!result.pending) toast.error(CONTINUE_ERROR_MESSAGES[result.error ?? 'unknown'])
70+
if (result.error) toast.error(CONTINUE_ERROR_MESSAGES[result.error])
7171
} catch {
7272
toast.error(CONTINUE_ERROR_MESSAGES.unknown)
7373
}

0 commit comments

Comments
 (0)