Refuse 'staging' channel synthesis on daily/local/pr CLI builds#17235
Merged
Conversation
Fixes #16652. PackagingService synthesized the 'staging' channel from the running CLI's build context: a stable-quality staging used a SHA-specific darc feed built from the CLI commit, and a Prerelease/Both staging without an override fell back to the shared daily feed. On a daily, local, or per-PR CLI neither produces a real staging feed, so 'aspire update --channel staging' silently resolved to daily package versions. Gate staging synthesis on the CLI's baked identity (CliExecutionContext. IdentityChannel from [AssemblyMetadata("AspireCliChannel", ...)]): - stable: SHA-specific darc-pub-microsoft-aspire-<hash> feed exists. - staging: dogfoods staging packages (per #17155). - daily, local, pr-<N>: refuse and surface a localized reason that names the running identity and points at 'overrideStagingFeed' or installing a staging CLI as recovery paths. Escape hatches preserved: - 'overrideStagingFeed' configuration always allows synthesis. - 'StagingChannelEnabled' feature flag continues to opt in for dev/test. UpdateCommand surfaces the packaging-service reason when --channel staging is requested but refused, instead of the generic 'No channel found matching staging' message that hid the actual fix. When staging IS synthesized, log the resolved feed URL, quality, and pinned version at Information so users can see what '--channel staging' actually selected (the 'show what was resolved' suggestion from the issue RCA). Adds IPackagingService.GetStagingChannelUnavailableReason() and a matching member on TestPackagingService that defaults to reporting staging as available so existing tests stay unchanged. 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 -- 17235Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 17235" |
…ewCommand - Cache GetStagingChannelUnavailableReason via Lazy<string?> so the formatted reason is computed once per process instead of on every GetChannelsAsync call. - Emit the refusal warning and resolved-staging info log at most once per process using Interlocked.Exchange flags. Many code paths invoke GetChannelsAsync repeatedly (NewCommand, IntegrationPackageSearchService, NuGetPackagePrefetcher, etc.); without this, a daily CLI with channel: staging pinned in aspire.config.json would spam the warning on every command. - Surface the staging-specific reason in NewCommand's error path so users scaffolding a project with channel: staging configured get the same actionable message UpdateCommand now produces. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes #16652: a daily Aspire CLI silently resolved aspire update --channel staging to daily package versions because PackagingService.CreateStagingChannel() synthesized the staging feed from either a non-existent SHA-specific darc feed or the shared daily feed. The PR gates staging-channel synthesis on the CLI's baked IdentityChannel (from [AssemblyMetadata("AspireCliChannel", ...)]) so daily/local/pr-N CLIs refuse staging synthesis and return a localized, actionable reason instead.
Changes:
- Adds
IPackagingService.GetStagingChannelUnavailableReason()and identity-based gating inPackagingService, with cached reason, one-shot refusal/resolution log lines, andoverrideStagingFeed/StagingChannelEnabledescape hatches preserved. UpdateCommandandNewCommandsurface the packaging-service reason when--channel staging(or configuredchannel: staging) is requested but refused, instead of the generic "No channel found matching" message.- New localized resource string
StagingChannelUnavailableOnDailyCli(resx + Designer + all 13 xlf files) and test coverage for the full identity matrix plus anUpdateCommandregression test.
Show a summary per file
| File | Description |
|---|---|
| src/Aspire.Cli/Packaging/PackagingService.cs | Adds identity-gated staging synthesis, cached refusal reason, and one-shot log lines |
| src/Aspire.Cli/Commands/UpdateCommand.cs | Surfaces staging-specific reason when --channel staging is refused |
| src/Aspire.Cli/Commands/NewCommand.cs | Mirrors the staging-specific error path for aspire new |
| src/Aspire.Cli/Resources/PackagingStrings.resx | New StagingChannelUnavailableOnDailyCli resource string |
| src/Aspire.Cli/Resources/PackagingStrings.Designer.cs | Generated accessor for the new resource |
| src/Aspire.Cli/Resources/xlf/PackagingStrings.*.xlf (13 files) | New trans-units (state="new") for all locales |
| tests/Aspire.Cli.Tests/Packaging/PackagingServiceTests.cs | Identity matrix coverage: stable, daily, local, pr-N, override, feature flag |
| tests/Aspire.Cli.Tests/Commands/UpdateCommandTests.cs | Regression test that --channel staging surfaces the staging reason |
| tests/Aspire.Cli.Tests/TestServices/TestPackagingService.cs | Adds matching GetStagingChannelUnavailableReason fake |
Copilot's findings
Files not reviewed (1)
- src/Aspire.Cli/Resources/PackagingStrings.Designer.cs: Language not supported
- Files reviewed: 20/21 changed files
- Comments generated: 0
radical
reviewed
May 19, 2026
JamesNK
reviewed
May 19, 2026
Member
JamesNK
left a comment
JamesNK
left a comment
There was a problem hiding this comment.
Reviewed the staging-channel gating logic, caching, concurrency guards, error surfacing in UpdateCommand/NewCommand, resource strings, and test coverage. The fix is well-structured and correctly addresses #16652.
1 issue flagged: test coverage gap where the feature flag escape hatch isn't actually exercised in isolation.
JamesNK
reviewed
May 19, 2026
JamesNK
reviewed
May 19, 2026
Addresses radical's review feedback on #17235. PrebuiltAppHostServer (and any sibling paths) previously treated a missing requested channel as a fallback to "all explicit channels", which silently restored integration packages from the shared daily feed even when the project pinned channel: staging. Refuse those calls with the same actionable GetStagingChannelUnavailableReason() the UpdateCommand/NewCommand paths now use, so the silent-downgrade hole this PR is meant to close is closed for the bundled AppHost restore path too. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
JamesNK
reviewed
May 19, 2026
JamesNK
reviewed
May 19, 2026
Addresses JamesNK's review feedback on #17235. The original feature-flag test set overrideStagingFeed for the test-host workaround, which short-circuited IsStagingChannelSynthesisAllowed before the feature-flag branch ever ran. Add a separate assertion that exercises GetStagingChannelUnavailableReason() on a service whose only opt-in is the StagingChannelEnabled feature flag, so a regression that removes the feature-flag branch would now be caught. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
JamesNK
reviewed
May 19, 2026
Addresses JamesNK's review feedback on #17235. The two remaining inline English literals in NewCommand.ResolveCliTemplateVersionAsync ("No package channels are available." and "No channel found matching '{0}'. Valid options are: {1}.") are now sourced from the resource file the same way the staging-unavailable message already is, so all three failure paths are localizable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…sons Addresses JamesNK's review feedback on #17235. Introduces a small StringComparisons helper so the canonical case-insensitive comparison rule for package-channel names lives in one place, and switches the existing call sites in PackagingService, UpdateCommand, NewCommand, PrebuiltAppHostServer, and KnownFeatures from raw StringComparison.OrdinalIgnoreCase to StringComparisons.ChannelName. Behaviour is unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The previous code lazily allocated a Lazy<T> via Interlocked.CompareExchange, which is redundant since Lazy<T> already provides thread-safe one-time initialization. Construct the Lazy eagerly in the constructor and let it do the deferral, per JamesNK's review on #17235. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Mirrors the same resource extraction that was done for NewCommand earlier in this PR. Adds NoChannelFoundMatching to UpdateCommandStrings.resx + Designer, regenerates xlf for all locales, and switches the ChannelNotFoundException construction site to string.Format against the resource. Per JamesNK on #17235. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Per JamesNK on #17235: replace the duplicated "overrideStagingFeed" literal in PackagingService (and the test fixtures) with PackagingService.OverrideStagingFeedConfigKey so a single source of truth governs the configuration name. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
JamesNK
approved these changes
May 19, 2026
Member
JamesNK
left a comment
JamesNK
left a comment
There was a problem hiding this comment.
No issues found. The fix correctly gates staging-channel synthesis on the CLI's baked identity channel, surfaces actionable error messages through all affected code paths (UpdateCommand, NewCommand, PrebuiltAppHostServer bundled restore), and preserves the existing escape hatches. Test coverage is comprehensive.
Contributor
|
❓ CLI E2E Tests unknown — 90 passed, 0 failed, 1 unknown (commit View all recordings
📹 Recordings uploaded automatically from CI run #26076137563 |
Contributor
|
✅ No documentation update needed. Documentation PR could not be created due to a tooling limitation: the |
radical
added a commit
to radical/aspire
that referenced
this pull request
May 19, 2026
…spire-empty-typescript-hive Resolved conflicts in PrebuiltAppHostServer.cs and its tests: - `GetNuGetSourcesAsync` and `TryCreateTemporaryNuGetConfigAsync` visibility widened to `internal` (from microsoft#17235) while keeping this branch's `packageSourceOverride` parameter. - Channel-name comparisons updated to `StringComparisons.ChannelName` to match the shared comparer introduced on main. - Kept both sets of tests in the overlap region: this branch's `--source` override tests plus microsoft#17235's staging-unavailable refusal tests. Adjusted the two new staging tests that call `GetNuGetSourcesAsync` directly to pass `packageSourceOverride: null` for the widened signature. Verified: `dotnet build tests/Aspire.Cli.Tests/Aspire.Cli.Tests.csproj` succeeds with 0 errors / 0 warnings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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: #16652
On a daily Aspire CLI,
aspire update --channel stagingsilently resolved to daily package versions. The packaging service was synthesizing thestagingchannel relative to the running CLI build:darc-pub-microsoft-aspire-<hash>). That feed only exists for stable release branch commits, never for daily ones.overrideStagingFeed, it fell back to the shared daily feed (pkgs.dev.azure.com/.../dotnet9/...), which contains daily packages.Either way, a daily CLI's
--channel stagingended up resolving daily versions and reporting them as "staging".Approach
Gate staging-channel synthesis on the CLI's baked identity (
CliExecutionContext.IdentityChannel, sourced from[AssemblyMetadata("AspireCliChannel", ...)]and validated byIdentityChannelReader):stableidentity: SHA-specificdarc-pub-microsoft-aspire-<hash>feed is real, allow synthesis.stagingidentity: dogfoods staging packages (per Auto-register staging channel for staging CLI #17155), allow synthesis.daily,local,pr-<N>identity: refuse and return a localized reason naming the running identity and pointing atoverrideStagingFeed(escape hatch) or installing a staging/stable CLI.Escape hatches preserved:
overrideStagingFeedconfiguration entry always allows synthesis regardless of identity.StagingChannelEnabledfeature flag continues to opt in for dev/test scenarios.UpdateCommandnow surfaces the packaging-service-provided reason when--channel stagingis requested but refused, instead of the genericNo channel found matching 'staging'message that hid the actual recovery action from users.When staging IS synthesized, the resolved feed URL, quality, and pinned version are logged at Information so users can see what
--channel stagingactually selected (the "show what was resolved" suggestion from the issue RCA).Why this differs from the closed PR #16717
The earlier PR keyed availability off the assembly version string (Arcade preview vs 3+ identifier daily) and required a new test seam (
internal:packaging:cliVersionForTesting).This PR keys off
CliExecutionContext.IdentityChannelinstead. It is deterministic, cleaner, doesn't need a new test seam, and naturally coverslocalandpr-<N>builds. The signal was already baked into the CLI assembly and validated centrally.Validation
Stable / staging-identity CLIs are unaffected.
Automated coverage
tests/Aspire.Cli.Tests/Packaging/PackagingServiceTests.csadds the full identity matrix: stable, staging, daily, local,pr-12345, daily +overrideStagingFeed, daily +StagingChannelEnabledfeature flag. The previous test that asserted the buggy local-CLI shared-feed behavior is rewritten to assert refusal.tests/Aspire.Cli.Tests/Commands/UpdateCommandTests.csadds a regression test verifying that--channel stagingsurfaces the staging-specific reason viaDisplayErrorand exitsFailedToUpgradeProject.TestPackagingServicegains a matching fake member that defaults to reporting staging as available, so existing tests stay unchanged.All 3,217
Aspire.Cli.Testspass locally.Checklist