Backport CLI cancellation fixes to release/13.4 - #17641
Conversation
…ignal bug (#17588) * Fix CLI Ctrl+C shutdown taking too long during AppHost startup - Make ConsoleCancellationManager.Cancel() non-blocking so Ctrl+C handler returns immediately - Pass cancellation token through to WaitAsync in CancelAppHostStartupAsync so Ctrl+C exits promptly instead of waiting for the full 5s timeout - Support second Ctrl+C for immediate force exit (Environment.Exit) - Add logging support to ConsoleCancellationManager via SetLogger() - Add comprehensive unit tests for ConsoleCancellationManager - Add integration test for RunCommand cancellation during startup timeout Fixes #17569 * Fix review comments: volatile logger, accurate comments, clearer warning * Clean up * Use WaitForSuccessPromptFailFastAsync in CLI E2E tests Replace WaitForSuccessPromptAsync with WaitForSuccessPromptFailFastAsync across all CLI E2E tests. The fail-fast variant detects error prompts immediately and throws instead of hanging for up to 500s waiting for a success prompt that will never arrive. This prevents 10+ minute CI timeouts when a command fails with a non-zero exit code. * Fix double-signal bug and consolidate test helpers - Fix ConsoleCancellationManager double-signal bug: move Console.CancelKeyPress to else branch so it only registers on platforms without PosixSignalRegistration. Previously both handlers fired for the same SIGINT, causing immediate force-kill. - Add SIGQUIT/Ctrl+Break registration for Windows parity. - Remove old WaitForSuccessPromptAsync (no fail-fast) and rename WaitForSuccessPromptFailFastAsync to WaitForSuccessPromptAsync. - Remove duplicate RunCommandFailFastAsync (identical to RunCommandAsync). * Fix stale comment and restrict SIGQUIT to Windows only * Remove unused legacy builder methods and update E2E skill docs * Don't force when debugging * More logging
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 17641Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 17641" |
There was a problem hiding this comment.
Pull request overview
This PR backports two already-merged main PRs (#17576 and #17588) to release/13.4, combined to avoid merge conflicts. It addresses CLI Ctrl+C/SIGTERM responsiveness issues (issue #17569) and introduces a TerminalRun IAsyncDisposable to guarantee diagnostics capture in CLI E2E tests.
Changes:
- Rewires
ConsoleCancellationManager: async forced-termination timeout, second-signal force-kill, SIGQUIT on Windows, and removes the double-signal bug by registeringConsole.CancelKeyPressonly on platforms withoutPosixSignalRegistration.RunCommand.CancelAppHostStartupAsyncnow plumbs the outer cancellation token toWaitAsyncso Ctrl+C exits the startup wait immediately. - Introduces
TerminalRun/CliE2ETestHelpers.StartRunand migrates ~10 E2E tests + several others away from manualexit/await pendingRunboilerplate; copies.aspire-diagnostics/from workspace totestresults/workspaces/<test>for CI artifacts. - Renames
WaitForSuccessPromptFailFastAsync→WaitForSuccessPromptAsyncand removes the duplicateRunCommandFailFastAsync, updating ~59 call sites across CLI E2E and deployment E2E tests; adds unit tests inConsoleCancellationManagerTestsand aRunCommandTestsCtrl+C test.
Reviewed changes
Copilot reviewed 88 out of 88 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/Aspire.Cli/ConsoleCancellationManager.cs | Async timeout, double-signal handling, SIGQUIT, logger, debugger bypass |
| src/Aspire.Cli/Program.cs | Wires logger into cancellation manager; updates termination log message |
| src/Aspire.Cli/Commands/RunCommand.cs | Threads cancellationToken through CancelAppHostStartupAsync to exit startup wait promptly on Ctrl+C |
| src/Aspire.Cli/Projects/ProcessGuestLauncher.cs | Adds debug/info logging around guest-process start, cancel, kill, exit |
| tests/Aspire.Cli.Tests/ConsoleCancellationManagerTests.cs | New unit tests: cancellation, second-signal force, timeout, non-blocking Cancel, dispose semantics |
| tests/Aspire.Cli.Tests/Commands/RunCommandTests.cs | New test: Ctrl+C during startup exits within 3s instead of waiting full 5s timeout |
| tests/Aspire.Cli.EndToEnd.Tests/Helpers/TerminalRun.cs | New IAsyncDisposable capturing diagnostics, exiting terminal, copying workspace diagnostics to test artifacts |
| tests/Aspire.Cli.EndToEnd.Tests/Helpers/CliE2ETestHelpers.cs | Adds StartRun(...) factory |
| tests/Aspire.Cli.EndToEnd.Tests/Helpers/CliE2EAutomatorHelpers.cs | Consolidates diagnostics under .aspire-diagnostics/; replaces RunCommandFailFastAsync calls |
| tests/Shared/Hex1bAutomatorTestHelpers.cs | Removes old WaitForSuccessPromptAsync; renames *FailFastAsync variants; removes RunCommandFailFastAsync |
| tests/Shared/Hex1bTestHelpers.cs | Removes unused WaitForSuccessPromptFailFast builder and InstallAspireBundleFromPullRequest helper |
| tests/Aspire.Cli.EndToEnd.Tests/*.cs (~50 files) | Mechanical: adopt StartRun, drop manual exit/pendingRun, rename *FailFastAsync → *Async |
| tests/Aspire.Deployment.EndToEnd.Tests/*.cs | Rename RunCommandFailFastAsync → RunCommandAsync |
| .github/skills/cli-e2e-testing/SKILL.md | Documents the StartRun/TerminalRun pattern and updated helper names |
|
❓ CLI E2E Tests unknown — 107 passed, 0 failed, 2 unknown (commit View all recordings
📹 Recordings uploaded automatically from CI run #26612978519 |
|
✅ No documentation update needed. Decision: Triggered signals (1): Rationale: The change to The existing |
Description
Backport of CLI cancellation fixes from main to
release/13.4. This PR contains changes from two PRs that are combined to avoid merge conflicts:From #17576 — Add TerminalRun IAsyncDisposable for consistent CLI E2E diagnostics capture
Adds a
TerminalRuntype (IAsyncDisposable) that wraps the CLI E2E terminal lifecycle, ensuringCaptureAspireDiagnosticsAsyncalways runs at the end of a test — even when the test fails. This replaces the manualexit/await pendingRunboilerplate and guarantees diagnostics are consistently captured across all tests.From #17588 — Fix CLI Ctrl+C/SIGTERM shutdown: responsive cancellation and double-signal bug
Fixes the CLI's Ctrl+C/SIGTERM handling to be responsive during all phases and prevents a double-signal bug that caused immediate force-kill instead of graceful shutdown.
Problems fixed:
PosixSignalRegistration(SIGINT)andConsole.CancelKeyPresswere registered simultaneously. On Linux, when SIGINT arrives both handlers fire for the same signal, each callingCancel(), so_cancelCalledreaches 2 immediately — triggering the force-kill path instead of graceful shutdown.Fixes #17569
Checklist