From 64e657072daf180859ebc45b3571b22c96bce75a Mon Sep 17 00:00:00 2001
From: Jinjing <6427696+AmethystLiang@users.noreply.github.com>
Date: Thu, 20 Aug 2026 12:41:30 -0700
Subject: [PATCH] Fix flaky CI tests by adding retry logic and increasing
 timeouts

Add Electron launch retry for CI runners where startup wedges before
reaching 'ready', with fresh profile per attempt to avoid mid-init state.
Increase skill install lock timeout from 100ms to 5s to account for
fsync cost plus retry duration on loaded CI runners.
---
 ...r-cookie-import-partition.electron.test.ts | 52 +++++++++++++------
 src/main/skills/skill-install-lock.test.ts    |  4 +-
 2 files changed, 40 insertions(+), 16 deletions(-)

diff --git a/src/main/browser/browser-cookie-import-partition.electron.test.ts b/src/main/browser/browser-cookie-import-partition.electron.test.ts
index c992421871e..194ed9a4eb7 100644
--- a/src/main/browser/browser-cookie-import-partition.electron.test.ts
+++ b/src/main/browser/browser-cookie-import-partition.electron.test.ts
@@ -16,6 +16,22 @@ afterAll(() => {
   }
 })
 
+// Why: on CI runners this Electron launch intermittently never reaches `ready` — the fixture's
+// watchdog fires with its step still 'starting', and the same signature hits sibling cookie
+// fixtures on unrelated PRs, including one whose watchdog allows 20s. Measured off-runner, `ready`
+// costs ~0.1-0.4s even at a quarter CPU, so the launch is wedged rather than slow and no watchdog
+// value fixes it. Relaunch once, and only for that signature: every assertion, import failure, and
+// crash after `ready` still fails on the first attempt.
+const FIXTURE_LAUNCH_ATTEMPTS = 2
+
+function neverReachedElectronReady(fixtureResult: string): boolean {
+  try {
+    return (JSON.parse(fixtureResult) as { step?: string }).step === 'timed out after starting'
+  } catch {
+    return false
+  }
+}
+
 type FixtureResult = {
   before: Record<string, unknown>
   after: Record<string, unknown>
@@ -182,21 +198,27 @@ async function runFixture(): Promise<FixtureResult> {
   })
   writeFileSync(fixturePath, buildFixtureMain(importPath, resultPath, sourceCookiesPath))
   const { ELECTRON_RUN_AS_NODE: _electronRunAsNode, ...env } = process.env
-  const electronArgs = [fixturePath, `--user-data-dir=${join(root, 'profile')}`]
   const executable = process.platform === 'linux' ? 'xvfb-run' : electronBinary
-  const args =
-    process.platform === 'linux'
-      ? ['--auto-servernum', electronBinary, ...electronArgs, '--no-sandbox']
-      : electronArgs
-  const run = spawnSync(executable, args, {
-    encoding: 'utf8',
-    env,
-    timeout: 60_000
-  })
-  const fixtureResult = existsSync(resultPath) ? readFileSync(resultPath, 'utf8') : 'no result'
-  expect(run.error).toBeUndefined()
-  expect(run.status, `${fixtureResult}\n${run.stdout}\n${run.stderr}`).toBe(0)
-  return JSON.parse(fixtureResult) as FixtureResult
+  for (let attempt = 1; ; attempt += 1) {
+    rmSync(resultPath, { force: true })
+    // Why a fresh profile per attempt: a launch that never reached `ready` may have left the
+    // Chromium profile mid-initialization, and reusing it would bias the retry.
+    const electronArgs = [fixturePath, `--user-data-dir=${join(root, `profile-${attempt}`)}`]
+    const run = spawnSync(
+      executable,
+      process.platform === 'linux'
+        ? ['--auto-servernum', electronBinary, ...electronArgs, '--no-sandbox']
+        : electronArgs,
+      { encoding: 'utf8', env, timeout: 60_000 }
+    )
+    const fixtureResult = existsSync(resultPath) ? readFileSync(resultPath, 'utf8') : 'no result'
+    if (attempt < FIXTURE_LAUNCH_ATTEMPTS && neverReachedElectronReady(fixtureResult)) {
+      continue
+    }
+    expect(run.error).toBeUndefined()
+    expect(run.status, `${fixtureResult}\n${run.stdout}\n${run.stderr}`).toBe(0)
+    return JSON.parse(fixtureResult) as FixtureResult
+  }
 }
 
 describe('native Chromium excluded partition cookie under Electron', () => {
@@ -220,5 +242,5 @@ describe('native Chromium excluded partition cookie under Electron', () => {
       'partitioned-google',
       'stale'
     ])
-  }, 90_000)
+  }, 150_000)
 })
diff --git a/src/main/skills/skill-install-lock.test.ts b/src/main/skills/skill-install-lock.test.ts
index 32bfb9b4e2b..08da5b58040 100644
--- a/src/main/skills/skill-install-lock.test.ts
+++ b/src/main/skills/skill-install-lock.test.ts
@@ -43,7 +43,9 @@ describe('skill install lock', () => {
       JSON.stringify({ token: 'dead-owner', pid: 2_147_483_647, createdAt: Date.now() })
     )
 
-    const release = await acquireSkillInstallLock({ path: lockPath, timeoutMs: 100 })
+    // Why: reclaiming costs a fsync plus one 50ms retry, so a 100ms budget expires on a
+    // loaded CI runner and surfaces the legacy file's rename ENOTDIR instead of reclaiming.
+    const release = await acquireSkillInstallLock({ path: lockPath, timeoutMs: 5_000 })
     expect((await readPublishedOwner(lockPath)).token).not.toBe('dead-owner')
     await release()
     await expect(readdir(lockPath)).rejects.toMatchObject({ code: 'ENOENT' })