Fix extension CLI teardown during AppHost build by ellahathaway · Pull Request #20021 · microsoft/aspire

Ella Hathaway (ellahathaway) · 2026-09-09T22:56:43Z

Description

Stopping a dynamic debug session could terminate the Aspire CLI while a cold single-file AppHost build was still running. The extension's stopCli RPC used immediate process exit, bypassing normal command cancellation and child-process cleanup. On Windows, the orphaned dotnet build process retained the fixture workspace as its working directory, so E2E teardown failed with EBUSY while removing the second dynamic-debug workspace.

This change routes extension stop requests through the shared ConsoleCancellationManager, allowing the CLI's existing cancellation and process-tree cleanup paths to run. RunCommand also keeps pending pre-build work owned by the active command handler during manager-triggered shutdown. That is important because the cancellation manager's graceful deadline and final drain are the process-level escape hatch; returning from the handler after a separate local timeout could otherwise make shutdown appear complete before nested build cleanup finished.

Direct callers that cancel RunCommand with an unrelated token still use the bounded five-second wait. Those invocations do not have the cancellation manager's process-level deadline, so waiting indefinitely there could hang embedded or test callers.

The manager-owned cancellation regression requests the stop RPC synchronously from the fake project's RunAsync callback. This ensures the command enters its cleanup wait before fake time advances, rather than relying on Task.Yield() to schedule the handler. The pending run is released and awaited in finally, and the signaling time provider is shared with AppHostLauncherTests.

Validation:

169 passed and two expected platform skips across RunCommandTests, AppHostLauncherTests, and ExtensionBackchannelTests (one Unix-only case and one case requiring Windows symlink privileges).
Negative control: temporarily restoring the five-second local timeout for manager-owned cancellation makes the focused regression fail at the timeout-policy assertion. The correct production code was restored and the affected classes were rerun successfully.
Earlier validation of the production fix: the Windows dynamic-debug-configuration shard passed three complete runs (9/9 tests), including validation with a temporary delayed AppHost build that reproduced the directory lock before the fix. The follow-up changes only refine test coverage and share its helper.

Checklist

Is this feature complete?
- Yes. Ready to ship.
- No. Follow-up changes expected.
Are you including unit tests for the changes and scenario tests if relevant?
- Yes
- No
Did you add public API?
- Yes
  - If yes, did you have an API Review for it?
    - Yes
    - No
  - Did you add <remarks /> and <code /> elements on your triple slash comments?
    - Yes
    - No
- No
Does the change make any security assumptions or guarantees?
- Yes
  - If yes, have you done a threat model and had a security review?
    - Yes
    - No
- No

Route extension stop requests through cooperative cancellation and wait for in-flight pre-build cleanup before the CLI exits. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

github-actions · 2026-09-09T22:57:17Z

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 20021

Or

Run remotely in PowerShell:

iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 20021"

Copilot

🟡 Changes recommended

The five-second cleanup timeout can still let a pending build outlive the CLI.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Routes extension stop requests through cooperative CLI cancellation and waits for AppHost build cleanup.

Changes:

Replaces immediate CLI termination with cancellation-manager signaling.
Adds pending build cleanup handling and regression tests.

File summaries

File	Description
`src/Aspire.Cli/Backchannel/ExtensionRpcTarget.cs`	Cooperatively cancels CLI execution.
`src/Aspire.Cli/Commands/RunCommand.cs`	Waits for canceled pre-build work.
`tests/Aspire.Cli.Tests/Backchannel/ExtensionBackchannelTests.cs`	Tests stop-request cancellation.
`tests/Aspire.Cli.Tests/Commands/RunCommandTests.cs`	Tests cleanup ordering.
`tests/Aspire.Cli.Tests/Utils/CliTestHelper.cs`	Supplies the cancellation manager.

Review details

Files reviewed: 5/5 changed files
Comments generated: 1
Review effort level: Balanced

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

🟢 Approval recommended

The cancellation lifecycle is correctly bounded and covered by focused regression tests.

Review details

Files reviewed: 5/5 changed files
Comments generated: 0 new
Review effort level: Balanced

github-actions · 2026-09-10T18:22:25Z

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

github-actions · 2026-09-10T18:38:55Z

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

github-actions · 2026-09-10T18:53:33Z

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

