Add BrowserUserDataMode to separate user data dir from profile#16457
Open
Add BrowserUserDataMode to separate user data dir from profile#16457
Conversation
Introduce a BrowserUserDataMode enum (Shared/Isolated) on the tracked browser launch path so callers can choose between launching against the browser's real user data directory (like clicking the browser icon) or a temporary, isolated user data directory. - WithBrowserLogs(...) gains a userDataMode parameter - Configuration key: Aspire:Hosting:BrowserLogs[:Resource]:UserDataMode - Default is Isolated to preserve current behavior - Setting Profile while UserDataMode is Isolated is rejected - Shared + omitted profile lets Chromium pick its default profile - Shared + profile resolves the profile inside the real user data dir - Surface effective user data mode as a resource property Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Flip BrowserUserDataMode default from Isolated to Shared so the tracked browser behaves like a normal browser launch by default. - Add a remarks section on BrowserUserDataMode.Shared explaining the Chromium singleton/SingletonLock + DevToolsActivePort interaction so callers understand why a second launch against an already-running real browser cannot establish a CDP endpoint. - Update the default-mode test accordingly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
davidfowl
requested review from
JamesNK,
danegsta,
karolz-ms and
mitchdenny
as code owners
Contributor
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 16457Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 16457" |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new user-data directory selection mode for tracked browser log sessions, separating “which Chromium user data dir to use” from “which profile inside that dir to use” to support either reusing real browser state or running from a clean temporary state.
Changes:
- Introduces
BrowserUserDataMode(Shareddefault /Isolated) and threads it throughWithBrowserLogs(...), configuration resolution, and resource properties. - Updates browser launch behavior to use the real user data directory in
Sharedmode, including profile directory resolution via directory entries and Chromium “Local State” metadata. - Expands unit tests to cover user data mode defaults/overrides and profile directory resolution behaviors.
Show a summary per file
| File | Description |
|---|---|
| tests/Aspire.Hosting.Tests/BrowserLogsSessionManagerTests.cs | Adds coverage for profile directory resolution (case-insensitive + Local State display-name mapping + ambiguity). |
| tests/Aspire.Hosting.Tests/BrowserLogsBuilderExtensionsTests.cs | Adds coverage for default Shared mode, config parsing, explicit override precedence, and Isolated + profile rejection. |
| src/Aspire.Hosting/BrowserLogs/BrowserLogsRunningSession.cs | Implements user-data-mode-aware user data dir selection and profile directory resolution; passes resolved directory name to --profile-directory. |
| src/Aspire.Hosting/BrowserLogs/BrowserLogsResource.cs | Adds the public BrowserUserDataMode enum and stores resolved/override mode on the resource. |
| src/Aspire.Hosting/BrowserLogs/BrowserLogsBuilderExtensions.cs | Extends public API + configuration parsing/validation and surfaces “User data mode” as a resource property. |
Copilot's findings
- Files reviewed: 5/5 changed files
- Comments generated: 1
Adds parsing for CDP events used to detect when a tracked target ends: - Target.targetDestroyed - Target.targetCrashed - Target.detachedFromTarget - Inspector.detached Also adds command-name constants (Target.closeTarget, Target.setDiscoverTargets) that the upcoming host abstraction will use to manage tracked targets without calling Browser.close. This is purely additive; nothing wires these events up yet. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Defines the IBrowserHost interface plus BrowserHostIdentity and the BrowserHostOwnership enum. The interface separates 'a browser instance to attach CDP to' from 'a tracked log session' so the same host can back many sessions and so disposal can do the right thing for adopted browsers. Key invariants documented in the file: - Adopted hosts must never close the user's browser - Profile directory participates in identity (different profiles = different process) - Acquire/ReleaseAsync refcount, last release disposes - Completion task surfaces process exit / CDP socket close so sessions can fail fast No callers yet; this commit only adds the contract. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…t routing Code-review feedback from gpt-5.4, gpt-5.3-codex, and claude-opus-4.7 on the Phase 2 foundation commits, plus a mechanical pass applying the same lens James used on the original feature PR (#16310). BrowserHostIdentity changes: - Removed ProfileDirectory from the identity. Chromium's singleton is keyed by user-data-dir, not by profile, so different profiles under the same user data root share a browser process. Profile selection is per-target, not per-host. The previous doc comment claiming otherwise was wrong. - Replaced record-struct synthesized equality (which is ordinal case-sensitive on strings) with explicit Equals/GetHashCode using StringComparer.OrdinalIgnoreCase on Windows / Ordinal elsewhere. Without this, paths that differ only in casing would create duplicate hosts on Windows. - Constructor now normalizes paths via Path.GetFullPath + TrimEndingDirectorySeparator so 'C:/foo' and 'C:\foo\' collapse to the same identity. - GetHashCode is null-safe against default(BrowserHostIdentity) since StringComparer.GetHashCode(null) throws. Protocol changes: - Documented above the new Target.* event records that the SUBJECT of these events lives in params (targetId / params.sessionId), not the envelope sessionId. The existing dispatch in BrowserLogsRunningSession filters on envelope sessionId only, so any future fanout layer that naively reuses that filter would silently drop these events. The comment makes the routing requirement unmissable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Apply the reliability fixes identified from the architecture review before continuing the larger adoption refactor: - Bound ChromeDevToolsConnection.DisposeAsync websocket close with a 3 second timeout so shutdown cannot hang indefinitely on a wedged browser. - Add Target.closeTarget and Target.setDiscoverTargets helpers and enable target discovery after each browser-CDP connection, including reconnects. - Add explicit Target lifecycle correlation helpers so future fanout routes targetDestroyed/targetCrashed by targetId and detachedFromTarget by params.sessionId instead of the usually-null envelope sessionId. - Make shared-mode default browser selection prefer Edge. Isolated mode keeps Chrome-first behavior. This avoids Chrome's default-profile remote-debugging guardrail for the default Shared experience. - Fail fast when Shared mode explicitly targets Google Chrome's default user data directory, with guidance to use Edge or Isolated mode. - Reshape the browser host contract around leases and target-session creation rather than public Acquire/Release refcount methods. This keeps exactly-once release and target ownership in the abstraction that will support adopted browsers. Adds targeted tests for the mode-aware default browser choice, browser host identity normalization/default safety, lease idempotency, and the Chrome shared-profile guard helper. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Refactor tracked browser logs around shared browser hosts and per-target sessions so shared user-data mode can reuse an existing debug-enabled Chromium instance without ever closing the user's browser. Add endpoint metadata discovery, owned/adopted host ownership, target lifecycle monitoring, and stale metadata handling. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add architecture comments for the shared/adopted browser pieces and target discovery. Surface browser host ownership and last-error diagnostics in resource state, preserve failed-session diagnostics, and log host/target/reconnect failures through resource logs. Harden endpoint metadata validation for malformed adoption hints. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Refresh ATS code-generation snapshots after adding BrowserUserDataMode to WithBrowserLogs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
Re-running the failed jobs in the CI workflow for this pull request because 1 job was identified as retry-safe transient failures in the CI run attempt.
|
Rename the per-tab browser logs session abstraction around pages, document the host/context/page model, and detect Chromium's Windows lockfile for non-debuggable shared profiles. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Track active registry lock users so BrowserHostRegistry can dispose its SemaphoreSlim without breaking late lease releases after registry disposal. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
🎬 CLI E2E Test Recordings — 75 recordings uploaded (commit View all recordings
📹 Recordings uploaded automatically from CI run #24943130688 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Follow-up to #16310 (tracked browser logs). This PR separates "which Chromium user data directory to use" from "which profile inside that directory to use", then finishes the shared-browser path so browser logs can run against the user's real browser state without racing Chromium's singleton process.
What's new
BrowserUserDataMode:Shared(default): use the browser's real user data directory so sessions can use real cookies, sign-in state, extensions, and profiles.Isolated: use a temporary user data directory for a clean, throwaway browser state.WithBrowserLogs(..., userDataMode: ...)and configuration viaAspire:Hosting:BrowserLogs[:{ResourceName}]:UserDataMode.BrowserConfiguration, with explicit builder overrides grouped inBrowserConfigurationOverrides, so initial resource properties and command-time launches use the same precedence.Shared+ Google Chrome default user data directory.BrowserUserDataModeand the newwithBrowserLogsargument.Shared-browser architecture
BrowserHostRegistry, keyed by normalized(browser executable, user data root), with exactly-onceBrowserHostLeaserelease.--remote-debugging-address=127.0.0.1and--remote-debugging-port=0, write endpoint metadata, and clean up the spawned browser when the last lease is released.aspire-debug-endpoint.jsondiscovery/validation. Metadata is treated as a hint, not truth: schema, executable path, user data root, PID liveness, profile compatibility, and/json/versionare validated before adoption; stale metadata is deleted.Target.closeTargetonly.Browser.closepath so adopted/shared browser sessions cannot accidentally close the user's browser.Target.targetDestroyed,Target.targetCrashed,Target.detachedFromTarget, andInspector.detached, using the event payload (params.targetId/params.sessionId) for correlation.Last errorclearing, and adopted-browser resource diagnostics.Behavior
UserDataModeprofileSharedShared--profile-directoryIsolatedIsolatedReliability notes
DevToolsActivePort.DevToolsActivePortand Aspire endpoint metadata are not reused blindly.Last errorproperty and unhealthy health report so the dashboard keeps the diagnostic visible even when another tracked browser session is still running.Validation
dotnet build src/Aspire.Hosting/Aspire.Hosting.csproj /p:TreatWarningsAsErrors=falsedotnet test tests/Aspire.Hosting.Tests/Aspire.Hosting.Tests.csproj -- --filter-class "*.BrowserLogsProtocolTests" --filter-class "*.BrowserLogsSessionManagerTests" --filter-class "*.BrowserLogsBuilderExtensionsTests" --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"— 57 passeddotnet test tests/Aspire.Hosting.Tests/Aspire.Hosting.Tests.csproj -- --filter-class "*.BrowserLogsBuilderExtensionsTests" --filter-class "*.BrowserLogsSessionManagerTests" --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"— passed after theBrowserConfigurationcleanupdotnet testforTwoPassScanning_GeneratesWithEnvironmentOnTestRedisBuilderin TypeScript, Go, Python, Java, and Rust code-generation test projects — 5 passedgit diff --checkplayground/BrowserTelemetry:web-browser-logsdefaults toSharedmode and Edge on macOS.Aspire__Hosting__BrowserLogs__UserDataMode=Isolated.open-tracked-browsersucceeded,web-browser-logsbecame healthy, and browser network/console logs flowed into the resource.Fixes # (issue)
Checklist
<remarks />and<code />elements on your triple slash comments?aspire.devissue: