fix(browser): import Chromium cookies on Windows despite the live DB lock#8130
Conversation
…lock On Windows the live partition Cookies DB is held with an exclusive lock by Electron's network service, so copyFileSync (and even readFileSync) fail with EBUSY. Cookie import copied that live DB into a staging file before populating it, so the copy failure aborted the whole import with "Could not create staging cookie database." — making Chromium cookie import impossible on Windows while Orca is running. Make staging best-effort: when the live DB can't be copied, skip the SQLite staging / cold-start-swap path and import cookies into the live session via cookies.set() (the primary path that already runs on every platform). macOS/Linux behavior is unchanged; on Windows only cookies that fail cookies.set() validation (rare non-ASCII values) are dropped instead of the entire import failing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
📝 WalkthroughWalkthroughChromium cookie importing now treats copying the live Cookies database into staging as best-effort. When staging is unavailable, the importer skips staging database operations and continues with in-memory 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 23aef0f8-18e6-455b-b71f-e21ade869cbf
📒 Files selected for processing (2)
src/main/browser/browser-cookie-import.test.tssrc/main/browser/browser-cookie-import.ts
| if (stagingAvailable && memoryFailed > 0) { | ||
| // Why: some cookies couldn't be loaded via cookies.set() (non-ASCII values | ||
| // or other validation failures). Keep the staging DB so the next cold start | ||
| // picks them up from SQLite where CookieMonster reads them without validation. | ||
| browserSessionRegistry.setPendingCookieImport(targetPartition, stagingCookiesPath) | ||
| diag(` staged at ${stagingCookiesPath} for ${memoryFailed} cookies that need restart`) | ||
| } else { | ||
| try { | ||
| unlinkSync(stagingCookiesPath) | ||
| } catch { | ||
| /* best-effort */ | ||
| if (stagingAvailable) { | ||
| try { | ||
| unlinkSync(stagingCookiesPath) | ||
| } catch { | ||
| /* best-effort */ | ||
| } | ||
| } | ||
| diag(` all cookies loaded in-memory — no restart needed`) | ||
| // Why: without staging (e.g. the Windows exclusive lock on the live DB), cookies | ||
| // that fail cookies.set() validation cannot be recovered via a cold-start swap. | ||
| diag( | ||
| stagingAvailable || memoryFailed === 0 | ||
| ? ` all cookies loaded in-memory — no restart needed` | ||
| : ` imported in-memory; ${memoryFailed} non-loadable cookies dropped (no staging)` | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="src/main/browser/browser-cookie-import.ts"
echo '--- context around lines 1680-1785 ---'
sed -n '1680,1785p' "$file" | cat -n
echo
echo '--- search for imported/memoryFailed/summary.importedCookies ---'
rg -n "imported|memoryFailed|importedCookies|stagingAvailable" "$file"Repository: stablyai/orca
Length of output: 6420
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="src/main/browser/browser-cookie-import.ts"
echo '--- exact lines around summary assignment ---'
sed -n '1760,1785p' "$file" | cat -n
echo
echo '--- definition / references for importedCookies ---'
rg -n "importedCookies|CookieImportSummary|summary =" src/main/browser/browser-cookie-import.tsRepository: stablyai/orca
Length of output: 1425
Count usable cookies in the no-staging path. summary.importedCookies still includes cookies that fail cookies.set() when staging is unavailable, so the reported count can exceed the number of cookies actually available after import. Use the in-memory success count there, or document that the field counts attempted imports.
|
Confirmed this exact failure on the latest public build. Environment:
Observed after the #7882-era fix: This matches the failure mode described in this PR. I do not think WSL is the direct root cause here, since the failing part appears to be the Windows-side Electron/Orca target partition cookie DB lock. But it would still be useful to include Windows desktop + WSL runtime/worktree setup in the smoke matrix, because that is the real environment where this browser handoff/import flow is being used and where the previous #6875 Windows reproduction also came from. |
2 participants
Problem
On Windows, importing cookies from Chrome/Chromium browsers fails with:
(After the
#7882fix, the earlierunable to open database file/ "Try closing Chrome first" step is passed — this is the next wall, and it blocks Windows regardless of whether the source browser is closed.)Root cause
importCookiesFromBrowsercopies the target partition's liveCookiesDB into a staging file before populating it:On Windows, Electron's network service holds that live DB open with an exclusive lock, so any external open of the file fails. I verified this empirically against a running Orca profile DB — both fail with
EBUSY:Because the copy is a hard prerequisite, its failure aborts the whole import — even though the in-memory
cookies.set()path (the primary import mechanism, already run further down) needs no file copy at all. This makes Chromium cookie import effectively impossible on Windows while Orca is running (and Orca must be running to perform the import). macOS/Linux are unaffected because their SQLite locks are advisory, so the copy succeeds.Fix
Make the SQLite staging step best-effort. When the live DB can't be copied, skip the staging/cold-start-swap path and import cookies into the live session via
cookies.set()— the path that already runs on every platform.cookies.set()validation).cookies.set()validation (rare non-ASCII values) can't be recovered via the cold-start swap and are dropped, instead of the entire import failing.All staging DB operations are guarded so the pipeline runs with
stagingDb === null.Testing
falls back to in-memory import when the live target DB cannot be copied— simulates the WindowsEBUSYon the staging copy and asserts the import still succeeds viacookies.set()and leaves no staging artifact. Confirmed it fails without this change (Could not create staging cookie database.→ok:false) and passes with it.browser-cookie-import.test.ts: 20/20 passing.tsgotypecheck,oxlint, andcheck:max-lines-ratchet: all green.Related: #6875 (the Windows reproduction was reported there but the issue was closed as macOS-only).