Ensure Aspire CLI bundle resolution - #19251
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 19251Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 19251" |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Ensures CLI bundle dependencies are prepared and propagated for direct AppHost builds and launches.
Changes:
- Runs bundle setup from installers and MSBuild when required.
- Carries validated DCP/dashboard paths through CLI inspection and caching.
- Adds regression coverage and localized diagnostics.
Reviewed changes
Copilot reviewed 31 out of 32 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
tests/Aspire.Hosting.Tests/MSBuildTests.cs |
Tests bundle preparation and failure diagnostics. |
tests/Aspire.Hosting.Sdk.Tests/AppHostSdkTargetsTests.cs |
Tests PATH/DNX setup behavior. |
tests/Aspire.Hosting.Sdk.Tests.FakeCommand/Program.cs |
Simulates bundle setup. |
tests/Aspire.Cli.Tests/Projects/DotNetAppHostProjectTests.cs |
Tests runtime-path precedence and validation. |
tests/Aspire.Cli.Tests/Projects/AppHostInfoResolverTests.cs |
Tests discovery metadata retrieval. |
tests/Aspire.Cli.Tests/Caching/AppHostInfoDiskCacheTests.cs |
Tests missing bundle-file invalidation. |
tests/Aspire.Acquisition.Tests/Scripts/ReleaseScriptShellTests.cs |
Verifies shell installer setup ordering. |
tests/Aspire.Acquisition.Tests/Scripts/ReleaseScriptPSFunctionTests.cs |
Tests PowerShell setup failure propagation. |
tests/Aspire.Acquisition.Tests/Scripts/ReleaseScriptPowerShellTests.cs |
Verifies PowerShell installer setup ordering. |
tests/Aspire.Acquisition.Tests/Scripts/ReleaseScriptFunctionTests.cs |
Tests shell setup failure propagation. |
src/Aspire.Hosting.Tasks/ResolveAspireCliBundle.cs |
Exposes the resolved Aspire home. |
src/Aspire.Hosting.AppHost/build/Aspire.Hosting.AppHost.in.targets |
Adds on-demand bundle setup and diagnostics. |
src/Aspire.Cli/Resources/xlf/RunCommandStrings.zh-Hant.xlf |
Adds localized-resource entry. |
src/Aspire.Cli/Resources/xlf/RunCommandStrings.zh-Hans.xlf |
Adds localized-resource entry. |
src/Aspire.Cli/Resources/xlf/RunCommandStrings.tr.xlf |
Adds localized-resource entry. |
src/Aspire.Cli/Resources/xlf/RunCommandStrings.ru.xlf |
Adds localized-resource entry. |
src/Aspire.Cli/Resources/xlf/RunCommandStrings.pt-BR.xlf |
Adds localized-resource entry. |
src/Aspire.Cli/Resources/xlf/RunCommandStrings.pl.xlf |
Adds localized-resource entry. |
src/Aspire.Cli/Resources/xlf/RunCommandStrings.ko.xlf |
Adds localized-resource entry. |
src/Aspire.Cli/Resources/xlf/RunCommandStrings.ja.xlf |
Adds localized-resource entry. |
src/Aspire.Cli/Resources/xlf/RunCommandStrings.it.xlf |
Adds localized-resource entry. |
src/Aspire.Cli/Resources/xlf/RunCommandStrings.fr.xlf |
Adds localized-resource entry. |
src/Aspire.Cli/Resources/xlf/RunCommandStrings.es.xlf |
Adds localized-resource entry. |
src/Aspire.Cli/Resources/xlf/RunCommandStrings.de.xlf |
Adds localized-resource entry. |
src/Aspire.Cli/Resources/xlf/RunCommandStrings.cs.xlf |
Adds localized-resource entry. |
src/Aspire.Cli/Resources/RunCommandStrings.resx |
Adds missing-path diagnostic. |
src/Aspire.Cli/Resources/RunCommandStrings.Designer.cs |
Exposes the new resource. |
src/Aspire.Cli/Projects/DotNetAppHostProject.cs |
Validates and injects resolved runtime paths. |
src/Aspire.Cli/Projects/AppHostInfoResolver.cs |
Captures build-resolved bundle metadata. |
src/Aspire.Cli/Caching/AppHostInfoDiskCache.cs |
Caches and validates bundle paths. |
eng/scripts/get-aspire-cli.sh |
Runs setup after shell installation. |
eng/scripts/get-aspire-cli.ps1 |
Runs setup after PowerShell installation. |
Files not reviewed (1)
- src/Aspire.Cli/Resources/RunCommandStrings.Designer.cs: Generated file
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
PR Testing ReportPR Information
Artifact Version Verification
Changes Analyzed
Change Categories
Test Scenarios Executed1. Focused source testsObjective: Validate the changed acquisition, resolver/cache/project, SDK-target, and MSBuild behavior from the exact PR source. Status: PASSED
Evidence: 2. Windows and Linux PR artifact installationObjective: Install the actual PR CLI artifacts in isolated locations and verify that each matches the PR head. Status: PASSED
Evidence: 3. Changed release installer scriptsObjective: Exercise the PowerShell and shell release installer implementations from the PR source, including eager bundle setup. Status: PASSED
Evidence: 4. Direct AppHost launch with no existing bundle layoutObjective: Reproduce the original IDE/direct-launch path with Status: PASSED
Evidence: 5. On-demand recovery after runtime deletionObjective: Verify stale/missing runtime files are recoverable on a subsequent direct launch. Status: PASSED
Evidence: 6. Invalid explicit CLI pathObjective: Ensure an invalid authoritative Coverage type: Unhappy path Status: PASSED (expected failure)
Evidence: 7. Fresh
|
| Scenario | Status | Notes |
|---|---|---|
| Focused source tests | PASSED | 269 passed, 1 platform skip |
| Windows/Linux PR artifact verification | PASSED | Both match 621c5850 |
| PowerShell/shell release installers | PASSED | Fresh bundle layouts extracted |
Direct dotnet run without layout |
PASSED | Layout recovered; dashboard HTTP 200 |
| Repeated recovery after deletion | PASSED | Layout recreated; dashboard HTTP 200 |
| Invalid explicit CLI path | PASSED | Expected actionable ASPIRE009 |
Fresh aspire start |
FAILED | MSB4057 before first restore |
Overall Result
ISSUES FOUND - PR NOT VERIFIED
The intended direct-launch and installer behavior works, but the new pre-build AppHost inspection breaks the standard fresh-project workflow before restore. The PR should not merge until AppHostInfoResolver avoids requesting package-defined targets before they are available, or ensures the project is restored before that probe. The cold-cache aspire new then aspire start path should remain covered by end-to-end validation.
Artifacts
- Report:
pr-19251-testing-report.md - Scenario evidence:
evidence/ - CI failure diagnostics:
ci-failures/ - Temporary root:
C:\Users\karolz\AppData\Local\Temp\aspire-pr-19251-621c585-copilot
PR Comment and Cleanup
- PR comment: Posted at Ensure Aspire CLI bundle resolution #19251 (comment)
- Temporary workspace: Removed after user confirmation
Karol Zadora-Przylecki (karolz-ms)
left a comment
Karol Zadora-Przylecki (karolz-ms)
left a comment
There was a problem hiding this comment.
Found 2 correctness issues.
Karol Zadora-Przylecki (karolz-ms)
left a comment
Karol Zadora-Przylecki (karolz-ms)
left a comment
There was a problem hiding this comment.
Code review of the CLI bundle resolution changes. 10 findings, all problems rather than style notes.
By category
- Correctness / behavioral regression (3): unusable bundle-layout paths shadow the new build-resolved fallback and hard-fail the launch; the new launch guard ignores env vars,
DcpPublisherconfig, and baked assembly metadata; publish/deploy is now blocked on a dashboard binary it never uses. - Diagnostics (1): MSBuild errors during project inspection are swallowed and reported as "not an AppHost".
- Behavior change in a probe path (1):
aspire run/aspire lsinspection can now executeaspire setupor adnxdownload, with two 120sExectimeouts. - Acquisition scripts (1): a transient bundle-setup failure aborts the whole install after the binary is written but before PATH configuration.
- Test reliability (1): the Windows branch of the new DNX test does not exit — it consumes the 120s
Exectimeout out of a 180s budget. - Build robustness (2):
Execlaunch failures surface as warnings that break-warnaserrorconsumers; the cmd shim caret-escaping omits%. - Test coverage (1): no coverage for the environment-provided-paths case, the publish guard, or the stale-layout regression that motivated this PR.
De-duplicated against feedback already on this PR — the cache-identity issue (raised twice on AppHostInfoDiskCache.cs), the MSB4057 missing-target problem on AppHostInfoResolver.cs:147, and the MSB4057 blocker in the PR testing comment are deliberately not repeated. My two comments in that same area are scoped to what survives a fix for it: the swallowed MSBuild diagnostics, and the Exec/dnx side effects that remain even for restored Aspire AppHosts.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 24 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- src/Aspire.Cli/Resources/RunCommandStrings.Designer.cs: Generated file
Suppressed comments (3)
src/Aspire.Cli/Projects/DotNetAppHostProject.cs:2480
- This only logs when an
AspireUseCliBundle=truelaunch has no current CLI layout;RunAsyncstill proceeds without verifying that DCP and dashboard are available from overrides or the built AppHost metadata. In particular,--no-buildcan launch an older output whose stamped paths were deleted and reproduce the lateOptionsValidationExceptionthis change is intended to replace. Carry the resolved paths through inspection and reject the launch with the actionable CLI diagnostic when neither source is usable.
_logger.LogDebug("AspireUseCliBundle is enabled, but the Aspire CLI bundle layout was not available from this CLI process. The AppHost will resolve configured, inherited, or assembly-metadata paths.");
src/Aspire.Hosting.AppHost/build/Aspire.Hosting.AppHost.in.targets:262
- The percent escaping is only applied to
.cmd/.batshims, but MSBuildExecwrites a temporary batch file for this native-executable branch too. On Windows, a validaspire.exepath containing a literal%NAME%segment is therefore environment-expanded and the setup command targets the wrong path. Escape%to%%for the Windows native path before constructing this command.
<_AspireCliSetupCommand Condition="'$(AspireCliBundlePath)' == '' and Exists('$(_AspireCliSetupPath)') and '$(_AspireCliSetupPathIsWindowsCommandShim)' != 'true'">"$(_AspireCliSetupPath)" setup</_AspireCliSetupCommand>
tests/Aspire.Hosting.Sdk.Tests/AppHostSdkTargetsTests.cs:930
- The fake DNX handles
setup --install-pathonly in the Unix script arm. On Windows,CreateFakeDnxAsyncstill just captures the arguments and exits successfully without creatingbundle/dcp/dcp.exeorbundle/managed/aspire-managed.exe; the updated cross-platform test then re-resolves an empty layout and fails with ASPIRE009. Add the equivalent setup case to the Windows batch implementation.
if [ "$1" = "--yes" ] && [ "$2" = "aspire.cli@13.5.0" ] && [ "$3" = "--" ] && [ "$4" = "setup" ] && [ "$5" = "--install-path" ]; then
mkdir -p "$6/bundle/dcp" "$6/bundle/managed"
: > "$6/bundle/dcp/dcp"
: > "$6/bundle/managed/aspire-managed"
exit 0
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
fc7f515 to
9e25792
Compare
This comment has been minimized.
This comment has been minimized.
Karol Zadora-Przylecki (karolz-ms)
left a comment
Karol Zadora-Przylecki (karolz-ms)
left a comment
There was a problem hiding this comment.
Reviewed the CLI bundle resolution changes end to end (MSBuild targets, CLI launch/publish paths, install scripts, and tests), taking the two earlier review rounds and the fixes in fc7f515f into account.
7 issues found:
- Bugs / correctness (3):
Exectimeout bypassing the ASPIRE009 diagnostic (verified empirically against MSBuild), empty environment variables treated as overrides inHasEnvironmentOverride, and a non-runnable command string embedded in the ASPIRE009 text. - Behavior consistency (2): unvalidated inherited runtime paths taking precedence over a validated layout, and the PowerShell installer throwing on unsupported host architectures where the shell installer degrades gracefully.
- Cross-platform parity (1): bundle setup / VS Code extension install ordering differs between
get-aspire-cli.ps1andget-aspire-cli.sh. - Test coverage (1): the
aspire setupfake extracts toASPIRE_HOMErather than next to the CLI binary, so the CLI-adjacent re-resolution path from #19227 isn't actually covered.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 24 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- src/Aspire.Cli/Resources/RunCommandStrings.Designer.cs: Generated file
Suppressed comments (2)
tests/Aspire.Cli.Tests/Caching/AppHostInfoDiskCacheTests.cs:131
runCommandandrunArgumentsare not project-only metadata for bundle AppHosts:ComputeRunArgumentsembeds the absolute Aspire/DNX command selected fromPATH.ComputeKeyAsyncdoes not fingerprintPATH(or other environment-provided MSBuild properties), so changing PATH between CLI invocations while the old CLI still exists returns this cache entry and direct launch runs the previously selected CLI. Include the invocation environment in the cache identity or stop persisting theRun*values, and add a regression that switches between two existing PATH CLIs.
"runCommand",
"runWorkingDirectory",
src/Aspire.Hosting.AppHost/build/Aspire.Hosting.AppHost.in.targets:300
- This recovery path still fails under
-warnaserrorwhen the PATH CLI times out.WarnAndContinueconverts Exec's timeout error (MSB5002) into a warning, which warnings-as-errors promotes even if the subsequent DNX fallback creates a valid layout. Suppress/demote the expected timeout diagnostic when fallback can recover, and run the existing timeout-fallback test with warnings treated as errors.
Timeout="$(_AspireCliBundleSetupTimeout)"
ContinueOnError="WarnAndContinue">
Damian Edwards (DamianEdwards)
left a comment
Damian Edwards (DamianEdwards)
left a comment
There was a problem hiding this comment.
Found 2 correctness issues in the bundle recovery target: invalid explicit-path diagnostics are suppressed, and a recovered timeout can still fail warnings-as-errors builds.
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Tests selector (audit mode)The full test matrix and all jobs still run in audit mode. The tests and jobs below are what selective CI would run under enforcement. 52 / 100 test projects · 7 jobs, from 27 changed files. Selected test projects (52 / 100)
Selected jobs (7)
How these were chosen — grouped by what changed
🔧 show 41
🔧 📄 📄 📦 affected project 📦 affected project 🔧 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 Job reasons
Selection computed for commit |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 26 out of 27 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- src/Aspire.Cli/Resources/RunCommandStrings.Designer.cs: Generated file
Suppressed comments (1)
src/Aspire.Cli/Projects/DotNetAppHostProject.cs:1515
- The PR description still says project inspection carries build-resolved DCP/dashboard paths into direct launches, but this code now inspects only
IsUsingCliBundleand resolves runtime paths from the current CLI layout; the new cache and run tests explicitly enforce that inspection paths are not persisted or injected. The earlier cache fix makes the code behavior intentional, so update the description to describe build-stamped assembly metadata plus run-time layout injection instead of inspection-carried paths.
var appHostInfo = canQueryCliBundleProperty
? await _appHostInfoResolver.GetAppHostInfoAsync(effectiveAppHostFile, cancellationToken)
: null;
var injectDcpAndDashboard = appHostInfo?.IsUsingCliBundle == true;
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Damian Edwards (DamianEdwards)
left a comment
Damian Edwards (DamianEdwards)
left a comment
There was a problem hiding this comment.
No new issues found, I think this is good to go!
f65a201
into
main
|
/backport to release/13.5 |
|
Started backporting to |
|
Pull request created: #1496
|
|
📝 Documentation has been drafted in microsoft/aspire.dev#1496 targeting Updated Note This draft PR needs human review before merging. |
Description
Fresh Aspire CLI installations can have a valid embedded bundle that has not been extracted yet. When an AppHost uses
AspireUseCliBundle=true, that previously allowed the build to complete without DCP or dashboard metadata, causing direct or IDE launches to fail before the first successful CLI run.This change ensures the bundle is available across installation and launch paths:
aspire setupthrough the selected PATH or paired DNX CLI only when needed, then resolve the layout again.User-facing behavior
After installing the Aspire CLI, users can build and launch an
AspireUseCliBundle=trueAppHost directly from an IDE without first running an Aspire CLI command. Installations that did not use the release scripts are recovered by the build targets on demand.Validation included:
13.5.0+cfbf1c432e94dfe4a3261593eeef8c93913079bbthrough both updated scripts.ASPIRE_DCP_PATHandASPIRE_DASHBOARD_PATHremoved, reaching a healthy dashboard.Fixes #19227
Checklist
<remarks />and<code />elements on your triple slash comments?