Merged
Conversation
Generalize failed-process output retention in ProcessUtil and ProcessResult, update publishing failure paths to surface retained output, and add regression coverage for compose-up failures and CircularBuffer duplicate-value assertions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
davidfowl
requested review from
JamesNK,
danegsta,
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 -- 16303Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 16303" |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves failure diagnostics across Aspire’s publishing/container runtime flows by retaining and surfacing a bounded tail of process stdout/stderr from the shared DCP process layer, and updates the shared CircularBuffer debug assertions to handle duplicate values correctly.
Changes:
- Add opt-in (and failure-default) retained process output to
ProcessSpec/ProcessResult, and use it to enrich exception messages for publish/container-runtime failures. - Rename “build output” terminology to “process output” across publishing exceptions and tests.
- Fix
CircularBufferdebug assertions for duplicate values and add regression tests.
Show a summary per file
| File | Description |
|---|---|
| tests/Aspire.Hosting.Tests/Publishing/ResourceContainerImageManagerTests.cs | Updates test to validate ProcessFailedException.ProcessOutput instead of BuildOutput. |
| tests/Aspire.Hosting.Tests/Publishing/ProcessFailedExceptionTests.cs | Renames variables/expectations to “process output” and validates truncation messaging. |
| tests/Aspire.Hosting.Tests/Publishing/ContainerRuntimeBaseTests.cs | Adds coverage ensuring registry login / compose up failures include captured output. |
| tests/Aspire.Hosting.Tests/Dcp/ProcessUtilTests.cs | Adds tests for default failure output capture, opt-in retention in ProcessResult, and truncation behavior. |
| tests/Aspire.Dashboard.Tests/CircularBufferTests.cs | Adds regression tests for duplicate-value behavior when full. |
| src/Shared/CircularBuffer.cs | Adjusts debug assertions to handle duplicates by counting occurrences. |
| src/Aspire.Hosting/Publishing/ResourceContainerImageManager.cs | Enables retained output for dotnet publish and passes it into ProcessFailedException. |
| src/Aspire.Hosting/Publishing/ProcessFailedException.cs | Renames surfaced output to “process output” and formats via shared formatter. |
| src/Aspire.Hosting/Publishing/DockerContainerRuntime.cs | Switches build/buildx failure paths to use ProcessResult retained output. |
| src/Aspire.Hosting/Publishing/PodmanContainerRuntime.cs | Switches build failure paths to use ProcessResult retained output. |
| src/Aspire.Hosting/Publishing/ContainerRuntimeBase.cs | Adds ExecuteContainerCommandWithResultAsync and includes formatted retained output in login/compose up failures. |
| src/Aspire.Hosting/Dcp/Process/ProcessUtil.cs | Implements retained output capture and appends formatted tail to non-zero-exit exceptions. |
| src/Aspire.Hosting/Dcp/Process/ProcessSpec.cs | Adds DefaultRetainedOutputLineCount and RetainedOutputLineCount knob. |
| src/Aspire.Hosting/Dcp/Process/ProcessResult.cs | Carries retained output + total line count and provides formatting helper. |
| src/Aspire.Hosting/Dcp/Process/ProcessOutputCapture.cs | Introduces shared output tail capture/formatting utility under Aspire.Hosting.Utils. |
| src/Aspire.Hosting.Docker/Aspire.Hosting.Docker.csproj | Links ProcessOutputCapture.cs into shared compile set. |
| src/Aspire.Hosting.Azure/Aspire.Hosting.Azure.csproj | Links ProcessOutputCapture.cs into shared compile set. |
| src/Aspire.Hosting.Azure.Kubernetes/Aspire.Hosting.Azure.Kubernetes.csproj | Links ProcessOutputCapture.cs into shared compile set. |
Copilot's findings
Comments suppressed due to low confidence (1)
src/Shared/CircularBuffer.cs:201
removedItemCount = CountOccurrences(removedItem)is executed unconditionally, but it exists only to support the subsequentDebug.Assert. SinceDebug.Assertcalls are removed in non-DEBUG builds, this leaves an unnecessary O(n) scan on everyAddonce the buffer is full. Wrap the counting (and assert) in#if DEBUG(or similar) so release builds keep O(1) behavior.
var removedItem = this[0];
var removedItemCount = CountOccurrences(removedItem);
- Files reviewed: 18/18 changed files
- Comments generated: 2
Tighten the updated tests to assert stable retained-output behavior instead of exact external tool wording. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Make CircularBuffer duplicate-count assertions debug-only so release builds keep O(1) behavior, and clarify the ProcessResult helper summary to describe non-zero-exit behavior accurately. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
Re-running the failed jobs in the CI workflow for this pull request because 1 job was identified as retry-safe transient failures in the CI run attempt.
|
Contributor
|
🎬 CLI E2E Test Recordings — 72 recordings uploaded (commit View recordings
📹 Recordings uploaded automatically from CI run #24616742709 |
mitchdenny
approved these changes
Apr 20, 2026
Member
mitchdenny
left a comment
mitchdenny
left a comment
There was a problem hiding this comment.
Clean, well-structured refactoring. Behavioral contracts are preserved across all call sites, thread safety is maintained, and the CircularBuffer assertion fix correctly handles duplicate values. Test coverage is thorough. No issues found.
JamesNK
approved these changes
Apr 20, 2026
Member
JamesNK
left a comment
JamesNK
left a comment
There was a problem hiding this comment.
Clean PR — no bugs, security issues, correctness errors, or convention violations found.
Verified: all callers migrated from BuildOutputCapture to the new ProcessOutputCapture/RetainedOutputLineCount pattern, linked .csproj includes are consistent, CircularBuffer debug assertions correctly handle duplicates, no credential leak risk in login output capture, and logging levels are preserved through the refactoring.
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
ProcessSpecopt into retained output and returning that retained output fromProcessResult.dotnet publish, Docker/Podman build, registry login, andcompose upfailures.CircularBufferdebug assertions so duplicate values do not trip incorrect invariants.Motivation/context:
az bicep buildand other process-driven deployment steps include the useful stderr/stdout tail by default.Dependencies:
Validation:
./dotnet.sh build src/Aspire.Hosting/Aspire.Hosting.csproj -c Release./dotnet.sh build src/Aspire.Hosting.Azure/Aspire.Hosting.Azure.csproj -c Release./dotnet.sh build src/Aspire.Hosting.Docker/Aspire.Hosting.Docker.csproj -c Release./dotnet.sh build src/Aspire.Hosting.Azure.Kubernetes/Aspire.Hosting.Azure.Kubernetes.csproj -c Release./dotnet.sh test tests/Aspire.Hosting.Tests/Aspire.Hosting.Tests.csproj -- --filter-class "*.ContainerRuntimeBaseTests" --filter-class "*.ProcessUtilTests" --filter-class "*.ProcessFailedExceptionTests" --filter-class "*.ResourceContainerImageManagerTests" --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"./dotnet.sh test tests/Aspire.Dashboard.Tests/Aspire.Dashboard.Tests.csproj -- --filter-class "*.CircularBufferTests" --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"Fixes # (issue)
Checklist
<remarks />and<code />elements on your triple slash comments?aspire.devissue: