[release/13.4] Fix five aspire ls bugs from #17620 (L1–L5)#17688
Conversation
Fixes #17615, #17620, #17621, #17624, #17626. - L1 (#17615): Remove the eager-migration block in ConfigurationHelper.RegisterSettingsFiles. Read commands like `aspire ls` no longer silently materialize an aspire.config.json next to a user's legacy .aspire/settings.json. Migration now happens lazily/explicitly via the existing write paths. - L2 (#17620): Drop the `silent` parameter from ProjectLocator.GetAppHostProjectFileFromSettingsAsync so the legacy branch unconditionally surfaces the migration warning, and surface the actual user-authored `.aspire/settings.json` path in the warning text rather than the auto-created `aspire.config.json` path. - L3 (#17621): Remove the dead post-emission `appHosts.Sort()` in LsCommand.FindAppHostsWithJsonStreamAsync (--stream emits candidates as they are discovered, so the sort had no effect on already-emitted output). Update the --stream option description and docs/specs/cli-output-formats.md to declare the arrival-ordered contract. - L4 (#17624): Add an IsValidConfiguredAppHostPath helper in ProjectLocator that rejects `\0` and Path.GetInvalidPathChars() before the path is passed to Path.IsPathRooted / Path.Combine. Wired into both the modern `aspire.config.json` (`appHost.path`) branch and the legacy `.aspire/settings.json` (`appHostPath`) branch. Validation is intentionally at the consumption point rather than in AspireConfigFile.Load, which has 12+ unrelated callers. Adds a new ConfiguredAppHostPathHasInvalidCharacters resource string and refreshes the xlf set via UpdateXlf. - L5 (#17626): Add PathNormalizer.ResolveSymlinks in src/Shared, a recursive segment-walker that canonicalizes intermediate symlinks (Directory.ResolveLinkTarget only reads exactly the path it is given, and returns the link target as stored on disk — so a single call on /tmp/x/y.cs does not unwrap /tmp -> /private/tmp, and following a link whose stored target is /var/.../app keeps the un-canonical /var prefix). The recursion has a hard depth limit of 40 and falls back to the un-resolved input on broken or circular links. Use it in AddSettingsAppHostCandidateAsync as a comparison key only — the surfaced AppHostProjectCandidate keeps its original FileInfo so the displayed path matches what the user authored in settings. Tests: 158 of 160 targeted tests pass (2 Windows-only skipped on macOS). New tests cover L1 (no migration on read), L2 (legacy warning references settings.json), L3 (arrival-order under --stream), L4 (NUL byte in modern and legacy branches), L5 (symlink dedupe via a node_modules-hosted link the discovery walk excludes), plus 5 unit tests on PathNormalizer.ResolveSymlinks itself. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
PR #17671 owns the Foundry ATS baseline update; this branch should only contain the AppHost and CLI fixes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 17688Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 17688" |
|
❓ CLI E2E Tests unknown — 109 passed, 0 failed, 2 unknown (commit View all recordings
📹 Recordings uploaded automatically from CI run #26661737572 |
Documents the aspire ls command which discovers and lists AppHost project candidates, including: - Table and JSON output formats - Streaming mode (--format json --stream) and its ordering behavior - The --all flag for including all candidates - Configuration file path validation error messages Fixes noted in microsoft/aspire#17688 (backport of fixes L1-L5): - Stream output is emitted in arrival order (not sorted); non-streaming JSON output is sorted by path - Guidance on sorting streamed output with jq Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Pull request created: #1134
|
|
📝 Documentation has been drafted in microsoft/aspire.dev#1134 targeting Created a new Files changed:
Note This draft PR needs human review before merging. |
Backport of #17631 to release/13.4
/cc @joperezr @adamint
Customer Impact
Testing
Risk
Regression?