Fix remaining issue 17244 items - #17507
Conversation
Preserve legacy auxiliary backchannel resource property serialization unless clients opt in to JSON-valued properties, and remove the unused dashboard start command filter. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 17507Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 17507" |
There was a problem hiding this comment.
Pull request overview
Updates the auxiliary backchannel contract to add client capability negotiation (aux.v4) so callers can opt into typed JSON resource properties while preserving legacy string/null property values for callers that don’t opt in. Also removes the dead dashboard menu filtering for the Start command so Start is shown whenever it’s visible in the resource command model.
Changes:
- Add
aux.v4capability andClientCapabilitiesto v2 resource get/watch requests to enable opt-in typed JSON resource properties. - Update AppHost backchannel RPC target to emit legacy string-valued properties by default and typed JSON properties for
aux.v4callers. - Remove
showStartCommandfiltering in the dashboard resource menu builder and update tests accordingly.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/Aspire.Hosting.Tests/Backchannel/BackchannelContractTests.cs | Extends contract test helpers to handle string[] properties. |
| tests/Aspire.Hosting.Tests/Backchannel/AuxiliaryBackchannelTests.cs | Updates capability assertions to include aux.v4. |
| tests/Aspire.Hosting.Tests/Backchannel/AuxiliaryBackchannelRpcTargetTests.cs | Adds tests validating legacy string mapping vs v4 typed JSON mapping for properties. |
| tests/Aspire.Dashboard.Tests/Model/ResourceMenuBuilderTests.cs | Updates test expectations to include Start as a normal visible command. |
| tests/Aspire.Cli.Tests/TestServices/TestAppHostAuxiliaryBackchannel.cs | Adds SupportsV4 to the CLI backchannel test double. |
| tests/Aspire.Cli.Tests/Backchannel/AppHostAuxiliaryBackchannelTests.cs | Adds tests ensuring CLI sends ClientCapabilities including aux.v4 for v2 get/watch calls. |
| src/Aspire.Hosting/Backchannel/BackchannelDataTypes.cs | Adds aux.v4 constant and ClientCapabilities fields to v2 resource request types. |
| src/Aspire.Hosting/Backchannel/AuxiliaryBackchannelRpcTarget.cs | Implements opt-in typed JSON properties and legacy string property conversion. |
| src/Aspire.Dashboard/Model/ResourceMenuBuilder.cs | Removes showStartCommand parameter and filtering; treats Start like other visible commands. |
| src/Aspire.Dashboard/Components/Controls/ResourceDetails.razor.cs | Updates call site after removing showStartCommand parameter. |
| src/Aspire.Cli/Backchannel/IAppHostAuxiliaryBackchannel.cs | Adds SupportsV4 capability property to the CLI backchannel interface. |
| src/Aspire.Cli/Backchannel/AppHostAuxiliaryBackchannel.cs | Sends client capabilities on v2 resource get/watch requests to opt in to typed JSON properties. |
| var snapshots = new List<ResourceSnapshot>(); | ||
| await foreach (var snapshot in backchannel.WatchResourceSnapshotsAsync(includeHidden: true).DefaultTimeout()) | ||
| { | ||
| snapshots.Add(snapshot); | ||
| } | ||
|
|
||
| var resource = Assert.Single(snapshots); | ||
| Assert.Equal("api", resource.Name); | ||
| Assert.NotNull(server.Target.WatchResourcesRequest); | ||
| Assert.Contains(AuxiliaryBackchannelCapabilities.V4, server.Target.WatchResourcesRequest.ClientCapabilities); | ||
| } |
PR Testing ReportPR Information
CLI Version Verification
Changes AnalyzedFiles Changed
Change Categories
Test Scenarios ExecutedScenario 1: Dogfood CLI install and version verificationObjective: Confirm the PR dogfood CLI artifact is available and matches the PR head commit. Steps:
Evidence:
Observations:
Scenario 2: CLI/AppHost backchannel typed resource propertiesObjective: Verify a real AppHost created with the PR templates can start under the PR CLI and that aspire describe receives typed JSON resource properties through the changed auxiliary backchannel path. Steps:
Evidence:
Observations:
Scenario 3: Non-empty output directory rejectionObjective: Verify template creation still fails safely for an invalid non-empty output directory while using the PR CLI/hive. Steps:
Evidence:
Expected Unhappy-Path Outcome:
Observations:
Summary
Overall Result✅ PR VERIFIED The PR dogfood CLI installed successfully after waiting for artifacts, matched the PR head commit, and passed targeted real-command scenarios covering the changed CLI/AppHost backchannel behavior plus dashboard resource visibility evidence. PR Comment StatusNot posted. Posting requires explicit confirmation. Cleanup / Inspection StatusArtifacts copied to /Users/adamratzman/.copilot/session-state/07fa9b16-c557-4cf9-90e3-b2a5571b252a/files/pr-17507-testing. The temp dogfood workspace can be removed after final reporting. |
| public bool SupportsV3 => _capabilities.Contains(AuxiliaryBackchannelCapabilities.V3); | ||
|
|
||
| /// <inheritdoc /> | ||
| public bool SupportsV4 => _capabilities.Contains(AuxiliaryBackchannelCapabilities.V4); |
There was a problem hiding this comment.
No. V3 is new. Use it.
James Newton-King (JamesNK)
left a comment
James Newton-King (JamesNK)
left a comment
There was a problem hiding this comment.
Looks good. Before merging, can you run the PR tester with an older released CLI against the new app host to verify backward compatibility? Specifically, confirm that an old CLI (without V4 capability) can still get resource data from the new app host — i.e., properties are serialized as strings for legacy callers.
Use the existing auxiliary backchannel V3 capability for typed resource properties and stop advertising a new V4 capability. Also limit the watch-resource capability test to the first streamed snapshot so it does not wait for a never-ending watch stream. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
James Newton-King (@JamesNK) I ran the backward-compatibility PR-tester scenario you requested. PR Testing ReportPR Information
Version Verification
Changes AnalyzedThis PR changes the Aspire CLI/AppHost auxiliary backchannel resource snapshot contract. James requested a backward-compatibility PR-tester pass confirming an older released CLI/client without the typed-property capability can still get resource data from the new AppHost with properties serialized as strings. Release tag evidence for the tested old CLI line is captured in Test Scenarios ExecutedScenario 1: Legacy released CLI with PR AppHost packagesObjective: Create/run an app with the released Aspire CLI while pinning the AppHost SDK/package to the PR build, then confirm the new AppHost starts successfully. Steps:
Evidence:
Scenario 2: Legacy resource snapshot request gets string propertiesObjective: Confirm a legacy caller using the old 13.3.5 request shape receives string-valued properties from the new PR AppHost. Steps:
Observed legacy properties: {
"number": "42",
"flag": "True",
"list": "one,two",
"json": "{\"name\":\"value\"}"
}Evidence:
Scenario 3: V3-capable request still gets typed JSONObjective: Sanity-check that typed properties remain available when the caller advertises the current typed-property capability. Steps:
Observed V3 properties: {
"number": 42,
"flag": true,
"list": ["one", "two"],
"json": { "name": "value" }
}Evidence:
Harness Notes
Summary
Overall ResultPR verified for James's backward-compatibility scenario. An older released Aspire CLI/client shape can get resource data from the new PR AppHost, and resource properties are serialized as strings for legacy callers. |
f984d4f
into
microsoft:main
|
Adam Ratzman (@adamint) main is 13.5, now if you want this in 13.4 then you need to backport. |
|
/backport to release/13.4 |
|
Started backporting to |
Description
Fixes the remaining items from #17244.
This preserves auxiliary backchannel compatibility for existing clients that deserialize
ResourceSnapshot.Propertiesas string values. Legacy resource snapshot calls and clients that do not opt in continue to receive string/null property values, while clients that advertise the newaux.v4capability receive JSON-valued properties for numbers, booleans, and arrays. The Aspire CLI now sends its client capabilities when using the v2 resource get/watch calls so it can opt in to the typed JSON behavior.This also removes the dead dashboard
showStartCommandmenu filter. Visible Start commands are now handled like other visible resource commands instead of being filtered by a duplicate call-site flag.User-facing behavior
Older auxiliary backchannel clients remain compatible with resource properties that contain non-string values. Newer clients can opt in to typed JSON resource properties by sending
aux.v4inClientCapabilitiesonGetResourcesRequestorWatchResourcesRequest.Dashboard resource action menus include the Start command whenever that command is visible in the resource command model.
Validation:
dotnet test --project tests/Aspire.Hosting.Tests/Aspire.Hosting.Tests.csproj --no-launch-profile -- --filter-method "*.GetResourceSnapshotsAsync_MapsNonStringPropertiesAsStringsForLegacyCallers" --filter-method "*.GetResourcesAsync_MapsNonStringPropertiesAsJsonForV4Callers" --filter-method "*.RequestWithTraceContext_PreservesRequestProperties" --filter-method "*.GetCapabilitiesAsyncReturnsCurrentCapabilities" --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"dotnet test --project tests/Aspire.Cli.Tests/Aspire.Cli.Tests.csproj --no-launch-profile -- --filter-class "*.AppHostAuxiliaryBackchannelTests" --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"dotnet test --project tests/Aspire.Dashboard.Tests/Aspire.Dashboard.Tests.csproj --no-launch-profile -- --filter-class "*.ResourceMenuBuilderTests" --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"git diff --checkFixes #17244
Checklist
<remarks />and<code />elements on your triple slash comments?