Skip to content

Add tracked browser logs and network capture#16310

Merged
davidfowl merged 12 commits intomainfrom
spike/browser-console-logs-poc
Apr 22, 2026
Merged

Add tracked browser logs and network capture#16310
davidfowl merged 12 commits intomainfrom
spike/browser-console-logs-poc

Conversation

@davidfowl
Copy link
Copy Markdown
Contributor

@davidfowl davidfowl commented Apr 19, 2026

Description

Adds WithBrowserLogs(...) to Aspire.Hosting so endpoint-capable resources can attach an internal *-browser-logs child resource. The child resource exposes open-tracked-browser, launches a Chromium-based browser in tracked mode, and streams browser activity back into Aspire’s resource log stream.

This PR includes:

  • the internal BrowserLogs runtime split across builder, resource, session manager, running session, websocket transport, protocol, and event-logging components;
  • command-driven child resource creation with .ExcludeFromManifest() and NameValidationPolicyAnnotation.None, so the child stays internal to the AppHost model;
  • browser/profile configuration from IConfiguration, including global and resource-scoped overrides plus launch-time re-resolution when open-tracked-browser runs;
  • install-based browser fallback that prefers an installed Chrome and then an installed Edge instead of using an OS-based default;
  • known-browser profile reuse without deleting real browser user data, including Linux Chromium profile-directory detection when the resolved executable is Chromium;
  • per-resource multi-session tracking with stable session IDs, aggregated health, and serialized Browser sessions metadata;
  • browser console, error, exception, and network request lifecycle capture from Chromium CDP;
  • reconnect and cleanup hardening, including stale DevToolsActivePort handling, post-dispose launch rejection, and failed-websocket-connect cleanup;
  • live session metadata with both browser-level and page-level CDP endpoints plus target IDs for downstream tools;
  • AspireExport support so withBrowserLogs(...) is emitted in the polyglot AppHost generators while the child resource remains out of manifest publishing.

Process management and dataflow

flowchart LR
    subgraph AppHost["Aspire AppHost"]
        Parent["Parent endpoint resource"]
        Child["BrowserLogs child resource"]
        Command["open-tracked-browser"]
        SessionMgr["Session manager"]
    end

    subgraph Session["One tracked browser session"]
        Create["Create session-000N"]
        Launch["Launch browser process"]
        Discover["Wait for DevToolsActivePort"]
        Attach["Attach target + navigate URL"]
        Running["Running session"]
        Cleanup["Stop / cleanup session"]
    end

    Dashboard["Dashboard / aspire logs / aspire describe"]
    Clients["playwright-cli / CDP clients"]

    Parent --> Child
    Child --> Command --> SessionMgr
    SessionMgr --> Create --> Launch --> Discover --> Attach --> Running
    Running -->|logs + health + active session state| SessionMgr
    SessionMgr --> Dashboard
    SessionMgr -->|Browser sessions property<br/>cdpEndpoint / targetUrl| Clients
    SessionMgr --> Cleanup
    Cleanup -->|close browser + release temp profile| Launch
Loading

How Aspire manages browser instances

  1. WithBrowserLogs(...) adds one internal *-browser-logs child resource beneath an endpoint-capable parent resource.
  2. The child resource is command-driven. Aspire does not auto-start a browser; each open-tracked-browser invocation asks BrowserLogsSessionManager to create a new tracked session.
  3. BrowserLogsSessionManager owns per-resource concurrency, assigns session-0001, session-0002, etc., aggregates active sessions, and publishes health/properties for the child resource.
  4. Each BrowserLogsRunningSession owns exactly one launched browser process plus its attached CDP target. It resolves browser/profile settings at launch time, launches the browser, reads DevToolsActivePort, reuses or creates the page target, enables the required CDP domains, and navigates to the parent resource URL.
  5. ChromeDevToolsConnection transports raw websocket frames, BrowserLogsProtocol parses and serializes CDP messages, and BrowserEventLogger turns runtime/log/network events into resource log lines.
  6. Session metadata is surfaced through the Browser sessions property as serialized JSON so CLI and dashboard consumers can read browser, browserExecutable, profile, processId, targetUrl, cdpEndpoint, pageCdpEndpoint, and targetId.
  7. On shutdown/dispose, Aspire stops active sessions, waits for completion observers, closes browser/CDP state, removes temporary profiles, and preserves configured real browser profiles without deleting user data.

