feat(desktop): browser profiles for the preview browser - #7254
feat(desktop): browser profiles for the preview browser#7254juliusmarminge wants to merge 13 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
github-actions
Bot
added
the
vouch:trusted
github-actions
Bot
added
the
size:XL
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
Thread transfer impact✅ Thread transfer remains within every enforced ceiling.
Baseline: unavailable · PR result: Scenario and decoded snapshot size10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.
Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed. |
There was a problem hiding this comment.
Reviewed the web UI changes (RightPanelTabs, PreviewChromeRow/PreviewView, PreviewMoreMenu, IntegrationsSettings, menu.tsx) against the shared primitive contracts. Two consistency findings, both inline.
Posted via Macroscope — UI Consistency
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
ApprovabilityVerdict: Needs human review 5 blocking correctness issues found. This PR introduces browser profiles as a new feature with significant new capability: isolated session partitions, profile management UI, and modified data-clearing paths. New features with this scope warrant human review. Additionally, an unresolved High-severity finding identifies that legacy tabs may target the wrong partition when clearing data. You can customize Macroscope's approvability policy. Learn more. |
b7395b6 to
fe1bc48
Compare
github-actions
Bot
added
the
📱 Native Change
fe1bc48 to
61d5a11
Compare
| // Built here rather than via `openPreviewSession` because this path | ||
| // maps the result differently, so the configured defaults have to be | ||
| // applied explicitly or file/link opens would ignore them. | ||
| viewport: browserDefaultOpenViewport(), |
There was a problem hiding this comment.
🟡 Medium browser/openFileInPreview.ts:54
Opening a file or link before client-settings hydration permanently creates the tab with the schema-default viewport and profile instead of the user's configured defaults. browserDefaultOpenViewport() and browserDefaultOpenProfileId() read the pre-hydration snapshot synchronously; resolve the hydrated defaults first, as openPreviewSession does, before invoking openPreview.
Also found in 1 other location(s)
apps/web/src/components/preview/openTerminalLinkInPreview.ts:93
openTerminalLinkInPreviewreads the viewport/profile via synchronousbrowserDefaultOpenViewport()andbrowserDefaultOpenProfileId(). Those accessors use the current settings snapshot, whileresolveBrowserDefaults()explicitly waits for client-settings hydration because pre-hydration values are schema defaults. If a terminal link is opened before hydration finishes, the new tab is permanently created with the schema-default viewport/profile instead of the user's configured defaults. This path should resolve hydrated defaults before invokingopenPreview.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/browser/openFileInPreview.ts around line 54:
Opening a file or link before client-settings hydration permanently creates the tab with the schema-default viewport and profile instead of the user's configured defaults. `browserDefaultOpenViewport()` and `browserDefaultOpenProfileId()` read the pre-hydration snapshot synchronously; resolve the hydrated defaults first, as `openPreviewSession` does, before invoking `openPreview`.
Also found in 1 other location(s):
- apps/web/src/components/preview/openTerminalLinkInPreview.ts:93 -- `openTerminalLinkInPreview` reads the viewport/profile via synchronous `browserDefaultOpenViewport()` and `browserDefaultOpenProfileId()`. Those accessors use the current settings snapshot, while `resolveBrowserDefaults()` explicitly waits for client-settings hydration because pre-hydration values are schema defaults. If a terminal link is opened before hydration finishes, the new tab is permanently created with the schema-default viewport/profile instead of the user's configured defaults. This path should resolve hydrated defaults before invoking `openPreview`.
| Electron only honours a partition before attach. | ||
| */} | ||
| <MenuSubTrigger | ||
| onClick={() => { |
There was a problem hiding this comment.
🟡 Medium components/RightPanelTabs.tsx:727
On touch-only devices, tapping MenuSubTrigger closes the root menu and immediately calls onAddBrowser(), so MenuSubPopup never becomes usable and every tap creates a Default-profile tab. Remove the trigger's default-browser action (or provide it as a separate item) so tapping the trigger opens the profile submenu.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/components/RightPanelTabs.tsx around line 727:
On touch-only devices, tapping `MenuSubTrigger` closes the root menu and immediately calls `onAddBrowser()`, so `MenuSubPopup` never becomes usable and every tap creates a Default-profile tab. Remove the trigger's default-browser action (or provide it as a separate item) so tapping the trigger opens the profile submenu.
There was a problem hiding this comment.
Two findings in the changed web UI. The MenuSubTrigger icon treatment added in this revision fixes the alignment of the new Browser sub-trigger, but the selector it uses also matches the trailing chevron on the two existing icon-less sub-triggers and overrides their ms-auto.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
One finding on the new profile badge in the preview chrome row: the truncate cap doesn't actually ellipsize because Badge is a flex container. Everything else in scope (the MenuSubTrigger icon-column fix, the MenuGroup/MenuGroupLabel wrapping in PreviewMoreMenu, the profile list rows reusing SettingsRow/ITEM_ROW_INNER_CLASSNAME/DraftInput/Button/AlertDialog, and the Select for the default profile) follows the existing primitives and settings conventions.
Posted via Macroscope — UI Consistency
0009cb1 to
1adce5c
Compare
github-actions
Bot
removed
the
📱 Native Change
| // Drop the partition's data too, otherwise a removed profile's cookies | ||
| // stay on disk with nothing in the UI pointing at them. | ||
| if (environmentId) { | ||
| void previewBridge?.clearCookies(environmentId, id).catch(() => undefined); |
There was a problem hiding this comment.
🟡 Medium settings/IntegrationsSettings.tsx:521
The profile is removed from settings even when clearCookies or clearCache fails, so its cookies or cache can remain on disk with no profile entry available for retry. Both IPC calls are fire-and-forget and their errors are suppressed; await the deletions and only update settings after they succeed, while surfacing failure to the user.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/components/settings/IntegrationsSettings.tsx around line 521:
The profile is removed from settings even when `clearCookies` or `clearCache` fails, so its cookies or cache can remain on disk with no profile entry available for retry. Both IPC calls are fire-and-forget and their errors are suppressed; await the deletions and only update settings after they succeed, while surfacing failure to the user.
1adce5c to
36b86ed
Compare
Groundwork for browser profiles: each profile maps to its own Electron session partition, so a tab opened under one profile cannot see another's cookies. Two profiles are built in and synthesized rather than stored, so they cannot be renamed away or deleted by hand-editing settings. Default keeps the bare environment id the browser already used as its partition scope, so upgrading does not strand existing logins in an orphaned partition. Incognito derives a non-persistent partition, which Chromium discards with the process. Incognito partitions omit the `persist:` prefix, so `will-attach-webview` — which only prefix-checks — now recognises both shapes; without that, incognito tabs would simply fail to attach. Clearing cookies or cache takes an optional profile. It previously reached every partition unconditionally, which under profiles would mean signing out of every profile from any one tab. Partition derivation stays in main. The attach gate only checks the partition string's prefix, so a renderer-supplied partition could attach to a session that never had the UA rewrite or permission handlers installed. `navigate` and `reportStatus` rebuild the snapshot field by field rather than spreading, so both carry the profile explicitly — otherwise a tab would move to another partition on its first navigation. The regression test was checked against a reverted fix to confirm it fails for that reason. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> feat(web): open preview tabs under a browser profile Threads the tab's profile from the server snapshot down to the Chromium guest, so a tab opened under one profile mounts against that profile's partition. The webview-config atom is now keyed by environment *and* profile. `Atom.family` keys on its argument, so the two are folded into one string rather than passed as an object, which would allocate a fresh entry on every render. Storage clearing in the three-dot menu names the tab's profile and scopes the call to it. Previously it cleared every partition, which under profiles would have signed the user out of every profile from any one tab. A configured default pointing at a deleted profile falls back to Default rather than opening tabs into a partition with nothing behind it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds the create/rename/remove list and a picker for which profile new tabs open under, including tabs an agent opens. Built-ins render without controls: they are synthesized rather than stored, so there is nothing to rename, and removing them would strand every tab already opened under them. A test covers the matching invariant — a hand-edited settings file cannot shadow Default or Incognito with a stored entry. Removing a profile clears its partition's cookies and cache, otherwise its logins would sit on disk with nothing in the UI pointing at them, and reassigns the default if it pointed at the removed profile. Names commit on blur via DraftInput rather than per keystroke, matching the dimension fields. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was no way to reach a non-default profile: tabs always opened under the configured default, and nothing in the chrome said which profile you were in. The "+" surface menu gains a "Browser in profile" submenu. The choice lives at open time because a tab's profile is fixed then — Electron only honours a partition before the guest attaches — so offering it on an already-open tab would promise a switch that cannot happen. The three-dot menu now names the tab's profile, which was otherwise invisible. Also routes the two paths that bypassed `openPreviewSession` — opening a file or an external link in the preview, and the terminal's "open in preview" — through the configured defaults. Both built their open input by hand, so they silently ignored the default viewport as well as the profile. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…nu row Widening `onAddBrowser` to `(profileId?: string)` broke the empty-state Browser card: it is passed straight to a DOM click handler, so React handed the MouseEvent in as the profile id and the open silently failed schema validation. Fixed structurally rather than at the call site — `onAddBrowser` goes back to taking no arguments, and choosing a profile is a separate `onAddBrowserInProfile(profileId)`. The type now rejects wiring the profile variant to a DOM handler, so this cannot recur; adding it surfaced all four call sites immediately. The "+" menu is one row again. The submenu trigger is itself clickable and opens the default profile, with hover or arrow revealing the rest, so the common case stays a single click. The menu is controlled so that action can dismiss it, which a submenu trigger does not do on its own. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`MenuSubTrigger` was the only interactive menu primitive without a cursor: `MenuItem`, `MenuCheckboxItem`, and `MenuRadioItem` all set `cursor-pointer` plus `data-disabled:cursor-not-allowed`, so it kept the default arrow and read as inert. It always was clickable — clicking opens the submenu — and it is now also an action in the add-surface menu, where the arrow cursor was actively misleading. Fixed in the primitive rather than at that one call site, since the gap applies to the other two submenu triggers (Diff scope, preview Appearance) as well. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A tab's profile was only discoverable by opening the three-dot menu, which is a poor place for something that changes what you are logged into. The chrome row gains a leading slot before the URL bar, and the preview names the tab's profile there. Only when it differs from the default: labelling every tab "Default" would be noise on the common case, while a tab running in another profile is exactly what needs calling out. Also gives the three-dot menu's profile heading a `MenuGroup` ancestor — `MenuGroupLabel` reads Base UI's group context and throws without one, which took the app to its error boundary as soon as the menu opened. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The More menu passed the tab's raw `profileId` to the clear actions. A tab created before profiles existed has none, and the IPC layer reads an absent profile as "every profile" — so a menu labelled "Clear cookies (Default)" wiped every partition. The view now resolves the default the same way it resolves the name it displays, and the prop is required so the gap cannot come back. Clearing also only touched sessions already in the in-memory map. Deriving the partition string does not create the session, so clearing a profile with no tab open this run reported success and deleted nothing; the handler now loads the session first. `resolveBrowserProfiles` additionally drops repeated ids, which map to one partition and would otherwise show as two isolated identities sharing every cookie, and reports a custom `incognito` profile as persistent, since persistence is keyed off the built-in id alone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The leading-icon rules added for the Browser sub-trigger were scoped with `:first-of-type`, which on a sub-trigger with no leading icon matches the trailing chevron instead — the compound selector outranks its `ms-auto` and took away the right alignment on the existing Appearance and Turn triggers. Scoping away from the last child leaves the chevron alone. The profile badge in the chrome row was unbounded while profile names run to 48 characters, so it took width from the URL input, the only flexible element there. It is capped and truncated. Removing a profile now confirms first, like every other destructive action in Settings, and Incognito is no longer offered as — or resolved to — the default profile: as a default it would open every new tab into storage discarded on close, and the settings list and the resolved default now agree on that. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`Badge` is an `inline-flex` with `whitespace-nowrap`, so `truncate` on the badge never reached the name inside it: a long profile name was hard-clipped at both ends with no ellipsis. The cap stays on the badge, the truncation moves to an inner span, and the full name is available as a title. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
One convention finding: the new browserProfile module (and its test) uses the consolidated import { Schema } from "effect" form instead of the subpath namespace import used by the rest of packages/contracts. Everything else in the diff (service interfaces on BrowserSession/PreviewManager, Schema.TaggedErrorClass errors with structural attributes, make/layer shape, dependency acquisition) matches the conventions.
Posted via Macroscope — Effect Service Conventions
36b86ed to
316224d
Compare
| // A tab created before profiles existed carries no profile of its own, so it | ||
| // runs in — and must clear — the configured default. Passing the snapshot's | ||
| // raw `undefined` through would reach the IPC layer as "every profile". | ||
| const activeProfileId = snapshot?.profileId ?? browserDefaults.profileId; |
There was a problem hiding this comment.
🟠 High preview/PreviewView.tsx:152
Legacy tabs without snapshot.profileId are labeled as the user's current configured default, so clearCookies and clearCache target that custom profile's partition instead of the tab's built-in Default partition; the active tab's data remains unchanged. Use the legacy built-in Default profile ID as the fallback rather than browserDefaults.profileId.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/components/preview/PreviewView.tsx around line 152:
Legacy tabs without `snapshot.profileId` are labeled as the user's current configured default, so `clearCookies` and `clearCache` target that custom profile's partition instead of the tab's built-in `Default` partition; the active tab's data remains unchanged. Use the legacy built-in `Default` profile ID as the fallback rather than `browserDefaults.profileId`.
| let name = "New profile"; | ||
| for (let index = 2; taken.has(name); index += 1) name = `New profile ${index}`; | ||
| updateSettings({ | ||
| browserProfiles: [ |
There was a problem hiding this comment.
🟡 Medium settings/IntegrationsSettings.tsx:499
Clicking Add profile twice before React rerenders creates only one profile instead of two, because both calls build browserProfiles from the same render-captured userProfiles array and the second updateSettings replaces the first patch. Disable or debounce the action while updating, or use an atomic updater based on the latest settings snapshot.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/components/settings/IntegrationsSettings.tsx around line 499:
Clicking **Add profile** twice before React rerenders creates only one profile instead of two, because both calls build `browserProfiles` from the same render-captured `userProfiles` array and the second `updateSettings` replaces the first patch. Disable or debounce the action while updating, or use an atomic updater based on the latest settings snapshot.
| // applied explicitly or file/link opens would ignore them. | ||
| viewport: browserDefaultOpenViewport(), | ||
| profileId: browserDefaultOpenProfileId(), | ||
| }, |
There was a problem hiding this comment.
Sync opens skip settings hydration
Medium Severity
File and terminal-link preview opens newly apply browserDefaultOpenViewport / browserDefaultOpenProfileId via the sync getters. Unlike openPreviewSession, they never await resolveBrowserDefaults(), so a tab opened before client settings hydrate is born on schema Default and never corrected.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 316224d. Configure here.
There was a problem hiding this comment.
Reviewed the changed web UI surfaces. The two issues flagged in earlier runs (MenuSubTrigger leading-icon column and the unbounded profile badge in the preview chrome row) are resolved — [&>svg:not(:last-child)] now excludes the chevron, and the badge is capped with truncation on an inner span, which does ellipsize because the span's overflow: hidden zeroes its automatic minimum size.
Two remaining layout notes on the new browser-profiles settings list, both about the row primitives it composes.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
One finding: the new browser-profiles list escapes the DesktopOnlyBrowserDefaults dimming on non-desktop clients, so it reads as live content inside the "only available in the desktop app" block.
Posted via Macroscope — UI Consistency
Built-in rows are a plain span and a badge rather than `h3`/`p` or disabled controls, so the block's own dimming never reached them: on web they were the only full-contrast content inside "only available in the desktop app". Also switches `browserProfile` to the subpath namespace import the rest of `packages/contracts` uses. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
One finding on the profile list's unavailable-state dimming. Everything else previously raised (badge cap/truncation, removal confirmation, sub-trigger icon column) looks addressed.
Posted via Macroscope — UI Consistency
The wrapper-level dim stacked with each control's own: the rename field and remove button composited to roughly 0.41 alpha while every other disabled control in the desktop-only block sits at 0.64. Only the built-in row's name and badge lack a disabled treatment, so the dim belongs there. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Reviewed the changed web UI (RightPanelTabs, PreviewMoreMenu, PreviewView, PreviewChromeRow, menu.tsx, IntegrationsSettings) against the shared primitives. The sub-trigger icon alignment, the capped profile badge, the confirmation dialog, and the disabled dimming all now line up with their neighbours. Two remaining spots render user-supplied profile names (up to BROWSER_PROFILE_NAME_MAX_LENGTH = 48) inside menu rows without the truncation treatment the rest of the app applies to user-named menu rows.
Posted via Macroscope — UI Consistency
Profile names are user-supplied and run to 48 characters. The Browser sub-menu rendered them bare inside an unbounded popup, so a long one widened it to fit-content and wrapped; it is now capped and truncated like the other name-bearing menus. The clear actions repeated the name their own group heading already shows, which drove the popup far past its width for no added information. The heading keeps the profile and the actions keep fixed-length labels. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
Bugbot Autofix is ON, but a cloud agent failed to start.
Reviewed by Cursor Bugbot for commit 3045949. Configure here.
There was a problem hiding this comment.
One new finding: the profile heading in PreviewMoreMenu can't ellipsize, so a long profile name still widens the popup. Previously reported findings that remain open (ITEM_ROW_INNER_CLASSNAME narrow-viewport stacking and the missing bottom spacing on the profile list in IntegrationsSettings.tsx) are not repeated here.
Posted via Macroscope — UI Consistency
Bare rows stack on narrow viewports with a larger gap inside a row than between rows, so the remove button read as belonging to the profile below. Each profile is now a bounded row, and the list carries the bottom spacing `SettingsRow` leaves to its children. `MenuGroupLabel` renders a block box, so `text-overflow` on an inline span inside it never applied and a long profile name pushed the popup past its width. The truncation sits on the label itself. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 participant
Adds browser profiles to the in-app preview browser. Each profile maps to its own Electron session partition, so a tab opened under one can't see another's cookies or logins.
Two profiles are built in and synthesized rather than stored, so they can't be renamed away or deleted by hand-editing settings:
Notes for review
Partition derivation stays in main. The
will-attach-webviewgate only prefix-checks the partition string, so a renderer-supplied partition could attach to a session that never had the UA rewrite or permission handlers installed.Incognito partitions omit the
persist:prefix, so that attach gate had to learn both shapes — without it, incognito tabs simply fail to attach.Clearing cookies/cache is now per-profile. It previously reached every partition unconditionally, which under profiles would mean signing out of every profile from any one tab.
navigateandreportStatuscarry the profile explicitly. Both rebuild the snapshot field-by-field rather than spreading, so a tab would otherwise move to another partition on its first navigation. The regression test was checked against a reverted fix to confirm it fails for that reason.A tab's profile is fixed at open. Electron only honours a
<webview>'s partition before the guest attaches, so switching would mean tearing the guest down and losing page state. The choice therefore lives in the open menu, and the chrome row names the profile when it differs from the default.Testing
Typecheck and lint clean; full suite passing. Exercised in the desktop app: profile creation, renaming, deletion, opening tabs under a chosen profile.
resolveBrowserProfileshas a test for the invariant that a hand-edited settings file cannot shadow a built-in id — without it, an entry with iddefaultwould capture every tab already opened under Default.Note
Add browser profiles to the desktop preview browser
browserProfilemodule, with built-indefault(persistent) andincognitoprofiles always present.profileIdin their snapshots, persisted across navigations and status updates; a badge in the preview chrome shows the active profile when it differs from the default.clearCookies,clearCache, andgetPreviewConfignow acceptenvironmentIdandprofileIdto scope operations to a specific partition; incognito profiles use ephemeral (non-persistent) Chromium partitions.clearCookiesandclearCachebridge calls now require a payload — zero-argument callers from older renderer code are no longer compatible.Macroscope summarized 7aa90aa.
Note
Medium Risk
Touches Electron session partitions and cookie/cache clearing semantics; IPC signatures changed (callers must pass
environmentId). Profile is fixed at tab open—mis-resolved defaults could clear wrong scope ifundefinedis passed where “all profiles” is intended.Overview
Adds browser profiles so each preview tab can use an isolated Electron session (cookies/logins). Built-in Default keeps the pre-profiles partition scope for upgrades; Incognito uses a non-persistent partition.
Desktop/main: Partition scope is derived in main from
environmentId+profileId(resolvePartitionScope).getPreviewConfig,clearCookies, andclearCachenow take environment and optional profile; per-profile clears load the session first so partitions with no open tab still clear.BrowserSessionadds ephemeral partition prefixes and optional partition filters on clear.Server: Preview open/navigate/status snapshots carry optional
profileIdso navigation does not drop the tab’s profile.Web: Settings to manage profiles and default profile; add-browser submenu to pick a profile at open time; preview chrome shows non-default profile and scopes clear actions. Open paths (
openPreviewSession, file/link opens) apply configured defaultprofileIdand viewport.Contracts: New
browserProfilemodule, settings fields, and IPC schema updates (DesktopPreviewClearDataInputSchema, optionalprofileIdon open/config).Reviewed by Cursor Bugbot for commit 7aa90aa. Bugbot is set up for automated code reviews on this repo. Configure here.