feat(cli): cleanup commands for Aspire CLI installs and installer scripts#17462
Draft
radical wants to merge 6 commits into
Draft
feat(cli): cleanup commands for Aspire CLI installs and installer scripts#17462radical wants to merge 6 commits into
radical wants to merge 6 commits into
Conversation
Contributor
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 17462Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 17462" |
radical
changed the title
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.
|
Move CLI install enumeration and per-install metadata reporting out of `aspire doctor` into a new `aspire installs` command surface, and align the install-metadata vocabulary on `source` everywhere so the CLI, sidecar spec, installer scripts, and documentation use the same term. `aspire installs list` reports every discovered Aspire CLI install and every orphan hive in a vertical, color-aware layout. `--format json` emits the same rows as a stable contract documented in `docs/specs/cli-output-formats.md`. `aspire installs --self --format json` replaces the hidden `aspire doctor --self` endpoint that install discovery uses to cross-check a peer install's reported metadata. `aspire doctor` no longer ships an install table or `--self` flag; it now reports environment checks only. Install discovery still discovers peers, but the peer probe now invokes `aspire installs --self`. The sidecar spec moves to `docs/specs/install-sources.md`; the `InstallSidecarReader`, `InstallationInfo`, scripts, homebrew template, localhive helpers, and tests all read and write `source` (formerly `route`). Homebrew installs are tagged with the explicit `homebrew` source value in the sidecar and user-facing output. The WinGet first-run install sidecar probe now runs before `aspire installs list` instead of `aspire doctor`, so PR users still get a fresh sidecar populated on the first invocation after `winget install Microsoft.Aspire`. On Windows, the shared `PathLookupHelper` preserves the executable casing recorded on disk after PATHEXT resolution, so `aspire.exe` does not render as `aspire.EXE` just because PATHEXT contains `.EXE`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The install-source sidecar's `source` field is the wire-level schema value: the release script, PR installer, dotnet-tool packaging, and the Homebrew cask all write the literal `brew` string (matching the cask's `postflight` block and the actual Homebrew CLI binary name). `BundleService.ComputeDefaultExtractDir`, `ParseInstallSource`, and the installer test theory rows consume the same wire value. The user-facing surfaces in `aspire installs list` — `GetInstallKind`, `GetCleanupHint`, `GetManagedBy`, and the matching test expectations — render the friendlier `homebrew` / `Homebrew` label because that's what users recognise. Match on `"brew"` at the wire layer, translate to `"homebrew"` at the display layer. The schema stays minimal, the cask sidecar stays a literal one-liner, and users still see the conventional spelling. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…xt factory
WingetFirstRunProbe is the writer that lets a winget portable install identify
itself as such on first run — winget has no install hook, so the running CLI
self-stamps `.aspire-install.json` after consulting Windows ARP. It was being
invoked from two unrelated consumer sites:
- InstallsCommand.ListCommand.ExecuteAsync, before discovery reads the sidecar
- BundleService.GetBundleExtractDirForCurrentProcess, before the extract dir
is computed from the sidecar source
Each consumer paid the same try/catch cost and each leaked installer-specific
knowledge into a layer that should be installer-agnostic ("commands shouldn't
depend on any specific installer"). Worse, the factory that builds
CliExecutionContext also reads the sidecar (via GetUsersAspirePath ->
CliPathHelper.GetAspireHomeDirectory) but ran *before* either consumer fired
the probe — so a fresh winget install on its first invocation derived its
Aspire home with the sidecar still unstamped.
Hoist the invocation into the CliExecutionContext DI singleton factory in
Program.cs, immediately before BuildCliExecutionContext. The factory body now
runs the probe once per CLI invocation, before any downstream code reads the
sidecar. BundleService drops its optional WingetFirstRunProbe? parameter and
InstallsCommand drops its ctor parameter, field, and explicit call site.
Tests:
- WindowsRegistryReaderTests (new, 7 tests, Windows-only) exercises the real
HKCU\Software\Microsoft\Windows\CurrentVersion\Uninstall hive: matching
entry returns true; wrong identifier / missing target / different target /
surrounded by noise all return false; path comparison is case-insensitive;
empty processPath returns false. Each test creates a uniquely-named
`Microsoft.Aspire.Tests.<guid>` subkey and deletes it in a using; a static
sweeper removes leftovers from any crashed prior run before tests start.
- WingetStartupProbeTests (new, 2 tests) pins the new TryRunWingetFirstRunProbe
contract: it resolves the probe from DI and invokes Run with the binary
directory of Environment.ProcessPath, and any IWindowsRegistryReader
exception is swallowed so CLI startup is never broken by a probe failure.
TryRunWingetFirstRunProbe is internal for testing; the factory's use of it
remains a one-line check at code review.
Net: -54/+50 production lines; +279 test lines; all existing tests still pass
(1366/1366 in the Acquisition + Commands + Bundles surface on macOS).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add an Aspire-owned cleanup surface for script and PR installs so users can remove stale Aspire CLI state without remembering install layouts. `aspire installs uninstall <id>` removes Aspire-owned install state for a discovered install or orphan hive, refusing managed installs (Homebrew, WinGet, dotnet tool) before any prompt and pointing the user back at the owning package manager. PR installs also clean their dogfood install directory; the shared script install under `~/.aspire/bin`, `~/.aspire/bundle`, and `~/.aspire/versions/<v>` is only removed when the caller passes `--remove-shared-install`. Lower-level commands let callers operate on the pieces individually: `aspire hives list/delete` for package hives, and `aspire uninstall --channel|--all` for direct channel cleanup. `aspire installs list` output now carries a per-row `Cleanup` hint pointing at the matching command (`aspire installs uninstall <id>` for Aspire-owned rows, package-manager command for managed rows). The release and PR acquisition scripts gain a `--uninstall` / `-Uninstall` mode so users can clean stale state even when no correct Aspire CLI binary is on PATH. The scripts infer the channel from the PR number (PR script) or `--quality` (release script), support `--dry-run` / `-WhatIf`, require `--yes` for destructive actions, and keep the shared script install untouched unless `--remove-shared-install` is passed explicitly. Refs microsoft#15614 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…aller uninstall paths `validate_channel` was only present in `get-aspire-cli.sh`; `get-aspire-cli-pr.sh` accepted any channel value and interpolated it straight into the `rm -rf` target. Both `get-aspire-cli.ps1` and `get-aspire-cli-pr.ps1` had no validation at all, so a `-Channel "../../some-dir"` value resolved outside the hives root and was passed to `Remove-Item -Recurse`. Mirror the existing `validate_channel` from `get-aspire-cli.sh` into `get-aspire-cli-pr.sh`, and add an equivalent `Test-UninstallChannel` to both PowerShell installer scripts. Reject channel names that contain path separators or `..` segments before composing any deletion path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Two paths in `aspire uninstall --remove-shared-install` and the matching `--uninstall --remove-shared-install` script flows reported success on failure:
- `TryGetBundleVersionTarget` caught `ResolveLinkTarget` exceptions and returned `false`, conflating "doesn't apply" with "couldn't resolve". The bundle symlink itself was then deleted on the way out and the user saw a clean operations list, while the on-disk `versions/<v>/` tree was silently stranded.
- `UninstallAsync` with `--remove-shared-install` enumerated the bundle version directory and deleted it without checking `BundleVersionLease.HasActiveLease`. `BundleService.TryCleanupStaleVersions` explicitly skips leased entries; without the equivalent check here, another running CLI / AppHost could lose files mid-execution.
- `Remove-CleanupPath` in `get-aspire-cli{,-pr}.ps1` called `Remove-Item -Recurse -Force` without `-ErrorAction Stop` and without a try/catch. With the script's default error action of `Continue`, `Remove-Item` failures (permission denied, file in use) were written to the error stream and execution fell through to the `Write-Message "removed: $Path"` line, producing a success-looking exit while the path was still on disk.
Refactor the shared-install cleanup so the bundle link target is resolved before the symlink is deleted; surface `ResolveLinkTarget` failures as a `Failed` cleanup operation; and skip the version directory with a clear reason when a `BundleVersionLease` is active. Wrap the PowerShell helper in try/catch with `-ErrorAction Stop` so a failed delete emits `failed: <path>` and propagates a non-zero exit.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
radical
force-pushed
the
radical/installs-cleanup
branch
from
67ef4a8 to
82397bb
Compare
Member
PR Testing ReportPR Information
CLI Version Verification
Changes AnalyzedFiles ChangedKey changed areas reviewed from the PR diff:
Change Categories
Test Scenarios ExecutedScenario 1: Dogfood install and version verificationObjective: Install the PR CLI from the dogfood command into an isolated temp path and verify the installed binary matches the PR head. Steps:
Evidence:
Observations:
Scenario 2: Basic CLI/template/AppHost smokeObjective: Verify the PR CLI can create and run a fresh Aspire app from the PR hive. Steps:
Evidence:
Observations:
Scenario 3: Install discovery/list outputObjective: Validate Steps:
Evidence:
Observations:
Scenario 4: Cleanup safety and exact-id behaviorObjective: Validate cleanup refuses unsafe or ambiguous operations without deleting the PR install unexpectedly. Steps:
Evidence:
Expected Unhappy-Path Outcome: Invalid or unsafe cleanup should produce a non-zero/clear failure and leave the install intact. Observations:
Scenario 5: Hive command lifecycleObjective: Validate hive listing, safe refusal, dry-run behavior, and forced isolated hive deletion. Steps:
Evidence:
Expected Unhappy-Path Outcome: Deleting a hive with a matching dogfood install should fail unless Observations:
Scenario 6: CLI and installer script uninstall validationObjective: Validate invalid channel hardening and PowerShell PR script cleanup behavior. Steps:
Evidence:
Expected Unhappy-Path Outcome: Invalid channel values should be rejected before path cleanup and return non-zero. Observations:
Summary
Overall Result✅ PR VERIFIED No blocking issues were found in the locally tested Windows/PowerShell path. The same-process Recommendations
Artifacts
|
IEvangelist
reviewed
May 27, 2026
Member
IEvangelist
left a comment
IEvangelist
left a comment
There was a problem hiding this comment.
Posted 1 code review comment.
| return CleanupOperation.Failed(target.FullName, "Could not determine the running CLI path; refusing to delete cleanup targets. Re-run from a fully-resolved CLI invocation."); | ||
| } | ||
|
|
||
| if (IsPathUnderTarget(currentProcessPath, target.FullName)) |
Member
There was a problem hiding this comment.
This guard runs before the dryRun branch below, so previewing cleanup can fail even though nothing would be deleted. For example, running aspire installs uninstall pr-17462 --dry-run from that PR install hits this path because the current CLI is inside the dogfood target and returns a failure instead of showing the planned removals. Move the dry-run case ahead of the self-delete guard (or make this guard non-fatal for dry-run) so --dry-run remains describe-only.
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
2 participants
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.
Builds on #17461. The description below covers the diff this PR adds on top of #17461.
What was missing. #17461 introduces
aspire installsfor discovering Aspire CLI state, but provides no way to remove that state. Cleaning up apr-<N>dogfood install, an orphan NuGet hive, or a leftover~/.aspire/bundlelayout still requires remembering the install layout by heart. The installer scripts have no uninstall mode either, so users can't clean up at all if the CLI onPATHis broken or absent.The fix. Add cleanup commands to the
aspire installssurface and to the install scripts:aspire installs uninstall <id> [--remove-shared-install] [--dry-run] [--yes]— removes Aspire-owned state for a discovered install or orphan hive listed byaspire installs list. Refuses managed installs (Homebrew, WinGet, dotnet tool) before any prompt and points the user back at the owning package manager.aspire installs listrows gain aCleanuphint pointing at the right command for that row's source.aspire hives list/aspire hives delete <name>— lower-level package-hive operations.aspire uninstall --channel <ch> | --all— lower-level channel cleanup (drives the sameCliCleanupServiceasinstalls uninstall).get-aspire-cli{,-pr}.{sh,ps1} --uninstall(or-Uninstall) — clean stale state even when no Aspire CLI is onPATH. The PR script infers the channel from--pr-number; the release script requires explicit--channelor--all. Both require--yesfor destruction and support--dry-run/-WhatIf.Destructive-path hardening:
..segments before being interpolated into any deletion path (CliCleanupService.ValidateChannel, plusvalidate_channel/Test-UninstallChannelin all four installer scripts).Remove-CleanupPathrunsRemove-Item -ErrorAction Stopinside try/catch so a failed delete emitsfailed: <path>and propagates a non-zero exit instead of falling through to aremoved:log line.Get-BundleVersionTarget(both PowerShell scripts) wrapsGet-Itemwith-ErrorAction Stop; an IO failure on the bundle symlink now throws rather than returning$nulland lettingRemove-BundleLayoutsilently strand theversions/<v>/tree.CliCleanupService.AddSharedInstallOperationsresolves the bundle symlink target before deleting anything. When another aspire process holds a lease on the bundle version, both the symlink and the version directory are skipped — deleting the symlink alone would still break the lease holder's ability to re-resolve~/.aspire/bundle. The exit code is non-zero outside--dry-runwhen the requested--remove-shared-installcould not be carried out in full.installs uninstall <id>requires an exact id match. A typoed-suffix id likepr-17416-2is rejected instead of silently stripping topr-17416and deleting the wrong hive.-Uninstallpath tracks$Script:QualityExplicit(mirrorsQUALITY_EXPLICITin the bash sibling), soget-aspire-cli.ps1 -Uninstall -Yeswith no other arguments errors out instead of inferringstablefrom the install-default-Quality.Refs #15614