Repository navigation
Warn on unsupported AspireCliInvocationMode values - #21545
Conversation
Unsupported values were silently treated as Path. Emit ASPIRE014 from _ResolveAspireCliInvocation so misspelled modes such as DnxPinnedd are visible. Fixes #19714 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 -- 21545Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 21545" |
github-actions
Bot
added
the
needs-area-label
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues are recorded for the invocation-mode warning and fallback changes.
No open issues 🎉
What changed in this PR
Addresses #19714 by warning about unsupported AspireCliInvocationMode values in AppHost CLI resolution while preserving the existing Path fallback behavior.
Changes:
- Adds suppressible warning
ASPIRE014for values other thanPath,Dnx, orDnxPinned, ignoring case. - Adds tests for invalid values, supported values, fallback resolution, and warning suppression.
- Documents the new diagnostic and fallback behavior.
| File | Description |
|---|---|
| tests/Aspire.Hosting.Sdk.Tests/AppHostSdkTargetsTests.cs | Adds validation, fallback, and suppression tests. |
| src/Aspire.Hosting.AppHost/build/Aspire.Hosting.AppHost.in.targets | Emits ASPIRE014 for unsupported invocation modes. |
| docs/list-of-diagnostics.md | Documents ASPIRE014 and the default Path fallback. |
Review effort: Balanced
|
CI note: 🤖 Posted by AspireAgent (AI Agent) |
David Fowler (davidfowl)
added
the
agent:working
Copilot SWE Agent
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Merged
Local tests: 🤖 Posted by AspireAgent (AI Agent) |
David Fowler (davidfowl)
removed
the
agent:working
Tests selector45 / 101 PR test projects · 3 PR jobs, from 2 changed files. Selected PR test projects (45 / 101)
Selected PR jobs (3)
How these were chosen — grouped by what changed
🔧 show 43
📦 affected project 🧪 Job reasons
Selection computed for commit |
Ankit Jain (radical)
left a comment
Ankit Jain (radical)
left a comment
There was a problem hiding this comment.
No confirmed issues found in review.
|
Pull request created: #2085
|
|
📝 Documentation has been drafted in microsoft/aspire.dev#2085 targeting Documents new ASPIRE014 warning. Triggered signal Note This draft PR needs human review before merging. |
Description
AspireCliInvocationModeonly recognizedDnxandDnxPinned; every other value (for exampleDnxPinneddorDefinitelyInvalid) silently fell through toPathresolution, so typos never surfaced.Root cause:
_ResolveAspireCliInvocationpassed the raw value toResolveAspireCliInvocation, which treats anything other thanDnx/DnxPinnedasPath. The only validation target (_ValidateAspireCliDnxInvocationMode) is conditioned on a DNX mode, so it was skipped for unrecognized values.Fix:
_ResolveAspireCliInvocationemits a new suppressibleASPIRE014warning when the value is notPath,DnxorDnxPinned(case-insensitive), and resolution continues inPathmode as before. I chose a warning instead of an error so that existing builds keep working while the mistake becomes visible.ASPIRE013is skipped because open PRs #21288 and #19880 both propose it.docs/list-of-diagnostics.mddocumentsASPIRE014.Fixes #19714
Before (main)
Playground
DotnetProject.AppHostwith-p:AspireUseCliBundle=true -p:AspireCliInvocationMode=DefinitelyInvalid -t:_ResolveAspireCliInvocation -v:diag:A full
dotnet buildof the AppHost with the same value also reported0 Warning(s).After
Valid values (
Path,DnxPinned) still resolve toAspireandDnxwithout a warning.End-to-end with Aspire
I ran
dotnet runonplayground/DotnetProject/DotnetProject.AppHostwithAspireCliInvocationMode=DefinitelyInvalidand the Aspire CLI built from this branch (17.0.0-dev) onPATH. The warning appears during the build, and the AppHost then launches throughaspire run(Path mode):(The dashboard was skipped locally with
SkipDashboardProjectReference=true.)Tests
New tests in
tests/Aspire.Hosting.Sdk.Tests/AppHostSdkTargetsTests.cs:ComputeRunArgumentsWarnsAndUsesPathModeWhenInvocationModeIsUnsupported(DefinitelyInvalid,DnxPinnedd): asserts thatASPIRE014is emitted and that the fakeaspirefromPATHis 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 fullAspire.Hosting.Sdk.Testsproject 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, soAspireUseCliBundleisn't enabled by default and the packaged tasks assembly isn't present. SettingAspireCliInvocationModein a playground project would therefore have no effect in a normal repo build. The playground proof above runs the existingDotnetProject.AppHostand passes those properties on the command line.Checklist
🤖 Posted by AspireAgent (AI Agent)