Skip to content

feat(cli): aspire doctor lists every Aspire CLI install on the machine#17105

Merged
radical merged 21 commits into
microsoft:mainfrom
radical:aspire-commands
May 21, 2026
Merged

feat(cli): aspire doctor lists every Aspire CLI install on the machine#17105
radical merged 21 commits into
microsoft:mainfrom
radical:aspire-commands

Conversation

@radical
Copy link
Copy Markdown
Member

@radical radical commented May 14, 2026

What changes for users

Three things land in this PR:

  1. NEW — aspire doctor lists every Aspire CLI install on the machine. One row for the running CLI, one row per peer. aspire doctor --format json adds the same rows as an installations array for tooling consumption.

  2. FIXED — aspire add returns integrations on dogfood and local-hive CLIs. Previously it returned zero results because dotnet package search doesn't support local folder sources. Internal framework packages (AppHost, Sdk, Orchestration.*, Testing, Msi) and deprecated packages also no longer leak into the picker. Same fix flows through aspire integration list / search and MCP list-integrations. Fixes aspire add integration picker leaks internal framework packages on PR / localhive CLI installs #17292.

  3. CHANGED — localhive.sh / localhive.ps1 no longer silently flip your global Aspire channel. They used to run aspire config set channel <hive> -g after each successful build, switching the user's default channel to whatever hive was last built. The scripts now print the path to the freshly-built binary and offer one-shell activation (bash prints the export PATH=... line; PowerShell adds the bin directory to the current session's $env:PATH):

    Run Aspire directly with: /path/to/.aspire/dogfood/localhive/bin/aspire
    For this shell only, run: export PATH="/path/to/.aspire/dogfood/localhive/bin:$PATH"
    

    They also stamp a .aspire-install.json sidecar so the install shows up as a localhive route in scenario 1's aspire doctor table.

Installations
=============
╭───────────────────────────────────────────┬────────────┬──────────┬───────────┬──────────────╮
│ Path                                      │ Version    │ Channel  │ Route     │ Path status  │
├───────────────────────────────────────────┼────────────┼──────────┼───────────┼──────────────┤
│ /usr/local/bin/aspire (current)           │ 13.4.0     │ stable   │ brew      │ active       │
│ /opt/homebrew/bin/aspire                  │ 13.3.0     │ stable   │ brew      │ shadowed     │
│ ~/.aspire/dogfood/pr-17105/bin/aspire     │ 13.4.0-pr… │ pr-17105 │ pr        │ not on path  │
╰───────────────────────────────────────────┴────────────┴──────────┴───────────┴──────────────╯

What was missing

There was no first-class way to answer "which aspire is on my PATH, what version is it, where did it come from, and is it the only one installed?" Users hitting stale dogfood builds, side-by-side dotnet-tool installs, or a script-installed CLI shadowed by a homebrew one had to inspect paths and sidecar JSON by hand.

localhive was also missing a formal route identity: locally-built CLIs weren't sidecar-stamped, BundleService didn't know about them, and docs/specs/install-routes.md had no entry.

How aspire doctor discovers installs

aspire doctor walks $PATH plus well-known install prefixes (~/.aspire/bin, ~/.aspire/dogfood/<channel>/bin, ~/.dotnet/tools/.store/aspire.cli/), reads each binary's .aspire-install.json sidecar, and asks sidecar-bearing peers to self-describe via a hidden aspire doctor --self subcommand. The table and the --format json shape are the new user-facing surface. localhive becomes a known install route end-to-end at the same time — sidecar source, bundle extract layout, route-aware Aspire-home selection, and a docs entry.

Non-obvious decisions

Peer probing is gated on sidecar presence, not on path heuristics. The alternative — probing every PATH binary named aspire — would spawn arbitrary executables a user happened to put on PATH. Sidecar-less candidates appear in the table as notProbed so the row is still visible without executing the binary.

Unknown sidecar source values are surfaced as-is, not rejected. So a future CLI version that ships a new route (e.g. an OS package manager) will appear in older parents' aspire doctor output before the parent is updated.

macOS firmlinks under /private/var are collapsed during path resolution. A binary observed as /var/.../aspire from PATH and /private/var/.../aspire from a sidecar resolve to the same row instead of double-counting.

The dotnet-tool store walk excludes reparse points. A symlink or junction cycle anywhere under ~/.dotnet/tools/.store/aspire.cli would otherwise make Directory.EnumerateFiles walk indefinitely. The legitimate tool-store layout has no symlinks, so this loses nothing real and removes a self-DoS surface.

Surprises and call-outs

  • aspire doctor --self is a hidden subcommand for peer probing only. It bypasses prereq checks and full discovery, emits a single-row table or single-element installations JSON for the running CLI, and is required when spawning a peer — without it the peer would run full discovery and recurse.
  • The peer probe spawns other binaries discovered on PATH. Bounded by per-peer timeout (5s), capture budget (1 MiB per stream), and process-tree kill on timeout/cancel. Peer stderr is stripped of control sequences before being surfaced.

