Honor configured channel in 'aspire update'#16716
Open
mitchdenny wants to merge 2 commits intomainfrom
Open
Conversation
Contributor
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 16716Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 16716" |
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes aspire update so project updates consult persisted channel configuration instead of always falling through to the implicit/default channel. This fits the CLI’s broader config-driven channel selection behavior used by other commands.
Changes:
- Centralizes channel resolution in
UpdateCommand.ExecuteAsyncand adds config lookup after CLI args. - Preserves explicit CLI precedence over configured values.
- Adds regression tests for local config, global config, precedence, fallback, and invalid configured channels.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/Aspire.Cli/Commands/UpdateCommand.cs |
Adds config-based channel resolution to project update flow. |
tests/Aspire.Cli.Tests/Commands/UpdateCommandTests.cs |
Adds unit tests for configured-channel update scenarios. |
Comment on lines
+160
to
+171
| // 2. local app config "channel" | ||
| // 3. global config "channel" | ||
| // 4. interactive channel prompt when appropriate (PR hives present) | ||
| // 5. implicit/default channel as the documented fallback | ||
| // Local-vs-global precedence for steps 2 and 3 is honored implicitly: | ||
| // ConfigurationHelper.RegisterSettingsFiles loads the global settings file before the | ||
| // local one, so IConfiguration (and therefore GetConfigurationAsync) returns local first. | ||
| var channelName = parseResult.GetValue(_channelOption) ?? parseResult.GetValue(_qualityOption); | ||
| var channelFromConfig = false; | ||
| if (string.IsNullOrWhiteSpace(channelName)) | ||
| { | ||
| channelName = await _configurationService.GetConfigurationAsync("channel", cancellationToken); |
Member
Author
There was a problem hiding this comment.
Good catch — confirmed and fixed in 46c5979.
IConfiguration is built once at startup via ConfigurationHelper.RegisterSettingsFiles(builder.Configuration, Environment.CurrentDirectory, globalSettingsFile) (Program.cs:295), so reading channel from it always anchored to the launch cwd, not the resolved AppHost project's tree. aspire update --apphost /elsewhere/AppHost.csproj would silently pick up channel from whichever aspire.config.json happened to live in the caller's cwd hierarchy.
Added IConfigurationService.GetConfigurationFromDirectoryAsync(key, startDirectory, ct) which:
- walks up from
startDirectoryfor the nearestaspire.config.json(or legacy.aspire/settings.json) usingConfigurationHelper.FindNearestConfigFilePath, - falls back to the global settings file (
globalSettingsFile.FullName) if the local file is absent or doesn't contain the key, - never consults the process-wide
IConfiguration, so the lookup is no longer anchored to the launch cwd, and - throws
InvalidOperationExceptionwithErrorStrings.InvalidJsonInConfigFileon JSON parse failure to matchConfigurationHelper.AddSettingsFile's startup-time behavior — keeping malformed-config diagnostics consistent across code paths.
UpdateCommand now passes projectFile.Directory (falling back to ExecutionContext.WorkingDirectory for safety) so the lookup is scoped to the resolved AppHost project's tree.
Three new tests in UpdateCommandTests cover the regression and the precedence rules (project-local-only, project-local-vs-cwd, project-local-without-channel-falls-back-to-global). All pass; existing UpdateCommandTests still pass (39/39).
Contributor
|
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.
|
mitchdenny
added a commit
that referenced
this pull request
May 4, 2026
…rectory Addresses Copilot reviewer feedback on PR #16716. The previous fix moved channel resolution to read from `IConfiguration`, but `IConfiguration` is rooted at `Environment.CurrentDirectory` at startup via `ConfigurationHelper.RegisterSettingsFiles`. That means `aspire update --apphost <path-to-other-app>/AppHost.csproj` ignored the target app's local `aspire.config.json` and read config from the caller's cwd tree instead, so the documented "local app-config in the project tree" precedence was still broken for explicit `--apphost` updates. Add `IConfigurationService.GetConfigurationFromDirectoryAsync(key, startDirectory)` which walks up from a caller-supplied directory for the nearest `aspire.config.json`, then falls back to the global settings file. The process-wide IConfiguration is intentionally not consulted, so the lookup is never anchored to the launch cwd. `UpdateCommand` now passes `projectFile.Directory` to scope the channel lookup to the resolved AppHost project's tree. Three new tests cover: - Project in another directory uses its own local config - Project-local config wins over cwd config - Project-local config without `channel` falls back to global Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
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.
|
Member
Is this true? I could've sworn being asked which channel I wanted to use when running aspire update before. |
Member
Author
|
The behavior has shifted a bit as folks have been tweaking the experience around working with channels. The general trend is towards asking less questions. |
'aspire update' previously consulted only the explicit --channel/--quality option and otherwise silently selected the implicit channel (or prompted only if PR hives were present). The local and global 'channel' configuration values were never read, so a user who ran 'aspire config set channel staging' (or had it saved by 'aspire update --self') would still get the implicit/NuGet-config based channel on subsequent 'aspire update' runs. UpdateCommand.ExecuteAsync now resolves the channel using the documented precedence: 1. explicit --channel / hidden --quality 2. local app config 'channel' (aspire.config.json / .aspire/settings.json) 3. global config 'channel' (~/.aspire/settings.global.json) 4. interactive channel prompt when PR hives are present 5. implicit/default channel as the documented fallback Local-vs-global precedence comes for free because RegisterSettingsFiles loads the global settings file before the local one, so IConfiguration (and IConfigurationService.GetConfigurationAsync) returns the local value when both exist. A configured channel that does not match any available channel now surfaces a ChannelNotFoundException instead of being silently ignored. The existing UpdateCommand_WithoutHives_UsesImplicitChannelWithoutPrompting test is preserved (no configured channel still falls back to the implicit channel) and joined by new regression coverage: * UpdateCommand_LocalConfiguredChannel_IsUsed * UpdateCommand_GlobalConfiguredChannel_IsUsed * UpdateCommand_ExplicitChannelOverridesConfiguredChannel * UpdateCommand_LocalConfiguredChannel_OverridesGlobalConfiguredChannel * UpdateCommand_WithoutHives_ConfiguredChannel_TakesPrecedenceOverImplicitFallback * UpdateCommand_ConfiguredChannelNotInChannelList_ThrowsChannelNotFound Fixes #16650 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…rectory Addresses Copilot reviewer feedback on PR #16716. The previous fix moved channel resolution to read from `IConfiguration`, but `IConfiguration` is rooted at `Environment.CurrentDirectory` at startup via `ConfigurationHelper.RegisterSettingsFiles`. That means `aspire update --apphost <path-to-other-app>/AppHost.csproj` ignored the target app's local `aspire.config.json` and read config from the caller's cwd tree instead, so the documented "local app-config in the project tree" precedence was still broken for explicit `--apphost` updates. Add `IConfigurationService.GetConfigurationFromDirectoryAsync(key, startDirectory)` which walks up from a caller-supplied directory for the nearest `aspire.config.json`, then falls back to the global settings file. The process-wide IConfiguration is intentionally not consulted, so the lookup is never anchored to the launch cwd. `UpdateCommand` now passes `projectFile.Directory` to scope the channel lookup to the resolved AppHost project's tree. Three new tests cover: - Project in another directory uses its own local config - Project-local config wins over cwd config - Project-local config without `channel` falls back to global Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
mitchdenny
force-pushed
the
mitchdenny/fix-16650-update-respect-channel-config
branch
from
46c5979 to
e3bf581
Compare
Contributor
|
🎬 CLI E2E Test Recordings — 74 recordings uploaded (commit View all recordings
📹 Recordings uploaded automatically from CI run #25359562337 |
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
Fixes #16650.
aspire updatepreviously only consulted--channel/--qualityfrom the command line. When neither was supplied it dropped straight through to the implicit/default channel (whateverNuGet.confighappened to point at), even when the user had explicitly setaspire config set channel stagingeither locally or globally. The CLI silently used a different channel than the user''s stated intent.This PR centralizes channel resolution in
UpdateCommand.ExecuteAsyncto use the documented precedence:--channel/ hidden--quality(CLI)channel(aspire.config.jsonin the project tree)channel(e.g.%APPDATA%\.aspire\settings.json)Local-vs-global precedence for steps 2 and 3 is preserved through the existing
IConfigurationregistration order:ConfigurationHelper.RegisterSettingsFilesloads the global settings file before the local one, soIConfigurationService.GetConfigurationAsync("channel", ...)returns the local value when both are set.aspire update --selfretains its existing prompt-and-persist behavior; project updates will now pick up the persisted channel on subsequent runs (closing the inconsistency called out in the issue).This is a 💥 blocking-release fix targeting
mainfirst; a backport PR torelease/13.3will follow.Validation
To verify locally, install the CLI from this PR:
Then in any Aspire app:
Expected: the CLI uses
staging. Previously it would silently fall through to the implicit/default channel. With--channelpassed explicitly, behavior is unchanged (CLI still wins). Clearingchannelfrom config and runningaspire updateagain should pick the implicit/default channel.Automated coverage
tests/Aspire.Cli.Tests/Commands/UpdateCommandTests.csadds regressions covering local config, global config, explicit-CLI-wins, and the no-config fallback paths.Checklist