Remove resource data from aspire ps; use aspire describe for resource streaming#17479
Conversation
Resource snapshot streaming from 'aspire ps --resources' overlapped heavily with 'aspire describe --follow --apphost <path>' and emitted the full resource set per AppHost on every change. Drop --resources (and --include-hidden, which only filtered the resource payload) so ps stays an AppHost-level summary similar to 'docker ps'. Consumers that need resource data should use 'aspire describe' instead. - Remove the --resources/--include-hidden options, AppHostDisplayInfo.Resources and the resource fetch/follow watcher plumbing from PsCommand. - Drop ResourcesOptionDescription from PsCommandStrings and reword FollowOptionDescription; regenerate xlf for all locales. - Drop --resources tests from PsCommandTests; remaining 28 tests pass. - Update docs/specs/cli-output-formats.md to remove the --resources example and point readers at 'aspire describe' for resource streaming. - VS Code extension: drop the _supportsResources branch in AppHostDataRepository (and its --resources fallback path), and update the matching tests. 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 -- 17479Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 17479" |
There was a problem hiding this comment.
Pull request overview
This PR narrows aspire ps back to an AppHost-level listing by removing the --resources and --include-hidden options and all related resource-fetch/watch plumbing, directing consumers to aspire describe for per-resource inspection and streaming.
Changes:
- Removed
psresource-related options, payload fields, and follow-mode resource watcher logic. - Updated localized CLI strings to reflect the new
ps --followsemantics and removed the resources option string. - Updated the VS Code extension and CLI output-format spec docs to no longer rely on
ps --resources, and adjusted unit tests accordingly.
Show a summary per file
| File | Description |
|---|---|
| tests/Aspire.Cli.Tests/Commands/PsCommandTests.cs | Removes ps --resources-specific tests and updates remaining coverage to match the new ps contract. |
| src/Aspire.Cli/Commands/PsCommand.cs | Removes --resources / --include-hidden options and resource snapshot plumbing from ps (including follow-mode resource watching). |
| src/Aspire.Cli/Resources/PsCommandStrings.resx | Removes the resources option description and updates follow description text. |
| src/Aspire.Cli/Resources/PsCommandStrings.Designer.cs | Regenerates strongly-typed resource accessors to match updated .resx. |
| src/Aspire.Cli/Resources/xlf/PsCommandStrings.zh-Hant.xlf | Updates follow description translation source; removes resources option string. |
| src/Aspire.Cli/Resources/xlf/PsCommandStrings.zh-Hans.xlf | Updates follow description translation source; removes resources option string. |
| src/Aspire.Cli/Resources/xlf/PsCommandStrings.tr.xlf | Updates follow description translation source; removes resources option string. |
| src/Aspire.Cli/Resources/xlf/PsCommandStrings.ru.xlf | Updates follow description translation source; removes resources option string. |
| src/Aspire.Cli/Resources/xlf/PsCommandStrings.pt-BR.xlf | Updates follow description translation source; removes resources option string. |
| src/Aspire.Cli/Resources/xlf/PsCommandStrings.pl.xlf | Updates follow description translation source; removes resources option string. |
| src/Aspire.Cli/Resources/xlf/PsCommandStrings.ko.xlf | Updates follow description translation source; removes resources option string. |
| src/Aspire.Cli/Resources/xlf/PsCommandStrings.ja.xlf | Updates follow description translation source; removes resources option string. |
| src/Aspire.Cli/Resources/xlf/PsCommandStrings.it.xlf | Updates follow description translation source; removes resources option string. |
| src/Aspire.Cli/Resources/xlf/PsCommandStrings.fr.xlf | Updates follow description translation source; removes resources option string. |
| src/Aspire.Cli/Resources/xlf/PsCommandStrings.es.xlf | Updates follow description translation source; removes resources option string. |
| src/Aspire.Cli/Resources/xlf/PsCommandStrings.de.xlf | Updates follow description translation source; removes resources option string. |
| src/Aspire.Cli/Resources/xlf/PsCommandStrings.cs.xlf | Updates follow description translation source; removes resources option string. |
| extension/src/views/AppHostDataRepository.ts | Stops passing --resources to ps and removes the resources-capability fallback logic. |
| extension/src/test/appHostDataRepository.test.ts | Updates extension tests for the new ps invocation behavior (no --resources). |
| docs/specs/cli-output-formats.md | Removes the ps --resources example and points readers to aspire describe for resource streaming. |
Copilot's findings
Files not reviewed (1)
- src/Aspire.Cli/Resources/PsCommandStrings.Designer.cs: Language not supported
- Files reviewed: 19/20 changed files
- Comments generated: 3
The previous commit removed --resources from 'aspire ps' but left the global (multi-AppHost) view in the VS Code extension with no source for per-AppHost resource data, since 'aspire describe --follow' is per-AppHost (takes a single --apphost). Add a per-AppHost describe fan-out in AppHostDataRepository: while the panel is visible in global mode, maintain one 'aspire describe --follow --apphost <path>' stream per AppHost discovered by 'ps', merging resources into appHost.resources so the tree provider's existing rendering path lights up again. Streams are reconciled on every ps update, torn down when an AppHost stops or when leaving global mode/hiding the panel, and restarted with exponential backoff if they exit unexpectedly. Update the global ps-follow test to feed resources via describe instead of ps, and add two new tests covering the fan-out reconciliation and the mode-switch teardown. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Update AppHostDataRepository tests so ps payload fixtures match the current aspire ps output shape. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The 'possibly unbuildable AppHost candidates do not force global mode' test races on Windows CI because the aspire ls exit handler awaits getConfiguredAppHostPathFromWorkspaceRoot, which probes for the workspace config files via vscode workspace fs. That probe can take more than the single setTimeout(0) tick that waitForAppHostDiscovery covers, so workspaceAppHostPath was still undefined when the assertion ran. Switch to the existing waitForCondition polling helper so the test waits until the path is actually populated. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
JamesNK
left a comment
There was a problem hiding this comment.
1 issue found (bug in error recovery path for global describe streams).
Node's spawn can emit error (e.g., ENOENT when the CLI binary is missing or its path changes) without a subsequent exit. The previous errorCallback only cleared stream.process, leaving a zombie entry in _globalDescribeStreams that blocked _reconcileGlobalDescribes from recreating the stream. Drop the entry (and clear cached resources + fire a change event so the UI reflects the removal) so the next ps reconcile recreates it from scratch. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
I tried testing it locally but it never found AppHosts. Have you tested it locally? I debugged a bit as it looks like |
|
@JamesNK are you running within the context of the workspace? If you aren't you'll need to press this button:
|
|
Yes, I have the aspire app open in a workspace. Going into global mode does make the AppHost appear in VS Code, but I would have expected it display in both modes because it's inside the workspace. extensions-workspace.mp4 |
|
That is possibly a good improvement, but I don't know if that is what Adam was originally going for with this feature. |
|
Either way this PR is about addressing using |
|
Sounds like we regressed something. |
|
@JamesNK @davidfowl looks like it was a regression but it happened a while ago. See issue I just created with analysis. If we want to fix it we should submit another PR, I don't think we should hold this one up. |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
In multi-AppHost workspace mode, non-selected running AppHosts had no resources displayed because only the selected AppHost got a workspace describe stream. This adds _reconcileWorkspaceDescribes() which fans out global describe streams for every running workspace AppHost that is not the selected one, so the tree view can show their resources. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
adamint
left a comment
There was a problem hiding this comment.
Adversarial review pass. Found 6 issues worth addressing:
- 1 high-severity bug: zombie
_globalDescribeStreamsentry when CLI path resolution fails (same class of bug @JamesNK flagged forerrorCallback). - 1 medium-severity bug: spurious change detection in
_handlePsOutputcauses_onDidChangeData.fire()on every ps update. - 1 low-severity bug: per-AppHost describe stderr silently dropped — even from the log channel.
- 1 test issue:
emit('close', 0)is a no-op becausespawnCliProcess(which wires the handler) is bypassed by the stub. - 1 naming concern:
_globalDescribeStreamsand friends are used by the workspace fan-out too — "Global" is misleading. - 1 scope concern: build-script changes (
yarn→corepack yarn@1.22.22) are unrelated to the PR title and not mentioned in the description.
Details inline.
Fixes three bugs and one test issue flagged in the adversarial review of #17479: 1. Drop zombie _globalDescribeStreams entry when getAspireCliExecutablePath rejects. Mirrors the errorCallback fix at line 766 — without this, a one-time CLI path resolution failure leaves a permanent map entry that blocks _reconcileGlobalDescribes from ever re-spawning the stream. 2. Compute the 'changed' flag in _handlePsOutput against a cached post-reconcile snapshot instead of stringifying the raw ps payload. Raw 'ps' no longer emits a 'resources' field after #17479, but _attachGlobalResourcesToAppHosts mutates this._appHosts to add it, so a direct JSON.stringify compare reported 'changed' on every ps update and fired _onDidChangeData (and setContext) spuriously. 3. Log per-AppHost describe stderr to extensionLogOutputChannel so users can diagnose missing resources for non-selected AppHosts (e.g., CLI too old to support 'describe --apphost <path>'). The error banner is still left untouched. 4. Remove dead emit('close', 0) calls in two new tests. The stub bypasses spawnCliProcess (which is where production wires the 'close' -> exitCallback handler), so the emit was a no-op and the following waitForMicrotasks() was likewise pointless. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
FYI @mitchdenny — I've also pushed the fix commit directly to this PR branch as 2ed8a3cd82 so you don't have to apply the patch manually. The PR HEAD is now |
|
/backport to release/13.4 |
|
Started backporting to |
|
@adamint backporting to git am output$ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patch
Applying: Remove --resources from aspire ps (#17472)
Applying: Restore resources in global multi-AppHost view via describe fan-out
Applying: Remove resources from ps test mocks
Applying: Remove stale aspire ps resources comment
Applying: Make Windows-flaky workspace AppHost discovery test deterministic
Using index info to reconstruct a base tree...
M extension/src/test/appHostDataRepository.test.ts
Falling back to patching base and 3-way merge...
Auto-merging extension/src/test/appHostDataRepository.test.ts
CONFLICT (content): Merge conflict in extension/src/test/appHostDataRepository.test.ts
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0005 Make Windows-flaky workspace AppHost discovery test deterministic
Error: The process '/usr/bin/git' failed with exit code 128 |
|
❓ CLI E2E Tests unknown — 107 passed, 0 failed, 2 unknown (commit View all recordings
📹 Recordings uploaded automatically from CI run #26553168827 |
…cribe Documents the removal of the --resources and --include-hidden options from aspire ps (microsoft/aspire#17479). The command is now strictly an AppHost-level summary; users who need per-resource details or streaming should use aspire describe. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Pull request created: #1109
|
|
📝 Documentation has been drafted in microsoft/aspire.dev#1109 targeting Updated Note This draft PR needs human review before merging. |
|
@adamint shall we backport? |


Description
aspire ps --resourcesoverlapped heavily withaspire describe --follow --apphost <path>and emitted the full resource set per AppHost on every change, makingpsbehave less like itsdocker ps/kubectl get podsnamesakes. This PR drops--resources(and--include-hidden, which only existed to filter the resource payload) sopsstays an AppHost-level summary. Consumers that need resource data should useaspire describeinstead.Highlights:
PsCommand: removed the--resources/--include-hiddenoptions,AppHostDisplayInfo.Resources, the resource fetch path, and the follow-mode resource watcher plumbing (file goes from ~575 to ~290 lines).ResourcesOptionDescriptionand rewordedFollowOptionDescription; regenerated all 13 xlf files.--resources-specific tests inPsCommandTests; the remaining 28 tests pass.docs/specs/cli-output-formats.md): removed the--resourcesexample and pointed readers ataspire describefor per-resource streaming.AppHostDataRepository): removed the_supportsResourcesfield and the--resourcesfallback branch in_fetchAppHosts; updated tests accordingly. 515 extension unit tests pass; lint clean. (2 pre-existing RPC tests fail onmaintoo because they require a packageddist/extension.js.)Behavior note: the extension's global-mode tree previously nested resources under each AppHost from the
ps --resourcespayload. Withpsno longer returning resources, that nesting goes away in global mode; per-AppHost resource streaming is already available viadescribe --apphost, which the extension uses in workspace mode.Fixes: #17472
Checklist
<remarks />and<code />elements on your triple slash comments?