feat(desktop): browser profiles for the preview browser - #7254
feat(desktop): browser profiles for the preview browser#7254juliusmarminge wants to merge 18 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: 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
Agents could always drive the preview browser, with no way to say no. Adds Settings → Integrations → Browser → Agent browser access, on by default. Turning it off withholds the MCP credential in `prepareMcpSession`. That is the only place a credential is minted and the `/mcp` endpoint accepts nothing else, so one branch covers all five providers and any external MCP client — every adapter already treats a missing session as "no MCP server". The Codex developer instructions describing `preview_*` drop out with the tools. That is derived from the session's actual MCP configuration via the existing `hasConfiguredMcpServer`, not from a second read of the setting, so the prompt cannot describe tools the turn doesn't have. Leaving the text in would be worse than saying nothing: it steers the model away from Playwright and agent-browser, so it would talk the agent out of its only remaining browser automation. An unreadable settings file denies rather than propagating the error, which would mean adding ServerSettingsError to a union every caller handles. Denying is the safe direction: an explicit "off" silently becoming "on" would violate the user's choice, while the reverse costs one toolset and is visible at once. The toggle applies to sessions started from then on; a running agent keeps the tools it was given, which the UI states rather than implying otherwise. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The check flagged an untested backend behavior change, and it was right — withholding the MCP credential is the whole feature, and nothing asserted it. Credential issuance is the observable that matters: it is the only place a credential is minted and `/mcp` accepts nothing else, so withholding it is what actually denies every provider and any external MCP client. Testing it needed a seam. The real issuer reads a module-global registry that only a running MCP server installs, so from a unit test the enabled and disabled paths look identical — both leave no session behind. Reading the stored session back would have passed either way, which is worse than no test. `ProviderServiceLiveOptions` already carries a test override for the canonical event logger, so the issuer follows that existing pattern. 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
| 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.
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>
A tab created before profiles existed carries no profile of its own and runs in the built-in `default` partition — the scope the browser used before profiles. It was labelled with, and cleared against, whatever profile is configured as the default now, so on a machine with a custom default the active tab's data was left untouched while another profile's was wiped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
c930bd4 to
3fa5af7
Compare
github-actions
Bot
added
size:XXL
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
BrowserProfilecontract with built-indefaultandincognitoprofiles, user-defined persistent profiles, and helpers for resolving and normalizing profile lists (browserProfile.ts).profileIdfixed at open time; the chrome shows a badge when running under a non-default profile, and "Clear cookies/cache" in the More menu is scoped to the active profile.clearCookiesandclearCacheIPC handlers now accept an optional profile scope instead of always clearing all partitions.enableAgentBrowserAccessserver setting that gates MCP credential issuance; when disabled, existing credentials are revoked and browser tool instructions are omitted from Codex developer prompts.clearCookies,clearCache, andgetPreviewConfigIPC calls now requireenvironmentIdandprofileIdarguments — callers using the old no-arg signature will break.Macroscope summarized 3fa5af7.
Note
Medium Risk
Changes Electron session partitioning and cookie/cache clearing semantics across desktop IPC and preview open paths; incorrect partition derivation or missing
profileIdon snapshot updates could leak or drop login state between profiles.Overview
Adds browser profiles so each preview tab can use an isolated Electron session (cookies/storage). Built-in Default keeps the pre-profiles partition scope so upgrades do not sign users out; Incognito uses a non-persistent partition.
Contracts & settings: New profile types,
browserProfiles/browserDefaultProfileIdin client settings, and optionalprofileIdon preview open snapshots and RPC inputs. Desktop: Partition scope is derived in main from environment + profile;getPreviewConfig,clearCookies, andclearCachetakeenvironmentIdand optionalprofileId, with per-profile clears loading the session first so deletes work after restart. Server:profileIdis stored on open and explicitly carried throughnavigate/reportStatussnapshot rebuilds.Web: Integrations settings for create/rename/delete profiles and default profile; add-browser submenu to pick a profile at open time; preview chrome shows a profile badge and scopes clear actions to the tab’s profile; webview config is cached per environment+profile.
Reviewed by Cursor Bugbot for commit c930bd4. Bugbot is set up for automated code reviews on this repo. Configure here.