fix(browser): serialize cookie imports per partition (STA-4601)

Two concurrent cookie imports on one partition can erase a session. Nothing
serialises imports per partition — neither the renderer IPC handler
(browser-session-profile-ipc.ts) nor the runtime RPC method
(orca-runtime-browser.ts) — and the clear lock covered only the clear itself,
so it was released before the writes and before any rollback.

Reachable interleaving:

  import A replaces cookies for its imported domains, releases the lock, and
  later fails a write; import B clears and writes; A's rollback then removes
  cookies B already wrote and reported as imported.

Path B has the same shape: A clears, B clears, then A writes its cookies on top
of B's jar.

Fix: one lock spans the whole live-jar transaction on both paths — the
clear/replace, the writes, and the rollback. withCookieClearLock becomes
acquireCookieMutationLock/withCookieMutationLock so path A can hold it across a
try/finally rather than a single callback.

Rebased onto #15030 (STA-4300), which rewrote this file: imports now write
through CDP identities, so path A's writes moved from an inline cookies.set()
loop into writeImportedCookies(), path B's into the same helper, and — the part
that matters for a lock — importValidatedCookies no longer holds the Electron
Session at all. It receives a CookieImportTarget that hides the Session behind
openWriteStore(), and openWriteStore() builds a FRESH adapter per call, so
keying the lock on anything reachable from it would serialise nothing. The
target therefore carries mutationLockOwner, set to session.fromPartition's
instance, which is the same object the native path locks on. That is what keeps
both paths on one key per partition.

Deliberately bounded: this covers the LIVE JAR only. Staging and cold-start
replay keep their existing semantics, so two other pre-existing hazards in that
subsystem are untouched and still open — the crash window between the clear and
the result, and a permanently-armed replay when replay keeps failing. Both need
the pending-image operations to become provable, which is a different change.

The lock is keyed per owner, so imports into different partitions still run
concurrently.

Mutation-proved against the rebased tree, every mutation structure-preserving
(brace/paren balance pinned) and every one re-run rather than carried over:
dropping path A's acquire reddens the file-import detector; keying path A's
acquire on a fresh object instead of mutationLockOwner reddens it too — the
lock-present-but-miskeyed shape this rebase risks; replacing path B's lock with
a passthrough reddens the native detector; doing both reddens both, which rules
out incidental serialisation; and neutering the cold-init probe's lock reddens
the new probe detector. That last detector is new: neutering the probe lock
previously left all 779 browser tests green, so the probe's protection was
unproven.

The concurrency suite's store stub also gained writeCookieIdentity and
getStoragePath. Without them the STA-4300 code throws a TypeError that
writeImportedCookies catches as a write rejection, so the imports would have
taken the failure path while the ordering assertions still passed. Each real
import test now asserts writeCookieIdentity was CALLED, and that cookies.set
never was.

src/main/browser: 72 files, 780 tests, green; typecheck, oxlint, type-aware
code quality, oxfmt and the max-lines ratchet all clean.

src/main/browser/browser-cookie-import-clear.ts

@@ -51,7 +51,7 @@ export type CookieClearSession = {
   restoreClearIdentities: CookieClearStore['restoreClearIdentities']
 }
 
-const clearLocks = new WeakMap<object, Promise<void>>()
+const mutationLocks = new WeakMap<object, Promise<void>>()
 
 function cookieClearKey(url: string, name: string): string {
   return JSON.stringify([url, name])
@@ -74,17 +74,30 @@ export function identitiesFromClearCookies(
   }))
 }
 
-export async function withCookieClearLock<T>(owner: object, run: () => Promise<T>): Promise<T> {
-  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<void>((resolve) => {
     release = resolve
   })
-  clearLocks.set(
+  mutationLocks.set(
     owner,
     previous.then(() => current)
   )
   await previous
+  return release
+}
+
+export async function withCookieMutationLock<T>(owner: object, run: () => Promise<T>): Promise<T> {
+  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<string> = new Set()
 ): Promise<void> {
-  return withCookieClearLock(targetSession, async () => {
+  return withCookieMutationLock(targetSession, async () => {
     const store = targetSession.cookies
     const initialCookies = await store.get({})
     if (initialCookies.length === 0) {

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<unknown>
+      remove: (url: string, name: string) => Promise<void>
+    }
+  }) => ({
+    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<typeof vi.fn>
+  let cookiesRemoveMock: ReturnType<typeof vi.fn>
+  let cookiesSetMock: ReturnType<typeof vi.fn>
+  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<string, unknown>; 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<void>((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<typeof vi.fn>
+  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<string, unknown>; url: string }[]) =>
+        items.map(({ cookie: entry, url }) => ({ url, ...entry }))
+      )
+    platformSpy = vi.spyOn(process, 'platform', 'get').mockReturnValue('darwin')
+
+    const firstWrite = new Promise<void>((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<typeof vi.fn>
+  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<string, unknown>; 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'))
+  })
+})

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()
     }