Checklist

  • Feature complete
  • Tests included
  • Security: peer probing spawns other binaries discovered on PATH; trust gate (sidecar presence) + bounded timeout + capture cap + kill-tree contain resource use.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 14, 2026

🚀 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 -- 17105

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 17105"

radical added a commit to radical/aspire that referenced this pull request May 15, 2026
Addresses review feedback on the acquisition code introduced in
PR microsoft#17105 to keep helpers and platform-conditionals consistent with
the rest of the CLI.

* Drop HomeDirectoryResolver and route every `~/.aspire` consumer
  through CliPathHelper.GetAspireHomeDirectory(), the canonical
  user-home accessor used everywhere else in the CLI (sentinel
  paths, logs, telemetry storage, certificates, project paths,
  CliExecutionContext, etc.). The dotnet-tool store probe in
  InstallationDiscovery walks under `~/.dotnet`, not `~/.aspire`,
  so a sibling CliPathHelper.GetUserProfileDirectory() is added per
  the review note for the raw user-profile case.

* Replace the custom $PATH walker in
  InstallationDiscovery.FindFirstAspireOnPath with
  PathLookupHelper.FindFullPathFromPath("aspire"). The shared helper
  already handles PATH splitting, PATHEXT extension probing on
  Windows, and the Unix executable-bit check (matching real shell
  semantics, where a non-executable file is not on PATH). The
  per-directory `aspire.exe` / `aspire` array literal is no longer
  needed; PATHEXT resolution covers Windows.

* Hoist the platform-specific aspire binary name into a single
  static readonly field per type (InstallationDiscovery and
  InstallationUninstaller) so the four remaining inline
  `OperatingSystem.IsWindows() ? "aspire.exe" : "aspire"`
  conditionals collapse to a named reference.

The WriteFakeBinary helper in InstallationDiscoveryDiscoverAllTests
now sets the executable bit on Unix so the new PATH walker can find
the test fixture. The previous custom walker accepted any
`File.Exists` hit regardless of mode bits, which masked the
incorrect setup.

Tested locally on macOS (arm64): full Aspire.Cli.Tests run passes
(3081 succeeded, 0 failed, 20 platform-skipped). Note: tests using
`EnvVarOverride("USERPROFILE", ...)` to redirect home only
effectively worked on Unix (via HOME, honored by GetFolderPath).
On Windows, USERPROFILE override is not honored by GetFolderPath,
so HomeDirectoryResolver was the only code path that respected the
override there. Watch the Windows CI job on this push; affected
tests may need to migrate to CliExecutionContext.HomeDirectory
injection as a follow-up if they regress.

Co-authored-by: Copilot <[email protected]>
@radical radical changed the title feat(cli): identity-aware CLI primitives + aspire info + aspire uninstall (PR γ) feat(cli): identity-aware CLI primitives + aspire info (PR γ) May 15, 2026
@radical radical changed the title feat(cli): identity-aware CLI primitives + aspire info (PR γ) feat(cli): add aspire info command and localhive install route May 16, 2026
radical added a commit that referenced this pull request May 16, 2026
The localhive install route added by #17105 keeps using the local channel
and hive package folder layout. Make that contract explicit in the package
channel tests by using PackageChannelNames.Local and covering both pinned
and unpinned local hive package enumeration.

Co-authored-by: Copilot <[email protected]>
@radical radical force-pushed the aspire-commands branch from 8d53715 to 01d9188 Compare May 16, 2026 06:44
Comment thread src/Aspire.Cli/Commands/InfoCommand.cs Outdated
@radical radical force-pushed the aspire-commands branch from 0b6128d to 13aa090 Compare May 19, 2026 07:11
@radical radical changed the title feat(cli): add aspire info command and localhive install route feat(cli): aspire doctor lists every Aspire CLI install on the machine May 19, 2026
@davidfowl
Copy link
Copy Markdown
Contributor

neeeds pr-testing report.

radical and others added 2 commits May 20, 2026 01:23
`aspire doctor` now lists every Aspire CLI installation discovered on
the machine, with one row for the running CLI and one per peer:

```
Installations
=============
╭───────────────────────────────────────────┬────────────┬──────────┬───────────┬──────────────╮
│ Path                                      │ Version    │ Channel  │ Route     │ Path status  │
├───────────────────────────────────────────┼────────────┼──────────┼───────────┼──────────────┤
│ /usr/local/bin/aspire (current)           │ 13.4.0     │ stable   │ brew      │ active       │
│ /opt/homebrew/bin/aspire                  │ 13.3.0     │ stable   │ brew      │ shadowed     │
│ ~/.aspire/dogfood/pr-17105/bin/aspire     │ 13.4.0-pr… │ pr-17105 │ pr        │ not on path  │
╰───────────────────────────────────────────┴────────────┴──────────┴───────────┴──────────────╯
```

