Fix --apphost directory resolution for commands#16406
Fix --apphost directory resolution for commands#16406adamint wants to merge 5 commits intomicrosoft:mainfrom
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 16406Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 16406" |
There was a problem hiding this comment.
Pull request overview
Aligns stop-side CLI commands with start behavior by normalizing explicit --apphost inputs (including directory paths) through IProjectLocator.UseOrFindAppHostProjectFileAsync(...) before attempting backchannel socket resolution, fixing #16360.
Changes:
- Updates
AppHostConnectionResolverto resolve explicit--apphostviaIProjectLocator, and to surface project-resolution failures distinctly from “no running AppHost found”. - Plumbs
IProjectLocatorinto commands that useAppHostConnectionResolverand adjusts exit-code handling to preserve existing UX. - Adds a localized interaction string for unsupported AppHosts and adds/updates unit + E2E coverage for directory-valued
--apphost.
Reviewed changes
Copilot reviewed 31 out of 32 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/Aspire.Cli.Tests/Commands/StopCommandTests.cs | Adds unit coverage for explicit --apphost project-resolution failure vs. successful resolution path. |
| tests/Aspire.Cli.EndToEnd.Tests/StopNonInteractiveTests.cs | Updates stop non-interactive E2E scenario to exercise directory-valued --apphost. |
| src/Aspire.Cli/Backchannel/AppHostConnectionResolver.cs | Resolves explicit --apphost through IProjectLocator and returns structured project-resolution errors (exit code + message). |
| src/Aspire.Cli/Projects/ProjectLocator.cs | Adds ProjectLocatorErrorHelper to centralize mapping from ProjectLocatorFailureReason to exit code + localized message. |
| src/Aspire.Cli/Commands/BaseCommand.cs | Switches to shared ProjectLocatorErrorHelper mapping for consistent error messages/exit codes. |
| src/Aspire.Cli/Commands/StopCommand.cs | Uses updated resolver and preserves “no running AppHost” UX while returning distinct exit codes for resolution failures. |
| src/Aspire.Cli/Commands/WaitCommand.cs | Plumbs IProjectLocator into resolver and returns project-resolution exit codes when applicable. |
| src/Aspire.Cli/Commands/ResourceCommand.cs | Plumbs IProjectLocator into resolver and returns project-resolution exit codes when applicable. |
| src/Aspire.Cli/Commands/LogsCommand.cs | Plumbs IProjectLocator into resolver and returns project-resolution exit codes when applicable. |
| src/Aspire.Cli/Commands/DescribeCommand.cs | Plumbs IProjectLocator into resolver and returns project-resolution exit codes when applicable. |
| src/Aspire.Cli/Commands/ExportCommand.cs | Plumbs IProjectLocator into resolver (connection resolution now normalizes explicit --apphost). |
| src/Aspire.Cli/Commands/McpToolsCommand.cs | Plumbs IProjectLocator into resolver and preserves “no running AppHost” UX vs. resolution errors. |
| src/Aspire.Cli/Commands/McpCallCommand.cs | Plumbs IProjectLocator into resolver and returns project-resolution exit codes when applicable. |
| src/Aspire.Cli/Commands/TelemetryLogsCommand.cs | Plumbs IProjectLocator into resolver (telemetry commands now normalize explicit --apphost). |
| src/Aspire.Cli/Commands/TelemetrySpansCommand.cs | Plumbs IProjectLocator into resolver (telemetry commands now normalize explicit --apphost). |
| src/Aspire.Cli/Commands/TelemetryTracesCommand.cs | Plumbs IProjectLocator into resolver (telemetry commands now normalize explicit --apphost). |
| src/Aspire.Cli/Commands/TelemetryCommandHelpers.cs | Treats project-resolution failures as errors (propagates exit code) while keeping “no running AppHost” non-fatal behavior. |
| src/Aspire.Cli/Resources/InteractionServiceStrings.resx | Adds NoSupportedAppHostsFound localized resource. |
| src/Aspire.Cli/Resources/InteractionServiceStrings.Designer.cs | Regenerates strongly-typed accessor for NoSupportedAppHostsFound. |
| src/Aspire.Cli/Resources/xlf/InteractionServiceStrings.cs.xlf | Adds NoSupportedAppHostsFound trans-unit. |
| src/Aspire.Cli/Resources/xlf/InteractionServiceStrings.de.xlf | Adds NoSupportedAppHostsFound trans-unit. |
| src/Aspire.Cli/Resources/xlf/InteractionServiceStrings.es.xlf | Adds NoSupportedAppHostsFound trans-unit. |
| src/Aspire.Cli/Resources/xlf/InteractionServiceStrings.fr.xlf | Adds NoSupportedAppHostsFound trans-unit. |
| src/Aspire.Cli/Resources/xlf/InteractionServiceStrings.it.xlf | Adds NoSupportedAppHostsFound trans-unit. |
| src/Aspire.Cli/Resources/xlf/InteractionServiceStrings.ja.xlf | Adds NoSupportedAppHostsFound trans-unit. |
| src/Aspire.Cli/Resources/xlf/InteractionServiceStrings.ko.xlf | Adds NoSupportedAppHostsFound trans-unit. |
| src/Aspire.Cli/Resources/xlf/InteractionServiceStrings.pl.xlf | Adds NoSupportedAppHostsFound trans-unit. |
| src/Aspire.Cli/Resources/xlf/InteractionServiceStrings.pt-BR.xlf | Adds NoSupportedAppHostsFound trans-unit. |
| src/Aspire.Cli/Resources/xlf/InteractionServiceStrings.ru.xlf | Adds NoSupportedAppHostsFound trans-unit. |
| src/Aspire.Cli/Resources/xlf/InteractionServiceStrings.tr.xlf | Adds NoSupportedAppHostsFound trans-unit. |
| src/Aspire.Cli/Resources/xlf/InteractionServiceStrings.zh-Hans.xlf | Adds NoSupportedAppHostsFound trans-unit. |
| src/Aspire.Cli/Resources/xlf/InteractionServiceStrings.zh-Hant.xlf | Adds NoSupportedAppHostsFound trans-unit. |
Files not reviewed (1)
- src/Aspire.Cli/Resources/InteractionServiceStrings.Designer.cs: Language not supported
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
radical
left a comment
There was a problem hiding this comment.
Looks good — clean extraction of the project-resolution logic into ProjectLocatorErrorHelper, consistent plumbing of IProjectLocator through all command constructors, and clear separation between "project not found" (hard error) and "AppHost not running" (informational) in the result model.
The new AppHostNotRunningAtPath message with relative paths is a nice UX improvement, and the test coverage (unit, integration, and E2E with directory resolution) is solid.
One non-blocking suggestion left inline about tightening the IsProjectResolutionError check.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Description
Fix
--apphostresolution for stop-side CLI commands.Fixes #16360
This change:
--apphostinputs viaIProjectLocator, with explicit directory-specific diagnostics for zero or multiple AppHosts--apphostReviewer testing steps using the built CLI:
./localhive.sh~/.aspire/bin/aspire --version~/.aspire/bin/aspire new aspire-starter --name ReviewApp --output /tmp/ReviewApp --non-interactivecd /tmp/ReviewApp/ReviewApp.AppHost && dotnet runcd /tmp && ~/.aspire/bin/aspire stop --non-interactive --apphost ReviewAppExpected result: the running AppHost is found and stopped via the solution directory path.
Validation performed:
./dotnet.sh test --project tests/Aspire.Cli.Tests/Aspire.Cli.Tests.csproj --no-launch-profile -- --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"Checklist
<remarks />and<code />elements on your triple slash comments?aspire.devissue: