Fix Aspire CLI versioning and socket management issues (13.5 milestone)#18261
Closed
mitchdenny wants to merge 7 commits into
Closed
Fix Aspire CLI versioning and socket management issues (13.5 milestone)#18261mitchdenny wants to merge 7 commits into
mitchdenny wants to merge 7 commits into
Conversation
During 'aspire update', WarnIfCliSdkVersionSkew reads the SDK version from disk. At that point the in-memory config has already been updated to the CLI's version but hasn't been saved yet, causing a false positive warning about version mismatch. Thread the target SDK version through GenerateCodeViaRpcAsync to WarnIfCliSdkVersionSkew. When the target aligns with the CLI version, skip the warning since the on-disk config is stale and about to be overwritten. Also extract IAppHostServerSessionFactory.Start() to enable unit testing the full UpdatePackagesAsync → BuildAndGenerateSdkAsync → GenerateCodeViaRpcAsync → WarnIfCliSdkVersionSkew code path without starting a real process. Fixes #18103
The <remarks> referenced FakeFailingAppHostServerProject but the test actually uses FakeSucceedingAppHostServerProject. It also stated the call would fail at regeneration, but with the fake session returning empty results the full flow succeeds.
…eractive mode When no AppHost is found in the current directory but other AppHosts are running on the system, commands like 'aspire describe --non-interactive' fell through to an interactive selection prompt, which threw and surfaced as 'An unexpected error occurred' with a generic exit code. AppHostConnectionResolver now checks ICliHostEnvironment.SupportsInteractiveInput before prompting: - Only out-of-scope AppHosts running: returns the command's standard not-found error with CliExitCodes.FailedToFindProject. - Multiple in-scope AppHosts: returns a new actionable error telling the user to pass --apphost, also with FailedToFindProject. Fixes #17619 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fixes issue where aspire stop leaves socket files behind, causing subsequent aspire add/describe commands to fail with connection timeouts. Resolves #17587: aspire add fails after aspire run --detach and aspire stop The socket file was not being deleted after successfully stopping an AppHost instance. Subsequent commands would attempt to connect to the stale socket path, resulting in timeout errors. Now we explicitly delete the socket file after the instance has been stopped and monitored for process termination. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…phost Fixes issue where aspire describe --apphost fails when path contains symlinks (e.g., /tmp on macOS which resolves to /private/tmp). Resolves #17618: aspire describe --apphost fails to find running AppHost when path traverses a symlink The socket lookup in AppHostConnectionResolver now canonicalizes the project file path before computing the backchannel socket key, matching the path canonicalization that occurs when the AppHost is started via ProjectLocator. This ensures both the running AppHost and the describe command use the same canonical path when looking up sockets, regardless of whether the user provided a symlinked path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 18261Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 18261" |
Contributor
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Contributor
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Member
Author
PR Testing Report: PR #18261PR Information
Artifact Version Verification
Changes AnalyzedFiles ChangedThe PR modified 41 files across three main categories: CLI Core Changes:
Command Changes (for IHostEnvironment support):
Supporting Files:
Change Categories
Issues FixedThis PR consolidates fixes for three 13.5 milestone issues:
Test Scenarios ExecutedScenario 1: Non-Interactive Mode Error Handling (Issue #17619)Objective: Verify Steps:
Evidence:
Result: Command fails gracefully with appropriate guidance instead of crashing or prompting for input. Scenario 2: Socket Cleanup After Stop (Issue #17587)Objective: Verify socket files are cleaned up after Steps:
Evidence:
Result: Socket cleanup works correctly; sequential CLI operations succeed without socket conflicts. Scenario 3: Symlink Path Resolution (Issue #17618)Objective: Verify Steps:
Evidence:
Result: Path canonicalization works correctly for symlinked AppHost paths. Scenario 4: Basic CLI Operations Smoke TestObjective: Verify CLI core functionality is not regressed (aspire new, aspire run, aspire add, aspire --version) Steps:
Evidence:
Result: Core CLI functionality works as expected with no regressions. CLI Version DetailsSummary
Overall Result✅ PR #18261 VERIFIED All 4 test scenarios passed successfully. The PR correctly implements:
Recommendations
Testing Environment
|
Contributor
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
The explicit --apphost socket lookup in AppHostConnectionResolver used PathNormalizer.ResolveToFilesystemPath, which only normalizes Windows casing and is a no-op on Linux/macOS. A running AppHost keys its backchannel socket off the symlink-resolved path (its process working directory is reported physically by the OS, e.g. /tmp -> /private/tmp on macOS), so the consumer never matched when the supplied path traversed a symlink and reported 'No AppHost is currently running'. Switch the socket-key computation to PathNormalizer.ResolveSymlinks so the consumer hashes the same canonical path as the producer. The user-facing error path still displays the original supplied path. Adds a regression test that fails on the previous behavior, and updates the dead-PID pruning test to key its socket off the resolved path (matching a real AppHost) since the macOS temp workspace lives under the symlinked /var -> /private/var. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
Author
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
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.
Description
This PR consolidates fixes for 3 Aspire CLI issues related to socket management and path canonicalization in the 13.5 milestone. Each issue was reproduced, fixed, and validated with targeted unit tests.
Fixed Issues
[bug] aspire describe --non-interactive crashes with 'An unexpected error occurred' when no AppHost in cwd #17619: aspire describe --non-interactive crashes with prompt when no AppHost found
AppHostConnectionResolverattempted interactive prompt even in non-interactive modeservices.HostEnvironmentparameter toAppHostConnectionResolverconstructor inTerminalAttachCommandandTerminalPsCommandso callers can pass theIHostEnvironmentfor non-interactive detectionaspire describe --non-interactivewithout the AppHost, and get a clean error message instead of a crash[AspireE2E]
aspire addfailed to add the integration package after stopping thedetachprocess #17587: aspire add fails after aspire run --detach and aspire stopaspire stopcompleted, leaving stale socket files that subsequentaspire addcommands would try to connect to, causing timeoutsRunningInstanceManager.StopRunningInstanceAsyncafter successfully stopping and monitoring the instanceaspire run --detach,aspire stop, and immediately runaspire addwithout errors[bug] aspire describe --apphost fails to find running AppHost when path traverses a symlink (e.g. /tmp on macOS) #17618: aspire describe --apphost fails when path traverses symlink (e.g., /tmp on macOS)
/tmpis a symlink to/private/tmp. Whenaspire describe --apphost /tmp/project/apphost.cswas run, the socket lookup path didn't match the canonicalized path used when the AppHost was started--apphostpath usingPathNormalizer.ResolveToFilesystemPathinAppHostConnectionResolverbefore socket lookup, matching the canonicalization that occurs inProjectLocator.UseOrFindAppHostProjectFileAsyncwhen starting the AppHost--apphostflag; the CLI will resolve them to the same canonical path as the running AppHostValidation
Fixes # 17619, # 17587, # 17618
Checklist
<remarks />and<code />elements on your triple slash comments?