`aspire doctor --format json` adds the rows as an `installations` array
for tooling consumption.

There was no first-class way to answer "which `aspire` is on my PATH,
what version is it, where did it come from, and is it the only one
installed?" Users hitting stale dogfood builds, side-by-side dotnet-tool
installs, or a script-installed CLI shadowed by a homebrew one had to
inspect paths and sidecar JSON by hand. `localhive` was also missing
formal route identity: locally-built CLIs weren't sidecar-stamped,
`BundleService` didn't know about them, and `docs/specs/install-routes.md`
had no entry.

`aspire doctor` walks `$PATH` plus well-known install prefixes
(`~/.aspire/bin`, `~/.aspire/dogfood/<channel>/bin`, `~/.dotnet/tools/.store/aspire.cli/`),
reads each binary's `.aspire-install.json` sidecar, and asks
sidecar-bearing peers to self-describe via a hidden `aspire doctor --self`
subcommand. The peer probe is bounded by a 5s timeout, a 1 MiB capture
budget per stream, and kills the process tree on timeout/cancel so a
broken peer cannot deadlock or exhaust memory in the parent. `localhive`
becomes a known install route end-to-end: sidecar source, bundle extract
layout, route-aware Aspire-home selection, and a docs entry.

Non-obvious decisions:

- **Peer probing is gated on sidecar presence, not on path heuristics.**
  The alternative — probing every PATH binary named `aspire` — would
  spawn arbitrary executables a user happened to put on PATH.
  Sidecar-less candidates appear in the table as `notProbed` so the row
  is still visible without executing the binary.
- **Unknown sidecar `source` values are surfaced as-is, not rejected.**
  So a future CLI version that ships a new route (e.g. an OS package
  manager) will appear in older parents' `aspire doctor` output before
  the parent is updated.
- The dotnet-tool store walk excludes reparse points
  (`AttributesToSkip = … | ReparsePoint`) so a symlink or junction cycle
  anywhere under the store cannot make discovery walk indefinitely.
- The dogfood candidate-source streams subdirectory enumeration via a
  manually-driven `IEnumerator` so the surrounding cancellation token
  is honored between subdirectory probes, matching the pattern already
  used by the dotnet-tool store walk.

Surprises and call-outs:

- **`aspire doctor --self` is a hidden subcommand** for peer probing
  only. It bypasses prereq checks and full discovery, emits a single-row
  table or single-element `installations` JSON for the running CLI, and
  is required when spawning a peer — without it the peer would run full
  discovery and recurse.
- **The peer probe spawns other binaries discovered on PATH.** Bounded
  by per-peer timeout, stdout byte cap, and process-tree kill on
  timeout. Peer stderr is stripped of control sequences before being
  surfaced.
- macOS firmlinks under `/private/var` are collapsed during path
  resolution so a binary observed as `/var/.../aspire` from PATH and
  `/private/var/.../aspire` from a sidecar resolve to the same row
  instead of double-counting.

Co-authored-by: Copilot <[email protected]>
`aspire add` returned zero integrations after a local-hive install:

```
$ aspire add
No Aspire integrations were found in the configured package sources.
```

The same emptiness affected `aspire integration list` / `search` and
the MCP `list-integrations` tool. Stable channels worked because they
hit nuget.org via `dotnet package search`. Dogfood / built-locally
channels were broken because `dotnet package search` does not support
local folder sources and returns no results.

The previous fix narrowed this to `PinnedVersion is not null`, but a
local hive without a pinned version still fell through to the broken
`dotnet package search` path, and even on the pinned path internal
framework packages (`AppHost`, `Sdk`, `Orchestration.*`, `Testing`,
`Msi`) and deprecated packages leaked into the picker.

Enumerate the local folder source directly whenever a scoped
`Aspire*` mapping points at an existing local directory, regardless of
whether a pinned version is set. The pinned version is now an
additional filter, not a precondition for the local-source path.
Filter results through a single `IsIntegrationPackageId` predicate
that keeps `Aspire.Hosting.*` and `CommunityToolkit.Aspire.Hosting.*`,
drops the internal framework prefixes listed above, drops
`DeprecatedPackages`, and applies the channel's quality gate to the
parsed `SemVersion`. Per-id deduplication keeps the highest version
using `SemVersion.PrecedenceComparer`. The reported `Source` is
normalized through `PathNormalizer.NormalizePathForStorage` so the
cache key matches what other channel code emits.

Co-authored-by: Copilot <[email protected]>
@radical radical force-pushed the aspire-commands branch from 27864e2 to e54cdfe Compare May 20, 2026 05:33
…nnel

`localhive.sh` and `localhive.ps1` no longer call
`aspire config set channel <hive> -g` after each successful build.

Building a hive — even just to test a one-off local fix — used to
silently flip the user's default Aspire channel to whatever hive was
last built. The change affected every future `aspire` invocation in
every shell with no obvious undo, and was particularly confusing on
dev machines that already had a stable install.