Validation

Validation included:

  • targeted Aspire.Hosting.Tests coverage for resource wiring, config refresh, session behavior, protocol parsing, target selection, diagnostics logging, and websocket cleanup on failed connect attempts;
  • ATS/codegen coverage plus refreshed TypeScript/Python/Go/Java/Rust snapshots for the exported withBrowserLogs(...) capability;
  • live playground/BrowserTelemetry runs confirming tracked browser launches plus real [network.*], console, and error lines in web-browser-logs;
  • live aspire describe web-browser-logs --format json validation confirming Browser sessions includes cdpEndpoint, pageCdpEndpoint, targetId, and targetUrl;
  • live playwright-cli validation against the BrowserTelemetry sample using the surfaced CDP metadata to attach to the tracked browser and confirm the page title/log flow;
  • reran the focused browser-logs test slice and rebuilt playground/BrowserTelemetry/BrowserTelemetry.AppHost after the latest feedback fixes, including the websocket-connect cleanup fix.

Checklist

  • Is this feature complete?
    • Yes. Ready to ship.
    • No. Follow-up changes expected.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
    • No
  • Did you add public API?
    • Yes
      • If yes, did you have an API Review for it?
        • Yes
        • No
      • Did you add `` and <code/> elements on your triple slash comments?
        • Yes
        • No
    • No
  • Does the change make any security assumptions or guarantees?
    • Yes
      • If yes, have you done a threat model and had a security review?
        • Yes
        • No
    • No
  • Does the change require an update in our Aspire docs?

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 19, 2026

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 16310

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 16310"

Add a child browser logs resource that can launch tracked Chromium sessions, stream console and network request events, and expose the builder through polyglot export.\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@davidfowl davidfowl force-pushed the spike/browser-console-logs-poc branch from 5c31105 to eab2c02 Compare April 19, 2026 17:36
@davidfowl davidfowl changed the title Add tracked browser logs and network capture WIP: Add tracked browser logs and network capture Apr 19, 2026
Add typed CDP protocol parsing, preserve connection failure reasons in resource logs, and strengthen reconnect handling for tracked browser sessions.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Member

@mitchdenny mitchdenny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

5 issues found (1 security, 2 bugs, 1 race condition, 1 formatting).

Comment thread src/Aspire.Hosting/BrowserLogsSessionManager.cs Outdated
Comment thread src/Aspire.Hosting/BrowserLogsSessionManager.cs Outdated
Comment thread src/Aspire.Hosting/BrowserLogsSessionManager.cs Outdated
Comment thread src/Aspire.Hosting/BrowserLogs/BrowserLogsSessionManager.cs
Comment thread src/Aspire.Hosting/BrowserLogsBuilderExtensions.cs Outdated
mitchdenny and others added 6 commits April 20, 2026 09:24
Fix indentation of <remarks> block in BrowserLogsBuilderExtensions.cs to
match surrounding XML doc tags.

Thread TimeProvider through RunningSession and its factory so timeout
calculations in WaitForBrowserEndpointAsync and TryReconnectAsync use the
injected provider instead of TimeProvider.System/DateTime.UtcNow.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Extract the running-session lifecycle, CDP websocket transport, and event/diagnostic logging into focused files so the browser connection layers can be tested in isolation. Also add targeted comments describing the browser and CDP quirks these layers handle.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove the global certificate-bypass flag from tracked browser launches and make session-manager disposal wait for completion observers without publishing shutdown updates through torn-down services.

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>
@davidfowl davidfowl changed the title WIP: Add tracked browser logs and network capture Add tracked browser logs and network capture Apr 20, 2026
@davidfowl davidfowl marked this pull request as ready for review April 20, 2026 07:14
Copilot AI review requested due to automatic review settings April 20, 2026 07:14
Comment thread src/Aspire.Hosting/BrowserLogs/BrowserLogsBuilderExtensions.cs
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new Aspire.Hosting capability (WithBrowserLogs(...)) that attaches a *-browser-logs child resource to endpoint-capable resources. The child resource can launch and track Chromium-based browser sessions (CDP), streaming console/errors/exceptions and network lifecycle activity into the dashboard logs, and surfaces live “Browser sessions” metadata/properties for downstream tooling.

Changes:

  • Introduces browser log resources, session orchestration, CDP websocket transport, protocol parsing, and event-to-log projection in src/Aspire.Hosting.
  • Adds unit tests for resource wiring/config refresh, diagnostics logging, protocol parsing, target selection, session lifecycle, and network log formatting.
  • Updates polyglot AppHost/codegen snapshots + capability scanning, and adds localized command strings + a playground demo to validate the end-to-end experience.
Show a summary per file
File Description
tests/Aspire.Hosting.Tests/BrowserLogsSessionManagerTests.cs Adds tests for endpoint parsing, user-data-dir resolution, target selection, and diagnostics log formatting.
tests/Aspire.Hosting.Tests/BrowserLogsProtocolTests.cs Adds focused protocol parsing tests for events and command error reporting.
tests/Aspire.Hosting.Tests/BrowserLogsBuilderExtensionsTests.cs Adds extensive tests for resource wiring, config precedence/refresh, session tracking, and network log projection.
tests/Aspire.Hosting.CodeGeneration.TypeScript.Tests/Snapshots/TwoPassScanningGeneratedAspire.verified.ts Updates generated TS SDK snapshot to include withBrowserLogs(...) capability and options.
tests/Aspire.Hosting.CodeGeneration.TypeScript.Tests/Snapshots/HostingContainerResourceCapabilities.verified.txt Updates capability list snapshot to include Aspire.Hosting/withBrowserLogs.
tests/Aspire.Hosting.CodeGeneration.TypeScript.Tests/AtsTypeScriptCodeGeneratorTests.cs Adds ATS scanner test validating the new capability signature.
tests/Aspire.Hosting.CodeGeneration.Rust.Tests/Snapshots/TwoPassScanningGeneratedAspire.verified.rs Updates generated Rust SDK snapshot to include with_browser_logs(...).
tests/Aspire.Hosting.CodeGeneration.Python.Tests/Snapshots/TwoPassScanningGeneratedAspire.verified.py Updates generated Python SDK snapshot to include with_browser_logs(...) and kwargs support.
tests/Aspire.Hosting.CodeGeneration.Java.Tests/Snapshots/TwoPassScanningGeneratedAspire.verified.java Updates generated Java SDK snapshot to include withBrowserLogs(...) and options DTO.
tests/Aspire.Hosting.CodeGeneration.Go.Tests/Snapshots/TwoPassScanningGeneratedAspire.verified.go Updates generated Go SDK snapshot to include WithBrowserLogs(...).
src/Aspire.Hosting/Resources/xlf/CommandStrings.zh-Hant.xlf Adds localization entries for the “Open tracked browser” command.
src/Aspire.Hosting/Resources/xlf/CommandStrings.zh-Hans.xlf Adds localization entries for the “Open tracked browser” command.
src/Aspire.Hosting/Resources/xlf/CommandStrings.tr.xlf Adds localization entries for the “Open tracked browser” command.
src/Aspire.Hosting/Resources/xlf/CommandStrings.ru.xlf Adds localization entries for the “Open tracked browser” command.
src/Aspire.Hosting/Resources/xlf/CommandStrings.pt-BR.xlf Adds localization entries for the “Open tracked browser” command.
src/Aspire.Hosting/Resources/xlf/CommandStrings.pl.xlf Adds localization entries for the “Open tracked browser” command.
src/Aspire.Hosting/Resources/xlf/CommandStrings.ko.xlf Adds localization entries for the “Open tracked browser” command.
src/Aspire.Hosting/Resources/xlf/CommandStrings.ja.xlf Adds localization entries for the “Open tracked browser” command.
src/Aspire.Hosting/Resources/xlf/CommandStrings.it.xlf Adds localization entries for the “Open tracked browser” command.
src/Aspire.Hosting/Resources/xlf/CommandStrings.fr.xlf Adds localization entries for the “Open tracked browser” command.
src/Aspire.Hosting/Resources/xlf/CommandStrings.es.xlf Adds localization entries for the “Open tracked browser” command.
src/Aspire.Hosting/Resources/xlf/CommandStrings.de.xlf Adds localization entries for the “Open tracked browser” command.
src/Aspire.Hosting/Resources/xlf/CommandStrings.cs.xlf Adds localization entries for the “Open tracked browser” command.
src/Aspire.Hosting/Resources/CommandStrings.resx Adds the new command name/description resources.
src/Aspire.Hosting/Resources/CommandStrings.Designer.cs Regenerates designer accessors for new resource strings.
src/Aspire.Hosting/IBrowserLogsSessionManager.cs Introduces internal session manager abstraction used by the command handler.
src/Aspire.Hosting/BrowserLogsSessionManager.cs Implements session orchestration + dashboard property/health projection for tracked sessions.
src/Aspire.Hosting/BrowserLogsRunningSession.cs Implements per-browser lifecycle: launch, endpoint discovery, CDP connect/attach, reconnect, cleanup.
src/Aspire.Hosting/BrowserLogsResource.cs Adds internal child resource model + config re-resolution for launch-time refresh.
src/Aspire.Hosting/BrowserLogsProtocol.cs Adds CDP frame parsing/serialization + typed envelopes for key CDP domains.
src/Aspire.Hosting/BrowserLogsEventLogger.cs Projects CDP events into consistent dashboard log lines + connection diagnostics.
src/Aspire.Hosting/BrowserLogsDevToolsConnection.cs Adds CDP websocket transport, command/response correlation, and receive loop.
src/Aspire.Hosting/BrowserLogsBuilderExtensions.cs Adds the public WithBrowserLogs(...) API and the dashboard command wiring/config resolution.
playground/BrowserTelemetry/BrowserTelemetry.Web/Scripts/index.js Expands sample UI logic to emit console, exception, and network activity for validation.
playground/BrowserTelemetry/BrowserTelemetry.Web/Pages/Index.cshtml Updates sample page UI with buttons/status area for browser logging demo.
playground/BrowserTelemetry/BrowserTelemetry.AppHost/BrowserTelemetry.AppHost.csproj Adds UserSecretsId to support configuration overrides in the playground.
playground/BrowserTelemetry/BrowserTelemetry.AppHost/AppHost.cs Enables browser logs in the playground by calling .WithBrowserLogs().

Copilot's findings

Files not reviewed (1)
  • src/Aspire.Hosting/Resources/CommandStrings.Designer.cs: Language not supported
  • Files reviewed: 36/37 changed files
  • Comments generated: 3

Comment thread src/Aspire.Hosting/BrowserLogs/BrowserLogsRunningSession.cs
Comment thread tests/Aspire.Hosting.Tests/BrowserLogsBuilderExtensionsTests.cs Outdated
Comment thread tests/Aspire.Hosting.Tests/BrowserLogsSessionManagerTests.cs Outdated
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment thread src/Aspire.Hosting/BrowserLogs/BrowserLogsBuilderExtensions.cs
Comment thread src/Aspire.Hosting/BrowserLogsBuilderExtensions.cs Outdated
Comment thread src/Aspire.Hosting/BrowserLogsBuilderExtensions.cs Outdated
Comment thread src/Aspire.Hosting/BrowserLogs/BrowserLogsBuilderExtensions.cs
Comment thread src/Aspire.Hosting/BrowserLogsDevToolsConnection.cs Outdated
Comment thread src/Aspire.Hosting/BrowserLogs/BrowserLogsDevToolsConnection.cs
Comment thread src/Aspire.Hosting/BrowserLogsEventLogger.cs Outdated
Comment thread src/Aspire.Hosting/BrowserLogsEventLogger.cs Outdated
Copy link
Copy Markdown
Member

@JamesNK JamesNK left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the new tracked browser logs feature. Found 4 issues:

  • 2 bugs: missing early-exit detection during endpoint polling + stale DevToolsActivePort file with persistent profiles
  • 1 resource leak: no disposal guard in StartSessionAsync allows post-dispose browser process leaks
  • 1 hang risk: unbounded CloseAsync in ChromeDevToolsConnection.DisposeAsync can block shutdown indefinitely

Comment thread src/Aspire.Hosting/BrowserLogs/BrowserLogsRunningSession.cs
Comment thread src/Aspire.Hosting/BrowserLogs/BrowserLogsSessionManager.cs
Comment thread src/Aspire.Hosting/BrowserLogs/BrowserLogsDevToolsConnection.cs
Comment thread src/Aspire.Hosting/BrowserLogs/BrowserLogsRunningSession.cs
Comment thread src/Aspire.Hosting/BrowserLogs/BrowserLogsProtocol.cs
Comment thread src/Aspire.Hosting/BrowserLogs/BrowserLogsBuilderExtensions.cs
Comment thread src/Aspire.Hosting/BrowserLogs/BrowserLogsSessionManager.cs
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Member

@JamesNK JamesNK left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thorough review of the new browser logs feature. Very well-structured code with clean separation of concerns across the CDP transport, protocol parsing, event logging, session lifecycle, and resource state management. One resource leak issue flagged (1 bug).

Comment thread src/Aspire.Hosting/BrowserLogs/BrowserLogsDevToolsConnection.cs
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown
Contributor

🎬 CLI E2E Test Recordings — 72 recordings uploaded (commit e28edf9)