Consolidate startup and pre-build cancellation into one helper with an explicit wait budget. Keep manager-owned shutdown under the central deadline, preserve bounded direct cancellation, and retain late task fault observation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

🟢 Approval recommended

The shutdown paths are correctly distinguished and covered by focused regression tests.

Review details

Files reviewed: 5/5 changed files
Comments generated: 0 new
Review effort level: Balanced

github-actions · 2026-09-10T20:13:12Z

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

github-actions · 2026-09-10T20:33:20Z

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

github-actions · 2026-09-10T20:48:56Z

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

Cancel synchronously at the fake project boundary and share the signaling time provider between command tests. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

github-actions · 2026-09-10T22:02:32Z

Tests selector

2 / 99 PR test projects · 4 PR jobs · 1 advisory-only target, from 7 changed files.

Selected PR test projects (2 / 99)

Aspire.Cli.EndToEnd.Tests, Aspire.Cli.Tests

Selected PR jobs (4)

cli-starter-validation, extension-e2e, polyglot, typescript-api-compat

Advisory workflow impact (1)

deployment-e2e (schedule/dispatch-only)

How these were chosen — grouped by what changed

📦 affected project Aspire.Cli
→ 1 test: Aspire.Cli.EndToEnd.Tests

🧪 tests/Aspire.Cli.Tests/Backchannel/ExtensionBackchannelTests.cs (changed test)
→ 1 directly: Aspire.Cli.Tests

🧪 tests/Aspire.Cli.Tests/Commands/AppHostLauncherTests.cs (changed test)
→ 1 directly: Aspire.Cli.Tests

🧪 tests/Aspire.Cli.Tests/Commands/RunCommandTests.cs (changed test)
→ 1 directly: Aspire.Cli.Tests

🧪 tests/Aspire.Cli.Tests/TestServices/SignalingFakeTimeProvider.cs (changed test)
→ 1 directly: Aspire.Cli.Tests

🧪 tests/Aspire.Cli.Tests/Utils/CliTestHelper.cs (changed test)
→ 1 directly: Aspire.Cli.Tests

Job reasons

Job	Triggered by
`cli-starter-validation`	affected project `Aspire.Cli`
`deployment-e2e`	affected project `Aspire.Cli`
`extension-e2e`	• `src/Aspire.Cli/Backchannel/ExtensionRpcTarget.cs`, `src/Aspire.Cli/Commands/RunCommand.cs`, `tests/Aspire.Cli.Tests/Backchannel/ExtensionBackchannelTests.cs`, `tests/Aspire.Cli.Tests/Commands/AppHostLauncherTests.cs`, `tests/Aspire.Cli.Tests/Commands/RunCommandTests.cs`, `tests/Aspire.Cli.Tests/TestServices/SignalingFakeTimeProvider.cs`, `tests/Aspire.Cli.Tests/Utils/CliTestHelper.cs` • affected project `Aspire.Cli`
`polyglot`	affected project `Aspire.Cli`
`typescript-api-compat`	affected project `Aspire.Cli`

Selection computed for commit d00bf50.

Copilot

🟢 Approval recommended

The cooperative shutdown behavior and both timeout paths are covered without unresolved correctness issues.

Review details

Files reviewed: 7/7 changed files
Comments generated: 0 new
Review effort level: Balanced

github-actions · 2026-09-10T22:27:22Z

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

github-actions · 2026-09-10T22:43:52Z

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

github-actions · 2026-09-10T23:01:12Z

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

James Newton-King (JamesNK) · 2026-09-11T03:24:52Z

        {
        }
-        catch (TimeoutException ex)
+        catch (TimeoutException ex) when (timeout != Timeout.InfiniteTimeSpan)


Timeout can be infinite? How does that then work with the WaitAsync above?

Also, how could there be a TimeoutException when timeout is infinite?

James Newton-King (JamesNK)

One lifecycle issue needs to be addressed before merge: extension cancellation after build completion can still let RunCommand finish before the pending AppHost run task has cleaned up. I confirmed it with a focused negative-control test and verified the minimal gate removal against the existing cancellation tests.

Proof: the PR's focused CLI classes pass (169 passed, 2 expected platform skips). The added post-build negative control fails on the PR head, then it and the three related cancellation tests pass after removing only the buildWaitCompleted gate. I did not rerun the full VS Code dynamic-debug E2E shard; that remains the real-path proof gap.

