Repository navigation
[release/17.0] Warn on unsupported AspireCliInvocationMode values - #22015
Closed
Mitch Denny (mitchdenny) wants to merge 1 commit into
Closed
Mitch Denny (mitchdenny) wants to merge 1 commit into
Mitch Denny (mitchdenny) wants to merge 1 commit into
Conversation
## Description `AspireCliInvocationMode` only recognized `Dnx` and `DnxPinned`; every other value (for example `DnxPinnedd` or `DefinitelyInvalid`) silently fell through to `Path` resolution, so typos never surfaced. **Root cause:** `_ResolveAspireCliInvocation` passed the raw value to `ResolveAspireCliInvocation`, which treats anything other than `Dnx`/`DnxPinned` as `Path`. The only validation target (`_ValidateAspireCliDnxInvocationMode`) is conditioned on a DNX mode, so it was skipped for unrecognized values. **Fix:** `_ResolveAspireCliInvocation` emits a new suppressible `ASPIRE014` warning when the value is not `Path`, `Dnx` or `DnxPinned` (case-insensitive), and resolution continues in `Path` mode as before. I chose a warning instead of an error so that existing builds keep working while the mistake becomes visible. `ASPIRE013` is skipped because open PRs #21288 and #19880 both propose it. `docs/list-of-diagnostics.md` documents `ASPIRE014`. Fixes #19714 ### Before (main) Playground `DotnetProject.AppHost` with `-p:AspireUseCliBundle=true -p:AspireCliInvocationMode=DefinitelyInvalid -t:_ResolveAspireCliInvocation -v:diag`: ```text Task Parameter:AspireCliInvocationMode=DefinitelyInvalid Aspire CLI invocation selection: Dnx (aspire: , dnx: /root/aspire/.dotnet/dnx) <- Path-mode fallback (no aspire on PATH) 0 Error(s) 0 Warning(s) ``` A full `dotnet build` of the AppHost with the same value also reported `0 Warning(s)`. ### After ```text Aspire.Hosting.AppHost.in.targets(221,5): warning ASPIRE014: DotnetProject.AppHost is configured with AspireCliInvocationMode=DefinitelyInvalid, which is not a supported value. Supported values are Path, Dnx, and DnxPinned. The default Path invocation mode will be used. Aspire CLI invocation selection: Dnx (aspire: , dnx: /root/aspire/.dotnet/dnx) 0 Error(s) 1 Warning(s) ``` Valid values (`Path`, `DnxPinned`) still resolve to `Aspire` and `Dnx` without a warning. ### End-to-end with Aspire I ran `dotnet run` on `playground/DotnetProject/DotnetProject.AppHost` with `AspireCliInvocationMode=DefinitelyInvalid` and the Aspire CLI built from this branch (`17.0.0-dev`) on `PATH`. The warning appears during the build, and the AppHost then launches through `aspire run` (Path mode): ```text Building... ...Aspire.Hosting.AppHost.in.targets(221,5): warning ASPIRE014: DotnetProject.AppHost is configured with AspireCliInvocationMode=DefinitelyInvalid, which is not a supported value. Supported values are Path, Dnx, and DnxPinned. The default Path invocation mode will be used. Connecting to AppHost... Starting dashboard... AppHost: DotnetProject.AppHost.csproj Press CTRL+C to stop the AppHost and exit. ``` (The dashboard was skipped locally with `SkipDashboardProjectReference=true`.) ### Tests New tests in `tests/Aspire.Hosting.Sdk.Tests/AppHostSdkTargetsTests.cs`: - `ComputeRunArgumentsWarnsAndUsesPathModeWhenInvocationModeIsUnsupported` (`DefinitelyInvalid`, `DnxPinnedd`): asserts that `ASPIRE014` is emitted and that the fake `aspire` from `PATH` is selected. - `ComputeRunArgumentsDoesNotWarnWhenInvocationModeIsSupported` (unset, `Path`, `pAtH`, `Dnx`, `DnxPinned`). - `UnsupportedInvocationModeWarningCanBeSuppressed` (`NoWarn=ASPIRE014`). Without the fix, the two unsupported-value cases fail (`total: 8, failed: 2`). With the fix, the full `Aspire.Hosting.Sdk.Tests` project passes: 98 total, 94 passed, 0 failed, 4 skipped (Windows-only). ### Playground No playground file changed. In-repo playground AppHosts don't import `Aspire.Hosting.AppHost.props`, so `AspireUseCliBundle` isn't enabled by default and the packaged tasks assembly isn't present. Setting `AspireCliInvocationMode` in a playground project would therefore have no effect in a normal repo build. The playground proof above runs the existing `DotnetProject.AppHost` and passes those properties on the command line. ## Checklist - Is this feature complete? - [x] Yes. Ready to ship. - [ ] No. Follow-up changes expected. - Are you including unit tests for the changes and scenario tests if relevant? - [x] Yes - [ ] No - Did you add public API? - [ ] Yes - [x] No - Does the change make any security assumptions or guarantees? - [ ] Yes - [x] No **🤖 Posted by AspireAgent (AI Agent)** Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> (cherry picked from commit 7b48978)
14 tasks
This comment has been minimized.
This comment has been minimized.
Mitch Denny (mitchdenny)
marked this pull request as ready for review
November 10, 2026 04:55
Member
Author
|
Approved for inclusion in release/17.0. Do not merge yet. |
Mitch Denny (mitchdenny)
requested review from
James Newton-King (JamesNK),
Adam Ratzman (adamint),
David Negstad (danegsta),
David Fowler (davidfowl),
Eric Erhardt (eerhardt),
Ella Hathaway (ellahathaway),
Karol Zadora-Przylecki (karolz-ms) and
Ankit Jain (radical)
as code owners
November 10, 2026 05:38
Mitch Denny (mitchdenny)
force-pushed
the
mitchdenny-backport-17-0-18-pr21545
branch
from
November 10, 2026 05:39
a67e74b to
6c7ac15
Compare
Contributor
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 22015Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 22015" |
This comment has been minimized.
This comment has been minimized.
Mitch Denny (mitchdenny)
force-pushed
the
mitchdenny-backport-17-0-18-pr21545
branch
from
November 10, 2026 06:56
6c7ac15 to
a67e74b
Compare
Mitch Denny (mitchdenny)
force-pushed
the
mitchdenny-backport-17-0-18-pr21545
branch
from
November 10, 2026 07:23
a67e74b to
6c7ac15
Compare
Contributor
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Mitch Denny (mitchdenny)
force-pushed
the
mitchdenny-backport-17-0-18-pr21545
branch
from
November 10, 2026 09:06
6c7ac15 to
a67e74b
Compare
Contributor
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Backport of #21545 to
release/17.0(cherry-picked from 7b48978 with no modifications). Layer 18 of 55 in the release/17.0 backport stack; stacked on #22014.Backport recommendation: No — New ASPIRE014 warning; enhancement.
Backport of #21545
Checklist
<remarks />and<code />elements on your triple slash comments?