The scripts now print the path to the freshly-built binary and offer
one-shell activation. Bash:

  log "Run Aspire directly with: $CLI_BIN_DIR/$CLI_EXE_NAME"
  log "For this shell only, run: export PATH=\"$CLI_BIN_DIR:\$PATH\""

The PowerShell variant goes one step further and assigns to
`$env:PATH` directly so the running session can invoke `aspire`
without a manual export. Neither variant touches `.bashrc` / `.zshrc`
or sets `[Environment]::SetEnvironmentVariable(..., 'User')` on
Windows.

The scripts also stamp a `.aspire-install.json` sidecar with
`{"source":"localhive"}` so the install is recognized as the
`localhive` route by `aspire doctor` and any other consumer of the
sidecar contract.

`NuGetConfigMerger`'s safe-to-remove-source heuristic and its
comments are generalized from "PR hive" to "CLI-managed hive feed"
so localhive feeds get the same cleanup treatment as PR-hive feeds
when the channel changes.

Adjacent fixes in the same file:

- `is_valid_hivename` rejects hive names with path separators,
  leading dots, or `..` segments. The hive name is concatenated into
  a `rm -rf` target, so any of those would let removal escape the
  hives directory.
- The Windows-targeted bundle picks `aspire.exe` (vs `aspire`) for
  publish copy, executable bit, and archive instructions, so
  building `--target-rid win-*` from a non-Windows host produces a
  runnable archive.

A regression test (`LocalHiveScriptFunctionTests.cs`) pins both
contracts going forward: no global-channel mutation, and no
persistent PATH mutation via shell profile files or User-scope
environment variables.

Co-authored-by: Copilot <[email protected]>
@radical

This comment was marked as outdated.

@radical radical marked this pull request as ready for review May 20, 2026 06:39
@radical radical requested review from Copilot and joperezr May 20, 2026 06:39
radical and others added 8 commits May 20, 2026 15:10
…into a shared helper

The dotnet-tool-store source inlined a try/MoveNext loop to swallow IOExceptions
raised both at open and mid-walk (iterator methods can't yield inside a catch),
and EnumerateDirectoriesSafe duplicated the same shape for the dogfood source.
Factor the common drive-the-enumerator-manually loop into
EnumerateFileSystemEntriesSafe and expose two thin wrappers — EnumerateDirectoriesSafe
and EnumerateFilesSafe — that pass the appropriate Directory.Enumerate* factory.

Cancellation now lives inside the helper's MoveNext step, so callers no longer need
the per-iteration ThrowIfCancellationRequested. The check sits inside the try
block but the catch filter only matches IOException / UnauthorizedAccessException /
SecurityException, so OperationCanceledException still propagates cleanly.

Co-authored-by: Copilot <[email protected]>
…parsing

TryExtractPrChannel hand-rolled a span scan for "-pr.<digits>" terminated by '.',
'+', or end-of-string. The shape is well within what [GeneratedRegex] expresses
cleanly, and source-gen keeps it AOT-safe. Behavior is preserved: the regex still
requires the leading hyphen, at least one ASCII digit, and a '.', '+', or
end-of-string terminator after the digits.

Co-authored-by: Copilot <[email protected]>
Doctor's `--format json` tests captured emitted JSON via
`TestInteractionService.DisplayedRawText` — an in-memory list — so they
never observed the actual console sink the production CLI writes to.
Every other `--format json` test in the CLI (LsCommandTests,
PsCommandTests, DescribeCommandTests, etc.) wires `OutputTextWriter` to
the real stdout console and runs `JsonDocument.Parse(textWriter.Logs)`
on the captured bytes. That pattern implicitly enforces "stdout-only is
parseable JSON": any non-JSON noise on stdout (status text, error
banners, update notifications) breaks `JsonDocument.Parse` and fails
the test. Doctor was the outlier.

This gap was the reason David Pine's report that
`aspire doctor --format json` "emits non-JSON on stdout" could not be
settled by pointing at an existing test. Locally, when stdout and
stderr are split, doctor's stdout IS pure parseable JSON — but the
test suite never proved it.

Refactor `RunDoctorJsonAsync` to use the same `OutputTextWriter`-based
capture as the rest of the CLI, drop the `TestInteractionService` swap
(the real `ConsoleInteractionService` is used so the BaseCommand
JSON-mode stream redirect actually runs), and migrate the 10 older
direct-style doctor JSON tests onto the helper. Net -69 lines.

Add `DoctorCommand_Json_VersionUpdateBanner_StaysOnStderr` as a direct
regression test for the original scenario: wire the post-command CLI
update notifier to fire `DisplayVersionUpdateNotification` through the
real interaction service, and assert that the banner lands on stderr
while stdout remains JsonDocument-parseable. Verified by temporarily
rerouting `DisplayVersionUpdateNotification` to `_outConsole` and
confirming the test fails with `JsonReaderException` at the expected
ESC (0x1B) byte position.

Co-authored-by: Copilot <[email protected]>
Symptom: after running `localhive.ps1 -Copy` (or `-Output`/`-Archive`,
which implies `-Copy`) more than once with different version suffixes,
the CLI inside the freshly-built hive resolves the *previous* run's
version instead of the one just built.

Root cause: the PowerShell copy path used `Get-ChildItem *.nupkg | Copy-Item`
with no filter, so every `.nupkg` left over in `artifacts/packages/<Config>/Shipping`
from previous packs landed in the new hive. `PackagingService.GetLocalHivePinnedVersion`
(src/Aspire.Cli/Packaging/PackagingService.cs:338-350) then pins the
hive to the *highest* SemVer-precedence package present — which is whichever
stale suffix happened to sort highest, not the suffix we just built.

The bash sibling already guards against this (localhive.sh:308-322): it
iterates `*$VERSION_SUFFIX*.nupkg`, counts copies, and the `set -e` script
fails out if the glob matches nothing.

Fix: give `Copy-PackagesToHive` an optional `-VersionSuffix` parameter.
When non-empty (the `-Copy` user-opt-in path), filter `.nupkg` matches
by suffix, count copies, log the count in the same shape as bash, and
hard-fail with `Write-Err` + `exit 1` if zero matched. When empty (the
symlink/junction-failure fallback path, which the user did not opt into),
copy everything — same as the bash fallback at localhive.sh:329-342 —
so a Windows host without symlink privilege still gets a working hive.

Co-authored-by: Copilot <[email protected]>
…PeerProbeResult to TestServices

Move three test helpers that were tucked at the bottom of
InstallationDiscoveryDiscoverAllTests.cs into the shared
tests/Aspire.Cli.Tests/TestServices/ folder alongside the existing
fakes (FakePeerInstallProbe, FakeIdentityChannelReader,
FakeInstallationDiscovery). Namespaces moved to
Aspire.Cli.Tests.TestServices; the test file already imports that
namespace, so call sites are unchanged.

The EnvVarMutatingTestCollection collection definition stays in place
in the test file — collection definitions are conventionally pinned at
the assembly root and have no benefit from being relocated.

This is a pure move with no behavior or signature changes. Sibling test
classes (AspireVersionCheck, DoctorCommandTests) currently reinvent the
env-var-restore pattern; co-locating these helpers with the rest of the
TestServices fakes makes them discoverable for adoption in follow-up
cleanups.

Co-authored-by: Copilot <[email protected]>
Add two behavior tests to InstallSidecarReaderTests pinning forward-compat
contracts that work today but were implicit:

- UTF-8 BOM tolerance. localhive.ps1 writes the sidecar with
  `Set-Content -Encoding UTF8`, which on Windows PowerShell 5.x emits a
  UTF-8 BOM (0xEF 0xBB 0xBF). JsonDocument.Parse strips the BOM today and
  both TryRead and ReadSourceField work, but nothing pinned that
  behavior. Two new tests assert TryRead returns Ok(Script) and
  ReadSourceField returns "script" when the sidecar starts with a BOM.

- Unknown / extra fields tolerated. A newer CLI may add fields to the
  sidecar; older parents must ignore them rather than reject. The
  JsonDocument-based reader already does this, but no test pinned it. A
  new test writes a sidecar with `futureField` and a nested object and
  asserts TryRead still returns Ok(Script).

Co-authored-by: Copilot <[email protected]>
…gration listing

A user who flips the `showDeprecatedPackages` feature flag to see deprecated
integrations on `aspire add` sees them on stable/staging/daily but silently
sees nothing change on local-hive or PR-hive channels:

  $ aspire config set showDeprecatedPackages true
  $ aspire add --channel pr-17105
  # deprecated packages still hidden

Root cause: two paths through the same intent diverged. The feed-based path
in `NuGetPackageCache` (src/Aspire.Cli/NuGet/NuGetPackageCache.cs:135, 207)
consulted `KnownFeatures.ShowDeprecatedPackages` and only stripped deprecated
ids when the flag was off. The local-folder path in
`PackageChannel.IsIntegrationPackageId` hardcoded
`DeprecatedPackages.IsDeprecated(packageId)` into its namespace-classifier
and ignored the flag entirely, so the user-visible contract for the flag
silently held on stable but not on local/PR hives.

Fix: thread `IFeatures` through `PackageChannel` and move the deprecation
filter out of `IsIntegrationPackageId` into the local-folder enumeration
pipeline, gated on `IFeatures.IsFeatureEnabled(KnownFeatures.ShowDeprecatedPackages,
defaultValue: false)`. `IsIntegrationPackageId` now classifies by namespace
only, matching what its name says.

