Decouple Aspire.Hosting.Dotnet from Aspire.Hosting internals - #18918
Conversation
github-actions
Bot
added
the
area-app-model
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 18918Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 18918" |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Decouples .NET, Go, Python, and JavaScript hosting packages from Aspire.Hosting internals.
Changes:
- Replaces production
InternalsVisibleTodependencies with public experimental debugging APIs and shared utilities. - Replaces the internal project-defaults interface with annotation-driven state.
- Updates language integrations, Kubernetes handling, and tests.
Reviewed changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
tests/Aspire.Hosting.Maui.Tests/MauiPlatformExtensionsTests.cs |
Uses public debugging APIs in tests. |
tests/Aspire.Hosting.Dotnet.Tests/DotnetProjectResourceTests.cs |
Tests annotation-based project behavior. |
src/Shared/ProjectPathResolver.cs |
Shares project-path resolution. |
src/Aspire.Hosting/Utils/ExtensionUtils.cs |
Removes the internal debugging helper. |
src/Aspire.Hosting/SupportsDebuggingAnnotation.cs |
Makes debug metadata publicly inspectable. |
src/Aspire.Hosting/ProjectResourceBuilderExtensions.cs |
Publicizes annotation-driven project defaults. |
src/Aspire.Hosting/IProjectMetadata.cs |
Uses the shared path resolver. |
src/Aspire.Hosting/Dcp/Model/ExecutableLaunchConfiguration.cs |
Removes internal launch DTOs. |
src/Aspire.Hosting/Dcp/Model/Executable.cs |
Uses public launch configuration types. |
src/Aspire.Hosting/Dcp/ExecutableCreator.cs |
Uses known launch-type constants. |
src/Aspire.Hosting/Aspire.Hosting.csproj |
Removes language-package IVTs. |
src/Aspire.Hosting/ApplicationModel/ProjectResource.cs |
Replaces the internal interface with an annotation. |
src/Aspire.Hosting/ApplicationModel/ProjectRebuilderResource.cs |
Generalizes the parent contract. |
src/Aspire.Hosting/ApplicationModel/ProjectLaunchDefaultsAnnotation.cs |
Stores project-default endpoint state. |
src/Aspire.Hosting/ApplicationModel/ExecutableLaunchConfiguration.cs |
Adds public experimental launch DTOs/constants. |
src/Aspire.Hosting/ApplicationModel/DebugSupportExtensions.cs |
Adds public debug-support inspection. |
src/Aspire.Hosting/ApplicationModel/CommandsConfigurationExtensions.cs |
Detects project defaults through annotations. |
src/Aspire.Hosting.RemoteHost/AtsCapabilityScanner.cs |
Generalizes an internal-interface comment. |
src/Aspire.Hosting.Python/PythonAppLaunchConfiguration.cs |
Uses public launch DTOs. |
src/Aspire.Hosting.Python/Aspire.Hosting.Python.csproj |
Source-shares the overload-priority attribute. |
src/Aspire.Hosting.Kubernetes/KubernetesResource.cs |
Reads default HTTPS state from the annotation. |
src/Aspire.Hosting.JavaScript/JavaScriptLaunchConfiguration.cs |
Uses public launch DTOs. |
src/Aspire.Hosting.JavaScript/BrowserLaunchConfiguration.cs |
Uses public launch DTOs. |
src/Aspire.Hosting.JavaScript/Aspire.Hosting.JavaScript.csproj |
Source-shares path normalization. |
src/Aspire.Hosting.Go/GoLaunchConfiguration.cs |
Uses public launch DTOs. |
src/Aspire.Hosting.Dotnet/DotnetProjectResource.cs |
Removes the internal project-default interface. |
src/Aspire.Hosting.Dotnet/DotnetProjectMetadata.cs |
Adds package-owned project metadata. |
src/Aspire.Hosting.Dotnet/DotnetProjectHostingExtensions.cs |
Uses public project/debug seams. |
src/Aspire.Hosting.Dotnet/Aspire.Hosting.Dotnet.csproj |
Source-shares required helpers and resources. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
PR Testing ReportPR Information
Artifact Version Verification
Changes AnalyzedFiles Changed
Change Categories
Test Scenarios ExecutedScenario 1: Source build and focused regression testsObjective: Prove that the affected packages compile without the removed language-package Coverage Type: Build and focused regression Status: PASS Steps:
Results:
Evidence:
Scenario 2: Fresh starter project lifecycleObjective: Validate normal .NET project orchestration through the PR artifact, including project metadata, endpoint wiring, hidden rebuild resources, and lifecycle commands. Coverage Type: Happy path end-to-end Status: PASS Steps:
Observations:
Evidence:
Scenario 3: Third-party language integration using only public APIsObjective: Verify the PR's main extensibility goal by implementing a custom project resource outside Coverage Type: Public API integration Status: PASS Steps:
Observations:
Evidence:
Scenario 4: Debug capability boundary and failure handlingObjective: Validate safe selection and fallback behavior exposed by the new public Coverage Type: Unhappy path and boundary Status: PASS Expected and observed outcomes:
Evidence:
Scenario 5: Invalid and ambiguous .NET project pathsObjective: Verify that the shared project-path resolver preserves safe failure behavior after moving out of Coverage Type: Unhappy path Status: PASS Cases:
Evidence:
Summary
Overall ResultPR VERIFIED The PR artifact matches the latest head commit, the repository builds, all 158 focused tests pass, normal project lifecycle behavior remains intact, and a third-party integration can use the new public APIs without access to |
Karol Zadora-Przylecki (karolz-ms)
left a comment
Karol Zadora-Przylecki (karolz-ms)
left a comment
There was a problem hiding this comment.
Code review finding — see the inline comment.
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 32 out of 32 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
src/Aspire.Hosting/ProjectResourceBuilderExtensions.cs:479
WithProjectDefaultsis now a public entry point whose documentedIProjectMetadataprecondition is not validated. A third-party resource without exactly one metadata annotation reachesGetConfiguration()below and fails withSequence contains no elements(ormore than one element), which gives no actionable indication of the contract violation. Validate the annotation count here before registering any callbacks or adding derived resources.
ArgumentNullException.ThrowIfNull(builder);
ArgumentNullException.ThrowIfNull(options);
src/Aspire.Hosting/Dcp/ExecutableCreator.cs:821
- This removes the previous
TryGetProjectLaunchConfigurationvalidation/defaulting and forwards the producer payload unchanged. BecauseWithDebugSupportis unconstrained, a caller can pair theprojectidentifier with an arbitrary DTO or aProjectLaunchConfigurationmissingproject_path; Aspire will still select IDE execution and suppress the process fallback, leaving the resource with a malformed launch request instead of failing clearly. Validate that project producers emit a matching, complete project configuration before attaching it.
supportsDebuggingAnnotation.LaunchConfigurationAnnotator(exe, GetProjectLaunchConfigurationMode());
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 33 out of 33 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
src/Aspire.Hosting/Dcp/ExecutableCreator.cs:821
- This drops the existing validation for the reserved
projectlaunch type.WithDebugSupport<T, TLaunchConfiguration>is unconstrained, so a producer can returnnullor a value that cannot be read asProjectLaunchConfiguration; that invalid payload is now sent to the IDE instead of producing the prior resource-specific error. Keep the producer output verbatim, but retain the compatibility check before returning.
supportsDebuggingAnnotation.LaunchConfigurationAnnotator(exe, GetProjectLaunchConfigurationMode());
src/Aspire.Hosting/ApplicationModel/DebugSupportExtensions.cs:111
- The stored producer can return
null(for example,WithDebugSupport<Foo?>is valid andSupportsDebuggingAnnotation.Createonly applies the null-forgiving operator), but this public API promises a non-nullobject. Returning that value directly violates the nullable contract and makes callers fail later while inspecting/serializing it. Reject a null producer result with a resource-specific exception (or make the API explicitly nullable).
return supportsDebuggingAnnotation.LaunchConfigurationProducer(mode);
src/Aspire.Hosting/ProjectResourceBuilderExtensions.cs:476
- This newly public fluent API has no
<example>showing the required setup order. In particular, callers must attach anIProjectMetadataannotation before invoking it, which is not obvious from a signature constrained only to environment/endpoints/args capabilities. Public extension methods insrc/are expected to include a practical XML-doc example; add one that constructs a custom .NET resource, attaches metadata, and then callsWithProjectDefaults.
public static IResourceBuilder<TProjectResource> WithProjectDefaults<TProjectResource>(this IResourceBuilder<TProjectResource> builder, ProjectResourceOptions options)
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 48 out of 48 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (4)
src/Aspire.Hosting/ApplicationModel/ExecutableLaunchConfiguration.cs:104
- This repeats the incorrect blanket fallback claim:
ProjectResourceusesProjectLaunchConfigurationand still advertises a process fallback when its command line is runnable. Limit this statement to executable-based project resources, or describe fallback as resource-shape dependent.
/// The IDE builds and launches the project itself, so resources using this launch configuration do not
/// get a process fallback. The resource must carry <see cref="IProjectMetadata"/>.
src/Aspire.Hosting/ResourceBuilderExtensions.cs:4766
- This replaces the shipped
WithDebugSupport(Func<string, TLaunchConfiguration>, ...)API (still present inapi/Aspire.Hosting.cs) rather than adding an async overload, so every existing third-party integration using the synchronous producer stops compiling and already-built consumers can fail to bind. Keep the synchronous overload as a compatibility wrapper and add this cancellation-aware overload alongside it.
src/Aspire.Hosting/ApplicationModel/ExecutableLaunchConfiguration.cs:47 - This newly public contract overstates the fallback rule. Core
ProjectResourceinstances using theprojectlaunch type explicitly receiveFallbackExecutionTypes = [Process]inExecutableCreator(and tests assert that behavior); only executable-based project resources such asDotnetProjectResourcesuppress the fallback. Narrow the remark so integration authors do not assume the type identifier alone always disables process fallback.
This issue also appears on line 103 of the same file.
/// This type is reserved for resources that carry <see cref="IProjectMetadata"/>. Aspire hands the
/// project path (and launch profile) to the IDE, which owns building and launching the project, so
/// no process fallback is offered for resources using this type.
src/Aspire.Hosting.Dotnet/DotnetProjectResource.cs:20
- Removing this marker interface changes the behavior of the public constructor path. A
DotnetProjectResourceadded directly withAddResourcewas previously recognized by core as a .NET project and received the project restart description/lifecycle command treatment; now onlyAddDotnetProject(...).WithProjectDefaults(...)adds the replacement annotation. The updated lifecycle test switches from direct construction toAddDotnetProject, so it no longer catches this compatibility regression. Preserve a marker that the resource can carry without IVT, or explicitly migrate the direct-construction path.
: ExecutableResource(name, "dotnet", workingDirectory), IResourceWithServiceDiscovery
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
…pass] - Remove stale type-level XML <param> tags for name/workingDirectory from DotnetProjectResource (CS1572) that broke Build packages, Hosting.Blazor, Hosting.Dotnet, Playground, and TypeScript API Compatibility CI jobs on PR #18918. - Remove unused 'using Aspire.Hosting.Dcp.Model;' from DotnetProjectHostingExtensions. - Add reflection regression tests asserting the exact ASPIREDOTNETPROJECT001 diagnostic ID and URL format on DotnetProjectResource and every public AddDotnetProject overload (Aspire.Hosting.Dotnet.Tests). - Add reflection regression tests asserting the exact ASPIREPROJECTS001 diagnostic ID and URL format on ProjectLaunchDefaultsAnnotation and WithProjectDefaults (Aspire.Hosting.Tests). Validation: ./restore.sh clean; targeted Aspire.Hosting.Dotnet build 0 errors/warnings; Aspire.Hosting.Dotnet.Tests 30/30 passed; Aspire.Hosting.Tests ProjectResourceBuilderExtensionTests 11/11 passed; full ./build.sh --build /p:SkipNativeBuild=true 0 errors/warnings. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 49 out of 49 changed files in this pull request and generated no new comments.
Suppressed comments (4)
src/Aspire.Hosting/ApplicationModel/ProjectLaunchDefaultsAnnotation.cs:42
- This documentation says repeated calls are idempotent, but
WithProjectDefaultschecksAppliedand throwsInvalidOperationExceptionon the second call. Describe the flag as rejecting repeated application rather than making it idempotent.
/// <remarks>
/// The flag is used to ensure that multiple calls to <see cref="ProjectResourceBuilderExtensions.WithProjectDefaults{TProjectResource}(IResourceBuilder{TProjectResource}, ProjectResourceOptions)"/>
/// are idempotent and don't add duplicate endpoints or environment variables.
/// </remarks>
src/Aspire.Hosting/ApplicationModel/ProjectLaunchDefaultsAnnotation.cs:14
- This public annotation does not itself apply the project defaults: adding it only marks the resource for a few core consumers, while
WithProjectDefaultsperforms the launch-profile, endpoint, environment, and rebuilder wiring. The current remarks tell third-party integrations that adding the annotation opts them into that behavior, which can leave a resource only partially configured. Direct callers should be pointed toWithProjectDefaultsand the marker-only behavior should be stated explicitly.
This issue also appears on line 39 of the same file.
/// <remarks>
/// This is a public annotation rather than an interface so resources in other assemblies can opt into the C# project-defaults behavior
/// without implementing a specific interface.
/// </remarks>
src/Aspire.Hosting/ProjectResourceBuilderExtensions.cs:459
- The documented
InvalidOperationExceptioncondition is incomplete: this method also throws that exception when project defaults were already applied. Include both cases so callers of the newly public API can understand the duplicate-application contract.
src/Aspire.Hosting/ApplicationModel/ExecutableLaunchConfiguration.cs:48 - The no-fallback statement is not true for all resources using this type.
ExecutableCreator.PrepareProjectExecutablesAsyncsetsFallbackExecutionTypes = [Process]for a project launch configuration whenever its debug support does not rewrite arguments (lines 285-297), while plain executable resources follow a different rule. Since this constant is now public integration guidance, document that fallback depends on the resource shape/argument configuration rather than promising none.
/// <remarks>
/// This type is reserved for resources that carry <see cref="IProjectMetadata"/>. Aspire hands the
/// project path (and launch profile) to the IDE, which owns building and launching the project, so
/// no process fallback is offered for resources using this type.
/// </remarks>
Tests selector (audit mode)The full test matrix and all jobs still run in audit mode. The tests and jobs below are what selective CI would run under enforcement. 50 / 100 test projects · 5 jobs, from 52 changed files. Selected test projects (50 / 100)
Selected jobs (5)
How these were chosen — grouped by what changed
🔧 show 27
🔧 🔧 🔧 🔧 🧪 📦 affected project 📦 affected project 🔧 🔧 🔧 🔧 🔧 🔧 🔧 🔧 🔧 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 Job reasons
Selection computed for commit |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 52 out of 52 changed files in this pull request and generated no new comments.
Suppressed comments (3)
src/Aspire.Hosting/ProjectResourceBuilderExtensions.cs:494
ProjectLaunchDefaultsAnnotationis public and appendable/replaceable, but this selects only the last instance to hold the internal state and duplicate-application flag. Appending a fresh annotation after defaults are applied shadowsDefaultHttpsEndpointfor the Radius/Kubernetes publishers; appending one before a second call also bypasses the “already applied” guard and permits partial duplicate wiring. Keep the mutable state in a canonical/internal annotation, or reject/validate duplicate marker annotations across all consumers.
src/Aspire.Hosting/Dcp/ExecutableCreator.cs:203- An asynchronous producer can observe
cancellationTokenand throwOperationCanceledException, but the broad fallback catch below converts that cancellation into a process fallback. Cancellation should stop resource creation rather than be logged as a launch-configuration failure; exclude cancellation exceptions from the fallback filter.
await supportsDebuggingAnnotation.LaunchConfigurationAnnotator(exe, mode, cancellationToken).ConfigureAwait(false);
src/Aspire.Hosting/ApplicationModel/ExecutableLaunchConfiguration.cs:47
- This newly public contract incorrectly states that
projectconfigurations never get a process fallback. A normalAddProjectexplicitly receivesFallbackExecutionTypes = [Process]when its arguments were not rewritten (ExecutableCreator.cs:285-292), andProjectResource_WithoutArgumentRewriting_OffersProcessFallback_InDebugSessionverifies that behavior. Document that fallback depends on the resource shape/argument validity; only executable-backed project resources that cannot reconstruct a runnable command categorically omit it.
/// This type is reserved for resources that carry <see cref="IProjectMetadata"/>. Aspire hands the
/// project path (and launch profile) to the IDE, which owns building and launching the project, so
/// no process fallback is offered for resources using this type.
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
1fad99a
into
main
This comment has been minimized.
This comment has been minimized.
|
Excellent! |
This comment has been minimized.
This comment has been minimized.
|
📝 Documentation has been drafted in microsoft/aspire.dev#1511 targeting Added a new diagnostics page for Note This draft PR needs human review before merging. |
* Add dashboard support to testing builders Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ffeff87e-f284-434d-87d3-843e21a7aebb * Finish testing dashboard support Reuse the canonical dashboard URL path, enforce authenticated testing defaults, and preserve cancellation and disposal semantics. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ffeff87e-f284-434d-87d3-843e21a7aebb * Address review findings for dashboard-in-tests support Lifecycle correctness: - BuildAsync no longer blocks on the released AppHost when the caller's token fires. It reclaims the late application on a background continuation and rethrows immediately, so cancellation is prompt even when the AppHost never finishes building. - DistributedApplicationFactory.OnBuiltCoreAsync disposes an application that arrives after the factory was disposed. Previously TrySetResult simply returned false and the built DistributedApplication, its service provider, and its orchestrator processes leaked for the lifetime of the test process. - DisposeAsync claims disposal with Interlocked.Exchange. The previous IsCancellationRequested read was not atomic with OnDisposed(), so concurrent disposers could both run teardown and race on the same application. - ObjectDisposedException now consistently reports IDistributedApplicationTestingBuilder rather than leaking the internal factory type name. Behavior: - DistributedApplicationOptions.DisableDashboard = false, the pre-existing spelling of "run the dashboard", now receives the same hardened testing defaults as the new EnableDashboard option instead of only one of them being hardened. - Dashboard endpoints are configured with an empty URL, which is how the product asks for a dynamically assigned port. The previous "http://127.0.0.1:0" parses to a literal fixed port 0 and was only dynamic while DcpPublisher:RandomizePorts stayed true. - A fresh browser token is generated per application and supplied through the command line, which has the highest precedence. Disabling anonymous access only closes the door if a credential exists, and an ambient ASPIRE_DASHBOARD_FRONTEND_BROWSERTOKEN would otherwise share one known token across every application on a CI agent. - DistributedApplicationTestingBuilderOptions.DefaultWaitBehavior lets a debugging session keep a stuck resource alive to inspect it, rather than always tearing the application down. Conventions: - Exception messages moved to Properties/Resources.resx with regenerated xlf, matching the rest of the file. - Commented the Aspire.Hosting InternalsVisibleTo grant, including why the shared file links had to be dropped. - Replaced vacuous Assert.NotNull-on-a-lambda assertions with assertions that exercise the overloads, and added coverage for per-application token isolation, DefaultWaitBehavior, publish-mode rejection through configureBuilder, and disposal of a late-arriving application. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: f3f649b5-52c4-4eb5-8d74-c4744e79faf4 * Fix dashboard testing regressions found by CI Restrict the hardened dashboard testing defaults to the explicit DistributedApplicationTestingBuilderOptions.EnableDashboard opt-in. Treating DistributedApplicationOptions.DisableDashboard = false as an equivalent spelling changed behavior for every caller already using it: - The args rebuild read hostBuilderOptions.Args, which the factory has already populated, so arguments a configureBuilder callback assigned to applicationOptions.Args were dropped. DashboardIsNotAddedInPublishMode lost "--publisher manifest", ran in run mode, and saw a dashboard resource it asserted was absent. - The appended dashboard settings overrode the caller's own configuration. GetDashboardUrlsAsync_ReturnsBaseUrl_WhenDashboardAllowsAnonymousAccess asked for anonymous access and got a browser token instead. Also merge caller args instead of choosing one array, so the new opt-in appends to whatever the callback left in place. Build Aspire.Dashboard from Aspire.Hosting.Testing.Tests. The tests that start a real dashboard resolve it from the repo-wide AspireDashboardDir, which was empty in the Hosting.Testing CI leg, failing all eight of them. Add ConcurrentDisposeAsyncRunsTeardownOnce, covering the interlocked disposal claim in DistributedApplicationFactory.DisposeAsync. With the claim reverted to a cancellation-token read the test fails consistently; with the claim in place it passes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Pin the AppHost browser token boundary with a test Review asked for the generated browser token to be restored after the AppHost configures itself, so an AppHost clearing AppHost:BrowserToken could not downgrade the dashboard to Unsecured authentication. Pinning it onto DashboardOptions works, but it removes the only way to reach the anonymous dashboard path once EnableDashboard has appended its own arguments. Two tests in this PR rely on that escape hatch through the returned builder, and both fail with the token pinned: GetDashboardLoginUrlAsyncThrowsWhenDashboardAllowsAnonymousAccess and CanonicalDashboardLoginUrlEscapesBrowserToken. The state is also not silent. DashboardUrlsHelper reports HasBrowserToken false and GetDashboardLoginUrlAsync throws rather than returning an unauthenticated URL. Cover the boundary instead, so neither side of it can drift unnoticed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Do not dispose an application a concurrent build still owns After a post-release cancellation, ReclaimApplicationInBackground awaited the shared application task and then disposed the factory unconditionally. The continuation state stayed ContinuationReleased, so a concurrent or retried BuildAsync kept waiting on that same task. When the application arrived both callers received it and the background continuation tore it down underneath the surviving one. Guard the continuation state and the outstanding build count together, so only the last caller to cancel reclaims the application, and reject later builds once it has been claimed rather than handing back an instance that is being disposed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Restore the browser token an AppHost cleared DistributedApplicationBuilder freezes the generated browser token into AppHost:BrowserToken while the builder is constructed, but DashboardOptions does not read that key until the application starts. AppHost code runs between those two points, so it could blank the key and DashboardEventHandlers would then launch the dashboard with Unsecured frontend authentication, silently downgrading the authenticated default EnableDashboard promises. Keep the generated token in DashboardTestingState and put it back once the AppHost entry point has finished configuring. The guard mirrors the string.IsNullOrEmpty check DashboardEventHandlers itself applies, so a deliberately chosen non-empty token is left alone, and it runs before the caller sees the builder, so the documented escape hatch to the anonymous dashboard is unaffected. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Close a disposal race that could leave the application running DisposeAsync read _appTcs.Task.IsCompletedSuccessfully and only called TrySetCanceled afterwards. OnBuiltCoreAsync disposes the application it built only when its own TrySetResult loses, so a TrySetResult landing between those two statements left neither side responsible: cancellation failed, DisposeAsync returned early, and OnBuiltCoreAsync saw a successful publish and ran OnBuilt. The host, its service provider, and the orchestrator processes it owns stayed alive for the rest of the test process. Claim the completion source first, then decide. TaskCompletionSource guarantees exactly one of TrySetCanceled and TrySetResult wins, so the two paths are now mutually exclusive and exhaustive: if cancellation wins there is nothing to tear down and OnBuiltCoreAsync disposes what it built; if it loses, the task is RanToCompletion and this method runs the full teardown. Also correct two doc claims that overstated what the code guarantees. Dashboard authentication is not fixed at construction - the generated browser token stays adjustable through the returned builder, which is what DashboardTestingLeavesTheBrowserTokenToTheCallersBuilder covers. And dynamic loopback ports prevent binding collisions, not cross-application access; the per-application browser token is what does that. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 487b899d-d79d-4349-a84b-a026861bde7f * Restore resource-service authentication after AppHost configuration The post-construction hardening restored only the browser token. The resource service sits in the same window: DistributedApplicationBuilder freezes AppHost:ResourceService:AuthMode and :ApiKey during construction, but DashboardServiceHost does not bind that section into ResourceServiceOptions until the application starts, so AppHost code running between those points can downgrade either value. Either half alone is enough to unauthenticate the endpoint. ResourceServiceApiKeyAuthenticationHandler only checks the API key header while AuthMode is ApiKey and otherwise succeeds for every request, and ValidateResourceServiceOptions stops requiring a key once the mode is Unsecured, so clearing the mode does not even fail the start. That left the application's resource model readable on the loopback resource-service endpoint by anything that could reach it. Restore both halves together whenever the effective configuration would not be an authenticated ApiKey pairing, and seed a generated key pre-construction the same way the browser token is seeded, so an ambient ASPIRE_DASHBOARD_RESOURCESERVICE_APIKEY cannot hand every test application on a CI agent the same credential. Six new tests cover both downgrade halves across both creation surfaces and the per-application key freshness. All six fail when the guard and the seeding are removed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 487b899d-d79d-4349-a84b-a026861bde7f * Keep the dashboard browser token out of AppHost logs in testing mode Enabling the dashboard for a test application generates a browser token, and the AppHost startup summary then wrote it to ILogger<DistributedApplication> as ".../login?t={token}" -- both in the "Login to the dashboard at" line dotnet watch keys off and in the summary's Login URL row. Under a test host that logger is test and CI output, so a live credential for that application's dashboard was landing in a retained log artifact. That also contradicted two things this PR states: that no token is written to logs, and that the login URL is deliberately not emitted to test output. DashboardOptions gains SuppressLoginUrlInStartupSummary, bound from AppHost:SuppressDashboardLoginUrlInStartupSummary, which the testing builder seeds alongside its other dashboard defaults. DashboardEventHandlers then withholds the token from the summary, which drops the login URL and the watch line while leaving the dashboard and OTLP endpoint rows intact. Nothing changes outside testing mode: the key is unset, so the summary is byte-identical. Tests still reach the URL through GetDashboardLoginUrlAsync, which is the supported accessor and unaffected. Proven end to end rather than at the seam: the new test captures AppHost logs with FakeLogCollector, reads the token back out of the URL GetDashboardLoginUrlAsync returns, and asserts it appears nowhere -- while still requiring the summary itself to be present, so suppressing everything would not pass. Disabling the branch fails it and prints the leaked "- Login URL: .../login?t=a682d0a3..." row. 59/59. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 487b899d-d79d-4349-a84b-a026861bde7f * Keep dashboard testing focused Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0d1cf760-b1bc-46ba-a6d4-628354b00f2c * Address remaining dashboard testing review feedback Dispose the suspended factory when builder creation fails, and keep dashboard runtime tests available on supported Podman hosts. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8698944a-ea7e-4b37-af6a-925165a54f2b * Suppress dashboard tokens in forwarded test logs Propagate a testing-only output setting to the child dashboard and wait for its forwarded summary in the regression test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8698944a-ea7e-4b37-af6a-925165a54f2b * Support running the dashboard from tests * Fix dashboard testing target host URL Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 522a7395-2ea1-4d1a-91b3-40050853b9f4 * Use direct dashboard endpoint host resolution Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 522a7395-2ea1-4d1a-91b3-40050853b9f4 * Drop the product-to-product IVT and promote the target-host helper Reaching EndpointHostHelpers.GetUrlWithTargetHostAsync from Aspire.Hosting.Testing needed access that the package did not have. The first attempt granted Aspire.Hosting.Testing an InternalsVisibleTo in Aspire.Hosting.csproj, which is the exact pattern #18918 removed from Aspire.Hosting.Dotnet and 453da9c removed from Aspire.Hosting.Rust: two independently restorable packages with a default >= dependency range, so a version mismatch fails at runtime rather than at compile time, and none of the coupling is visible to API review. The grant also forced removing five $(SharedDir) Compile items from Aspire.Hosting.Testing, because source-shared internals and IVT-exposed internals are the same type twice (CS0436) - churn in the opposite direction from what the Rust change established. Every dependency the helper actually needs is already public: EndpointReference.EndpointAnnotation, EndpointReference.GetValueAsync, EndpointAnnotation.TargetHost, and EndpointHostHelpers.IsLocalhostTld. The internal modifier on this one method was incidental, and the class around it is already public with eight documented public methods. Make the method public, drop the IVT, and restore the shared Compile items. Aspire.Hosting.csproj now has no diff at all against the PR base. Cover the substitution rule in EndpointHostHelpersTests, which is deterministic and needs neither a container runtime nor a real socket, unlike the GetDashboardUrlAsync integration test that pins the call site. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 522a7395-2ea1-4d1a-91b3-40050853b9f4 * Substitute the localhost TLD only for loopback addresses Making GetUrlWithTargetHostAsync public widened its contract to every EndpointReference, including one carrying a non-local ContextNetworkID. EndpointReference resolves those against the container network, so the same endpoint yields container1.dev.internal:10005 rather than a loopback address - and the method then replaced that host with the .localhost target host. A *.localhost name always resolves to the caller's own loopback, so the rewritten URL pointed a container at itself. Substitute only when the resolved address is itself loopback, which is exactly the case the method documents: DCP allocates "localhost" because that is what the service binds to, and the TLD is the name the user expects to see. All three production callers - the dashboard startup summary, the CLI backchannel API URL, and GetDashboardUrlAsync - resolve host-facing loopback endpoints, so their behavior is unchanged. Verified by a test that pins the container-network address; it reproduces the rewrite before the fix. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 522a7395-2ea1-4d1a-91b3-40050853b9f4 * Validate the endpoint argument on the public target-host helper GetUrlWithTargetHostAsync became public in an earlier commit on this branch, so `endpoint` is now an external API input. It was dereferenced without validation, which surfaces a NullReferenceException instead of the repository-standard ArgumentNullException when a null reaches it from a language without nullable reference type enforcement. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 522a7395-2ea1-4d1a-91b3-40050853b9f4 * Describe the endpoint URL resolution on the helper type summary EndpointHostHelpers is a shipped public type whose summary described it as validating localhost addresses. GetUrlWithTargetHostAsync became public on this branch, so the type now also resolves endpoint URLs against a configured target host and the summary no longer matched what the type exposes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 522a7395-2ea1-4d1a-91b3-40050853b9f4 * Clarify dashboard testing isolation and wait behavior Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Adam Ratzman <adamint@github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ffeff87e-f284-434d-87d3-843e21a7aebb Copilot-Session: f3f649b5-52c4-4eb5-8d74-c4744e79faf4 Copilot-Session: 487b899d-d79d-4349-a84b-a026861bde7f Copilot-Session: 0d1cf760-b1bc-46ba-a6d4-628354b00f2c Copilot-Session: 8698944a-ea7e-4b37-af6a-925165a54f2b Copilot-Session: 522a7395-2ea1-4d1a-91b3-40050853b9f4
Description
The is an attempt to make all language packages (Dotnet, Go, Python, and JavaScript) independent from Aspire.Hosting internals (all
InternalsVisibleToproject items related to these language packages are removed from Aspire.Hosting project). As a result, they can now be truly independently evolved, and 3rd party language support for Aspire becomes possible too.The cost is adding more publis APIs, primarily related to debugging.
I have included the problem analysis and description of the change that this PR implements after the PR checklist.
Checklist
<remarks />and<code />elements on your triple slash comments?Coupling inventory
Build-time:
Aspire.Hosting.Dotnet→Aspire.HostinginternalsAspire.Hosting.csproj:135-136grants IVT toAspire.Hosting.Dotnet+.Tests.Aspire.Hosting.Dotnet.csprojhas a plainProjectReferenceand no$(SharedDir)compile items.Utils.PathNormalizersrc/Shared/PathNormalizer.cspathargUtils.CommandLineArgsParsersrc/Shared/CommandLineArgsParser.cscommandLineArgsLaunchProfileExtensions.GetEffectiveLaunchProfile+NamedLaunchProfilesrc/Shared/LaunchProfiles/Utils.DotnetSdkUtils.TryGetVersionAsyncsrc/Aspire.Hosting/Utils/ProjectMetadata(internal impl of publicIProjectMetadata)src/Aspire.Hosting/IProjectMetadata.cs.csprojresolution + annotationUtils.ExtensionUtils.SupportsDebugging+SupportsDebuggingAnnotationdotnet runargsDcp.Model.ProjectLaunchConfiguration : ExecutableLaunchConfigurationsrc/Aspire.Hosting/Dcp/Model/WithDebugSupport(..., "project")ProjectResourceBuilderExtensions.WithProjectDefaults<T>(internal)ProjectResourceBuilderExtensions.cs:465ASPNETCORE_URLS/ rebuild wiringIProjectLaunchDefaultsResource(internal interface)ApplicationModel/DotnetProjectResourceItems 6 and 7 are the seam all language packages already need (Go/Python/JS use it today).
Runtime:
Aspire.Hosting→ the package's resourceCore never names
DotnetProjectResource(only in comments) — no assembly cycle. It reaches it via:IProjectMetadataannotation (public) —ExecutableCreator,ResourceSnapshotBuilderSupportsDebuggingAnnotation.LaunchConfigurationType == "project"(internal + magic string)IProjectLaunchDefaultsResourceruntime type test (internal) —CommandsConfigurationExtensions(Restart description + Rebuild command),
ProjectRebuilderResourceSo the internal interface is a genuinely bidirectional contract carried by IVT.
Other consumers
Aspire.Hosting.Blazor→Aspire.Hosting.DotnetProjectReference, public API only.Aspire.Hosting.AppHost(opt-in), no hardcoded package lists, noapi/*.csyet.4. Proposed approach — all four tiers, ending with IVT removal
Goal:
src/Aspire.Hosting/Aspire.Hosting.csprojno longer grantsInternalsVisibleTotoAspire.Hosting.Dotnet(the.Testsgrant stays — test-only IVT is normal). Tiers are ordered soevery one of them builds green on its own and can be reviewed/committed separately.
Tier 1 — mechanical, no API change (removes items 1-4)
Follow the
Aspire.Hosting.Azure.Functionsprecedent (which needs no IVT):src/Aspire.Hosting.Dotnet/Aspire.Hosting.Dotnet.csproj:<Compile Include="$(SharedDir)PathNormalizer.cs" />,$(SharedDir)CommandLineArgsParser.cs,$(SharedDir)\LaunchProfiles\*.cs, plus the linkedLaunchProfileStrings.Designer.cs+EmbeddedResource.resx(required byLaunchProfileExtensions) — copy the exact item group fromAspire.Hosting.Azure.Functions.csproj.src/Aspire.Hosting/Utils/DotnetSdkUtils.cs→src/Shared/and link it into both assemblies.Core call sites to keep compiling:
DotnetToolResourceExtensions.cs:210,ProjectResourceBuilderExtensions.cs:441.Tier 2 — small and local (removes item 5)
DotnetProjectMetadata : IProjectMetadata(IProjectMetadatais alreadypublic; core only ever does
TryGetLastAnnotation<IProjectMetadata>/ casts to the interface —verified, no concrete-type tests anywhere).
.csproj" resolution (ProjectMetadata.ResolveProjectPath) intosrc/Sharedand link it into both so the rule isn't duplicated.Tier 3 — public debug seam; pays off 4× (removes items 6-7, and for Go/Python/JS too)
Key finding:
WithDebugSupport<T, TLaunchConfiguration>is already public andTLaunchConfigurationis unconstrained — the producer's result is only JSON-serialized into theDCP
Executableannotation. So the sole blocker is that the DTO shape is internal.ExecutableLaunchConfiguration(base) andProjectLaunchConfigurationout ofAspire.Hosting.Dcp.Modelinto public[Experimental("ASPIREEXTENSION001")]types underAspire.Hosting.ApplicationModel, preserving every[JsonPropertyName](type,mode,project_path,launch_profile,disable_launch_profile) — this is the IDE/extension launchcontract, not a DCP implementation detail. Keep
ExecutableLaunchModeconstants public alongside.SupportsDebuggingAnnotationpublic[Experimental("ASPIREEXTENSION001")]with publicLaunchConfigurationType/RewritesArgumentsForDebugging, but keepLaunchConfigurationAnnotatorand
Create<T>internal (they reference the internal DCPExecutabletype).SupportsDebugging(this IResource, IConfiguration, out SupportsDebuggingAnnotation?)query out of internal
Utils.ExtensionUtilsinto a public[Experimental]extension class.KnownLaunchConfigurationTypes.Projectconstant and replace the"project"magicstring in
ExecutableCreator,ProjectResourceBuilderExtensions, and the package.Aspire.Hosting.Go/.Python/.JavaScripttothe public types so their IVT grants can be dropped too.
Tier 4 — the deep one (removes items 8-9). Design: make the contract annotation-driven
Consistent with how core already treats
IProjectMetadata/SupportsDebuggingAnnotation.Interface member usage is small and localized (9 call sites in
ProjectResourceBuilderExtensions,2 in
ProjectResource, 2 inCommandsConfigurationExtensions, 1 inProjectRebuilderResource).ProjectLaunchDefaultsAnnotationholdingKestrelEndpointAnnotationHosts(
Dictionary<EndpointAnnotation, string>) andDefaultHttpsEndpoint;WithProjectDefaultsadds it.Move
HasKestrelEndpoints/ShouldInjectEndpointEnvironmentto internal helpers over theannotation +
IResource.WithProjectDefaults<T>to public[Experimental]with a multi-interface constraint(
where T : class, IResourceWithEnvironment, IResourceWithEndpoints, IResourceWithArgs) instead ofwhere T : IProjectLaunchDefaultsResource.resource is IProjectLaunchDefaultsResourcetests(
CommandsConfigurationExtensionsRestart description + Rebuild command) withresource.HasAnnotationOfType<ProjectLaunchDefaultsAnnotation>().ProjectRebuilderResource : IResourceWithParent<IProjectLaunchDefaultsResource>toIResourceWithParent<IResource>.IProjectLaunchDefaultsResourceand the explicit implementations onProjectResourceandDotnetProjectResource.DotnetProjectResourceimplements nothing internal; the whole contract becomesordinary public API covered by
api/*.cs+ package validation.Tier 5 — remove the IVT and prove it
<InternalsVisibleTo Include="Aspire.Hosting.Dotnet" />fromAspire.Hosting.csproj:135../build.shmust succeed — that is the proof the coupling is gone.Alternatives considered and rejected: promoting
IProjectLaunchDefaultsResourceas-is (leaks amutable
Dictionary<EndpointAnnotation, string>into public API); duplicating the project-defaultslogic into the package via shared source (state is shared with core's DCP/rebuild/snapshot paths —
would diverge); duplicating the launch-configuration DTO shape in each language package (silent JSON
contract drift).