fix(browser): isolate embedded profiles per session - #1526
Conversation
github-actions
Bot
added
app
|
Warning Review limit reached
Next review available in: 20 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughThe embedded browser now assigns persistent profiles per conversation, maps profiles through persisted main-process state, creates profile-specific Electron partitions, and clears data across legacy and active profile partitions. ChangesBrowser profile isolation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Conversation
participant browserControllers
participant BrowserProfileRegistry
participant BrowserViewController
participant ElectronPartitions
Conversation->>browserControllers: request browser controller
browserControllers->>BrowserProfileRegistry: profileFor(conversation)
browserControllers->>BrowserViewController: create with profileID
BrowserViewController->>ElectronPartitions: create profile-specific WebContentsView
BrowserViewController->>BrowserProfileRegistry: adopt(sessionID, profileID) on retarget
Conversation->>browserControllers: clear browser data
browserControllers->>BrowserProfileRegistry: profileIDs()
browserControllers->>ElectronPartitions: clear legacy and profile partitions
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Suggested priority: P2 (includes user-path files (packages/app/src/context/platform.tsx, packages/desktop-electron/src/main/browser/controller-automation.ts, packages/desktop-electron/src/main/browser/controller.ts, packages/desktop-electron/src/main/browser/options.test.ts, packages/desktop-electron/src/main/browser/options.ts, packages/desktop-electron/src/main/browser/profile-registry.test.ts, packages/desktop-electron/src/main/browser/profile-registry.ts, packages/desktop-electron/src/main/ipc/browser.ts, packages/desktop-electron/src/main/runtime-namespace.ts)).
P1/P0 are reserved for maintainer confirmation. Please relabel manually if this is a release blocker, security issue, data-loss risk, or updater/runtime failure.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/desktop-electron/src/main/browser/profile-registry.ts`:
- Around line 42-61: Update profileFor to persist newly created draft profiles
under a unique unadopted:${created} key instead of leaving them untracked.
Update adopt to locate and remove that temporary key when adopting the profile,
then persist the profile under sessionID; preserve existing handling for invalid
or already non-draft sessions. Adjust the unadopted draft assertion in
profile-registry.test.ts to expect the temporary storage key.
In `@packages/desktop-electron/src/main/ipc/browser.ts`:
- Around line 83-98: Update the browser:clear-data handler to avoid
session.fromPartition for inactive profiles: clear storage and cache through
Electron only for partitions belonging to active browserControllers, while
removing inactive partition directories directly with fs.promises.rm using the
userData/Partitions path and recursive, force options. Process cleanup
sequentially rather than with Promise.all, preserve cleanup of the legacy
partition, and continue reloading loaded controllers afterward.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 07555b75-9259-4cef-80a3-ec1312f5a631
📒 Files selected for processing (9)
packages/app/src/context/platform.tsxpackages/desktop-electron/src/main/browser/controller-automation.tspackages/desktop-electron/src/main/browser/controller.tspackages/desktop-electron/src/main/browser/options.test.tspackages/desktop-electron/src/main/browser/options.tspackages/desktop-electron/src/main/browser/profile-registry.test.tspackages/desktop-electron/src/main/browser/profile-registry.tspackages/desktop-electron/src/main/ipc/browser.tspackages/desktop-electron/src/main/runtime-namespace.ts
1 participant
Summary
Why
The browser controllers, pages, and CDP connections were conversation-owned, but every
WebContentsViewstill used the samepersist:pawwork-browserpartition. Electron reuses oneSessionfor pages with the same partition, so cookies, local storage, IndexedDB, service workers, cache, and permission handlers remained shared. Two conversations operating the same site concurrently could therefore mutate each other's browser state.The profile partition is now part of the conversation ownership boundary instead of an app-wide singleton.
Related Issue
Related to #1333. This concurrency bug was reported directly and does not have a dedicated issue.
Human Review Status
Pending
Review Focus
profile-registry.ts, especially duplicate-map repair.Risk Notes
How To Verify
Screenshots or Recordings
Not applicable; this changes Electron storage ownership without changing visible UI.
Checklist
bug,enhancement,task,documentation. Type labels are author-added; the labeler bot does NOT assign them. Add the label in the GitHub UI, then tick this.app,ui,platform,harness,ci. The labeler bot assigns these on PR open based on changed paths. Confirm the bot's choice (or override if wrong), then tick this.P0,P1,P2,P3. The priority-triage bot suggests one on PR open. Confirm or override, then tick this.Pending,Approved by @<reviewer>, orNot required: <reason>(default isPending; "not required" is restricted to bot-authored low-risk PRs).dev, and my PR title and commit messages use Conventional Commits in English.Summary by CodeRabbit
New Features
Bug Fixes