Repository navigation
Fix Windows AppHost cleanup job routing - #20494
David Negstad (danegsta) merged 3 commits into
Conversation
Co-authored-by: Copilot App <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 -- 20494Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 20494" |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The launch-mode routing is consistent with the documented Windows job behavior and has focused regression coverage.
Review effort: Balanced
Findings: None
What changed in this PR
Fixes Windows AppHost cleanup by avoiding the incompatible outer kill-on-close job for dotnet run and dotnet watch, while retaining protection for direct launches.
Changes:
- Adjusts job routing based on launch mode.
- Documents the nested-job constraint.
- Adds focused routing and Windows process-topology tests.
| File | Description |
|---|---|
src/Aspire.Cli/Projects/DotNetAppHostProject.cs |
Selects parent-exit protection by launch path. |
src/Aspire.Cli/Processes/WindowsConsoleProcessJob.cs |
Documents nested-job breakaway behavior. |
tests/Aspire.Cli.Tests/Projects/DotNetAppHostProjectTests.cs |
Verifies fallback and direct-launch routing. |
tests/Aspire.Cli.Tests/Projects/AppHostServerSessionTests.cs |
Verifies server job protection remains enabled. |
tests/Aspire.Cli.Tests/Processes/WindowsConsoleProcessJobTests.cs |
Tests the real Windows nested-job topology. |
tests/Aspire.Cli.Tests/TestServices/ProcessTestHelpers.cs |
Adds process and file-handshake helpers. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The regression probe does not emulate DCP’s breakaway operation, so it can pass without exercising the reported nested-job incompatibility.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 1
The synthetic child never attempted DCP's breakaway behavior and tested ordinary job inheritance rather than the Aspire routing fix. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Avoid coupling process cleanup assertions to a transient AppHost that the preceding test deletes while discovery refreshes. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Description
On Windows, session-scoped containers could survive AppHost shutdown because DCP was terminated before it finished cleanup. When a .NET AppHost runs through
dotnet run, DCP inherits both the Aspire CLI's outer kill-on-close job and .NET ProcessReaper's inner non-breakaway job. DCP cannot escape that nested job chain, so closing the CLI job kills it during cleanup.This change omits the Aspire CLI kill-on-close job only for
dotnet runanddotnet watchfallback paths. Those paths retain console isolation and cooperative CLI orphan detection. Direct .NET launches still use the CLI job because they do not introduce ProcessReaper, and TypeScript/polyglot AppHost server and guest processes retain their existing job protection.Focused CLI tests assert the job selection for .NET fallback and direct launches and verify that TypeScript/polyglot server protection remains enabled.
User-facing behavior
Existing shutdown flows such as Ctrl+C and
aspire stopcan now allow DCP to finish removing session-scoped resources for .NET AppHosts on Windows. Persistent resources remain unaffected, and no new command-line options are required.Validation
Fixes #20495
Checklist
<remarks />and<code />elements on your triple slash comments?