diff --git a/src/main/browser/browser-cookie-import-clear.ts b/src/main/browser/browser-cookie-import-clear.ts index a01e3aa1d10..9f364d097d0 100644 --- a/src/main/browser/browser-cookie-import-clear.ts +++ b/src/main/browser/browser-cookie-import-clear.ts @@ -51,7 +51,7 @@ export type CookieClearSession = { restoreClearIdentities: CookieClearStore['restoreClearIdentities'] } -const clearLocks = new WeakMap>() +const mutationLocks = new WeakMap>() function cookieClearKey(url: string, name: string): string { return JSON.stringify([url, name]) @@ -74,17 +74,30 @@ export function identitiesFromClearCookies( })) } -export async function withCookieClearLock(owner: object, run: () => Promise): Promise { - const previous = clearLocks.get(owner) ?? Promise.resolve() +/** + * Serialises every live-jar mutation for one owner. + * + * Why (STA-4601): an import's clear, its writes, and its rollback are one transaction. Holding the + * lock for the clear alone lets a second import interleave between them, so a stale rollback can + * remove cookies the newer import already reported as written. Callers that need the lock across a + * try/finally take it directly; callers with a single callback use the wrapper below. + */ +export async function acquireCookieMutationLock(owner: object): Promise<() => void> { + const previous = mutationLocks.get(owner) ?? Promise.resolve() let release!: () => void const current = new Promise((resolve) => { release = resolve }) - clearLocks.set( + mutationLocks.set( owner, previous.then(() => current) ) await previous + return release +} + +export async function withCookieMutationLock(owner: object, run: () => Promise): Promise { + const release = await acquireCookieMutationLock(owner) try { return await run() } finally { @@ -165,11 +178,22 @@ async function restoreClearedCookies( ) } +/** + * Clears the transplantable cookies from a jar. + * + * Why (STA-4601): this takes the mutation lock on the object it is PASSED, which serialises direct + * callers that hand it a real Session — pinned by "serializes concurrent clears on the same + * session" in the atomicity suite. It does NOT serialise the importer, because both import paths + * build a fresh adapter object per call, so the key is new every time and this lock is a no-op for + * them. That is deliberate and safe: the importer holds the real per-partition lock, keyed on the + * Electron Session, across its whole clear-and-write transaction — a scope this function cannot + * see. Do not remove the importer's outer lock on the assumption that this one covers it. + */ export async function removeTransplantableCookies( targetSession: CookieClearSession, preserveFamilies: ReadonlySet = new Set() ): Promise { - return withCookieClearLock(targetSession, async () => { + return withCookieMutationLock(targetSession, async () => { const store = targetSession.cookies const initialCookies = await store.get({}) if (initialCookies.length === 0) { diff --git a/src/main/browser/browser-cookie-import-concurrency.test.ts b/src/main/browser/browser-cookie-import-concurrency.test.ts new file mode 100644 index 00000000000..6044be60ec6 --- /dev/null +++ b/src/main/browser/browser-cookie-import-concurrency.test.ts @@ -0,0 +1,464 @@ +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const { + appGetPathMock, + sessionFromPartitionMock, + snapshotClearIdentitiesMock, + writeCookieIdentityMock +} = vi.hoisted(() => ({ + appGetPathMock: vi.fn(), + sessionFromPartitionMock: vi.fn(), + snapshotClearIdentitiesMock: vi.fn(), + writeCookieIdentityMock: vi.fn() +})) + +vi.mock('electron', () => ({ + app: { getPath: appGetPathMock }, + dialog: { showOpenDialog: vi.fn() }, + session: { fromPartition: sessionFromPartitionMock } +})) +// Why (STA-4300): both import paths now write through writeCookieIdentity, not cookies.set. A stub +// without that method throws a TypeError that writeImportedCookies CATCHES as a write rejection, so +// the import would silently take the failure path and the ordering assertions would go vacuous. +// Every test below therefore also asserts writeCookieIdentity was actually CALLED. +vi.mock('./browser-cookie-clear-store', () => ({ + openCookieClearStore: (targetSession: { + cookies: { + get: (filter: object) => Promise + remove: (url: string, name: string) => Promise + } + }) => ({ + get: (filter: object) => targetSession.cookies.get(filter), + remove: (url: string, name: string) => targetSession.cookies.remove(url, name), + snapshotClearIdentities: snapshotClearIdentitiesMock, + restoreClearIdentities: async () => undefined, + writeCookieIdentity: writeCookieIdentityMock, + dispose: () => undefined + }) +})) + +import { importCookiesFromBrowser, importCookiesFromFile } from './browser-cookie-import' +import { createChromiumCookieTestDatabase } from './browser-cookie-import-test-database' +import { acquireCookieMutationLock, withCookieMutationLock } from './browser-cookie-import-clear' + +/** + * STA-4601: two imports on one partition must not interleave on the live jar. + * + * Nothing serialises imports per partition — neither the renderer IPC handler nor the runtime RPC + * method — so before this change the lock covered the clear alone and was released before the + * writes and the rollback. That let import A's rollback remove cookies import B had already + * written and reported as imported. + * + * These tests drive the lock primitive directly with deterministic interleavings rather than + * racing two real imports, so a failure names the ordering rule that broke instead of flaking. + */ +describe('cookie mutation lock', () => { + it('serialises two transactions on the same owner', async () => { + const owner = {} + const order: string[] = [] + + const first = withCookieMutationLock(owner, async () => { + order.push('A:clear') + await Promise.resolve() + order.push('A:write') + }) + // Why: started before A resolves, so an unserialised implementation interleaves here. + const second = withCookieMutationLock(owner, async () => { + order.push('B:clear') + await Promise.resolve() + order.push('B:write') + }) + + await Promise.all([first, second]) + + expect(order).toEqual(['A:clear', 'A:write', 'B:clear', 'B:write']) + }) + + it('does not serialise across different owners', async () => { + // Why: the lock is per partition. Two different profiles must still import concurrently. + const order: string[] = [] + const a = withCookieMutationLock({}, async () => { + order.push('a:start') + await Promise.resolve() + order.push('a:end') + }) + const b = withCookieMutationLock({}, async () => { + order.push('b:start') + await Promise.resolve() + order.push('b:end') + }) + + await Promise.all([a, b]) + + expect(order).toContain('a:end') + expect(order).toContain('b:end') + // Interleaved rather than strictly sequential. + expect(order).not.toEqual(['a:start', 'a:end', 'b:start', 'b:end']) + }) + + it('releases the lock when the transaction throws, so the next import is not wedged', async () => { + const owner = {} + + await expect( + withCookieMutationLock(owner, async () => { + throw new Error('clear rejected') + }) + ).rejects.toThrow('clear rejected') + + // Why: a failed import must not leave the partition permanently locked. + await expect(withCookieMutationLock(owner, async () => 'second ran')).resolves.toBe( + 'second ran' + ) + }) + + it('holds across an explicit acquire/release so a rollback stays inside the transaction', async () => { + // Why: path A takes the lock directly rather than through a callback, because its rollback runs + // in a finally block. A stale rollback outside the lock is the STA-4601 defect. + const owner = {} + const order: string[] = [] + + const release = await acquireCookieMutationLock(owner) + const queued = withCookieMutationLock(owner, async () => { + order.push('B:clear') + }) + + order.push('A:replace') + await Promise.resolve() + order.push('A:rollback') + release() + + await queued + + expect(order).toEqual(['A:replace', 'A:rollback', 'B:clear']) + }) + + it('leaves the lock usable if a holder releases twice', async () => { + const owner = {} + const release = await acquireCookieMutationLock(owner) + release() + release() + + await expect(withCookieMutationLock(owner, async () => 'ok')).resolves.toBe('ok') + }) +}) + +/** + * The primitive tests above prove the lock works. They would ALL still pass if the importer stopped + * calling it, because they never run an import — so on their own they are not a detector for the + * bug this change fixes. These tests drive two real concurrent imports into one partition and + * assert the second cannot begin until the first has finished writing. + */ +describe('two concurrent imports into one partition', () => { + let tmpDir: string + let events: string[] + let cookiesGetMock: ReturnType + let cookiesRemoveMock: ReturnType + let cookiesSetMock: ReturnType + let releaseFirstWrite: () => void + + beforeEach(() => { + tmpDir = mkdtempSync(join(tmpdir(), 'orca-cookie-concurrency-')) + appGetPathMock.mockReturnValue(join(tmpDir, 'userData')) + events = [] + // Why: the clear asserts its snapshot covers the removal plan, so a stub returning [] fails + // the import before it ever reaches a write — and the ordering assertion would pass vacuously. + snapshotClearIdentitiesMock + .mockReset() + .mockImplementation(async (items: { cookie: Record; url: string }[]) => + items.map(({ cookie: entry, url }) => ({ url, ...entry })) + ) + + cookiesGetMock = vi.fn().mockResolvedValue([ + { domain: '.a.example', name: 'old-a', value: 'v', path: '/', secure: true }, + { domain: '.b.example', name: 'old-b', value: 'v', path: '/', secure: true } + ]) + cookiesRemoveMock = vi.fn(async (_url: string, name: string) => { + events.push(`remove:${name}`) + }) + const firstWrite = new Promise((resolve) => { + releaseFirstWrite = resolve + }) + writeCookieIdentityMock.mockReset().mockImplementation(async (identity: { name: string }) => { + events.push(`set:${identity.name}`) + // Why: hold the FIRST import inside its write so a second import has a real opportunity to + // interleave. Without the lock it takes that opportunity; with it, it waits. + if (identity.name === 'new-a') { + await firstWrite + } + }) + // Why (STA-4300): the importer must never reach cookies.set — it drops partitionKey. Kept as a + // guard so a regression back onto it shows up here instead of silently passing. + cookiesSetMock = vi.fn() + + // Why: Electron returns the SAME Session instance for a given partition string, which is what + // makes a per-session lock a per-partition lock. Returning a fresh object per call here would + // make this test pass against an unlocked implementation. + const stableSession = { + cookies: { get: cookiesGetMock, remove: cookiesRemoveMock, set: cookiesSetMock } + } + sessionFromPartitionMock.mockReset().mockReturnValue(stableSession) + }) + + afterEach(() => { + rmSync(tmpDir, { recursive: true, force: true }) + }) + + function writeSource(name: string, domain: string): string { + const filePath = join(tmpDir, `${name}.json`) + writeFileSync( + filePath, + JSON.stringify([{ domain, name, value: 'imported', path: '/', secure: true }]) + ) + return filePath + } + + it('does not let the second import touch the jar until the first has finished writing', async () => { + const fileA = writeSource('new-a', '.a.example') + const fileB = writeSource('new-b', '.b.example') + + const first = importCookiesFromFile(fileA, 'persist:concurrency-test') + // Let the first import reach its write and block there. + await new Promise((resolve) => setTimeout(resolve, 5)) + const second = importCookiesFromFile(fileB, 'persist:concurrency-test') + await new Promise((resolve) => setTimeout(resolve, 20)) + + // At this instant the first import is still inside its write. Nothing from the second may have + // touched the jar yet — this is the assertion that fails if the call sites drop the lock. + expect(events).not.toContain('remove:old-b') + expect(events).not.toContain('set:new-b') + + releaseFirstWrite() + const [resultA, resultB] = await Promise.all([first, second]) + + // Why: if either import failed, the ordering assertions below would pass vacuously. + expect(resultA.ok).toBe(true) + expect(resultB.ok).toBe(true) + // Why: a store stub missing writeCookieIdentity throws a TypeError that writeImportedCookies + // swallows as a rejection, so BOTH imports would report zero writes while the ordering + // assertions still held. Pin that the write path was actually entered, once per import. + expect(writeCookieIdentityMock).toHaveBeenCalledTimes(2) + expect(cookiesSetMock).not.toHaveBeenCalled() + + // And the full ordering is serial, not interleaved. + expect(events.indexOf('set:new-a')).toBeLessThan(events.indexOf('remove:old-b')) + }) +}) + +/** + * Path B (native Chromium) has its own lock acquisition, and the file-import test above cannot + * reach it. Without this, deleting path B's withCookieMutationLock leaves every test green — which + * a review round confirmed by mutation before this was added. + */ +describe('two concurrent NATIVE imports into one partition', () => { + let tmpDir: string + let events: string[] + let cookiesSetMock: ReturnType + let releaseFirstWrite: () => void + let platformSpy: { mockRestore: () => void } + + function chromeBrowser(cookiesPath: string) { + return { + family: 'chrome' as const, + label: 'Google Chrome', + cookiesPath, + keychainService: 'Chrome Safe Storage', + keychainAccount: 'Chrome', + profiles: [{ name: 'Default', directory: 'Default' }], + selectedProfile: 'Default' + } + } + + beforeEach(() => { + tmpDir = mkdtempSync(join(tmpdir(), 'orca-cookie-native-concurrency-')) + appGetPathMock.mockReturnValue(join(tmpDir, 'userData')) + events = [] + // Why: an EMPTY jar makes removeTransplantableCookies return before it clears, so the clear + // would never run and a clearData assertion would pass vacuously. Seed it. + snapshotClearIdentitiesMock + .mockReset() + .mockImplementation(async (items: { cookie: Record; url: string }[]) => + items.map(({ cookie: entry, url }) => ({ url, ...entry })) + ) + platformSpy = vi.spyOn(process, 'platform', 'get').mockReturnValue('darwin') + + const firstWrite = new Promise((resolve) => { + releaseFirstWrite = resolve + }) + writeCookieIdentityMock.mockReset().mockImplementation(async (identity: { name: string }) => { + events.push(`set:${identity.name}`) + if (identity.name === 'first') { + await firstWrite + } + }) + // Why (STA-4300): same guard as the file path — the native writes go through CDP identities. + cookiesSetMock = vi.fn() + const stableSession = { + cookies: { + get: vi + .fn() + .mockResolvedValue([ + { domain: '.stale.example', name: 'stale', value: 'v', path: '/', secure: true } + ]), + remove: vi.fn().mockResolvedValue(undefined), + set: cookiesSetMock, + flushStore: vi.fn().mockResolvedValue(undefined) + }, + clearData: vi.fn(async () => { + events.push('clearData') + }), + // Why (STA-4300): the importer asks the Session where its storage lives instead of rebuilding + // the path from the partition string, so the stub has to answer. + getStoragePath: () => join(tmpDir, 'userData', 'Partitions', 'native-conc'), + setUserAgent: vi.fn() + } + sessionFromPartitionMock.mockReset().mockReturnValue(stableSession) + }) + + afterEach(() => { + platformSpy.mockRestore() + rmSync(tmpDir, { recursive: true, force: true }) + }) + + it('does not let a second native import clear while the first is still writing', async () => { + const sourceA = join(tmpDir, 'ChromeA', 'Default', 'Network', 'Cookies') + const sourceB = join(tmpDir, 'ChromeB', 'Default', 'Network', 'Cookies') + createChromiumCookieTestDatabase(sourceA, [ + { domain: '.a.example', name: 'first', value: 'v' } + ]).close() + createChromiumCookieTestDatabase(sourceB, [ + { domain: '.b.example', name: 'second', value: 'v' } + ]).close() + // Why: a live target DB so the cold-init probe does not run and change the event ordering. + createChromiumCookieTestDatabase( + join(tmpDir, 'userData', 'Partitions', 'native-conc', 'Network', 'Cookies'), + [] + ).close() + + const first = importCookiesFromBrowser(chromeBrowser(sourceA), 'persist:native-conc') + await new Promise((resolve) => setTimeout(resolve, 10)) + const second = importCookiesFromBrowser(chromeBrowser(sourceB), 'persist:native-conc') + await new Promise((resolve) => setTimeout(resolve, 25)) + + // The first import is parked inside its write. The second must not have cleared yet — this is + // the assertion that fails if path B stops taking the lock. + expect(events.filter((e) => e === 'clearData')).toHaveLength(1) + expect(events).not.toContain('set:second') + + releaseFirstWrite() + const [resultA, resultB] = await Promise.all([first, second]) + + // Why: a failed import would make the ordering assertions pass vacuously. + expect(resultA.ok).toBe(true) + expect(resultB.ok).toBe(true) + // Why: same swallowed-TypeError trap as the file path — assert the write path was ATTEMPTED, + // once per import, not just that the end state looks right. + expect(writeCookieIdentityMock).toHaveBeenCalledTimes(2) + expect(cookiesSetMock).not.toHaveBeenCalled() + expect(events.indexOf('set:first')).toBeLessThan(events.lastIndexOf('clearData')) + }) +}) + +/** + * The cold-init probe writes and then DELETES https://localhost/__init on the live jar, and an + * import can legitimately write that exact coordinate (normalizeCookieImportDomain accepts + * `localhost` and deriveUrl produces that URL). Unlocked, the probe's remove() would erase a cookie + * a concurrent import had just reported as imported. + * + * Nothing else in the suite reaches this call site — the other native test pre-creates the target + * DB so the probe never runs — so without this, dropping the probe's lock leaves every test green. + */ +describe('cold-init probe on a partition that has never stored a cookie', () => { + let tmpDir: string + let events: string[] + let cookiesSetMock: ReturnType + let platformSpy: { mockRestore: () => void } + let stableSession: object + + beforeEach(() => { + tmpDir = mkdtempSync(join(tmpdir(), 'orca-cookie-probe-')) + appGetPathMock.mockReturnValue(join(tmpDir, 'userData')) + events = [] + platformSpy = vi.spyOn(process, 'platform', 'get').mockReturnValue('darwin') + snapshotClearIdentitiesMock + .mockReset() + .mockImplementation(async (items: { cookie: Record; url: string }[]) => + items.map(({ cookie: entry, url }) => ({ url, ...entry })) + ) + writeCookieIdentityMock.mockReset().mockResolvedValue(undefined) + + const partitionDir = join(tmpDir, 'userData', 'Partitions', 'probe-conc') + cookiesSetMock = vi.fn(async (cookie: { name: string }) => { + events.push(`set:${cookie.name}`) + }) + stableSession = { + cookies: { + get: vi.fn().mockResolvedValue([]), + remove: vi.fn(async (_url: string, name: string) => { + events.push(`remove:${name}`) + }), + set: cookiesSetMock, + // Why: the real probe's flushStore is what creates the file. It must NOT create it on the + // earlier pre-staging flush, or the DB already exists when the probe is chosen and the + // probe never runs — which is exactly how this detector went vacuous the first time. + flushStore: vi.fn(async () => { + if (events.includes('set:__init')) { + createChromiumCookieTestDatabase(join(partitionDir, 'Network', 'Cookies'), []).close() + } + }) + }, + clearData: vi.fn(async () => { + events.push('clearData') + }), + getStoragePath: () => partitionDir, + setUserAgent: vi.fn() + } + sessionFromPartitionMock.mockReset().mockReturnValue(stableSession) + }) + + afterEach(() => { + platformSpy.mockRestore() + rmSync(tmpDir, { recursive: true, force: true }) + }) + + it('cannot write or delete __init while another transaction holds the partition', async () => { + const source = join(tmpDir, 'Chrome', 'Default', 'Network', 'Cookies') + createChromiumCookieTestDatabase(source, [ + { domain: '.probe.example', name: 'probe', value: 'v' } + ]).close() + + // Why: stand in for an import already mid-transaction on this partition. + const release = await acquireCookieMutationLock(stableSession) + + const running = importCookiesFromBrowser( + { + family: 'chrome' as const, + label: 'Google Chrome', + cookiesPath: source, + keychainService: 'Chrome Safe Storage', + keychainAccount: 'Chrome', + profiles: [{ name: 'Default', directory: 'Default' }], + selectedProfile: 'Default' + }, + 'persist:probe-conc' + ) + await new Promise((resolve) => setTimeout(resolve, 20)) + + // This is the assertion that fails if the probe stops taking the lock. + expect(events).not.toContain('set:__init') + expect(events).not.toContain('remove:__init') + + release() + await running + + // Why: if the import had bailed before the probe, the assertions above would pass vacuously. + // Pin that the probe actually ran once released. + expect(cookiesSetMock).toHaveBeenCalledWith( + expect.objectContaining({ name: '__init', url: 'https://localhost' }) + ) + expect(events.indexOf('set:__init')).toBeGreaterThanOrEqual(0) + expect(events.indexOf('remove:__init')).toBeGreaterThan(events.indexOf('set:__init')) + }) +}) diff --git a/src/main/browser/browser-cookie-import.ts b/src/main/browser/browser-cookie-import.ts index 36a6e4a9312..326c6b0a873 100644 --- a/src/main/browser/browser-cookie-import.ts +++ b/src/main/browser/browser-cookie-import.ts @@ -85,8 +85,9 @@ import { type ReplacedImportedDomainCookies } from './browser-cookie-import-policy' import { + acquireCookieMutationLock, removeTransplantableCookies, - withCookieClearLock, + withCookieMutationLock, type CookieClearStore, type CookieImportWriteStore } from './browser-cookie-import-clear' @@ -583,6 +584,11 @@ type CookieImportSessionStore = CookieClearStore & CookieImportWriteStore & { di type CookieImportTarget = { partition: string + // Why (STA-4601): the live-jar lock is keyed on an object, and this path no longer holds the + // Session that STA-4300 moved behind openWriteStore. session.fromPartition returns the SAME + // instance for one partition string, so carrying that instance here is what keeps this path's + // lock and the native path's lock on ONE key — a fresh object per call would serialise nothing. + mutationLockOwner: object openWriteStore: () => CookieImportSessionStore } @@ -594,6 +600,7 @@ function cookieImportTarget(targetPartition: string): CookieImportTarget { const targetSession = session.fromPartition(targetPartition) return { partition: targetPartition, + mutationLockOwner: targetSession, openWriteStore: () => openCookieClearStore(targetSession) } } @@ -664,6 +671,12 @@ async function importValidatedCookies( const cookieClearStore = plan.writes.length > 0 ? target.openWriteStore() : null if (cookieClearStore) { + // Why (STA-4601): the replace, the writes, and the rollback are one live-jar transaction. + // Releasing after the replace lets a second import interleave, so this run's rollback could + // remove cookies the newer import already wrote and reported as imported. Taken AFTER the + // store is opened on purpose — openWriteStore only builds the adapter, it attaches no + // debugger, so holding it while queued cannot deadlock against the holder. + const releaseMutationLock = await acquireCookieMutationLock(target.mutationLockOwner) let replaced: ReplacedImportedDomainCookies | null = null try { if (mode === 'replace-imported-domains') { @@ -724,7 +737,11 @@ async function importValidatedCookies( } } } finally { - cookieClearStore.dispose() + try { + cookieClearStore.dispose() + } finally { + releaseMutationLock() + } } } @@ -1586,14 +1603,23 @@ export async function importCookiesFromBrowser( let liveCookiesPath = resolveChromiumCookiesPath(partitionDir) // Why: Electron creates the Cookies file only after a cookie is stored; a throwaway set/remove forces DB init for unused profiles. + // Why (STA-4601): this probe MUTATES the live jar, so it runs under the same per-partition lock as + // the import itself. An earlier revision left it outside on the argument that no import writes + // https://localhost/__init — that was wrong. normalizeCookieImportDomain accepts `localhost`, + // cookie names are unrestricted, and deriveUrl produces exactly this URL, so an import CAN write + // that coordinate. Unlocked, this probe's remove() would delete a cookie a concurrent import had + // just written and reported as imported. The cost is negligible: the probe only runs for a + // partition that has never stored a cookie, so it is at most a one-time wait per profile. if (!liveCookiesPath) { - try { - await targetSession.cookies.set({ url: 'https://localhost', name: '__init', value: '1' }) - await targetSession.cookies.remove('https://localhost', '__init') - await targetSession.cookies.flushStore() - } catch { - // ignore — the set/remove may fail but flushStore should still create the file - } + await withCookieMutationLock(targetSession, async () => { + try { + await targetSession.cookies.set({ url: 'https://localhost', name: '__init', value: '1' }) + await targetSession.cookies.remove('https://localhost', '__init') + await targetSession.cookies.flushStore() + } catch { + // ignore — the set/remove may fail but flushStore should still create the file + } + }) liveCookiesPath = resolveChromiumCookiesPath(partitionDir) } @@ -2014,10 +2040,12 @@ export async function importCookiesFromBrowser( // the same CDP identities — cookies.set() cannot express the partition either one reads. const cookieClearStore = openCookieClearStore(targetSession) try { - // Why: this lock covers only the live jar; staging and cold-start replay keep their existing - // semantics while clear and writes can no longer interleave with another import. - await withCookieClearLock(targetSession, () => - removeTransplantableCookies( + // Why (STA-4601): the clear and the writes that repopulate the jar are one transaction. With + // the lock held for the clear alone, a second import could clear between them and this run + // would write its cookies on top of the newer import's jar. The lock covers the LIVE JAR + // only — staging and cold-start replay keep their existing semantics. + await withCookieMutationLock(targetSession, async () => { + await removeTransplantableCookies( { cookies: cookieClearStore, clearData: (options) => targetSession.clearData(options), @@ -2030,28 +2058,28 @@ export async function importCookiesFromBrowser( // snapshot taken from it, so they are never submitted to any mutation. nativePlan.skippedFamilies ) - ) - diag( - ` cleared existing session cookies before loading ${decryptedCookies.length} imported cookies` - ) + diag( + ` cleared existing session cookies before loading ${decryptedCookies.length} imported cookies` + ) - const writable: SourceCookieToWrite[] = [] - for (const cookie of decryptedCookies) { - const url = deriveUrl(cookie.domain, cookie.secure) - if (!url) { - memoryFailed++ - continue + const writable: SourceCookieToWrite[] = [] + for (const cookie of decryptedCookies) { + const url = deriveUrl(cookie.domain, cookie.secure) + if (!url) { + memoryFailed++ + continue + } + writable.push({ ...cookie, url }) } - writable.push({ ...cookie, url }) - } - // Why: a rejected cookie here falls back to the staged cold-start replay rather than - // unwinding the import, so one failure must not stop the rest from loading. - const phase = await writeImportedCookies(cookieClearStore, writable, { - stopOnFailure: false, - log: diag + // Why: a rejected cookie here falls back to the staged cold-start replay rather than + // unwinding the import, so one failure must not stop the rest from loading. + const phase = await writeImportedCookies(cookieClearStore, writable, { + stopOnFailure: false, + log: diag + }) + memoryLoaded = phase.importedCount + memoryFailed += phase.writeRejected }) - memoryLoaded = phase.importedCount - memoryFailed += phase.writeRejected } finally { cookieClearStore.dispose() }