Support live workspace AppHost updates#16836
Conversation
Fix multi-AppHost workspace discovery so the tree defaults to global view when more than one AppHost candidate is reported, while preserving single-AppHost workspace behavior and scoping CodeLens resource state to the document's AppHost. 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 -- 16836Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 16836" |
Keep the describe --follow compatibility messaging changes with their localization support, and classify non-unsupported AppHost exits as AppHost-version issues instead of CLI-version issues. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Updates the VS Code Aspire extension’s behavior in multi-AppHost workspaces by defaulting to global polling (instead of workspace describe --follow) when multiple AppHost candidates are discovered, and by scoping CodeLens resource state to the AppHost associated with the current document to avoid cross-AppHost state bleed.
Changes:
- Switches to global view automatically when
aspire extension get-apphostsreports multiple AppHost candidates (while preserving a selected AppHost path as a workspace fallback). - Enhances workspace
describe --followhandling by capturing non-NDJSON output/stderr to provide more actionable compatibility errors when no JSON data is produced. - Updates CodeLens state resolution to only use resource state from the AppHost that corresponds to the current document; adds/extends unit tests for these scenarios.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| extension/src/views/AppHostDataRepository.ts | Multi-AppHost detection and auto-switch to global view; extended describe --follow failure diagnostics and error selection logic. |
| extension/src/editor/AspireCodeLensProvider.ts | Scopes resource CodeLens state to the AppHost matching the current document to prevent cross-AppHost resource-state borrowing. |
| extension/src/test/aspireCodeLensProvider.test.ts | Adds coverage ensuring resource lenses aren’t emitted using state from a different running AppHost. |
| extension/src/test/appHostDataRepository.test.ts | Adds coverage for multi-AppHost auto-switch to global polling and new describe-watch compatibility error scenarios. |
Update the describe --follow no-data comment so it matches the intentional compatibility hint behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Match CodeLens resource scoping for gutter decorations so stopped AppHost files do not show state from another running AppHost with the same resource names. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
There’s a new command I want this to use which is aspire ls |
Switch extension AppHost discovery to aspire ls JSON output and use candidate status to ignore possibly-unbuildable AppHosts for workspace/global mode decisions. Also avoid showing the AppHost describe compatibility banner when describe exits successfully without resource data. 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>
adamint
changed the title
|
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.
Matched test failure patterns (26 tests)
|
# Conflicts: # src/Aspire.Hosting/Backchannel/BackchannelDataTypes.cs # tests/Aspire.Cli.Tests/Backchannel/ResourceSnapshotMapperTests.cs
…o fix/issue-16810-global-view
PR Testing ReportPR Information
CLI Version Verification
Changes AnalyzedThe PR changes VS Code extension live AppHost/resource update handling and CLI backchannel/ Test Scenarios ExecutedScenario 1: Dogfood CLI install and version checkObjective: Verify the PR dogfood artifact is available and corresponds to the current PR head. Result: Passed Evidence:
Scenario 2: Fresh AppHost lifecycle and
|
| Scenario | Status | Notes |
|---|---|---|
| Dogfood CLI install/version check | Passed | Artifact version matched PR head commit suffix. |
Fresh AppHost lifecycle and table ps output |
Passed | Create/build/start/list/stop worked. |
ps --format json --resources parseability |
Failed | Human-readable status text is emitted before the JSON payload. |
Overall Result
Issue found. The new JSON/resource output path returns resource data, but stdout is not directly parseable as JSON because aspire ps writes a status line before the JSON payload.
Avoid rendering scan status while producing JSON output so non-interactive ps output remains parseable. Add regression coverage and lifecycle tests for follow/process cleanup paths. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| function createAppHostProjectSearchResult(appHostCandidates: AppHostCandidate[], selectedProjectFile: string | null): AppHostProjectSearchResult { | ||
| const buildableCandidates = appHostCandidates.filter(isBuildableAppHostCandidate); | ||
| const allProjectFileCandidates = buildableCandidates.map(candidate => candidate.path); | ||
| const selectedCandidate = selectedProjectFile && buildableCandidates.some(candidate => isSamePath(candidate.path, selectedProjectFile)) |
There was a problem hiding this comment.
What happens if an existing workspace has multiple buildable AppHosts but its configured appHost.path points outside the aspire ls result set, for example to a sibling/parent AppHost or one otherwise excluded from ambient discovery? The old extension get-apphosts path intentionally preserved that configured selection and added it to all_project_file_candidates even when it was outside the discovered list, but this refactor drops it here unless it matches one of the buildable aspire ls candidates. In that case the extension loses the user's default AppHost, never starts describe --follow --apphost for it, and workspace mode filters ps down to the wrong candidate set. This silently breaks brownfield workspaces that relied on a valid configured AppHost outside the ambient search results. The fix direction is to preserve a configured selected path even when it is not in appHostCandidates (and include it in the candidate paths used downstream), matching the old ProjectLocator contract.
There was a problem hiding this comment.
Fixed in 5bc88da. Configured AppHost paths from aspire.config.json/.aspire/settings.json are now preserved even when they are outside the aspire ls result set, and included in the downstream candidate paths used by workspace describe/ps handling. Added regression coverage for discovery and repository workspace behavior.
This is weird. When format is JSON then all non-JSON content should automatically go to stderr. I assume this must work when run during manual testing. |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Wait for the async workspace AppHost discovery state instead of assuming a single timer/microtask flush is enough. This avoids the Windows CI race where the assertion can run before vscode.workspace.fs.readFile completes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| private static readonly Option<bool> s_includeHiddenOption = new("--include-hidden") | ||
| { | ||
| Description = SharedCommandStrings.IncludeHiddenOptionDescription | ||
| }; |
There was a problem hiding this comment.
Why does ps now include resources?
Why not use describe to get resources?
There was a problem hiding this comment.
describe works well once we have a specific AppHost, and we still use it for workspace-mode resource streaming. The global view needs one ambient command that can return every running AppHost with its resources; doing that with describe would mean ps first, then one describe process per AppHost, plus reconciling AppHost add/remove races across those streams.
I’ll keep ps --resources for that scenario and make the streaming contract clearer with the follow-up change.
There was a problem hiding this comment.
But I don't think ps should include resources. We have something that streams resources today and that's describe.
then one describe process per AppHost
Which will probably either be 0 or 1. You'll need to support more running at once, but that won't be typical. Having two ongoing CLI calls doesn't sound like a big deal to me.
PR testing resultsI ran the PR testing workflow against the dogfood CLI for this PR. Result: Issues found. The PR-specific Passed
Failed
This appears outside the PR’s changed CLI/extension area, but it is a dogfood smoke failure from the PR hive in this local environment. |
JamesNK
left a comment
JamesNK
left a comment
There was a problem hiding this comment.
Noting the streaming pattern inconsistency between ps --follow (full snapshots) vs describe --follow / logs --follow (incremental per-item NDJSON).
| async Task WriteSnapshotAsync() | ||
| { | ||
| var appHostInfos = await GatherAppHostInfosAsync(currentConnections, includeResources, includeHidden, followCancellationToken).ConfigureAwait(false); | ||
| var json = JsonSerializer.Serialize(appHostInfos, PsCommandJsonContext.CompactRelaxedEscaping.ListAppHostDisplayInfo); | ||
| if (!string.Equals(json, lastJson, StringComparison.Ordinal)) | ||
| { | ||
| lastJson = json; | ||
| _interactionService.DisplayRawText(json, ConsoleOutput.Standard); |
There was a problem hiding this comment.
Inconsistency with other --follow commands.
ps --follow emits a full JSON array snapshot of all AppHosts (with all their resources) on every change, while the existing --follow implementations stream incrementally:
describe --follow: NDJSON where each line is a singleResourceJsonfor the resource that changed (per-resource delta).logs --follow: NDJSON where each line is a single log entry as it arrives.ps --follow(this PR): NDJSON where each line is the entireList<AppHostDisplayInfo>array re-serialized.
I understand the motivation — the extension consumer replaces its entire _appHosts array on each line, so full snapshots simplify the client. But this creates an inconsistent streaming contract across the CLI:
- Consumers of
describe --followmust maintain a map and merge per-resource updates; consumers ofps --followget the full truth on every line. Should these commands follow the same pattern for consistency? - As the number of AppHosts/resources grows, re-serializing everything on each single resource state change (e.g., a health check toggling) could produce large repeated output.
describe --followavoids this by only emitting the changed resource.
Worth considering whether ps --follow should either:
- Stream per-AppHost deltas (like
describestreams per-resource), or - Document explicitly that this command intentionally uses full-snapshot semantics and why (so future maintainers don't try to "fix" it to match
describe).
At minimum, a code comment here explaining the design choice (full snapshot for simpler client-side replacement) would help.
There was a problem hiding this comment.
I think you can fix this by streaming just app host instances that are new or change, and have a column for status. e.g. running/stopped
Because app hosts can disappear (i.e. be stopped) then that must be communicated to consumers which can be done by returning the app host one last time with a status of stopped.
There was a problem hiding this comment.
Agreed, I’ll change ps --follow to stream per-AppHost updates instead of full snapshots. I’ll add a status field so removed AppHosts can be emitted once more as stopped, and resource changes can emit the owning AppHost with updated resources.
|
Fixed the JSON stdout contamination in the current push. |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
I got this working locally just fine. Took me a moment to figure out how to flick into the global vs. home view. The onther thing that caught me out was that if I create an app host in two sub directories and launch each of then, then open vscode rooted on the directory above, it doesn't show them unless I flip to the global view. As a follow up you could make it automatically detect that sub-folders have apphosts running (and there isn't a single apphost in the current immediate scope) and then flip to show that workspace view. It might be that we need three views. Global, Tree, Immediate dir (terms aren't exactly what I'm going for but you get the idea). |
Resolve conflicts from main reverting parser-backed AppHost parsing (microsoft#17413). Take main's regex-based csharp parser and drop the tree-sitter helpers + orphan emscripten typedef, while preserving this branch's workspace-AppHost-aware gutter/lens filtering and new tests (now sync to match the reverted parser interface). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
I made some changes to how the extension finds apphosts here #17408. I think this logic watch the workspace and re-run ls to avoid having the logic for "what is an apphost" be in multiple places. |
Resolve XLF conflicts from PR microsoft#17459 (Remove Logs column from aspire ps). Drop HeaderCliLog trans-units from every locale and keep this branch's FollowOptionDescription / FollowRequiresJson additions; the orphan HeaderCliLog resx/Designer entries had already been auto-merged away. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Description
Fixes #16810
This updates the VS Code extension and Aspire CLI AppHost discovery flow for workspaces with multiple AppHosts:
aspire lsfinds multiple buildable workspace AppHosts.aspire describe --follow --apphost <path>for the selected/default workspace AppHost so workspace resources stay scoped to the intended AppHost.aspire ps --follow --format json --resources, emitting newline-delimited full AppHost snapshots for live running-AppHost/resource updates without increasing polling frequency.aspire pspolling whenps --followor--resourcesis unavailable.describe --followresources whenpshas not populated resources for the selected AppHost yet, including empty resource arrays.Validation:
./restore.shdotnet test --project tests/Aspire.Cli.Tests/Aspire.Cli.Tests.csproj --no-launch-profile -- --filter-class "*.PsCommandTests" --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"dotnet build /t:UpdateXlf src/Aspire.Cli/Aspire.Cli.csprojcd extension && npm run compile-tests && ./node_modules/.bin/vscode-test --run out/test/appHostDataRepository.test.js --run out/test/appHostTreeView.test.js --run out/test/aspireCodeLensProvider.test.js && npm run lintcd extension && ./node_modules/.bin/vscode-test --run out/test/rpc/interactionServiceTests.test.jsgit diff --check13.4.0-pr.16836.gf509da86aspire ps --format json --resourcesreturned a running AppHost with 4 resources.aspire ps --follow --format json --resourcesemitted a full snapshot with 4 resources.Checklist
<remarks />and<code />elements on your triple slash comments?aspire.devissue: