Expose the evaluated project assembly name for debugger attach - #19136
Expose the evaluated project assembly name for debugger attach#19136Adam Ratzman (adamint) wants to merge 10 commits into
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 19136Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 19136" |
Review recordPosting as a comment rather than a review since this is my own PR. Scope: Full review, 4 lanes (public API surface, hosting core, packaging/SDK targets, cross-platform), followed by one incremental re-review over the fix delta. Fixed
DisprovedOne lane raised a HIGH claiming the probe added a duplicate An earlier premise that the probe needed protection from Declined, with rationale
DeferredThe SDK targets tests each spawn Known constraint worth recordingThe probe target must stay pure MSBuild. |
There was a problem hiding this comment.
Pull request overview
Exposes MSBuild-evaluated project assembly names for debugger process matching.
Changes:
- Adds optional
IProjectMetadata.AssemblyName. - Resolves names during AppHost builds and exposes
project.assemblyName. - Adds dashboard metadata, localization, and cross-layer tests.
Show a summary per file
| File | Description |
|---|---|
tests/Aspire.Hosting.Tests/ProjectResourceBuilderExtensionTests.cs |
Tests default and path-based metadata. |
tests/Aspire.Hosting.Tests/Dcp/ResourceSnapshotBuilderTests.cs |
Tests snapshot assembly-name behavior. |
tests/Aspire.Hosting.Sdk.Tests/AppHostSdkTargetsTests.cs |
Tests MSBuild evaluation scenarios. |
tests/Aspire.Dashboard.Tests/Model/KnownPropertyLookupTests.cs |
Tests property lookup. |
tests/Aspire.Cli.Tests/Backchannel/ResourceSnapshotMapperTests.cs |
Tests backchannel serialization. |
src/Shared/Model/KnownProperties.cs |
Defines the snapshot property key. |
src/Aspire.Hosting/Resources/xlf/MessageStrings.zh-Hant.xlf |
Updates localization resources. |
src/Aspire.Hosting/Resources/xlf/MessageStrings.zh-Hans.xlf |
Updates localization resources. |
src/Aspire.Hosting/Resources/xlf/MessageStrings.tr.xlf |
Updates localization resources. |
src/Aspire.Hosting/Resources/xlf/MessageStrings.ru.xlf |
Updates localization resources. |
src/Aspire.Hosting/Resources/xlf/MessageStrings.pt-BR.xlf |
Updates localization resources. |
src/Aspire.Hosting/Resources/xlf/MessageStrings.pl.xlf |
Updates localization resources. |
src/Aspire.Hosting/Resources/xlf/MessageStrings.ko.xlf |
Updates localization resources. |
src/Aspire.Hosting/Resources/xlf/MessageStrings.ja.xlf |
Updates localization resources. |
src/Aspire.Hosting/Resources/xlf/MessageStrings.it.xlf |
Updates localization resources. |
src/Aspire.Hosting/Resources/xlf/MessageStrings.fr.xlf |
Updates localization resources. |
src/Aspire.Hosting/Resources/xlf/MessageStrings.es.xlf |
Updates localization resources. |
src/Aspire.Hosting/Resources/xlf/MessageStrings.de.xlf |
Updates localization resources. |
src/Aspire.Hosting/Resources/xlf/MessageStrings.cs.xlf |
Updates localization resources. |
src/Aspire.Hosting/Resources/MessageStrings.resx |
Adds the display-name resource. |
src/Aspire.Hosting/Resources/MessageStrings.Designer.cs |
Exposes the generated resource accessor. |
src/Aspire.Hosting/IProjectMetadata.cs |
Adds optional assembly-name metadata. |
src/Aspire.Hosting/Dcp/ResourceSnapshotBuilder.cs |
Publishes the snapshot property. |
src/Aspire.Hosting/Dashboard/ResourcePropertySnapshotMetadata.cs |
Configures dashboard presentation. |
src/Aspire.Hosting.AppHost/build/Aspire.Hosting.AppHost.in.targets |
Evaluates and generates assembly metadata. |
src/Aspire.Cli/Projects/DotNetBasedAppHostServerProject.cs |
Opts generated server projects out. |
Review details
Files not reviewed (1)
- src/Aspire.Hosting/Resources/MessageStrings.Designer.cs: Generated file
- Files reviewed: 25/26 changed files
- Comments generated: 1
- Review effort level: Balanced
|
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. |
There was a problem hiding this comment.
Review details
Files not reviewed (1)
- src/Aspire.Hosting/Resources/MessageStrings.Designer.cs: Generated file
Suppressed comments (2)
tests/Aspire.Hosting.Sdk.Tests/AppHostSdkTargetsTests.cs:338
- This test does not exercise the failure path it claims to cover. Setting
BuildingProject=trueonly changesContinueOnError; the generated worker still lets both probes succeed, so this test would also pass if failures were accidentally tolerated. Inject a failingBeforeTargets="GetTargetPath"(or equivalent probe failure) and assert that the MSBuild invocation exits nonzero whenBuildingProject=true.
// The probe passes ContinueOnError="!$(BuildingProject)", so a real build makes a failed
// reference probe fatal while a design-time build stays tolerant. Every other test here runs
// a bare -t: invocation, which never executes BuildOnlySettings and therefore leaves
// BuildingProject at its default of false. Forcing it to true covers the strict path.
src/Aspire.Hosting.AppHost/build/Aspire.Hosting.AppHost.in.targets:115
- A wrong non-empty value does not leave consumers in the same state as omission. The attach consumer prefers
reportedAssemblyNamewith??, so a first-TFM name shadows its fallback even if the IDE launches another TFM whose conditionedAssemblyNamediffers. For a cross-targeting reference withoutSetTargetFramework, omit this property unless the selected launch TFM is known (or all TFMs resolve to the same name).
%(TargetFrameworks) is a single semicolon-joined value such as "net8.0;net9.0"; the first entry is used because
it is the one a cross-targeting reference reports first and Aspire never negotiates a TFM for these references,
so there is no better signal about which inner build the AppHost will end up launching. Guessing is worth it
here: a name resolved from the wrong inner build leaves a consumer exactly where omitting the property would
have - falling back to its own guess - while the common case, where every TFM produces the same assembly name,
resolves correctly.
- Files reviewed: 25/26 changed files
- Comments generated: 0 new
- Review effort level: Balanced
There was a problem hiding this comment.
Review details
Files not reviewed (1)
- src/Aspire.Hosting/Resources/MessageStrings.Designer.cs: Generated file
Suppressed comments (3)
tests/Aspire.Hosting.Sdk.Tests/AppHostSdkTargetsTests.cs:335
- This test never makes either probe fail; it only verifies that a successful probe also succeeds with
BuildingProject=true. Changing bothContinueOnErrorvalues to always tolerate failures would still pass, so the newly introduced fatal-build behavior is not covered. Add a referenced target that deliberately errors duringGetTargetFrameworksorGetTargetPathand assert a nonzero build result whenBuildingProject=true.
public async Task ProjectMetadataResolvesAssemblyNameWhenProbeFailuresAreFatal()
{
using var workspace = TemporaryWorkspace.Create(outputHelper);
// The probe passes ContinueOnError="!$(BuildingProject)", so a real build makes a failed
src/Aspire.Hosting/IProjectMetadata.cs:48
- The remarks incorrectly state that third-party implementations return
null. Existing implementations inherit that default, but new or updated implementations can override this member, andResourceSnapshotBuilderconsumes such values from anyIProjectMetadata. Document the default behavior without excluding custom providers.
/// Implementations that are not produced by the AppHost build - for example metadata created from a project
/// path at runtime, file-based apps, or third-party implementations - return <see langword="null"/>. Consumers
/// must therefore treat the value as an optional hint and fall back to their existing behavior when it is absent.
src/Shared/Model/KnownProperties.cs:62
- This property is not restricted to
ProjectReferenceresources.ResourceSnapshotBuilderemits it for anyIProjectMetadataimplementation that supplies a nonblank value, including annotation-based/custom resources. The comment should describe generated metadata as one provider rather than claiming exclusivity.
/// The MSBuild-evaluated assembly name of the project, baked into the generated project metadata at
/// AppHost build time. Only present for project resources added through a ProjectReference; the absence
/// of the property is the signal that the producer could not determine the name.
- Files reviewed: 26/27 changed files
- Comments generated: 1
- Review effort level: Balanced
|
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. |
The install hint recovered a resource's language by regex-parsing AppHost source, but the app model already computes exactly that: SupportsDebuggingAnnotation.LaunchConfigurationType is one of python/go/bun/project/maui/azure-functions. Publish it as a resource snapshot property and read it in the extension. Hosting: ResourceSnapshotBuilder emits resource.launchConfigurationType for project and executable resources that carry SupportsDebuggingAnnotation, and removes a stale value when the annotation goes away, so absence stays the "no debug support" signal. This follows the mechanism PR microsoft#19136 uses for project.assemblyName. Extension: hints are now keyed by launch configuration type off the existing languages/*.ts table instead of a parallel table keyed by C# method name. That fixes two coverage gaps: only .cs and JS/TS AppHosts had parsers registered, so a Python AppHost got no hint at all - the exact target scenario - and the method-name key silently missed any Add* method not in the table, including third-party integrations. Coverage goes from 3 method names to all 5 installable debug adapters, for any AppHost language. This removes DebuggerInstallHintWatcher entirely along with its parse cache, retry loop, reopen-on-close handling and hasPendingNotifications. The CodeLens reads the same snapshot it already had in hand, so it no longer needs the parsed method name either. Drop the affected-resource count from the toast. The count was a snapshot that went stale as soon as another resource started, the toast is coalesced per extension id anyway, and the actionable fact is that the debug adapter is missing. This also removes the singular/plural string pair, which vscode.l10n cannot express as a single message. Reuse the existing dontShowAgainLabel instead of a second localized string that differed only in case. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
The install hint recovered a resource's language by regex-parsing AppHost source, but the app model already computes exactly that: SupportsDebuggingAnnotation.LaunchConfigurationType is one of python/go/bun/project/maui/azure-functions. Publish it as a resource snapshot property and read it in the extension. Hosting: ResourceSnapshotBuilder emits resource.launchConfigurationType for project and executable resources that carry SupportsDebuggingAnnotation, and removes a stale value when the annotation goes away, so absence stays the "no debug support" signal. This follows the mechanism PR microsoft#19136 uses for project.assemblyName. Extension: hints are now keyed by launch configuration type off the existing languages/*.ts table instead of a parallel table keyed by C# method name. That fixes two coverage gaps: only .cs and JS/TS AppHosts had parsers registered, so a Python AppHost got no hint at all - the exact target scenario - and the method-name key silently missed any Add* method not in the table, including third-party integrations. Coverage goes from 3 method names to all 5 installable debug adapters, for any AppHost language. This removes DebuggerInstallHintWatcher entirely along with its parse cache, retry loop, reopen-on-close handling and hasPendingNotifications. The CodeLens reads the same snapshot it already had in hand, so it no longer needs the parsed method name either. Drop the affected-resource count from the toast. The count was a snapshot that went stale as soon as another resource started, the toast is coalesced per extension id anyway, and the actionable fact is that the debug adapter is missing. This also removes the singular/plural string pair, which vscode.l10n cannot express as a single message. Reuse the existing dontShowAgainLabel instead of a second localized string that differed only in case. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Review details
Files not reviewed (1)
- src/Aspire.Hosting/Resources/MessageStrings.Designer.cs: Generated file
Suppressed comments (1)
tests/Aspire.Hosting.Sdk.Tests/AppHostSdkTargetsTests.cs:355
- This test never makes either
GetTargetFrameworksorGetTargetPathfail, soContinueOnErroris never exercised; it would still pass if both probe calls were changed to always continue. Since real-build probe failures are intentionally build-fatal, add a case whose referenced project emits an error from one of those probe targets and assert thatBuildingProject=trueproduces a nonzero MSBuild exit code (and, ideally, that the tolerant mode continues).
public async Task ProjectMetadataResolvesAssemblyNameWhenProbeFailuresAreFatal()
- Files reviewed: 26/27 changed files
- Comments generated: 0 new
- Review effort level: Balanced
|
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. |
The install hint recovered a resource's language by regex-parsing AppHost source, but the app model already computes exactly that: SupportsDebuggingAnnotation.LaunchConfigurationType is one of python/go/bun/project/maui/azure-functions. Publish it as a resource snapshot property and read it in the extension. Hosting: ResourceSnapshotBuilder emits resource.launchConfigurationType for project and executable resources that carry SupportsDebuggingAnnotation, and removes a stale value when the annotation goes away, so absence stays the "no debug support" signal. This follows the mechanism PR microsoft#19136 uses for project.assemblyName. Extension: hints are now keyed by launch configuration type off the existing languages/*.ts table instead of a parallel table keyed by C# method name. That fixes two coverage gaps: only .cs and JS/TS AppHosts had parsers registered, so a Python AppHost got no hint at all - the exact target scenario - and the method-name key silently missed any Add* method not in the table, including third-party integrations. Coverage goes from 3 method names to all 5 installable debug adapters, for any AppHost language. This removes DebuggerInstallHintWatcher entirely along with its parse cache, retry loop, reopen-on-close handling and hasPendingNotifications. The CodeLens reads the same snapshot it already had in hand, so it no longer needs the parsed method name either. Drop the affected-resource count from the toast. The count was a snapshot that went stale as soon as another resource started, the toast is coalesced per extension id anyway, and the actionable fact is that the debug adapter is missing. This also removes the singular/plural string pair, which vscode.l10n cannot express as a single message. Reuse the existing dontShowAgainLabel instead of a second localized string that differed only in case. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
An AppHost that sets AssemblyName - commonly through an imported Directory.Build.props - launches an assembly whose name has no relation to the project file name. Consumers that need to identify the running process (debugger attach being the motivating case in microsoft#18937) read the project file name and target something that does not exist. Bake the MSBuild-evaluated assembly name into the generated IProjectMetadata while the AppHost is built, and project it onto the resource snapshot as an optional "project.assemblyName" property. There is no run-time evaluation: the value is resolved by the same MSBuild project-reference machinery the AppHost SDK already drives, so nothing is re-evaluated when a resource restarts and no subprocess is spawned. The property is additive in both directions. IProjectMetadata gains a default interface member returning null, so metadata created from a path, file-based apps, and third-party implementations stay source and binary compatible; the snapshot property is written only when a name resolved, so its absence is the capability signal for consumers. This does not close microsoft#18602. Replicas and descendant process identity still need a DCP-side contract; this is the fallback for the common single-instance case where the assembly name is enough to find the process. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ffeff87e-f284-434d-87d3-843e21a7aebb
Carry the ProjectReference configuration, platform, and global-property removal metadata into the GetTargetPath evaluation used for generated assembly names. Preserve an explicitly selected target framework rather than replacing it with the first declared framework. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ffeff87e-f284-434d-87d3-843e21a7aebb
Resolve assembly names from the SDK-prepared project reference contract so solution-selected configuration and platform values match the selected output. Preserve explicit target frameworks while retaining all other global-property removals, and add regressions for both cases. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ffeff87e-f284-434d-87d3-843e21a7aebb
Remove the unreachable dashboard legacy metadata. The dashboard resolves project.assemblyName through the producer-supplied metadata that the AppHost already sends, so the LegacyResourcePropertyMetadata arm and its localized resource string could never be reached. Deleting them leaves src/Aspire.Dashboard with no diff at all for this feature. Fail the probe loudly during a real build. Both MSBuild calls now use ContinueOnError="!$(BuildingProject)", matching _ValidateAspireHostProjectResources, so a genuinely broken project reference surfaces as an error instead of silently producing metadata without an assembly name. Strengthen the snapshot tests. The two omission tests now assert the complete property set with Assert.Equal rather than the absence of a single name, so a future project property has to be acknowledged instead of slipping through. This immediately caught two resource properties missing from the expected set. Also pin ManagePackageVersionsCentrally=false in the SDK test workspace so an ambient Directory.Packages.props cannot leak into the generated metadata, and document the SkipAspireProjectResourceAssemblyName opt-out in the targets file. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4d5b4985-cae3-4ee6-b159-53ae9385b638
The re-review pointed out that ContinueOnError="!$(BuildingProject)" was never actually exercised. Every SDK test invokes the metadata target directly with -t:, which never runs BuildOnlySettings, so BuildingProject stays at its default of false and the probe ran tolerant in all of them. Add a test that forces BuildingProject=true so the fatal-failure path is covered, and separately verify that a real `dotnet build` of tests/testproject/TestProject.AppHost still succeeds with 0 warnings and 0 errors. Skip the probe for the AppHost server project the CLI generates. Its ProjectReferences are Aspire.Hosting.* libraries rather than app resources anyone attaches a debugger to, several of them are multi-targeted, and it is built by a real `dotnet build`, so the probe would be pure cost on a newly fatal path. This matches the SkipValidateAspireHostProjectResources already set there for the same reason. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4d5b4985-cae3-4ee6-b159-53ae9385b638
The project snapshot defines the *absence* of `project.assemblyName` as the capability signal telling consumers whether the evaluated assembly name can be relied on. Snapshots are merged into the previously published one and SetResourcePropertyRange only adds or replaces entries, so omitting the property when the metadata is blank left any earlier value in place. Consumers would then read a stale assembly name and believe the capability is present - exactly the failure mode the contract exists to prevent. Remove the known property from the carried-forward snapshot in the blank branch and cover it with a regression test that seeds a previous snapshot containing an assembly name. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…erty The AppHost targets now emit an AssemblyName property into the generated project metadata, so MSBuildTests.ValidateMetadataSources failed on Hosting-6 with a VerifyException. Accept the regenerated snapshot. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…unctions CreateAspireProjectMetadataSources passed %(_AspireProjectResource.Identity) straight into GetFullPath and GetFileNameWithoutExtension. MSBuild substitutes %(...) into the unexpanded argument text of a property function, so an apostrophe in the value terminates the argument list, the parse is abandoned, and the whole expression is emitted back as a literal string. A project under a directory such as O'Brien therefore generated a ProjectPath of "$([System.IO.Path]::GetFullPath(../O'Brien/Worker.csproj))" and a source file named after the unparsed ClassName expression, which fails the AppHost build. Batch the target and route both values through properties first. $(...) is expanded only after the arguments are parsed, so the apostrophe never reaches the parser. This mirrors the property indirection already used for the assembly name in _SetAspireProjectMetadataAssemblyNames. The broken correlation also silently suppressed the new project.assemblyName property for such projects, which would have read as "the assembly name is not available" rather than as a failure. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ions The evaluated project assembly name is becoming the source of truth for the debugger attach target, so the cases where a naive implementation guesses wrong need to be pinned by tests: - No AssemblyName anywhere, so the value falls back to the project file name. This is the overwhelmingly common shape and was previously uncovered. - AssemblyName inherited from a Directory.Build.props several levels above the project, which only an MSBuild evaluation can see. The existing coverage only placed the props file directly next to the project. - TargetName deliberately diverging from AssemblyName. TargetName is what the built output is named and therefore what a debugger attaches to, so it has to win. Asserting the divergence stops a future refactor from quietly switching to AssemblyName. Per-configuration, per-TFM and per-platform conditions were already covered. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Review details
Files not reviewed (1)
- src/Aspire.Hosting/Resources/MessageStrings.Designer.cs: Generated file
Suppressed comments (1)
tests/Aspire.Hosting.Sdk.Tests/AppHostSdkTargetsTests.cs:425
- This test does not exercise the claimed fatal-failure path: both probes succeed, so
BuildingProject=truehas no observable effect and the test would still pass ifContinueOnErrorwere always enabled. Make one referenced target fail and assert that the MSBuild invocation exits nonzero in strict mode (and, if desired, remains tolerant whenBuildingProject=false).
public async Task ProjectMetadataResolvesAssemblyNameWhenProbeFailuresAreFatal()
{
using var workspace = TemporaryWorkspace.Create(outputHelper);
// The probe passes ContinueOnError="!$(BuildingProject)", so a real build makes a failed
- Files reviewed: 26/27 changed files
- Comments generated: 1
- Review effort level: Balanced
589cb0f to
9794bc1
Compare
There was a problem hiding this comment.
Review details
Files not reviewed (1)
- src/Aspire.Hosting/Resources/MessageStrings.Designer.cs: Generated file
Suppressed comments (1)
tests/Aspire.Hosting.Sdk.Tests/AppHostSdkTargetsTests.cs:428
- This test never causes either probe to fail, so
ContinueOnErroris not exercised: it would pass unchanged if the production value were hard-coded to eithertrueorfalse. Since the new real-build behavior intentionally makes probe failures fatal, inject a referenced-project target that fails duringGetTargetFrameworks/GetTargetPathand assert a non-zero build result withBuildingProject=true(ideally paired with the tolerant case).
// The probe passes ContinueOnError="!$(BuildingProject)", so a real build makes a failed
// reference probe fatal while a design-time build stays tolerant. Every other test here runs
// a bare -t: invocation, which never executes BuildOnlySettings and therefore leaves
// BuildingProject at its default of false. Forcing it to true covers the strict path.
- Files reviewed: 26/27 changed files
- Comments generated: 0 new
- Review effort level: Balanced
A project reference carries BuildReference=false when the active solution configuration excludes it from the build, and ResolveProjectReferences skips those references entirely. The assembly name probe only filtered on IsAspireProjectResource, so it evaluated references the build was told not to touch. That costs a fresh evaluation with no cached GetTargetPath result to serve it, and lets a project that was never meant to participate in the build fail the AppHost build. Filter the prepared reference list on BuildReference as well. Both probes derive from that list, so filtering at the source covers the GetTargetFrameworks and GetTargetPath calls. ProjectMetadataSkipsAssemblyNameForReferenceDisabledInSolutionConfiguration covers this; it fails without the filter, emitting an assembly name for the disabled reference. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The install hint recovered a resource's language by regex-parsing AppHost source, but the app model already computes exactly that: SupportsDebuggingAnnotation.LaunchConfigurationType is one of python/go/bun/project/maui/azure-functions. Publish it as a resource snapshot property and read it in the extension. Hosting: ResourceSnapshotBuilder emits resource.launchConfigurationType for project and executable resources that carry SupportsDebuggingAnnotation, and removes a stale value when the annotation goes away, so absence stays the "no debug support" signal. This follows the mechanism PR microsoft#19136 uses for project.assemblyName. Extension: hints are now keyed by launch configuration type off the existing languages/*.ts table instead of a parallel table keyed by C# method name. That fixes two coverage gaps: only .cs and JS/TS AppHosts had parsers registered, so a Python AppHost got no hint at all - the exact target scenario - and the method-name key silently missed any Add* method not in the table, including third-party integrations. Coverage goes from 3 method names to all 5 installable debug adapters, for any AppHost language. This removes DebuggerInstallHintWatcher entirely along with its parse cache, retry loop, reopen-on-close handling and hasPendingNotifications. The CodeLens reads the same snapshot it already had in hand, so it no longer needs the parsed method name either. Drop the affected-resource count from the toast. The count was a snapshot that went stale as soon as another resource started, the toast is coalesced per extension id anyway, and the actionable fact is that the debug adapter is missing. This also removes the singular/plural string pair, which vscode.l10n cannot express as a single message. Reuse the existing dontShowAgainLabel instead of a second localized string that differed only in case. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Review details
Files not reviewed (1)
- src/Aspire.Hosting/Resources/MessageStrings.Designer.cs: Generated file
Suppressed comments (2)
tests/Aspire.Hosting.Sdk.Tests/AppHostSdkTargetsTests.cs:445
- This test never makes either MSBuild probe fail; the helper asserts a zero exit code and the test only proves that a successful probe still works with
BuildingProject=true. It would continue passing if the fatalContinueOnErrorbehavior regressed. Trigger a failingGetTargetFrameworksorGetTargetPathprobe and assert that the real-build invocation exits nonzero (and, if the tolerant path is part of the contract, that the design-time invocation continues).
public async Task ProjectMetadataResolvesAssemblyNameWhenProbeFailuresAreFatal()
src/Aspire.Hosting.AppHost/build/Aspire.Hosting.AppHost.in.targets:134
- The tests added here all create a single project reference, so this per-item pairing logic is not exercised with multiple references. A batching regression that applies one reference's framework/name metadata to another would still pass, even though multi-project AppHosts are the normal case. Add a test with at least two references having distinct assembly names and assert each generated metadata file receives its own value.
The item list is updated from itself on purpose. Updating one list from another (Update="@(OtherList)") does not
batch per item - the last batch's values are applied to every item - so the self-update is what keeps each
reference's TargetFrameworks metadata paired with its own project.
- Files reviewed: 26/27 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Description
A .NET project resource is launched as
dotnet run <project>, so the PID Aspire reports belongs to thedotnet runlauncher rather than to the worker process a debugger actually wants to attach to. A consumer that wants to find the worker among the launcher's children has nothing reliable to match on: the project file name is often not the assembly name, becauseAssemblyNameis frequently set in an importedDirectory.Build.propsrather than in the project file itself.This exposes the MSBuild-evaluated assembly name so a consumer can match
<AssemblyName>.dll/<AssemblyName>among the descendant processes.How it works
IProjectMetadatagainsstring? AssemblyName => nullas a default interface member, matching the existingSuppressBuildandIsFileBasedAppDIMs. Path-based metadata, file-based apps, and third-party implementations stay source and binary compatible and simply returnnull.IProjectMetadataclass. Nothing runs at application run time.ResourceSnapshotBuilderwrites aproject.assemblyNamesnapshot property only when a name resolved. Absence is the capability signal, so a consumer can tell "no name available" from "name is empty".SkipAspireProjectResourceAssemblyName=trueopts out.The probe uses the SDK's own two-phase idiom:
GetTargetFrameworksto pick a TFM, thenGetTargetPathto get the built output path, deriving the name from that path. It runs over prepared references (_MSBuildProjectReferenceExistent) so it inherits the same configuration, platform, and global-property removals the SDK already computed for each reference. BecauseResolveProjectReferencesalready asks forGetTargetPathwith the same effective global properties, MSBuild serves the probe's request from its result cache.Measured cost: exactly one extra
GetTargetFrameworkscall per project reference, and zero extraGetTargetPathcalls. I verified this with an instrumented probe-counting harness rather than assuming it.This does not close #18602. Replicas and descendant PIDs still need a DCP-side contract; this is the smaller, self-contained half.
Fixes #18937
Validation
Aspire.Hosting.Sdk.Tests*.ProjectMetadata*Aspire.Hosting.TestsResourceSnapshotBuilderTests+ProjectResourceBuilderExtensionTestsAspire.Dashboard.TestsResourceViewModelTests+KnownPropertyLookupTests+ResourceSnapshotMapperTestsdotnet buildoftests/testproject/TestProject.AppHostAssemblyNameplayground/DotnetProject/DotnetProject.AppHostThe SDK tests cover assembly names arriving from an imported
Directory.Build.props,Configuration- andTargetFramework-conditioned values, per-referenceSetConfiguration/SetPlatform, solution-prepared references,GlobalPropertiesToRemove, multi-targeted references, C# escaping of exotic names, and the opt-out.Review notes
This went through a 4-lane review panel plus an incremental re-review. Things worth knowing:
ProjectPath,AspireProjectMetadataTypeName,AspireGeneratedClassesVisibility) would go stale in the IDE until a real build. That regression was not worth the ~1 cheap target per reference it saved.ContinueOnError="!$(BuildingProject)"), matching_ValidateAspireHostProjectResourcesin the same file and the SDK's ownGetTargetFrameworks/GetTargetPathcalls. The re-review caught that this path was never exercised — every SDK test uses a bare-t:invocation, which never runsBuildOnlySettings— so there is now a test that forcesBuildingProject=true, plus the realdotnet buildabove.ProjectReferences areAspire.Hosting.*libraries, not app resources anyone attaches a debugger to.Findings I declined, with rationale:
AssemblyNameon a multi-targeted executable project resource is vanishingly rare. Recorded in a comment in the targets file.RemovePropertiesis duplicated with_ValidateAspireHostProjectResources. That target consumes raw_AspireProjectResourceitems rather than prepared references and is skipped in-repo, so refactoring it carries more risk than value.AssemblyNamechange.WriteAspireProjectMetadataSourcesrunsBeforeTargets="CoreCompile"on everydotnet build, so the gap is VS FUTDC only.Deferred as a follow-up: the SDK targets tests each spawn
dotnet msbuild -restore, which is real CI cost, but that is pre-existing harness design rather than something this PR introduced.Checklist
<remarks />and<code />elements on your triple slash comments?