Fail cleanly when AppHost selection needs a prompt in non-interactive mode (#17619)#18297
Conversation
…eractive mode When no AppHost is found in the current directory but other AppHosts are running on the system, commands like 'aspire describe --non-interactive' fell through to an interactive selection prompt, which threw and surfaced as 'An unexpected error occurred' with a generic exit code. AppHostConnectionResolver now checks ICliHostEnvironment.SupportsInteractiveInput before prompting: - Only out-of-scope AppHosts running: returns the command's standard not-found error with CliExitCodes.FailedToFindProject. - Multiple in-scope AppHosts: returns a new actionable error telling the user to pass --apphost, also with FailedToFindProject. Fixes #17619 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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 -- 18297Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 18297" |
There was a problem hiding this comment.
Pull request overview
This PR fixes a crash in aspire describe --non-interactive (and other backchannel commands) that occurred when the CLI attempted an interactive AppHost-selection prompt in non-interactive mode. The fix adds early detection of non-interactive mode in AppHostConnectionResolver and returns clean, actionable error messages instead of letting the prompt throw.
Changes:
- Added non-interactive guards in
AppHostConnectionResolver.ResolveConnectionAsyncfor two prompt paths (multiple in-scope AppHosts and only out-of-scope AppHosts), returning descriptive errors withCliExitCodes.FailedToFindProject. - Wired
ICliHostEnvironmentthroughCommonCommandServicesand all 12 command call sites that constructAppHostConnectionResolver. - Added a new
MultipleAppHostsNonInteractiveresource string with all localization.xlfplaceholders, and two targeted unit tests.
Show a summary per file
| File | Description |
|---|---|
| src/Aspire.Cli/Backchannel/AppHostConnectionResolver.cs | Core fix: checks hostEnvironment.SupportsInteractiveInput before prompting; accepts new ICliHostEnvironment parameter |
| src/Aspire.Cli/Commands/CommonCommandServices.cs | Adds ICliHostEnvironment hostEnvironment as a required constructor parameter with a public property |
| src/Aspire.Cli/Commands/DescribeCommand.cs | Passes services.HostEnvironment to resolver |
| src/Aspire.Cli/Commands/ExportCommand.cs | Passes services.HostEnvironment to resolver |
| src/Aspire.Cli/Commands/LogsCommand.cs | Passes services.HostEnvironment to resolver |
| src/Aspire.Cli/Commands/StopCommand.cs | Passes services.HostEnvironment to resolver |
| src/Aspire.Cli/Commands/ResourceCommand.cs | Passes services.HostEnvironment to resolver |
| src/Aspire.Cli/Commands/WaitCommand.cs | Passes services.HostEnvironment to resolver |
| src/Aspire.Cli/Commands/McpCallCommand.cs | Passes services.HostEnvironment to resolver |
| src/Aspire.Cli/Commands/McpToolsCommand.cs | Passes services.HostEnvironment to resolver |
| src/Aspire.Cli/Commands/TelemetryLogsCommand.cs | Passes services.HostEnvironment to resolver |
| src/Aspire.Cli/Commands/TelemetrySpansCommand.cs | Passes services.HostEnvironment to resolver |
| src/Aspire.Cli/Commands/TelemetryTracesCommand.cs | Passes services.HostEnvironment to resolver |
| src/Aspire.Cli/Commands/TerminalAttachCommand.cs | Passes services.HostEnvironment to resolver |
| src/Aspire.Cli/Commands/TerminalPsCommand.cs | Passes services.HostEnvironment to resolver |
| src/Aspire.Cli/Resources/SharedCommandStrings.resx | New MultipleAppHostsNonInteractive resource string |
| src/Aspire.Cli/Resources/SharedCommandStrings.Designer.cs | Generated property for new resource string |
| src/Aspire.Cli/Resources/xlf/*.xlf (13 files) | Localization placeholders for the new string |
| tests/Aspire.Cli.Tests/Backchannel/AppHostConnectionResolverTests.cs | Two new tests for non-interactive branches + updates existing tests with new parameter |
| tests/Aspire.Cli.Tests/NuGet/NuGetPackagePrefetcherTests.cs | Adds 8th null! for the new CommonCommandServices parameter |
Copilot's findings
Files not reviewed (1)
- src/Aspire.Cli/Resources/SharedCommandStrings.Designer.cs: Generated file
- Files reviewed: 31/32 changed files
- Comments generated: 0
JamesNK
left a comment
There was a problem hiding this comment.
Clean fix. Both non-interactive prompt paths are correctly guarded, all call sites updated consistently, DI wiring is intact, and the two new tests cover both branches. No issues found.
PR Testing note — #18297 (merged)This PR is already MERGED ( |
Description
Fixes #17619 —
aspire describe --non-interactive(and other backchannel commands) crashes by attempting an interactive AppHost-selection prompt when no AppHost can be unambiguously resolved.Root cause
When
AppHostConnectionResolverfound multiple in-scope AppHosts, or only out-of-scope AppHosts, it always fell through to an interactive selection prompt. In non-interactive mode there is no console to prompt on, so the prompt threw instead of returning a clean error.Fix
AppHostConnectionResolvernow takes anICliHostEnvironmentand checksSupportsInteractiveInputbefore prompting:MultipleAppHostsNonInteractive) withFailedToFindProjectinstead of throwing.ICliHostEnvironmentthrough every caller (DescribeCommand,ExportCommand,LogsCommand,StopCommand, telemetry/MCP/resource/wait commands, and the terminal attach/ps commands).MultipleAppHostsNonInteractiveresource string (+ generated.xlfplaceholders).Tests
Adds
AppHostConnectionResolverTestscoverage for both non-interactive branches (only out-of-scope → not found; multiple in-scope → actionable error).Fixes #17619