`IFeatures` is added as a required (no-default) parameter on
`PackageChannel.CreateExplicitChannel` and `CreateImplicitChannel` — a
silent default would have masked exactly this kind of bug. Production
callers (`PackagingService`) already have `_features` injected; test call
sites were updated to pass `new TestFeatures()` (the existing shared fake
under `tests/Aspire.Cli.Tests/TestServices/`).

Co-authored-by: Copilot <[email protected]>
…H discovery candidates

`InstallationDiscovery.DiscoverAllAsync` called
`CliPathHelper.ResolveSymlinkToFullPath` on every candidate, including
$PATH hits. But `FindAllAspireOnPath` already resolves the canonical path
for every $PATH match and stores it on `InstallationPathHit.CanonicalPath`,
which `PathInstallationCandidateSource` was throwing away — so the discovery
walk re-paid the resolve for the same file. Aside from the redundant
syscall, the two resolves open a small TOCTOU window: if a symlink under
the binary is swapped between the two calls, the dedup `seen` HashSet
ends up keyed on a different canonical the second time and a peer can
fail to be deduped against self (or another candidate).

Plumb the pre-resolved canonical through the candidate model:
`InstallationDiscoveryCandidate` gets an optional `CanonicalPath` hint,
`PathInstallationCandidateSource` populates it from
`InstallationPathHit.CanonicalPath`, and `DiscoverAllAsync` prefers the
hint when non-empty before falling back to the existing
`ResolveSymlinkToFullPath` call. Other candidate sources (release prefix,
dogfood, dotnet-tool store) emit raw paths with no hint and continue to
resolve at the `DiscoverAllAsync` site, unchanged. Empty-string ->
skip-candidate semantics are preserved on both branches.

Tests: existing `InstallationDiscoveryDiscoverAllTests` (45) still pass.
Added a new test
`DiscoverAllAsync_UsesPreResolvedCanonicalHint_WithoutReResolving` that
injects a custom `IInstallationCandidateSource` yielding a candidate
whose `CanonicalPath` is a deliberately-different non-existent path —
no real symlink involved. The resulting row's `CanonicalPath` matches
the hint verbatim, which is only possible if `DiscoverAllAsync` used
the hint rather than re-resolving.

Co-authored-by: Copilot <[email protected]>
@radical

This comment was marked as outdated.

Resolved conflicts:
- src/Aspire.Cli/Bundles/BundleService.cs: keep main's
  `GetBundleExtractDirForCurrentProcess()` helper extraction;
  switch the remaining `ResolveSymlinks` call to
  `CliPathHelper.ResolveSymlinkOrOriginalPath` (the helper our PR
  removed in favor of the shared utility).
- src/Aspire.Cli/Packaging/PackagingService.cs: keep main's
  `PackageSources.NuGetOrg` constant and `CreateLocalHiveChannel`
  helper; propagate this branch's `_features` argument into every
  `CreateExplicitChannel` call (including the helper) and switch the
  helper's path normalization from `.Replace('\\', '/')` to
  `PathNormalizer.NormalizePathForStorage`.
- tests/Aspire.Cli.Tests/Commands/NewCommandTemplateConfigPersistenceTests.cs:
  combine main's `PackageSources.NuGetOrg` with this branch's
  `features: new TestFeatures()` argument.
Comment thread tests/Aspire.Cli.Tests/Utils/CliPathHelperTests.cs Outdated
Comment thread tests/Aspire.Cli.Tests/Utils/CliPathHelperTests.cs Outdated
Comment thread src/Shared/PathLookupHelper.cs Outdated
radical and others added 3 commits May 20, 2026 17:38
…stallProbeTests

Two Windows-only PeerInstallProbeTests rows failed on CI with no actionable
diagnostic — the assertion was `Assert.IsType<Ok>(result)`, which discards the
`Failed.Reason` (which carries the spawned peer's stderr, exit code, and the
specific failure path the probe took) and reports only "expected Ok, got
Failed". The probe itself logs at Debug level when it decides each failure
path, but the tests pass `NullLogger`, so those breadcrumbs are dropped too.

Wire the diagnostics into the xunit test output so the next CI failure
(transient or otherwise) is debuggable:

- Route `PeerInstallProbe`'s `ILogger<>` through `XunitLoggerProvider` so the
  probe's LogDebug calls land in `ITestOutputHelper`. Keep the
  `ILoggerFactory` alive as a class field (disposed at test-class teardown)
  so logs aren't truncated by an early dispose mid-probe.
- Add `AssertProbeOk(result)` helper that fails with the raw `Failed.Reason`
  text instead of "expected Ok, got Failed". Replace all 8 Ok-assertion
  sites.
- Dump each rendered fake-peer script body to test output with begin/end
  markers, so the exact script the probe spawned is recoverable from a
  failed-run log (Windows vs Unix script shape differs).

xunit only surfaces captured `ITestOutputHelper` output on failing tests, so
passing runs stay quiet.

Co-authored-by: Copilot <[email protected]>
…plicative update banner)

`aspire doctor --format json` was the only `--format json` command that
still emitted the "newer Aspire CLI version available" notifier banner
on stderr. Every other JSON-emitting command (ApiGet, ApiList,
DocsSearch, DocsList) already overrides `UpdateNotificationsEnabled` to
suppress that banner.

The doctor command additionally reports the same update information
inside `checks[]` via the `cli-version` check, so emitting the banner
on stderr is duplicative and inconsistent with the rest of the JSON
surface.

Override `UpdateNotificationsEnabled => false` on DoctorCommand to
match the existing convention. The `cli-version` check still surfaces
"newer version available" inside the JSON payload via an independent
call to `IUpdateNotifier.GetVersionStatusAsync()`.

Flip the existing regression test
(`DoctorCommand_Json_VersionUpdateBanner_StaysOnStderr` ->
`DoctorCommand_Json_VersionUpdateBanner_IsSuppressed`) to assert the
notifier callback is never invoked and the version string never
appears on stderr.

Co-authored-by: Copilot <[email protected]>
…Platform attribute)

Two minor cleanups raised during self-review on the PR:

- Rename the private `PathLookupHelper.FileExists(Func<string,bool>,
  string)` helper to `FileExistsSafe` so the name reflects the
  exception-swallowing behavior (the helper catches IO/permission
  exceptions from the underlying probe). Add a brief doc comment.

- Convert four platform-conditional tests in `CliPathHelperTests`
  from inline `Assert.SkipUnless(...)` calls to the
  `[SkipOnPlatform(...)]` attribute. This matches the convention used
  elsewhere in the test suite and keeps the platform gate at the test
  declaration instead of the test body.

Co-authored-by: Copilot <[email protected]>
Comment thread localhive.ps1
@radical
Copy link
Copy Markdown
Member Author

radical commented May 20, 2026

Local dogfood test report — macOS arm64

Installed the PR CLI via the dogfood script (get-aspire-cli-pr.sh 17105 --install-path <temp> --skip-path --skip-extension), confirmed version 13.4.0-pr.17105.g4aa03d1f (matches head 4aa03d1f), and exercised the four user-facing surfaces from the PR description.

Note: head-commit CI was still building when I started; waited ~3 min for Build native CLI archive (macOS) / osx-arm64 to complete on run 26193211897, then retried.

Results

Scenario Result
aspire doctor (text) — clean install, no peer on PATH
aspire doctor (text) — with ~/.aspire/bin/aspire (no sidecar) on PATH
aspire doctor --format json
aspire doctor --self (text + JSON, hidden)
aspire integration list on PR install ✅ — 119 packages, internals excluded

Highlights

Trust gate works as designed. With ~/.aspire/bin/aspire (a pre-existing host install without a sidecar) on PATH, doctor renders the peer row without executing the binary:

│ Path                                          │ Version                   │ Channel      │ Route        │ PATH status │
├───────────────────────────────────────────────┼───────────────────────────┼──────────────┼──────────────┼─────────────┤
│ /var/folders/.../dogfood/pr-17105/bin/aspire  │ 13.4.0-pr.17105.g4aa03d1f │ pr-17105     │ pr           │ not on PATH │
│ /Users/<me>/.aspire/bin/aspire                │ (not probed)              │ (not probed) │ (not probed) │ active      │

JSON shape is clean. Stdout = pure JSON ({checks, summary, installations}), stderr = only the Checking Aspire environment... status spinner. No duplicative update banner. Probed rows carry path, canonicalPath, version, channel, route, pathStatus, status: ok; unprobed rows carry status: notProbed + a descriptive statusReason.

--self is correct for peer probing. Hidden from --help, returns checks: [] + single-element installations, stderr is empty — no recursion, no chatter for the parent to strip.

aspire add fix verified on a dogfood install. Created a fresh AppHost from the PR hive and ran aspire integration list: 119 integration packages returned (would have been 0 before this PR). Aspire.Hosting.AppHost, Aspire.AppHost.Sdk, and Aspire.Hosting.Testing are physically present in the hive directory but correctly filtered out of the user-facing list. PR-built integrations show 13.4.0-pr.17105.g4aa03d1f; pinned/older ones (e.g. aws 13.2.0, clickhouse 13.1.2, elasticsearch 13.3.0, azure-aifoundry 13.1.3-preview…) appear at their pinned versions.

Two non-bug observations worth a follow-up note

  1. JSON has no explicit isCurrent field. Text output uses a (current) suffix; the JSON consumer can only identify the running CLI by matching canonicalPath against /proc/self/exe (or equivalent) or by relying on row order. Worth a sentence in docs/specs/install-routes.md so tooling integrators don't guess.

  2. --install-path re-roots AspireHome and shifts the well-known-prefix walk. With --install-path /some/dir, the discovery walk's "release prefix" and "dogfood prefix" both root at /some/dir, not at ~/.aspire. The dotnet-tool store walk still uses real $HOME, and $PATH discovery is unaffected. In a normal user install this is invisible (AspireHome IS ~/.aspire), but if someone troubleshoots "why isn't doctor finding my other install" in a sandboxed/CI setup, the answer in some cases is "your ASPIRE_CLI_HOME is somewhere unusual." Not a bug, just discoverable behavior.

Scenarios 1–4 in the PR description all pass cleanly on macOS arm64.

…chives

When `localhive.ps1 -Archive` builds a `win-*` zip on a non-Windows host,
`Compress-Archive -Path "$Output/*"` enumerates inputs via the PowerShell
provider, which treats files whose name starts with `.` as hidden and
excludes them from `<dir>/*` wildcard expansion. The portable layout
includes `bin/.aspire-install.json` — the localhive route sidecar that
`aspire doctor` and route-aware Aspire-home selection both rely on (see
`docs/specs/install-routes.md`). Dropping the sidecar silently downgrades
the extracted install on the target machine to a sidecar-less peer that
appears as `(not probed)` in doctor output and does not participate in
route identity. Verified on macOS arm64: `Compress-Archive -Path '*'`
produces a zip without `bin/.aspire-install.json`;
`[System.IO.Compression.ZipFile]::CreateFromDirectory` includes it.

Switch the win-* archive path to `ZipFile::CreateFromDirectory`, which
walks the filesystem directly and includes dotfiles unconditionally. The
existing non-win `tar -czf <dir> .` path is unaffected. Match the prior
`-Force` semantics by removing any pre-existing archive at the same path.

Add a post-archive verification step: when the portable layout has a
sidecar on disk, the new zip MUST contain `bin/.aspire-install.json`.
This is a belt-and-suspenders guard against future regressions
(e.g. a switch back to `Compress-Archive` or a wildcard-based copy).

Add a source-contract test that asserts `localhive.ps1` does not invoke
`Compress-Archive` and does call
`[System.IO.Compression.ZipFile]::CreateFromDirectory`. The test strips
PowerShell line comments before matching, so the rationale-comment in
the script can keep mentioning the cmdlet without false-positives.

Co-authored-by: Copilot <[email protected]>
@radical radical enabled auto-merge (squash) May 20, 2026 23:10
@JamesNK
Copy link
Copy Markdown
Member

JamesNK commented May 21, 2026

Wait before merging. I want to test this to make sure UI is consistent with the rest of the CLI.

@radical radical merged commit 4ec5445 into microsoft:main May 21, 2026
599 of 603 checks passed
@microsoft-github-policy-service microsoft-github-policy-service Bot added this to the 13.4 milestone May 21, 2026
@radical
Copy link
Copy Markdown
Member Author

radical commented May 21, 2026

Wait before merging. I want to test this to make sure UI is consistent with the rest of the CLI.

@JamesNK sorry this got merged at the same time as your comment! I can do a follow up PR for any feedback.

@JamesNK
Copy link
Copy Markdown
Member

JamesNK commented May 21, 2026

  • Don't use rounded corners in the table. No other table has rounded corners.
  • The table headers aren't bold. See other tables for the helper method used to add table headings.
  • Why are some file names aspire.EXE? They aren't captalized when I look at them on the file system.
  • Status column should have colors. Green for active, yellow for everything else?
image

@JamesNK
Copy link
Copy Markdown
Member

JamesNK commented May 21, 2026

What's the difference between path and canonical path? They're always the same for me:

image

@JamesNK
Copy link
Copy Markdown
Member

JamesNK commented May 21, 2026

Route is install route? It's not obvious what it means. Source feels like a better name.

Why did you choose route? Is there prior art from other CLIs/apps about how bits were installed?

@radical
Copy link
Copy Markdown
Member Author

radical commented May 21, 2026

path is the discovered/user-facing path. canonicalPath is the resolved identity path we use internally and expose in JSON: it follows symlinks, normalizes to a full path, and handles macOS firmlink normalization.

For normal installs they’ll often be identical. They differ when the CLI is reached through a symlink/shim or when macOS reports /private/var/... for the same file a PATH walk sees as /var/.... The human table only shows path; canonicalPath is there so JSON consumers can see the stable path doctor used for deduping and PATH status.

@radical
Copy link
Copy Markdown
Member Author

radical commented May 21, 2026

Good point. “Route” was something that made sense to me as “how this particular CLI got installed.” The sidecar has a source value, and internally that maps to an install route/layout we use for bundle extraction and Aspire home selection.

But for the doctor table, “Source” is probably clearer because the column is really telling the user where/how that CLI install came from (brew, winget, script, dotnet-tool, etc.), and it matches the sidecar field name. I’ll rename the human-facing column to “Source”.

@radical radical deleted the aspire-commands branch May 21, 2026 00:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

aspire add integration picker leaks internal framework packages on PR / localhive CLI installs

6 participants