Add command result support for resource commands#15622
Add command result support for resource commands#15622
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 15622Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 15622" |
|
Something I've been playing around with in my JWT scenario is writing the output directly to the user's clipboard - something worth thinking about. Works reasonably well for secrets |
davidfowl
force-pushed
the
davidfowl/command-results
branch
2 times, most recently
from
2f9c523 to
fc149ed
Compare
davidfowl
requested review from
JamesNK,
adamint,
danegsta,
karolz-ms and
mitchdenny
as code owners
There was a problem hiding this comment.
Pull request overview
Adds end-to-end support for resource commands to return structured result data (text/JSON) back to callers (Dashboard, CLI, MCP), flowing from hosting model → backchannel/gRPC → UI/CLI.
Changes:
- Introduces
CommandResultFormatand extendsExecuteCommandResult/CommandResults.Success(...)to carry result payload + format. - Extends dashboard gRPC/backchannel contracts and mapping to transport
result/result_format. - Updates Dashboard/CLI/MCP to surface command results, plus adds/updates tests and regenerated polyglot codegen snapshots.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Aspire.Hosting.Tests/ResourceCommandServiceTests.cs | Adds hosting tests for result propagation and replica aggregation behavior. |
| tests/Aspire.Hosting.CodeGeneration.TypeScript.Tests/Snapshots/TwoPassScanningGeneratedAspire.verified.ts | Updates TS generated API snapshots for new fields/enum. |
| tests/Aspire.Hosting.CodeGeneration.Rust.Tests/Snapshots/TwoPassScanningGeneratedAspire.verified.rs | Updates Rust generated API snapshots for new fields/enum. |
| tests/Aspire.Hosting.CodeGeneration.Python.Tests/Snapshots/TwoPassScanningGeneratedAspire.verified.py | Updates Python generated API snapshots for new fields/enum. |
| tests/Aspire.Hosting.CodeGeneration.Java.Tests/Snapshots/TwoPassScanningGeneratedAspire.verified.java | Updates Java generated API snapshots for new fields/enum. |
| tests/Aspire.Hosting.CodeGeneration.Go.Tests/Snapshots/TwoPassScanningGeneratedAspire.verified.go | Updates Go generated API snapshots for new fields/enum. |
| tests/Aspire.Cli.Tests/TestServices/TestInteractionService.cs | Adds a hook to capture DisplayMessage calls in CLI tests. |
| tests/Aspire.Cli.Tests/Mcp/ExecuteResourceCommandToolTests.cs | Verifies MCP tool returns additional content block when a command returns result data. |
| tests/Aspire.Cli.Tests/Commands/ResourceCommandHelperTests.cs | Adds CLI tests around displaying command results. |
| src/Aspire.Hosting/Dashboard/proto/dashboard_service.proto | Adds result and result_format fields + CommandResultFormat enum to the dashboard proto. |
| src/Aspire.Hosting/Dashboard/DashboardServiceData.cs | Extends command execution tuple to include result payload + format. |
| src/Aspire.Hosting/Dashboard/DashboardService.cs | Maps hosting command results into gRPC response fields. |
| src/Aspire.Hosting/Backchannel/BackchannelDataTypes.cs | Adds backchannel DTO properties for result payload + format. |
| src/Aspire.Hosting/Backchannel/AuxiliaryBackchannelRpcTarget.cs | Maps hosting ExecuteCommandResult to backchannel result fields. |
| src/Aspire.Hosting/ApplicationModel/ResourceCommandService.cs | Aggregates replica results and returns the first successful result payload/format. |
| src/Aspire.Hosting/ApplicationModel/ResourceCommandAnnotation.cs | Adds CommandResultFormat, ExecuteCommandResult.Result/ResultFormat, and CommandResults.Success(...) overload. |
| src/Aspire.Dashboard/ServiceClient/Partials.cs | Maps gRPC result fields into dashboard view model. |
| src/Aspire.Dashboard/Model/ResourceCommandResponseViewModel.cs | Adds result payload + format to the dashboard model and defines CommandResultFormat. |
| src/Aspire.Dashboard/Model/DashboardCommandExecutor.cs | Opens a text visualizer dialog on successful command results (locks to JSON when applicable). |
| src/Aspire.Cli/Mcp/Tools/ExecuteResourceCommandTool.cs | Returns command result as an additional MCP TextContentBlock on success. |
| src/Aspire.Cli/Commands/ResourceCommandHelper.cs | Displays command result in CLI after successful execution. |
| playground/Stress/Stress.AppHost/Program.cs | Adds sample commands returning JSON/text results in the stress playground. |
| Assert.Equal(0, exitCode); | ||
| Assert.NotNull(capturedMessage); | ||
| // Verify the brackets are escaped so Spectre doesn't interpret them as markup | ||
| // EscapeMarkup doubles [ and ] so [" becomes [[" | ||
| Assert.Contains("[[", capturedMessage); | ||
| Assert.Contains("]]", capturedMessage); | ||
| } |
There was a problem hiding this comment.
This test asserts that ResourceCommandHelper escapes Spectre markup (expects [[/]]), but in production the escaping is performed by ConsoleInteractionService.DisplayMessage when allowMarkup is false. With the current helper implementation, real output will be double-escaped; if the helper is fixed to stop pre-escaping, this test should be updated to reflect the actual contract (helper passes raw result; interaction service handles escaping), or TestInteractionService.DisplayMessage should mimic the production escaping behavior to catch double-escaping regressions.
There was a problem hiding this comment.
Fixed — this test was rewritten to verify raw text output (no escaping). The helper now uses DisplayRawText instead of DisplayMessage, so no markup escaping is involved.
davidfowl
force-pushed
the
davidfowl/command-results
branch
from
6c96bb0 to
4619b34
Compare
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
davidfowl
force-pushed
the
davidfowl/command-results
branch
from
4619b34 to
0898d6d
Compare
|
🎬 CLI E2E Test Recordings — 52 recordings uploaded (commit View recordings
📹 Recordings uploaded automatically from CI run #23632067527 |
| if (response.Result is not null) | ||
| { | ||
| interactionService.DisplayRawText(response.Result, ConsoleOutput.Standard); | ||
| } |
There was a problem hiding this comment.
Why only display result on success? I can imagine wanting to display output on error.
There was a problem hiding this comment.
Fixed — result is now displayed on both success and error paths.
| var content = new List<TextContentBlock> | ||
| { | ||
| new() { Text = $"Command '{commandName}' executed successfully on resource '{resourceName}'." } | ||
| }; | ||
|
|
||
| if (response.Result is not null) | ||
| { | ||
| content.Add(new TextContentBlock { Text = response.Result }); | ||
| } |
There was a problem hiding this comment.
As noted in previous comment, result is only available for success result.
There was a problem hiding this comment.
Fixed — MCP tool now returns result on error too via CallToolResult with IsError = true (instead of throwing).
| public sealed class DashboardCommandExecutor( | ||
| IDashboardClient dashboardClient, | ||
| IDialogService dialogService, | ||
| IServiceProvider serviceProvider, |
There was a problem hiding this comment.
Why not pass in DashboardDialogService directly? It and the containing type have the same lifetime (scoped) so shouldn't it be fine?
There was a problem hiding this comment.
I had a whole debate with the agent about blazor, and the fact that this is a singleton resolving a scoped service (which is badness)
There was a problem hiding this comment.
You're right — DashboardCommandExecutor is registered as scoped (TryAddScoped), not singleton. Fixed to inject DashboardDialogService directly.
| if (response.Result is not null) | ||
| { | ||
| var fixedFormat = response.ResultFormat == CommandResultFormat.Json ? DashboardUIHelpers.JsonFormat : null; | ||
| await TextVisualizerDialog.OpenDialogAsync(new OpenTextVisualizerDialogOptions | ||
| { | ||
| DialogService = serviceProvider.GetRequiredService<DashboardDialogService>(), | ||
| ValueDescription = command.GetDisplayName(), | ||
| Value = response.Result, | ||
| FixedFormat = fixedFormat | ||
| }).ConfigureAwait(false); | ||
| } |
There was a problem hiding this comment.
The await TextVisualizerDialog.OpenDialogAsync(...) blocks until the user dismisses the dialog. This runs before the toast update logic below, so while the dialog is open the toast is either stuck in the "Executing command…" progress state or auto-closes via closeToastCts. The success toast update is deferred until after the dialog is dismissed.
Consider either moving this call to after the toast update block, or fire-and-forgetting the dialog so the toast updates immediately.
There was a problem hiding this comment.
I tested and I didn't see this. And this wouldn't be a concern after switching to accessing result via persistent notification.
There was a problem hiding this comment.
Acknowledged — toast ordering isn't a visible issue in practice and will be moot with the persistent notification approach.
| { | ||
| interactionService.DisplayRawText(response.Result, ConsoleOutput.Standard); | ||
| } |
There was a problem hiding this comment.
This result handling only exists in ExecuteGenericCommandAsync. The sibling path via ExecuteResourceCommandAsync → HandleResponse doesn't check response.Result, so if a well-known command (start/stop/restart) ever returns result data, it will be silently discarded. Worth either forwarding results in HandleResponse too, or documenting that well-known commands intentionally ignore result data.
There was a problem hiding this comment.
Fixed — HandleResponse now also outputs response.Result when present, so well-known commands won't silently discard result data.
- Display result data on both success and error paths in CLI and MCP - Add result handling to HandleResponse for well-known commands - MCP tool returns CallToolResult with IsError instead of throwing Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add Failure(errorMessage, result, resultFormat) overload to CommandResults - Add test for error path with result data in ResourceCommandHelperTests - Add 'Validate Config' (JSON error) and 'Check Health' (text error) demo commands to Stress playground Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Both DashboardCommandExecutor and DashboardDialogService are scoped, so direct injection is safe and simpler. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Description
Adds
ResultandResultFormatproperties toExecuteCommandResult, allowing resource commands to return structured output data (text or JSON) back to the caller. The result flows through the entire pipeline: model → proto/gRPC → backchannel → Dashboard UI → CLI → MCP tools.Model: New
CommandResultFormatenum (None,Text,Json),Result/ResultFormatonExecuteCommandResult, andCommandResults.Success(result, format)overload.Proto/gRPC:
ResourceCommandResponsegetsoptional string resultandCommandResultFormat result_formatfields.Backchannel:
ExecuteResourceCommandResponsegainsResultandResultFormat(string) properties.Dashboard: On command success with result data, opens
TextVisualizerDialogwith syntax highlighting. JSON results lock the viewer to JSON mode.CLI: Result written to stdout via
DisplayRawText(result, ConsoleOutput.Standard). Status messages routed to stderr, enabling clean piping (e.g.,aspire resource myResource generate-token | jq .token). MCP tool returns result as additionalTextContentBlock.Replica aggregation: Takes the first successful replica's result when multiple replicas return data.
Playground: Added "Generate Token" (JSON) and "Get Connection String" (text) demo commands on
stress-telemetryservice.Fixes #15610
Checklist
<remarks />and<code />elements on your triple slash comments?aspire.devissue: