fix(cli): persist channel in aspire.config.json on init and update#17452
fix(cli): persist channel in aspire.config.json on init and update#17452mitchdenny wants to merge 5 commits into
Conversation
`aspire init` was not writing the top-level `channel` key into the scaffolded `aspire.config.json`. As a result, when a non-stable CLI build (daily / staging / pr-<N> / local) ran `aspire init`, subsequent `aspire add` / `integration list` / `integration search` calls had no channel context and defaulted to implicit nuget.org versions that did not line up with the CLI build the user was dogfooding. This change passes `CliExecutionContext.IdentityChannel` through to: - the single-file C# init path's `DropAspireConfig` helper, which now writes `settings["channel"]` when absent - the polyglot init path's `ScaffoldContext.Channel`, which the existing `ScaffoldingService` already persists into the polyglot template's `aspire.config.json` Pre-existing `channel` values are preserved (the write is gated on `settings["channel"] is null`) so user-edited or migrated configs are not clobbered. Related to #17295. 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 -- 17452Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 17452" |
There was a problem hiding this comment.
Pull request overview
This PR updates aspire init to persist the running CLI’s identity channel (e.g., daily, staging, pr-<N>, local) into the generated aspire.config.json, so subsequent package/integration commands resolve against the intended channel instead of falling back to implicit defaults.
Changes:
- Single-file C# init now passes
CliExecutionContext.IdentityChannelintoDropAspireConfigand writeschannelwhen it’s missing. - Polyglot init now passes
CliExecutionContext.IdentityChannelintoScaffoldContext.Channelso scaffolding persists it. - Adds unit tests covering channel persistence + preservation for single-file init.
Show a summary per file
| File | Description |
|---|---|
| src/Aspire.Cli/Commands/InitCommand.cs | Threads identity channel into init scaffolding/config generation and conditionally writes aspire.config.json#channel. |
| tests/Aspire.Cli.Tests/Commands/InitCommandTests.cs | Adds regression coverage for writing and preserving aspire.config.json#channel in single-file init mode. |
Copilot's findings
- Files reviewed: 2/2 changed files
- Comments generated: 2
JamesNK
left a comment
JamesNK
left a comment
There was a problem hiding this comment.
1 comment: stale architectural comment in ScaffoldingService contradicts the new polyglot init behavior.
The first pass blindly persisted CliExecutionContext.IdentityChannel into aspire.config.json#channel. That violates the documented invariant in ScaffoldingService.cs:84-92 and creates two real regressions: * Persisting 'local' (or any unregistered identity like a stale 'pr-<N>') pins a name no package source mapping can satisfy, zeroing out polyglot 'aspire add' discovery via IntegrationPackageSearchService's name filter (line 28-30). * Persisting an implicit channel pins the default fallback into the project file — exact regression scope of #17295. Mirror NewCommand.cs:316-402: resolve the identity through IPackagingService.GetChannelsAsync, match by StringComparisons.ChannelName, and only persist when the matched channel is PackageChannelType.Explicit. For the polyglot path also pre-check aspire.config.json#channel before scaffolding — ScaffoldingService writes context.Channel unconditionally when non-empty, so without this guard a user-edited value would be silently overwritten on subsequent 'aspire init' runs. Adds coverage for: unregistered identity (single-file + polyglot), implicit-channel identity (single-file), and polyglot channel pass-through + preserve-existing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Self-review post-mortem (commit
|
|
/deployment-test |
|
🚀 Deployment tests starting on PR #17452... This will deploy to real Azure infrastructure. Results will be posted here when complete. |
|
❌ Deployment E2E Tests failed — 36 passed, 4 failed, 0 cancelled View test results and recordings
|
Deployment test failures — pre-existing on
|
| Test | This PR | main (26351886139, ~21h ago) | main (26323258106, ~1d ago) |
|---|---|---|---|
FrontDoorDeploymentTests.DeployReactTemplateWithFrontDoor |
❌ | ❌ | ❌ |
TypeScriptJavaScriptHostingDeploymentTests |
❌ | ❌ | ❌ |
TypeScriptAzureContainerAppJobDeploymentTests |
❌ | ❌ | ❌ |
AksAzureKubernetesEnvironmentCertManagerTypeScriptDeploymentTests |
❌ | ❌ | ❌ |
Cross-checked the changes wouldn't be implicated anyway:
FrontDoorDeploymentTestsusesaspire new(notaspire init); this PR touches onlyInitCommand.AksAzureKubernetesEnvironmentCertManagerTypeScriptusesaspire newwith the Express/React template and fails readingapphost.ts— the template now produces.mts, but again, unrelated to this PR.- The TS hosting/jobs tests time out on later
aspire addpolyglot scaffolding steps, well afteraspire initwould have run.
This PR is good to go from a deployment-test perspective — the failures are tracked separately as ongoing TypeScript/template drift issues on main.
…Command Mention InitCommand's polyglot path alongside NewCommand in the channel-persistence invariant comment in ScaffoldingService.cs. Both call sites resolve the identity channel through IPackagingService and only pass an Explicit channel name, so the invariant the comment guards still holds; the comment was just stale on the list of callers that satisfy it. Addresses review feedback from @JamesNK on PR #17452. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Thanks for the review! @JamesNK — fixed in fb4772a. The @copilot-pull-request-reviewer:
|
When the user runs 'aspire update --channel <x>' against a C# AppHost (single-file apphost.cs or aspire-empty .csproj), the resolved Explicit channel was only being written into NuGet.config — the project's aspire.config.json#channel pin was left at whatever the create-time value was. That meant subsequent 'aspire add' / 'aspire update' commands on the same project kept resolving against the stale channel. The polyglot (TypeScript) update path in GuestAppHostProject already handled this; ProjectUpdater did not. This commit mirrors that logic in ProjectUpdater: after analysis, when the selected channel is Explicit and differs from the persisted value, enqueue a new ChannelUpdateStep that re-loads aspire.config.json on apply, sets Channel, and saves. Implicit channels are intentionally left alone. Also adds 3 E2E regression tests covering the remaining cells of the create-path × language matrix (C# init, C# new aspire-empty, TS init) to complement the existing TS new-empty deep test. Fixes part of #17295 (extension to update path). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
mitchdenny
changed the title
aspire init
|
Expanded the PR to also fix What changed in this push (04aa66b):
PR description + title updated to reflect the expanded scope. |
… tests Each E2E test runs in its own Docker container, so the global feature flag set inside the container never leaks to other tests. Drop the try/finally cleanup dance. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
JamesNK
left a comment
JamesNK
left a comment
There was a problem hiding this comment.
LGTM — well-thought-out fix for the channel persistence gap on both init and update paths. Good test coverage across the create-path × language matrix.
2 comments: 1 correctness issue (missing null guard in ProjectUpdater when aspire.config.json is absent), 1 misleading comment about channel type classification.
| // If no aspire.config.json is present (legacy split layouts), the rewrite is skipped. | ||
| if (channel.Type == PackageChannelType.Explicit && projectFile.Directory is { } projectDirectory) | ||
| { | ||
| var existingConfig = AspireConfigFile.Load(projectDirectory.FullName); |
There was a problem hiding this comment.
The comment above says "If no aspire.config.json is present (legacy split layouts), the rewrite is skipped" — but the code doesn't enforce that. When AspireConfigFile.Load() returns null (no file), existingChannel becomes null, and !string.Equals(null, channel.Name, ...) evaluates to true, so a ChannelUpdateStep is enqueued. The callback's LoadOrCreate then creates a new aspire.config.json for projects that never had one.
Consider guarding with a null check:
var existingConfig = AspireConfigFile.Load(projectDirectory.FullName);
if (existingConfig is not null)
{
var existingChannel = existingConfig.Channel;
if (!string.Equals(existingChannel, channel.Name, StringComparisons.CliInputOrOutput))
{
// ...enqueue step...
}
}| // so subsequent commands like `aspire add` resolve packages against the matching | ||
| // channel. Resolve through PackagingService and only persist when the identity | ||
| // matches a registered Explicit channel — mirrors `NewCommand.cs:316-402`. This | ||
| // avoids pinning `stable` (Implicit → restricts polyglot discovery, see |
There was a problem hiding this comment.
Nit: this comment (and the matching <item> in the ResolvePersistableChannelNameAsync XML doc) says "stable" is an example of an Implicit channel, but in production PackagingService.GetChannelsAsync creates "stable" via CreateExplicitChannel(PackageChannelNames.Stable, ...) — so "stable" IS Explicit and WILL be persisted by this code (confirmed by the [InlineData("stable")] test case).
The only Implicit channel is "default" (from CreateImplicitChannel), and no CLI ever has that as its identity. The real channels filtered out are unregistered identities like "local" or stale pr-<N>. Consider updating the comment to avoid confusing future maintainers about which channels are actually skipped.
|
❓ CLI E2E Tests unknown — 98 passed, 0 failed, 6 unknown (commit View all recordings
📹 Recordings uploaded automatically from CI run #26382537317 |
|
F1: stable polyglot + private NuGet sources After this PR, stable CLI's Vanilla stable users (nuget.org only) are unaffected — actually slightly better off (5 CommunityToolkit packages downgraded from Does this sound like a case we want to fix in this PR, or accept and let #17295's eventual fix paper over it? Repro
./eng/scripts/get-aspire-cli-pr.sh 17452 \
--install-path /tmp/aspire-pr17452 --skip-extension --skip-path
PR_CLI=/tmp/aspire-pr17452/dogfood/pr-17452/bin/aspire
# Compare init output: current installed CLI (pre-PR, no channel) vs this PR's CLI:
mkdir -p /tmp/before /tmp/after
(cd /tmp/before && aspire init --language typescript --suppress-agent-init --non-interactive --nologo)
(cd /tmp/after && $PR_CLI init --language typescript --suppress-agent-init --non-interactive --nologo)
diff /tmp/before/aspire.config.json /tmp/after/aspire.config.json
# /tmp/after gets "channel": "pr-17452" added.
# Version-aware diff of what `aspire add` will see:
for v in before after; do
$PR_CLI integration list --apphost /tmp/$v/apphost.mts --format json --non-interactive --nologo \
| jq -r '.[] | "\(.package) @ \(.version)"' | sort > /tmp/$v.txt
done
diff /tmp/before.txt /tmp/after.txtFor a stable user the diff shape is: 19 |
Description
When a non-stable Aspire CLI (daily / staging /
pr-<N>/ local) ranaspire init, the scaffoldedaspire.config.jsonwas written without a top-levelchannelkey. Downstream commands (aspire add,aspire integration list,aspire integration search) then had no channel context for the project and defaulted to the implicit nuget.org channel. The result: a daily-CLI user who ranaspire initfollowed byaspire add orleanswould be offered the stable Orleans package version rather than the daily one that matches the CLI build they are actually dogfooding.A parallel gap existed on the update path:
aspire update --channel <x>against a C# AppHost rewrote NuGet.config and bumped packages, but leftaspire.config.json#channelat its create-time value, so subsequentaspire add/aspire updateruns against the same project kept resolving against the stale channel. The polyglot (TypeScript) update path inGuestAppHostProjectalready handled this;ProjectUpdaterdid not.This PR fixes both:
Init path
Passes
CliExecutionContext.IdentityChannelthrough to:DropAspireConfighelper, which now writessettings["channel"]when it is not already presentScaffoldContext.Channel, which the existingScaffoldingServicealready persists into the polyglot template'saspire.config.json(covers TypeScript apphosts as well as any future polyglot languages)Pre-existing
channelvalues are preserved (the write is gated onsettings["channel"] is nullfor C# and a pre-check in the polyglot path) so user-edited or migrated configs are not clobbered. Only channels that are registered asExplicitin theIPackagingServiceare persisted, mirroringNewCommand's existing resolution logic — so implicit defaults (e.g. plain stable) are not pinned. Project-mode C# init is unchanged because theaspire-apphosttemplate owns its ownaspire.config.jsonand that mode already produces no top-level config file from init.Update path
Adds a new
ChannelUpdateStepinProjectUpdaterthat mirrorsGuestAppHostProject.UpdatePackagesInternalAsync: after analysis, when the selected channel is Explicit and differs from the persisted value, the step re-loadsaspire.config.jsonon apply, setsChannel, and saves. Implicit channels are intentionally left alone. The pre-confirm summary lists the channel change alongside any package updates so users see it in the prompt.Fixes #17295.
User-facing usage
C# single-file apphost
Running a daily CLI build:
Before this change:
{ "appHost": { "path": "apphost.cs" }, "profiles": { "https": { /* ... */ }, "http": { /* ... */ } } }After this change:
{ "appHost": { "path": "apphost.cs" }, "channel": "daily", "profiles": { "https": { /* ... */ }, "http": { /* ... */ } } }TypeScript polyglot apphost
Running a daily CLI build with
--language typescript:Before this change:
{ "appHost": { "path": "apphost.mts" } }After this change:
{ "appHost": { "path": "apphost.mts" }, "channel": "daily" }aspire update --channel <x>now rewritesaspire.config.json#channelFor any AppHost layout — C# single-file, C#
aspire-emptyproject mode, TypeScript single-file, TypeScript project mode — runningaspire update --channel stable(or--channel daily) against a project pinned to a different Explicit channel now updatesaspire.config.json#channelin place, alongside the SDK/package updates.aspire add orleansfrom any of these workspaces now resolves Orleans packages against the channel the user just switched to rather than the stale create-time channel.Tests
Existing init tests (C# and polyglot)
ProjectUpdaterTests: added unit tests forChannelUpdateStep.GetFormattedDisplayText(with and without an existing channel value), and extendedUpdateProjectFileAsync_CanUpdateFromStableToDailyto assert thataspire.config.json#channelis rewritten from"stable"to"daily"ChannelUpdateWorkflowTests: added three E2E regression tests so the create-path × language matrix is fully covered:UpdateProjectChannelToStable_CSharpSingleFileInit_RewritesAspireConfigChannelUpdateProjectChannelToStable_CSharpEmptyAppHost_RewritesAspireConfigChannelUpdateProjectChannelToStable_TypeScriptSingleFileInit_RewritesAspireConfigChannelThe existing
UpdateProjectChannelToStable_TypeScript_PicksUpStablePackagescovers the TSaspire newcell with package-version assertions.Checklist