View recordings
Test Recording
AddPackageInteractiveWhileAppHostRunningDetached ▶️ View Recording
AddPackageWhileAppHostRunningDetached ▶️ View Recording
AgentCommands_AllHelpOutputs_AreCorrect ▶️ View Recording
AgentInitCommand_DefaultSelection_InstallsSkillOnly ▶️ View Recording
AgentInitCommand_MigratesDeprecatedConfig ▶️ View Recording
AspireAddPackageVersionToDirectoryPackagesProps ▶️ View Recording
AspireUpdateRemovesAppHostPackageVersionFromDirectoryPackagesProps ▶️ View Recording
Banner_DisplayedOnFirstRun ▶️ View Recording
Banner_DisplayedWithExplicitFlag ▶️ View Recording
Banner_NotDisplayedWithNoLogoFlag ▶️ View Recording
CertificatesClean_RemovesCertificates ▶️ View Recording
CertificatesTrust_WithNoCert_CreatesAndTrustsCertificate ▶️ View Recording
CertificatesTrust_WithUntrustedCert_TrustsCertificate ▶️ View Recording
ConfigSetGet_CreatesNestedJsonFormat ▶️ View Recording
CreateAndRunAspireStarterProject ▶️ View Recording
CreateAndRunAspireStarterProjectWithBundle ▶️ View Recording
CreateAndRunEmptyAppHostProject ▶️ View Recording
CreateAndRunJavaEmptyAppHostProject ▶️ View Recording
CreateAndRunJsReactProject ▶️ View Recording
CreateAndRunPythonReactProject ▶️ View Recording
CreateAndRunTypeScriptEmptyAppHostProject ▶️ View Recording
CreateAndRunTypeScriptStarterProject ▶️ View Recording
CreateJavaAppHostWithViteApp ▶️ View Recording
CreateTypeScriptAppHostWithViteApp ▶️ View Recording
DashboardRunWithOtelTracesReturnsNoTraces ▶️ View Recording
DeployK8sBasicApiService ▶️ View Recording
DeployK8sWithGarnet ▶️ View Recording
DeployK8sWithMongoDB ▶️ View Recording
DeployK8sWithMySql ▶️ View Recording
DeployK8sWithPostgres ▶️ View Recording
DeployK8sWithRabbitMQ ▶️ View Recording
DeployK8sWithRedis ▶️ View Recording
DeployK8sWithSqlServer ▶️ View Recording
DeployK8sWithValkey ▶️ View Recording
DeployTypeScriptAppToKubernetes ▶️ View Recording
DescribeCommandResolvesReplicaNames ▶️ View Recording
DescribeCommandShowsRunningResources ▶️ View Recording
DetachFormatJsonProducesValidJson ▶️ View Recording
DetachFormatJsonProducesValidJsonWhenRestartingExistingInstance ▶️ View Recording
DoListStepsShowsPipelineSteps ▶️ View Recording
DoctorCommand_DetectsDeprecatedAgentConfig ▶️ View Recording
DoctorCommand_WithSslCertDir_ShowsTrusted ▶️ View Recording
DoctorCommand_WithoutSslCertDir_ShowsPartiallyTrusted ▶️ View Recording
GlobalMigration_HandlesCommentsAndTrailingCommas ▶️ View Recording
GlobalMigration_HandlesMalformedLegacyJson ▶️ View Recording
GlobalMigration_PreservesAllValueTypes ▶️ View Recording
GlobalMigration_SkipsWhenNewConfigExists ▶️ View Recording
GlobalSettings_MigratedFromLegacyFormat ▶️ View Recording
InitTypeScriptAppHost_AugmentsExistingViteRepoAtRoot ▶️ View Recording
InvalidAppHostPathWithComments_IsHealedOnRun ▶️ View Recording
LegacySettingsMigration_AdjustsRelativeAppHostPath ▶️ View Recording
LogsCommandShowsResourceLogs ▶️ View Recording
OtelLogsReturnsStructuredLogsFromStarterApp ▶️ View Recording
PsCommandListsRunningAppHost ▶️ View Recording
PsFormatJsonOutputsOnlyJsonToStdout ▶️ View Recording
PublishWithConfigureEnvFileUpdatesEnvOutput ▶️ View Recording
PublishWithDockerComposeServiceCallbackSucceeds ▶️ View Recording
PublishWithoutOutputPathUsesAppHostDirectoryDefault ▶️ View Recording
RestoreGeneratesSdkFiles ▶️ View Recording
RestoreRefreshesGeneratedSdkAfterAddingIntegration ▶️ View Recording
RestoreSupportsConfigOnlyHelperPackageAndCrossPackageTypes ▶️ View Recording
RunFromParentDirectory_UsesExistingConfigNearAppHost ▶️ View Recording
SecretCrudOnDotNetAppHost ▶️ View Recording
SecretCrudOnTypeScriptAppHost ▶️ View Recording
StagingChannel_ConfigureAndVerifySettings_ThenSwitchChannels ▶️ View Recording
StartAndWaitForTypeScriptSqlServerAppHostWithNativeAssets ▶️ View Recording
StopAllAppHostsFromAppHostDirectory ▶️ View Recording
StopAllAppHostsFromUnrelatedDirectory ▶️ View Recording
StopNonInteractiveMultipleAppHostsShowsError ▶️ View Recording
StopNonInteractiveSingleAppHost ▶️ View Recording
StopWithNoRunningAppHostExitsSuccessfully ▶️ View Recording
UnAwaitedChainsCompileWithAutoResolvePromises ▶️ View Recording

📹 Recordings uploaded automatically from CI run #24783280098

@davidfowl davidfowl merged commit 4ee4faf into main Apr 22, 2026
561 of 565 checks passed
@github-actions github-actions Bot added this to the 13.3 milestone Apr 22, 2026
davidfowl added a commit that referenced this pull request Apr 25, 2026
…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants