Skip to content

[release/17.0] Warn on unsupported AspireCliInvocationMode values - #22015

Closed
Mitch Denny (mitchdenny) wants to merge 1 commit into
mitchdenny-backport-17-0-17-pr21830from
mitchdenny-backport-17-0-18-pr21545
Closed

Mitch Denny (mitchdenny) wants to merge 1 commit into
mitchdenny-backport-17-0-17-pr21830from
mitchdenny-backport-17-0-18-pr21545

Conversation

@mitchdenny

Copy link
Copy Markdown
Member

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

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

## 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)
@github-actions

This comment has been minimized.

@mitchdenny
Mitch Denny (mitchdenny) marked this pull request as ready for review November 10, 2026 04:55
@mitchdenny

Copy link
Copy Markdown
Member Author

Approved for inclusion in release/17.0. Do not merge yet.

@github-actions

Copy link
Copy Markdown
Contributor

🚀 Dogfood this PR with:

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

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

Or

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

@github-actions

This comment has been minimized.

@mitchdenny
Mitch Denny (mitchdenny) force-pushed the mitchdenny-backport-17-0-18-pr21545 branch from 6c7ac15 to a67e74b Compare November 10, 2026 06:56
@mitchdenny
Mitch Denny (mitchdenny) force-pushed the mitchdenny-backport-17-0-18-pr21545 branch from a67e74b to 6c7ac15 Compare November 10, 2026 07:23
@github-actions

Copy link
Copy Markdown
Contributor

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

@github-actions

Copy link
Copy Markdown
Contributor

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants