Skip to content

Commit c762bb8

Browse files
committed
fix(browser): polish new tabs and local password imports
1 parent 7b4e737 commit c762bb8

19 files changed

Lines changed: 752 additions & 234 deletions

File tree

apps/desktop/src/main/browser-agent/session.test.ts

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4661,12 +4661,14 @@ describe('reopening a closed tab', () => {
46614661
})
46624662

46634663
describe('importAgentCookies', () => {
4664-
/** Points the mocked partition at a cookie jar and returns its `set` spy. */
4665-
function withCookieJar(set: ReturnType<typeof vi.fn>): SessionModule {
4664+
function withCookieJar(
4665+
set: ReturnType<typeof vi.fn>,
4666+
flushStore = vi.fn(async () => {})
4667+
): SessionModule {
46664668
// The partition is resolved per call, not captured at module load, so
46674669
// re-mocking it here is enough — no module reload required.
46684670
vi.mocked(electronSession.fromPartition).mockReturnValue({
4669-
cookies: { set },
4671+
cookies: { set, flushStore },
46704672
} as unknown as ReturnType<typeof electronSession.fromPartition>)
46714673
return sessionModule
46724674
}
@@ -4683,14 +4685,17 @@ describe('importAgentCookies', () => {
46834685

46844686
it('writes every cookie into the dedicated browser profile', async () => {
46854687
const set = vi.fn(async () => {})
4686-
const session = withCookieJar(set)
4688+
const flushStore = vi.fn(async () => {})
4689+
const session = withCookieJar(set, flushStore)
46874690

46884691
const result = await session.importAgentCookies([cookie('a'), cookie('b')])
46894692

46904693
expect(result).toEqual({ imported: 2, failed: 0 })
46914694
expect(electronSession.fromPartition).toHaveBeenCalledWith('persist:sim-browser-agent')
46924695
expect(set).toHaveBeenCalledTimes(2)
46934696
expect(set).toHaveBeenNthCalledWith(1, cookie('a'))
4697+
expect(flushStore).toHaveBeenCalledOnce()
4698+
expect(flushStore.mock.invocationCallOrder[0]).toBeGreaterThan(set.mock.invocationCallOrder[1])
46944699
})
46954700

46964701
it('counts a rejected cookie without losing the rest', async () => {
@@ -4709,9 +4714,22 @@ describe('importAgentCookies', () => {
47094714

47104715
it('does nothing when there is nothing to import', async () => {
47114716
const set = vi.fn(async () => {})
4712-
const session = withCookieJar(set)
4717+
const flushStore = vi.fn(async () => {})
4718+
const session = withCookieJar(set, flushStore)
47134719

47144720
await expect(session.importAgentCookies([])).resolves.toEqual({ imported: 0, failed: 0 })
47154721
expect(set).not.toHaveBeenCalled()
4722+
expect(flushStore).not.toHaveBeenCalled()
4723+
})
4724+
4725+
it('does not report a durable import when flushing to disk fails', async () => {
4726+
const session = withCookieJar(
4727+
vi.fn(async () => {}),
4728+
vi.fn(async () => {
4729+
throw new Error('Disk unavailable')
4730+
})
4731+
)
4732+
4733+
await expect(session.importAgentCookies([cookie('a')])).rejects.toThrow('Disk unavailable')
47164734
})
47174735
})

apps/desktop/src/main/browser-agent/session.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1196,6 +1196,7 @@ export async function importAgentCookies(
11961196
failed += 1
11971197
}
11981198
}
1199+
if (imported > 0) await jar.flushStore()
11991200
return { imported, failed }
12001201
}
12011202

apps/desktop/src/main/browser-credentials/vault.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,25 @@ const CANDIDATES = [
3939
]
4040

4141
describe('CredentialVault', () => {
42+
it('retains imported logins when the vault is reopened and does not duplicate a re-import', async () => {
43+
const original = new CredentialVault(vaultPath, encryption())
44+
await original.importCredentials(CANDIDATES, 'replace')
45+
const metadata = await original.list()
46+
47+
const reopened = new CredentialVault(vaultPath, encryption())
48+
expect(await reopened.list()).toEqual(metadata)
49+
expect(await reopened.readForFill(metadata[0].id, metadata[0].origin)).toEqual({
50+
username: 'ada',
51+
password: 'hunter2',
52+
})
53+
expect(await reopened.importCredentials(CANDIDATES, 'replace')).toEqual({
54+
added: 0,
55+
updated: 0,
56+
skipped: 2,
57+
})
58+
expect(await reopened.list()).toEqual(metadata)
59+
})
60+
4261
it('stores and lists credentials without their passwords', async () => {
4362
const vault = new CredentialVault(vaultPath, encryption())
4463

apps/desktop/src/main/browser-import/import-service.test.ts

Lines changed: 26 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -616,26 +616,33 @@ describe('importChromePasswords', () => {
616616
expect(importCredentials).toHaveBeenCalledWith([expect.any(Object)], 'replace')
617617
})
618618

619-
it('keeps credentials from one password store when the other is unreadable', async () => {
620-
const localPath = '/arc/Default/Login Data'
621-
const accountPath = '/arc/Default/Login Data For Account'
622-
const deps = createDeps({
623-
listProfiles: async () => [
624-
{ ...PROFILES[1], id: 'arc:Default', loginDataPaths: [localPath, accountPath] },
625-
],
626-
readPasswords: async (path) => {
627-
if (path === accountPath) {
628-
throw new ImportFailure('unsupported-schema', 'unknown account-store schema')
629-
}
630-
return readPasswords()
631-
},
632-
})
619+
it.each(['local', 'account'])(
620+
'reports a partial import when the %s password store is unreadable',
621+
async (failedStore) => {
622+
const localPath = '/arc/Default/Login Data'
623+
const accountPath = '/arc/Default/Login Data For Account'
624+
const deps = createDeps({
625+
listProfiles: async () => [
626+
{ ...PROFILES[1], id: 'arc:Default', loginDataPaths: [localPath, accountPath] },
627+
],
628+
readPasswords: async (path) => {
629+
if (path === (failedStore === 'local' ? localPath : accountPath)) {
630+
throw new ImportFailure('unsupported-schema', 'unknown account-store schema')
631+
}
632+
return readPasswords()
633+
},
634+
})
633635

634-
await expect(importChromePasswords('arc:Default', 'replace', deps)).resolves.toMatchObject({
635-
passwordsAdded: 1,
636-
passwordsSkipped: 0,
637-
})
638-
})
636+
await expect(importChromeData('arc:Default', 'replace', deps)).resolves.toMatchObject({
637+
cookies: { cookiesImported: 1 },
638+
passwords: {
639+
passwordsAdded: 1,
640+
passwordsSkipped: 0,
641+
error: 'unsupported-schema',
642+
},
643+
})
644+
}
645+
)
639646

640647
it('surfaces a failed password store when the other store only has unreadable rows', async () => {
641648
const localPath = '/arc/Default/Login Data'

apps/desktop/src/main/browser-import/import-service.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,7 @@ async function runPasswordImport(
302302
passwordsAdded: outcome.added,
303303
passwordsUpdated: outcome.updated,
304304
passwordsSkipped: outcome.skipped + read.skipped,
305+
...(read.error ? { error: read.error } : {}),
305306
}
306307
logger.info('Chrome password import finished', {
307308
added: result.passwordsAdded,
@@ -321,14 +322,15 @@ async function runPasswordImport(
321322
* source order breaks ties deterministically before applying the vault policy.
322323
*
323324
* One damaged store does not discard credentials already read from the other.
325+
* A partial read carries its failure to the UI alongside the imported counts.
324326
* If no store can produce any useful signal, the first concrete reader error
325327
* is surfaced instead of reporting a misleading successful import of zero.
326328
*/
327329
async function readProfilePasswords(
328330
paths: readonly string[],
329331
key: Buffer,
330332
deps: ImportServiceDeps
331-
): Promise<ReadPasswordsResult> {
333+
): Promise<ReadPasswordsResult & { error?: BrowserPasswordImportResult['error'] }> {
332334
const combined: ReadPasswordsResult = { credentials: [], skipped: 0, rowsSeen: 0 }
333335
const credentialIndexes = new Map<string, number>()
334336
let successfulReads = 0
@@ -379,6 +381,7 @@ async function readProfilePasswords(
379381
if (firstFailure !== undefined) {
380382
// Category only: database names and paths are deliberately absent.
381383
logger.warn('Could not read every password store in the selected browser profile')
384+
return { ...combined, error: categorize(firstFailure, 'password') }
382385
}
383386
return combined
384387
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import { Globe } from '@sim/emcn/icons'
2+
import { EmptyState } from '@/components/empty-state/empty-state'
3+
4+
export function BrowserEmptyState() {
5+
return (
6+
<section aria-label='New tab' className='absolute inset-0 flex overflow-auto bg-[var(--bg)]'>
7+
<EmptyState
8+
graphic={<Globe className='size-5 text-[var(--text-icon)]' aria-hidden='true' />}
9+
title='Browse the web'
10+
description='Search or enter a website in the address bar above.'
11+
/>
12+
</section>
13+
)
14+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
1+
/**
2+
* @vitest-environment jsdom
3+
*/
4+
import { act } from 'react'
5+
import type { BrowserPageState } from '@sim/browser-protocol'
6+
import type { BrowserToolbarCommand } from '@sim/desktop-bridge'
7+
import { createRoot, type Root } from 'react-dom/client'
8+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
9+
10+
const { desktop, navigateToSettings, removeResource } = vi.hoisted(() => ({
11+
navigateToSettings: vi.fn(),
12+
removeResource: vi.fn(),
13+
desktop: {
14+
settings: { getPreferences: vi.fn(async () => ({ browserTheme: 'app' })) },
15+
browserCredentials: { list: vi.fn(async () => []), onFillAvailability: vi.fn(() => () => {}) },
16+
browserImport: {
17+
listChromeProfiles: vi.fn(async () => []),
18+
listSites: vi.fn(async () => []),
19+
importFromChrome: vi.fn(),
20+
},
21+
browserAgent: {
22+
supportsAtomicPanelOcclusion: true,
23+
setTheme: vi.fn(),
24+
setPanelBounds: vi.fn(),
25+
setPanelFocused: vi.fn(),
26+
setPanelOccluded: vi.fn(async () => true),
27+
capturePanelSnapshot: vi.fn(async () => null),
28+
getKnownSessions: vi.fn(async () => ({ sessions: [] })),
29+
getDownloadsState: vi.fn(async () => ({ downloads: [] })),
30+
onAppearanceThemeChanged: vi.fn(() => () => {}),
31+
onToolbarCommand: vi.fn(
32+
(_callback: (command: BrowserToolbarCommand, scopeId: string) => void) => () => {}
33+
),
34+
onAddToChat: vi.fn(() => () => {}),
35+
onFocusOmnibox: vi.fn(() => () => {}),
36+
onOpenFind: vi.fn(() => () => {}),
37+
onCloseFind: vi.fn(() => () => {}),
38+
onDownloadsState: vi.fn(() => () => {}),
39+
},
40+
},
41+
}))
42+
43+
vi.mock('@/lib/desktop', () => ({ getDesktopBridge: () => desktop }))
44+
vi.mock('@/hooks/use-settings-navigation', () => ({
45+
useSettingsNavigation: () => ({ navigateToSettings }),
46+
}))
47+
vi.mock('@/app/workspace/[workspaceId]/home/components/mothership-resources-context', () => ({
48+
useMothershipResources: () => ({ removeResource }),
49+
}))
50+
51+
import { BrowserSession } from '@/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/browser-session'
52+
import { useBrowserSessionStore } from '@/stores/browser-session/store'
53+
54+
const PAGE: BrowserPageState = {
55+
scopeId: 'browser-ui-test',
56+
tabId: 'tab-1',
57+
url: 'about:blank',
58+
title: '',
59+
loading: false,
60+
canGoBack: false,
61+
canGoForward: false,
62+
}
63+
let container: HTMLDivElement
64+
let root: Root
65+
66+
async function render(page: BrowserPageState = PAGE, visible = true) {
67+
await act(async () => {
68+
useBrowserSessionStore.getState().setPageState(page)
69+
root.render(<BrowserSession visible={visible} scopeId={PAGE.scopeId} />)
70+
})
71+
await act(async () => vi.advanceTimersByTime(20))
72+
}
73+
74+
beforeEach(() => {
75+
;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true
76+
vi.useFakeTimers()
77+
vi.stubGlobal(
78+
'ResizeObserver',
79+
class {
80+
observe() {}
81+
disconnect() {}
82+
unobserve() {}
83+
}
84+
)
85+
vi.clearAllMocks()
86+
useBrowserSessionStore.setState({ sessions: {} })
87+
container = document.createElement('div')
88+
document.body.appendChild(container)
89+
root = createRoot(container)
90+
vi.spyOn(Element.prototype, 'getBoundingClientRect').mockReturnValue({
91+
x: 0,
92+
y: 0,
93+
width: 700,
94+
height: 600,
95+
top: 0,
96+
bottom: 600,
97+
left: 0,
98+
right: 700,
99+
toJSON: () => ({}),
100+
})
101+
})
102+
103+
afterEach(() => {
104+
act(() => root.unmount())
105+
container.remove()
106+
vi.useRealTimers()
107+
vi.restoreAllMocks()
108+
})
109+
110+
describe('browser empty state and in-place import', () => {
111+
it('replaces the native blank page with shared guidance and no extra actions', async () => {
112+
await render()
113+
const emptyState = container.querySelector('section[aria-label="New tab"]')
114+
expect(emptyState?.textContent).toContain('Browse the web')
115+
expect(emptyState?.textContent).toContain('Search or enter a website in the address bar above.')
116+
expect(emptyState?.querySelector('button')).toBeNull()
117+
expect(desktop.browserAgent.setPanelBounds).toHaveBeenLastCalledWith(null, null, PAGE.scopeId)
118+
expect(container.querySelector('input')).not.toBeNull()
119+
})
120+
121+
it('hands the surface back to the native page for navigation and restores it on a blank tab', async () => {
122+
await render()
123+
await render({ ...PAGE, loading: true })
124+
expect(container.querySelector('section[aria-label="New tab"]')).toBeNull()
125+
expect(desktop.browserAgent.setPanelBounds.mock.calls.at(-1)?.[0]).toEqual(
126+
expect.objectContaining({ width: 700, height: 600 })
127+
)
128+
await render({ ...PAGE, url: 'https://example.com' })
129+
expect(container.querySelector('section[aria-label="New tab"]')).toBeNull()
130+
await render()
131+
expect(container.querySelector('section[aria-label="New tab"]')).not.toBeNull()
132+
expect(desktop.browserAgent.setPanelBounds).toHaveBeenLastCalledWith(null, null, PAGE.scopeId)
133+
})
134+
135+
it('does not replace load errors with the new-tab state', async () => {
136+
await render({
137+
...PAGE,
138+
issue: {
139+
kind: 'load-error',
140+
url: 'https://missing.example',
141+
code: -105,
142+
description: 'ERR_NAME_NOT_RESOLVED',
143+
},
144+
})
145+
expect(container.textContent).toContain("This site can't be reached")
146+
expect(container.querySelector('section[aria-label="New tab"]')).toBeNull()
147+
})
148+
149+
it('opens import in place from the native toolbar command', async () => {
150+
await render()
151+
const callback = desktop.browserAgent.onToolbarCommand.mock.calls[0][0]
152+
await act(async () => callback('import', PAGE.scopeId))
153+
expect(document.querySelector('[role="dialog"]')?.textContent).toContain(
154+
'Import from your browser'
155+
)
156+
expect(navigateToSettings).not.toHaveBeenCalled()
157+
expect(desktop.browserImport.listChromeProfiles).toHaveBeenCalledOnce()
158+
expect(document.querySelector<HTMLElement>('[role="dialog"]')?.style.visibility).not.toBe(
159+
'hidden'
160+
)
161+
expect(desktop.browserAgent.setPanelOccluded).toHaveBeenCalledWith(true, PAGE.scopeId, true)
162+
})
163+
})

0 commit comments

Comments
 (0)