James Newton-King (JamesNK) · 2026-09-11T03:47:31Z

+            if (!buildWaitCompleted &&
+                runCts is not null &&
+                runTask is not null &&
+                !runTask.IsCompleted)


This gate leaves the same ownership gap after the build finishes. If stopCli arrives while WaitForAppHostStartupAsync is waiting for the backchannel/dashboard, buildWaitCompleted is already true, so the cancellation catch returns and the finally disposes runCts while runTask is still incomplete. I reproduced that with a focused test that completes BuildCompletionSource, waits for the ConnectingToAppHost status, sends stopCli, and blocks runTask: the command completed before cleanup. Removing only the buildWaitCompleted check made that test and the three existing cancellation tests pass. Please drain any incomplete runTask here; the existing infinite manager-owned versus five-second direct-caller policy still provides the intended bounds.

Fix extension CLI teardown during AppHost build

0424e9b

Route extension stop requests through cooperative cancellation and wait for in-flight pre-build cleanup before the CLI exits. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

github-actions Bot requested a balanced review from Copilot September 9, 2026 22:57

github-actions Bot added the area-cli label Sep 9, 2026

Copilot started reviewing on behalf of Ella Hathaway (ellahathaway) September 9, 2026 22:57 View session

This comment has been minimized.

Sign in to view

Copilot AI reviewed Sep 9, 2026

View reviewed changes

Comment thread src/Aspire.Cli/Commands/RunCommand.cs

Keep extension build cleanup under shutdown deadline

7e269d6

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

aspire-repo-bot Bot requested a balanced review from Copilot September 10, 2026 17:55

Copilot started reviewing on behalf of Ella Hathaway (ellahathaway) September 10, 2026 17:55 View session

This comment has been minimized.

Sign in to view

Copilot AI reviewed Sep 10, 2026

View reviewed changes

aspire-repo-bot Bot requested a balanced review from Copilot September 10, 2026 19:46

Copilot started reviewing on behalf of Ella Hathaway (ellahathaway) September 10, 2026 19:46 View session

This comment has been minimized.

Sign in to view

Copilot AI reviewed Sep 10, 2026

View reviewed changes

Make CLI teardown regression coverage deterministic

d00bf50

Cancel synchronously at the fake project boundary and share the signaling time provider between command tests. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

aspire-repo-bot Bot requested a balanced review from Copilot September 10, 2026 21:59

Ella Hathaway (ellahathaway) marked this pull request as ready for review September 10, 2026 22:00

Ella Hathaway (ellahathaway) requested review from James Newton-King (JamesNK), David Fowler (davidfowl) and Mitch Denny (mitchdenny) as code owners September 10, 2026 22:00

Copilot started reviewing on behalf of Ella Hathaway (ellahathaway) September 10, 2026 22:00 View session

Copilot AI reviewed Sep 10, 2026

View reviewed changes

James Newton-King (JamesNK) reviewed Sep 11, 2026

View reviewed changes

Uh oh!

Conversation

Ella Hathaway (ellahathaway) commented Sep 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

github-actions Bot commented Sep 9, 2026

Uh oh!

This comment has been minimized.

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

This comment has been minimized.

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

github-actions Bot commented Sep 10, 2026

Uh oh!

github-actions Bot commented Sep 10, 2026

Uh oh!

github-actions Bot commented Sep 10, 2026

Uh oh!

This comment has been minimized.

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

github-actions Bot commented Sep 10, 2026

Uh oh!

github-actions Bot commented Sep 10, 2026

Uh oh!

github-actions Bot commented Sep 10, 2026

Uh oh!

github-actions Bot commented Sep 10, 2026

Tests selector

Selected PR test projects (2 / 99)

Selected PR jobs (4)

Advisory workflow impact (1)

Job reasons

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

github-actions Bot commented Sep 10, 2026

Uh oh!

github-actions Bot commented Sep 10, 2026

Uh oh!

github-actions Bot commented Sep 10, 2026

Uh oh!

James Newton-King (JamesNK) Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

James Newton-King (JamesNK) left a comment

Choose a reason for hiding this comment

Uh oh!

James Newton-King (JamesNK) Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Ella Hathaway (ellahathaway) commented Sep 9, 2026 •

edited

Loading

James Newton-King (JamesNK) Sep 11, 2026 •

edited

Loading