Honor resource command confirmations in CLI#16669
Open
maddymontaquila wants to merge 1 commit intomainfrom
Open
Honor resource command confirmations in CLI#16669maddymontaquila wants to merge 1 commit intomainfrom
maddymontaquila wants to merge 1 commit intomainfrom
Conversation
Flow resource command confirmation metadata through the backchannel and prompt before executing confirmed commands from aspire resource. Add --non-interactive as an explicit confirmation override for automation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
maddymontaquila
requested review from
JamesNK,
danegsta,
davidfowl,
karolz-ms and
mitchdenny
as code owners
Contributor
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 16669Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 16669" |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR makes aspire resource <resource> <command> honor per-command confirmation metadata emitted by the AppHost. The CLI now reads a ConfirmationMessage from resource snapshots, prompts the user before executing confirmed commands, and introduces a --non-interactive automation override intended to accept confirmations without prompting.
Changes:
- Flow a new
ConfirmationMessagefield from Hosting backchannel snapshots through the CLI resource JSON model. - Add confirmation gating to
ResourceCommandHelperprior to invoking backchannel command execution. - Add/extend CLI unit tests, update skill guidance docs, and update localization resources for the new option description.
Reviewed changes
Copilot reviewed 25 out of 26 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Aspire.Cli.Tests/TestServices/TestAppHostAuxiliaryBackchannel.cs | Adds handler/call-count support to validate whether commands were executed in tests. |
| tests/Aspire.Cli.Tests/Commands/ResourceCommandTests.cs | Adds a parsing/invocation test for --non-interactive on resource. |
| tests/Aspire.Cli.Tests/Commands/ResourceCommandHelperTests.cs | Adds coverage for confirmed/declined/non-interactive confirmation flows. |
| tests/Aspire.Cli.Tests/Backchannel/ResourceSnapshotMapperTests.cs | Verifies ConfirmationMessage is mapped into CLI resource JSON. |
| src/Shared/Model/Serialization/ResourceJson.cs | Extends ResourceCommandJson with ConfirmationMessage. |
| src/Aspire.Hosting/Backchannel/BackchannelDataTypes.cs | Extends ResourceSnapshotCommand with ConfirmationMessage. |
| src/Aspire.Hosting/Backchannel/AuxiliaryBackchannelRpcTarget.cs | Flows ConfirmationMessage into snapshot RPC payloads. |
| src/Aspire.Cli/Commands/ResourceCommandHelper.cs | Adds confirmation lookup + prompt prior to executing resource commands. |
| src/Aspire.Cli/Commands/ResourceCommand.cs | Adds --non-interactive option and passes a confirmation binding to helper methods. |
| src/Aspire.Cli/Backchannel/ResourceSnapshotMapper.cs | Maps ConfirmationMessage into ResourceCommandJson. |
| src/Aspire.Cli/Resources/ResourceCommandStrings.resx | Adds localized string for --non-interactive description. |
| src/Aspire.Cli/Resources/ResourceCommandStrings.Designer.cs | Adds strongly-typed accessor for the new resource string. |
| src/Aspire.Cli/Resources/xlf/ResourceCommandStrings.cs.xlf | Adds NonInteractiveOptionDescription translation unit. |
| src/Aspire.Cli/Resources/xlf/ResourceCommandStrings.de.xlf | Adds NonInteractiveOptionDescription translation unit. |
| src/Aspire.Cli/Resources/xlf/ResourceCommandStrings.es.xlf | Adds NonInteractiveOptionDescription translation unit. |
| src/Aspire.Cli/Resources/xlf/ResourceCommandStrings.fr.xlf | Adds NonInteractiveOptionDescription translation unit. |
| src/Aspire.Cli/Resources/xlf/ResourceCommandStrings.it.xlf | Adds NonInteractiveOptionDescription translation unit. |
| src/Aspire.Cli/Resources/xlf/ResourceCommandStrings.ja.xlf | Adds NonInteractiveOptionDescription translation unit. |
| src/Aspire.Cli/Resources/xlf/ResourceCommandStrings.ko.xlf | Adds NonInteractiveOptionDescription translation unit. |
| src/Aspire.Cli/Resources/xlf/ResourceCommandStrings.pl.xlf | Adds NonInteractiveOptionDescription translation unit. |
| src/Aspire.Cli/Resources/xlf/ResourceCommandStrings.pt-BR.xlf | Adds NonInteractiveOptionDescription translation unit. |
| src/Aspire.Cli/Resources/xlf/ResourceCommandStrings.ru.xlf | Adds NonInteractiveOptionDescription translation unit. |
| src/Aspire.Cli/Resources/xlf/ResourceCommandStrings.tr.xlf | Adds NonInteractiveOptionDescription translation unit. |
| src/Aspire.Cli/Resources/xlf/ResourceCommandStrings.zh-Hans.xlf | Adds NonInteractiveOptionDescription translation unit. |
| src/Aspire.Cli/Resources/xlf/ResourceCommandStrings.zh-Hant.xlf | Adds NonInteractiveOptionDescription translation unit. |
| .agents/skills/aspire/references/resource-management.md | Updates agent guidance to mention confirmation prompts and --non-interactive. |
Files not reviewed (1)
- src/Aspire.Cli/Resources/ResourceCommandStrings.Designer.cs: Language not supported
Comment on lines
+36
to
+39
| private static readonly Option<bool> s_nonInteractiveOption = new("--non-interactive") | ||
| { | ||
| Description = ResourceCommandStrings.NonInteractiveOptionDescription | ||
| }; |
There was a problem hiding this comment.
--non-interactive is already defined as a recursive global option on RootCommand (see RootCommand.NonInteractiveOption). Adding a second Option<bool>("--non-interactive") on ResourceCommand can create ambiguous binding depending on where the flag appears (e.g. aspire --non-interactive resource ... may bind to the root option while this code reads the resource-scoped option), which would break the intended auto-confirm behavior in non-interactive runs. Consider removing the resource-scoped option and instead using RootCommand.NonInteractiveOption for PromptBinding.Create(...) (or introduce a distinct option name for the resource-confirmation override).
Suggested change
| private static readonly Option<bool> s_nonInteractiveOption = new("--non-interactive") | |
| { | |
| Description = ResourceCommandStrings.NonInteractiveOptionDescription | |
| }; | |
| private static readonly Option<bool> s_nonInteractiveOption = RootCommand.NonInteractiveOption; |
| return null; | ||
| } | ||
|
|
||
| var command = resources[0].Commands.FirstOrDefault(c => string.Equals(c.Name, commandName, StringComparison.OrdinalIgnoreCase)); |
There was a problem hiding this comment.
GetConfirmationMessageAsync looks up the command by name but doesn't consider the command state. If a command exists but is Disabled/Hidden, the CLI will still prompt for confirmation and then the backchannel execution will fail anyway, which is a confusing flow. Consider filtering to State == "Enabled" (or whatever enabled value is expected) before prompting, or skipping confirmation when the command isn't executable.
Suggested change
| var command = resources[0].Commands.FirstOrDefault(c => string.Equals(c.Name, commandName, StringComparison.OrdinalIgnoreCase)); | |
| var command = resources[0].Commands.FirstOrDefault(c => | |
| string.Equals(c.Name, commandName, StringComparison.OrdinalIgnoreCase) && | |
| string.Equals(c.State, "Enabled", StringComparison.OrdinalIgnoreCase)); |
Contributor
|
🎬 CLI E2E Test Recordings — 76 recordings uploaded (commit View all recordings
📹 Recordings uploaded automatically from CI run #25229282820 |
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
Make
aspire resource <resource> <command>honor resource command confirmation messages before executing commands. The CLI now readsConfirmationMessagefrom resource snapshots, prompts before invoking commands that require confirmation, and supports--non-interactiveas an explicit automation override that accepts the confirmation without prompting.This also flows confirmation metadata through the AppHost backchannel and CLI resource JSON model, updates the generated Aspire agent skill guidance, and adds focused CLI tests for confirmed, declined, and non-interactive command execution.
Validation:
dotnet test --project tests\Aspire.Cli.Tests\Aspire.Cli.Tests.csproj --no-launch-profile -- --filter-class "*.ResourceCommandHelperTests" --filter-class "*.ResourceCommandTests" --filter-class "*.ResourceSnapshotMapperTests" --filter-class "*.AgentInitCommandTests" --filter-class "*.CommonAgentApplicatorsTests" --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"dotnet build /t:UpdateXlf src\Aspire.Cli\Aspire.Cli.csprojFixes # (issue)
Checklist
<remarks />and<code />elements on your triple slash comments?aspire.devissue: