Stop running AppHost before adding packages#14573
Stop running AppHost before adding packages#14573mitchdenny wants to merge 7 commits intorelease/13.2from
Conversation
When running 'aspire add' while an AppHost is running in detached mode, the project file is locked by the build server, causing 'dotnet add package' to fail. The add command now stops any running AppHost instance before attempting to add the package, using the same running instance detection pattern used by the run command. Fixes part of #14238 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 14573Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 14573" |
There was a problem hiding this comment.
Pull request overview
This PR updates the aspire add workflow to avoid dotnet add package failing when the AppHost project is locked by a detached running AppHost instance, by proactively stopping any detected running instance before modifying the project.
Changes:
- Calls
project.CheckAndHandleRunningInstanceAsync(...)inAddCommandbefore invokingAddPackageAsync(). - Adds inline comments explaining why a running AppHost can block package installation.
🎬 CLI E2E Test RecordingsThe following terminal recordings are available for commit
📹 Recordings uploaded automatically from CI run #22209757033 |
Validates that 'aspire add' succeeds when an AppHost is running in detached mode. The add command should automatically stop the running AppHost to release file locks before modifying the project. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The aspire add --non-interactive flag doesn't suppress the version selection prompt. Updated the test to wait for and accept the default version before continuing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Gate CheckAndHandleRunningInstanceAsync behind KnownFeatures.RunningInstanceDetectionEnabled feature flag, matching RunCommand behavior - Check RunningInstanceResult.StopFailed return value and fail early with a clear error message instead of proceeding - Add E2E test for interactive aspire add flow (no integration argument) while AppHost is running in detached mode Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| // which prevents 'dotnet add package' from modifying the project. | ||
| if (_features.IsFeatureEnabled(KnownFeatures.RunningInstanceDetectionEnabled, defaultValue: true)) | ||
| { | ||
| var runningInstanceResult = await project.CheckAndHandleRunningInstanceAsync( |
There was a problem hiding this comment.
Isn't FindAndStopRunningInstanceAsync a better name? Check and handle are pretty generic descriptors.
| .Find($"Enter the project name ({workspace.WorkspaceRoot.Name}): "); | ||
|
|
||
| var waitingForOutputPathPrompt = new CellPatternSearcher() | ||
| .Find("Enter the output path:"); |
There was a problem hiding this comment.
The hardcoded text in these E2E tests worries me. When we change text (which will be often) tests will fail, and the E2E tests don't give friendly error messages.
They don't say something actionable like "Enter the output path: was not found", but have a message like "Timeout after 30 seconds".
| if (runningInstanceResult == RunningInstanceResult.StopFailed) | ||
| { | ||
| InteractionService.DisplayError(AddCommandStrings.UnableToStopRunningInstances); | ||
| return ExitCodeConstants.FailedToAddPackage; | ||
| } |
There was a problem hiding this comment.
How come this is only written when there is a failure to stop but not success to stop? Shouldn't the previous method either log about both or about none?
The aspire add command now auto-stops the running AppHost, so the subsequent aspire stop cleanup command returns exit code 7 (no running instances found). Use a generic prompt pattern that accepts both OK and ERR exit codes for the cleanup step. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The RightText() method checks for immediately-adjacent text, but the ERR prompt has intermediate text between the counter and '] $'. Instead, wait for the known aspire stop output messages which correctly handles both 'no running instances' and 'stopped successfully' outcomes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ceAsync Address PR feedback from JamesNK: the method name is more descriptive about what the method actually does — finds running instances and stops them. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| // which prevents 'dotnet add package' from modifying the project. | ||
| if (_features.IsFeatureEnabled(KnownFeatures.RunningInstanceDetectionEnabled, defaultValue: true)) | ||
| { | ||
| var runningInstanceResult = await project.FindAndStopRunningInstanceAsync( |
There was a problem hiding this comment.
Should this prompt to the user with something like: "A running AppHost was detected. Would you like to stop it to be able to add the integration?" Or something along those lines? I understand this is behind a feature flag, but I just feel like it might be worth it to prevent folks from shooting themselves in the foot (especially given things like logs and telemetry are likely lost after stopping.)
There was a problem hiding this comment.
I realize this is the same thing that happens if a user calls aspire run again on a detached project, so perhaps that's something we can consider as a follow up for potential UX issues. Also, with a subsequent aspire run I guess the current behavior might even be expected to some degree, while perhaps aspire add is not as intuitive since folks would like not assume we are keeping a handle on the project.
| ExecutionContext.HomeDirectory, | ||
| cancellationToken); | ||
|
|
||
| if (runningInstanceResult == RunningInstanceResult.StopFailed) |
There was a problem hiding this comment.
I noticed that in the RunCommand we actually still go ahead with the command even if stopping fails, unlike here. Assuming that is intentional as we know this will break otherwise.
joperezr
left a comment
There was a problem hiding this comment.
Some minor feedback from me, but nothing really blocking.
Summary
When running
aspire addwhile an AppHost is running in detached mode, the project file is locked by the build server, causingdotnet add packageto fail. The add command now stops any running AppHost instance before attempting to add the package.Changes
project.CheckAndHandleRunningInstanceAsync()inAddCommandbefore invokingAddPackageAsync()RunCommandCheckAndHandleRunningInstanceAsyncmethod displays user-visible messages about stopping the instance (viaRunningInstanceManager)Addresses part of #14238