[v3 acquisition] PR1: bake CLI channel into assembly metadata#16820
Draft
radical wants to merge 65 commits intomicrosoft:mainfrom
Draft
[v3 acquisition] PR1: bake CLI channel into assembly metadata#16820radical wants to merge 65 commits intomicrosoft:mainfrom
radical wants to merge 65 commits intomicrosoft:mainfrom
Conversation
Contributor
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 16820Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 16820" |
Add a build-wide MSBuild property AspireCliChannel (default 'daily' for
local devs and ./build.sh) and bake its value into the Aspire.Cli
assembly via <AssemblyMetadata>. CI overrides via /p:AspireCliChannel,
which Basher wires into eng/pipelines/templates/build_sign_native.yml.
The runtime IdentityChannelReader (PR1-S4) reads this metadata to know
which acquisition channel produced the binary it is running from.
Tests: tests/Aspire.Cli.Tests/Packaging/CliMetadataPackagingTests.cs
asserts the AssemblyMetadataAttribute with Key 'AspireCliChannel' is
present with a non-empty value. PR1-S5 will tighten this to validate
the value is one of {stable, staging, daily, pr}.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds a new pwsh step `computeCliChannel` in parallel with the existing `computeChannel` step in both azure-pipelines.yml and azure-pipelines-unofficial.yml. The new step exports an output variable `aspireCliChannel` whose value is pr / stable / staging / daily computed from Build.Reason, DotNetFinalVersionKind, and Build.SourceBranch. The existing `computeChannel` / `installerChannel` flow that feeds the prepare_installers stage is untouched. Wires `/p:AspireCliChannel=$(aspireCliChannel)` into the `Build native packages` script in build_sign_native.yml so the AzDO-computed channel overrides the local-dev default (`daily`) defined by the <AspireCliChannel> property in src/Aspire.Cli/Aspire.Cli.csproj when the Aspire.Cli project is packed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…aging,daily,pr}) Adds a stricter smoke test alongside CliMetadataPackagingTests (which only checks presence + non-empty). This test asserts the AspireCliChannel assembly metadata value is one of the expected enum values: stable, staging, daily, pr. Replaces the withdrawn Change #16 AzDO post-pack verification: catches any case where MSBuild fails to set AspireCliChannel correctly (CI misconfiguration that defaults to empty or invalid values). Pure assembly reflection — no filesystem, env vars, or external services. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add two new immutable properties to CliExecutionContext:
- Channel (string, default "daily") — one of {stable, staging, daily, pr}
Resolved at process start; immutable thereafter.
- PrNumber (int?, default null) — non-null only when Channel == "pr"
Both are exposed as primary-constructor parameters with safe defaults so
existing call sites (50+, in src + tests) continue to compile unchanged.
PR1-S11 will replace the defaults at the production construction site
(Program.cs) by wiring IIdentityChannelReader (PR1-S4).
Tests (tests/Aspire.Cli.Tests/CliExecutionContextTests.cs, 5 methods,
10 cases when theories expanded):
- Channel_DefaultsToDaily_WhenNotSpecified
- Channel_AndPrNumber_AreReadable_WhenConstructedWithNullPrNumber
- PrNumber_IsNull_ForNonPrChannels (Theory: stable, staging, daily)
- PrNumber_IsSet_WhenChannelIsPr
- Channel_Getter_ReturnsExactValuePassedToConstructor
(Theory: stable, staging, daily, pr)
AOT-safe (no reflection); no new packages.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… fix)
build_sign_native runs BEFORE the build stage, so the aspireCliChannel
job-output set by build.Windows.computeCliChannel is not yet defined when
/p:AspireCliChannel=$(aspireCliChannel) is consumed at line ~146.
Add a computeCliChannel pwsh step at the top of each per-RID job's steps:
section that mirrors the algorithm from azure-pipelines{,-unofficial}.yml
(PullRequest -> pr; release stabilization -> stable; release branch ->
staging; main / fallback -> daily). Variable is job-scoped (no
isOutput=true) since the consumer is a later step in the same job.
Reads DotNetFinalVersionKind via the in-repo dotnet wrapper ($(dotnetScript))
after preSteps' restore has set up the SDK. azure-pipelines{,-unofficial}.yml
still keep their own computeCliChannel step for the prepare_installers
stage's downstream consumers; that flow is untouched.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Under acquisition-coherence v3 the global channel field in
~/.aspire/aspire.config.json is no longer written by install scripts.
Channel is resolved at runtime by the CLI based on the installed bundle's
identity (set via /p:AspireCliChannel during pack).
Release scripts (get-aspire-cli.{sh,ps1}): drop the call site that picks
between Save-GlobalSettings/save_global_settings and
Remove-GlobalSettings/remove_global_settings for the 'channel' key.
PR scripts (get-aspire-cli-pr.{sh,ps1}): drop the two hive-label channel
writes (one in install_from_local_dir / Start-InstallFromLocalDir, one in
download_and_install_from_pr / Start-DownloadAndInstall).
save_global_settings / Save-GlobalSettings / remove_global_settings /
Remove-GlobalSettings function definitions are kept intact — they're
generic helpers that may write other keys (e.g. updateMode) in future
subtasks. All other script logic untouched.
Cross-platform parity preserved (.sh and .ps1 mutated identically).
Smoke tests: bash -n + pwsh ParseFile clean for all four scripts;
--help paths still work.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Drops the `if (string.IsNullOrEmpty(channelName)) { channelName = await
_configurationService.GetConfigurationAsync("channel", ct); }` block from:
- src/Aspire.Cli/Projects/DotNetBasedAppHostServerProject.cs (lines 330-333)
- src/Aspire.Cli/Projects/PrebuiltAppHostServer.cs (lines 346-350)
- src/Aspire.Cli/Commands/NewCommand.cs (lines 328-331)
With the global-channel writers gone (PR1-S6 + PR1-S8/S9), these reads were
dead code that would silently keep picking up leftover global channel state
and re-introduce the cross-route contamination surface (G1).
Existing fall-through logic handles "no preference":
- DotNetBased / PrebuiltAppHostServer: `channels.Where(c => c.Type == Explicit)`
- NewCommand: `channels.FirstOrDefault(c => c.Type is Implicit)`
PrebuiltAppHostServer.ResolveChannelNameAsync had no remaining awaits after
the removal — converted to sync `ResolveChannelName()` (single private call
site updated) to avoid CS1998. Same idiom landed in the parallel PR4 work.
DotNetBasedAppHostServerProject and NewCommand keep their `IConfigurationService`
field (other consumers / future-PR cleanup); only the channel read line is gone.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Drops the legacy global identity-channel field from the ~/.aspire/globalsettings.json → ~/.aspire/aspire.config.json migration block in Program.GetGlobalSettingsPath. Other migrated fields (AppHost path, language, sdk version, features, packages, profiles) remain untouched via AspireConfigFile.FromLegacy. Implementation: explicit `config.Channel = null` after FromLegacy returns, with a comment documenting the deliberate narrowing. This keeps AspireConfigFile.FromLegacy's signature intact so the per-project legacy migration path (.aspire/settings.json → aspire.config.json, called from AspireConfigFile.cs:189) continues to migrate the per-project channel field — that is project state, not CLI install state, and stays per agreed-design-v3. The CLI's identity channel is now baked into the binary via the AspireCliChannel assembly metadata (PR1-S2) and is never read from global config, which closes the cross-route contamination surface (G1) once PR1-S6/S7/S9 land alongside. Migration error handling and the "newPath will be created on first write" fallback are unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ssemblyMetadata
Adds src/Aspire.Cli/Acquisition/IdentityChannelReader.cs containing both
IIdentityChannelReader and the default IdentityChannelReader implementation.
The new signature is parameterless (string ReadChannel()) and reads the value
baked into the running CLI assembly via [AssemblyMetadata("AspireCliChannel")]
(emitted by the csproj on PR1-S2). Returns one of {stable, staging, daily, pr};
throws InvalidOperationException when the metadata is missing or empty (build
bug — should have been caught by the AssemblyMetadataChannelTests smoke).
ParsePrNumber(string) helper extracts the PR number from the
AssemblyInformationalVersionAttribute value (e.g. "0.0.0-pr12345.<sha>" -> 12345).
Returns null when the marker is absent or no digits follow it; preview/release
versions like "1.2.3-preview.5" are correctly rejected (false-positive guarded).
AOT-safe: uses Assembly.GetCustomAttributes<AssemblyMetadataAttribute>() over a
sealed, build-time-known attribute; no JSON, no dynamic type loading.
Testability: production ctor takes an optional Assembly?; null falls back to
Assembly.GetEntryAssembly(). Tests can pass a fake assembly directly.
ParsePrNumber is internal static (visible to Aspire.Cli.Tests via the existing
InternalsVisibleTo) so it is unit-testable in isolation.
DI wiring and call-site sweep are PR1-S11.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Drops the global identity-channel write (and its paired
DeleteConfigurationAsync on stable) from
UpdateCommand.ExecuteSelfUpdateAsync. After PR1-S2 baked the channel into
the binary as AssemblyMetadata("AspireCliChannel"), there is no longer a
useful reason for `aspire update --self` to mutate global config — the
freshly extracted binary already carries its own channel identity.
Behavior preserved:
- Channel is still resolved (--channel / --quality / interactive prompt)
for the purpose of selecting which artifact to download.
- DownloadLatestCliAsync(channel) is still invoked.
- ExtractAndUpdateAsync is still invoked.
- Cancellation + exception handling and exit codes are unchanged.
Removed:
- DeleteConfigurationAsync("channel", isGlobal: true) when channel == stable.
- SetConfigurationAsync("channel", channel, isGlobal: true) for non-stable.
- The accompanying _logger.LogDebug lines that announced the writes.
This closes the last remaining writer of `~/.aspire/aspire.config.json#channel`,
making PR1-S7's read-fallback removal safe (no leftover global state for
project readers to silently inherit).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…Channel
For new projects (and the `aspire update packages` apply path), seed the
per-project `aspire.config.json#channel` from the channel baked into the
running CLI (`CliExecutionContext.Channel`) instead of relying on
interactive prompts or global config (which is gone after PR1-S6/S7/S9).
Pattern across all 5 reseed sites: explicit user input (or prepare-resolved
explicit channel) wins; otherwise fall back to `CliExecutionContext.Channel`.
Files:
- src/Aspire.Cli/Scaffolding/ScaffoldingService.cs
- Constructor now injects `CliExecutionContext` (DI auto-resolves; the
only external new-up site is CliTestHelper.ScaffoldingServiceFactory,
updated below).
- Initial early-save (pre-prepare) now seeds channel from explicit
`context.Channel` or baked `_cliExecutionContext.Channel`. The
save-guard now also fires when we have a baked-channel seed so the
apphost prepare step sees the channel.
- Post-prepare save now writes `prepareResult.ChannelName ??
_cliExecutionContext.Channel` (no longer guarded on non-null
prepareResult.ChannelName — we always have a channel to persist).
- src/Aspire.Cli/Templating/CliTemplateFactory.PythonStarterTemplate.cs
- src/Aspire.Cli/Templating/CliTemplateFactory.GoStarterTemplate.cs
- Reuse the existing `_executionContext` field (already injected on
`CliTemplateFactory`). When `inputs.Channel` is empty, seed from
`_executionContext.Channel`. Channel is now always written when
non-empty (was previously only when `inputs.Channel` was non-empty).
- src/Aspire.Cli/Templating/CliTemplateFactory.TypeScriptStarterTemplate.cs
- Same pattern. Always sets `config.Channel` from explicit-or-baked.
- src/Aspire.Cli/Projects/GuestAppHostProject.cs
- Constructor now injects `CliExecutionContext` (positional, before
optional `TimeProvider`). DI auto-resolves; tests updated below.
- Channel write at line 347 now: `buildResult.ChannelName ??
_executionContext.Channel`. The if-guard around the SaveConfiguration
is gone — we always have a channel and always save.
- Channel write at line 1203 (update-packages apply path): explicit
branch unchanged; new else branch writes `_executionContext.Channel`
when the resolved channel is implicit.
Test factories updated mechanically to keep build green (no new test
behavior — Livingston owns PR1's reseed test coverage):
- tests/Aspire.Cli.Tests/Utils/CliTestHelper.cs (ScaffoldingServiceFactory)
- tests/Aspire.Cli.Tests/Projects/GuestAppHostProjectTests.cs
(CreateGuestAppHostProject helper)
Both build clean (0 warnings, 0 errors) for src and tests projects with
`/p:SkipNativeBuild=true`.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…lbackRemoval, ChannelReseed, UpdateCommand, CliBootstrap
Adds five test files exercising the PR1 channel-coherence implementation:
- tests/Aspire.Cli.Tests/Acquisition/IdentityChannelReaderTests.cs (NEW, 9 tests)
Exercises IdentityChannelReader(Assembly) with dynamic AssemblyBuilder fakes:
* 4 channels (stable, staging, daily, pr) round-trip through ReadChannel
* Missing AspireCliChannel metadata throws InvalidOperationException with
the assembly name in the message
* Empty AspireCliChannel value throws (per PR1-S4 AC d)
Covers ParsePrNumber via direct internal-static call (InternalsVisibleTo):
* 0.0.0-pr12345.deadbeef -> 12345
* 1.2.3-preview.5 -> null (no -pr marker)
* 0.0.0-pr.5 -> null (no digits between -pr and dot)
* null input -> null
* 0.0.0-pr0 -> 0 (no dot suffix is valid)
- tests/Aspire.Cli.Tests/Configuration/GlobalChannelFallbackRemovalTests.cs (NEW, 6 tests)
Verifies PR1-S7's removal of the global-channel read fallback from the 3
project readers. Behavioral tripwire on PrebuiltAppHostServer.ResolveChannelName:
injects a TestConfigurationService whose OnGetConfiguration throws,
reflectively invokes the (now sync) private ResolveChannelName, and asserts
it returns null on an empty workspace and the per-project channel when
aspire.config.json#channel is set. Also locks:
* ResolveChannelName is sync (no async-equivalent on the type)
* IConfigurationService field still present on all 3 readers (DI safe)
* No async-resolve method survives on any of the 3 reader types
- tests/Aspire.Cli.Tests/Scaffolding/ChannelReseedTests.cs (NEW, 13 tests)
Locks PR1-S10's reseed pattern via inline-equivalent assertions that round-trip
through AspireConfigFile (Save then Load):
* 4-channel theory: empty input -> CliExecutionContext.Channel wins
* 3 mixed pairs: explicit input (e.g. 'pr') wins over context (e.g. 'stable')
* post-prepare null result falls back to context channel
* post-prepare explicit result overrides context channel
* 3 blank-input variants (null, empty, whitespace) all fall back to context
Plus structural reflection on the 3 reseed-site types (ScaffoldingService,
GuestAppHostProject, CliTemplateFactory) to confirm CliExecutionContext field
is wired in as the reseed source.
- tests/Aspire.Cli.Tests/Commands/UpdateCommandTests.cs (EXTEND, +5 tests)
Verifies PR1-S9's --self channel-write removal:
* Theory across 4 channel/quality variants: SetConfigurationAsync('channel', _, isGlobal: true)
is never invoked during 'update --self'
* Stable channel no longer triggers DeleteConfigurationAsync('channel', isGlobal: true)
Uses Aspire.Cli.Tests.TestServices.TestConfigurationService (fully qualified
to avoid clash with the unrelated public TestConfigurationService in
ConfigCommandTests.cs in the same namespace).
- tests/Aspire.Cli.Tests/CliBootstrapTests.cs (NEW, 3 tests)
Locks IIdentityChannelReader / IdentityChannelReader type contracts
(parameterless ReadChannel returning string, optional Assembly? ctor
defaulting to null). Confirms the reader works on the running CLI assembly
and returns one of the 4 valid channels. Includes a snapshot test pinning
the fact that the production CLI does NOT yet wire IIdentityChannelReader
into DI / Program.cs — flagged via decision drop
livingston-pr1-bootstrap-wire-needed.md to Ocean for triage.
Test results: 87 tests pass / 0 fail / 0 skip across the 5 files.
Full UpdateCommandTests class: 44/44 still pass after the extension.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ttrs, whitespace, case sensitivity Extends IdentityChannelReaderTests with 4 edge cases that probe the contract of the AspireCliChannel assembly metadata read path: - ReadChannel_ChannelMetadataValueIsUnknownString_ReturnedVerbatim Reader returns 'foobar' (or any string) verbatim — it does not validate the value. Invalid values are caught at build time by AssemblyMetadataChannelTests (PR1-S5). Documents the intentional 'trust the build' behavior. - ReadChannel_AssemblyHasMultipleChannelMetadataAttributes_ReturnsFirstNonEmpty MSBuild misconfiguration could emit two AspireCliChannel attributes. The reader uses FirstOrDefault, so the first attribute wins. Locks ordering so a future LINQ change doesn't silently flip behavior. - ReadChannel_ChannelMetadataValueIsWhitespeOnly_ReturnedVerbatim Production reader uses string.IsNullOrEmpty (not IsNullOrWhiteSpace), so a whitespace-only value is returned. Documents the current behavior — the build-time smoke test catches this in practice. Tightening to IsNullOrWhiteSpace would be a deliberate behavior change, not a bug fix. - ReadChannel_KeyLookupIsCaseSensitive_DifferentCaseTreatedAsMissing The metadata key lookup uses StringComparison.Ordinal. An attribute keyed 'aspirecliChannel' (lowercase 'a') does NOT match 'AspireCliChannel' and ReadChannel throws as if the metadata were missing. Test count: 9 -> 15 (+4 TG1 edge cases + 2 helper overload tests). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…, overflow, mixed-suffixes Extends IdentityChannelReaderTests.ParsePrNumber coverage with 10 edge cases that probe input validation and integer-overflow safety: - ParsePrNumber_EmptyString_ReturnsNull string.Empty short-circuits at the IsNullOrEmpty guard. - ParsePrNumber_ReleaseVersionWithoutSuffix_ReturnsNull '0.0.0' has no '-pr' marker; IndexOf returns -1. - ParsePrNumber_PrMarkerWithoutTrailingDigits_ReturnsNull '0.0.0-pr' has the marker but no digits follow; the start==end guard hits. - ParsePrNumber_PrMarkerFollowedByHyphenThenDigits_ReturnsNull '0.0.0-pr-12345' — after '-pr' the next char is '-', not a digit, so null. Documents that the reader requires digits IMMEDIATELY adjacent to '-pr', not separated by punctuation. - ParsePrNumber_DigitsFollowedByLetters_StopsAtFirstNonDigit '0.0.0-pr12345abc' — the digit walk stops at 'a', and '12345' parses to 12345. Documents the lenient behavior: any non-digit acts as a delimiter. - ParsePrNumber_MaxIntPrNumber_Parses int.MaxValue (2147483647) parses cleanly via int.TryParse. - ParsePrNumber_OverflowsInt_ReturnsNull (most important) '0.0.0-pr2147483648' is int.MaxValue + 1. int.TryParse with NumberStyles.None returns false on overflow; the reader must propagate that as null without throwing OverflowException. Locks defensive behavior. - ParsePrNumber_PrMarkerFollowedByLettersOnly_ReturnsNull '0.0.0-prabc.def' — letters after '-pr', start==end, so null. - ParsePrNumber_MarkerEmbeddedInRcSuffix_ParsesEmbeddedDigits '1.0.0-rc.1.pr12345' contains 'pr12345' but not '-pr12345'. IndexOf is anchored on the literal '-pr' substring, not a regex match on 'pr', so this returns null. Documents the literal-substring anchoring. - ParsePrNumber_RealCliInformationalVersion_DoesNotThrow Defensive smoke: feed the actual running CLI's InformationalVersion (whatever it is today — likely a non-PR build) into ParsePrNumber and assert it never throws. Returns null or a non-negative int. Test count: 15 -> 25 (+10 TG2 parsing edges). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ExecutionContext
Closes the PR1 wiring gap noted in livingston-pr1-bootstrap-wire-needed.md.
Before this commit, IIdentityChannelReader had no consumer in production DI,
so CliExecutionContext.Channel always resolved to its constructor default
("daily") regardless of the AspireCliChannel value baked into the assembly.
That made the entire PR1-S10 reseed chain write "daily" for every CLI build.
Changes in src/Aspire.Cli/Program.cs:
1. Register IIdentityChannelReader as a singleton (default IdentityChannelReader,
reads from Assembly.GetEntryAssembly()) immediately before the
CliExecutionContext factory so it is resolvable from sp inside the lambda.
2. In the CliExecutionContext factory, resolve the reader, call ReadChannel(),
and parse the PR number from the assembly's AssemblyInformationalVersion via
IdentityChannelReader.ParsePrNumber.
3. Thread the resolved channel and prNumber through BuildCliExecutionContext
into the CliExecutionContext constructor as named arguments.
The CliExecutionContext constructor still defaults channel to "daily" and
prNumber to null; CliTestHelper.CreateDefaultCliExecutionContextFactory keeps
its explicit "daily" default so test ergonomics are unchanged. Only the
production startup path now overrides those defaults from the baked metadata.
AOT-safe: AssemblyMetadataAttribute and AssemblyInformationalVersionAttribute
are sealed framework attributes; GetCustomAttributes<T>() over them is preserved
by the trimmer / native compiler. No reflection-based JSON, no dynamic loading.
Build: dotnet build src/Aspire.Cli/Aspire.Cli.csproj /p:SkipNativeBuild=true
0 Warning(s), 0 Error(s).
Note: Livingston is replacing the snapshot test in tests/Aspire.Cli.Tests/
CliBootstrapTests.cs (which asserted the unwired state) with positive coverage
in parallel on the same branch. Her test will commit shortly after this one.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ityChannelReader DI wiring The CliBootstrapTests previously contained a snapshot test (IIdentityChannelReader_NotYetRegisteredInProductionDI_BootstrapWiringIsPendingFollowUp) that asserted, via reflection, the absence of any production consumer of IIdentityChannelReader. With PR1-S12 (1dc0dde) wiring the reader into Program.BuildApplicationAsync's DI container, that snapshot now flips and must be replaced with positive coverage of the wiring contract. New coverage in tests/Aspire.Cli.Tests/CliBootstrapTests.cs: * IIdentityChannelReader_TypeExists_AndProductionImplementationShape (kept, lightly renamed) — locks the interface + IdentityChannelReader ctor signature so the production factory delegate stays bound to a stable contract. * IdentityChannelReader_OnRunningCliAssembly_ReturnsKnownChannel (kept) — the Aspire.Cli assembly's baked AspireCliChannel value resolves to one of stable/staging/daily/pr. * BuildApplication_RegistersIIdentityChannelReader_AsIdentityChannelReaderInstance (new) — host.Services.GetRequiredService<IIdentityChannelReader>() resolves to an IdentityChannelReader instance (AC #1 from livingston-pr1-bootstrap-wire-needed.md step 5). * BuildApplication_PopulatesCliExecutionContextChannel_FromIdentityChannelReader (new) — context.Channel matches reader.ReadChannel(); the context's channel was sourced from the reader, not from the constructor default (AC #2). This is the assertion that catches a regression where the PR1-S10 reseed chain would silently write "daily" for every CLI build regardless of the baked channel. * BuildApplication_LocallyBuiltCli_HasDailyChannelAndNullPrNumber (new) — for a locally-built CLI (default csproj AspireCliChannel=daily, no -pr suffix in InformationalVersion), context.Channel == "daily" and context.PrNumber is null (AC #3 + the additional PrNumber assertion from the spec). * BuildApplication_CliExecutionContextChannel_MatchesAssemblyMetadataAttribute (new) — end-to-end coherence: the channel flowing through DI equals the value baked into the entry assembly's [AssemblyMetadata("AspireCliChannel")] via reflection — independent of the constant "daily". The new tests use the same pattern as the existing TelemetryConfigurationTests.BuildHostAsync — they invoke the real Program.BuildApplicationAsync so the assertions exercise the production factory delegate, not a duplicated test copy. Test-csproj change (Aspire.Cli.Tests.csproj): The default IdentityChannelReader reads AspireCliChannel from Assembly.GetEntryAssembly(). In production this is Aspire.Cli.dll (which has the metadata baked in by Aspire.Cli.csproj). Under `dotnet test` the entry assembly is the test host (Aspire.Cli.Tests.dll), which had no metadata, so the bootstrap factory threw on first resolution of CliExecutionContext. Adding <AssemblyMetadata Include="AspireCliChannel" Value="daily" /> to the test csproj mirrors production, so any test that resolves CliExecutionContext from a real Program.BuildApplicationAsync host gets a coherent "daily" channel value (and PrNumber null, since Aspire.Cli's InformationalVersion under dev builds has no -pr<N> suffix). Note on a follow-up concern: Linus's PR1-S12 wiring uses Assembly.GetEntryAssembly() (via the default IdentityChannelReader ctor) for Channel but typeof(Program).Assembly for InformationalVersion / PrNumber. In production both resolve to Aspire.Cli.dll and that is fine, but the asymmetry is fragile under any future hosting scenario where GetEntryAssembly() != typeof(Program).Assembly (tests, custom hosts). This isn't blocking PR1 closure — flagging it for a follow-up decision drop after the PR1 wave merges. Build + test: dotnet test --project tests/Aspire.Cli.Tests/Aspire.Cli.Tests.csproj \ --no-launch-profile -- \ --filter-class "*.CliBootstrapTests" \ --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true" -> 6 passed, 0 failed (1.2s) Also re-ran adjacent suites that share BuildApplicationAsync to confirm no regression: AssemblyMetadataChannelTests + CliExecutionContextTests + CliBootstrapTests + TelemetryConfigurationTests = 26 passed, 0 failed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…eChannelName PR1-S7 (commit 65ad8c7) renamed PrebuiltAppHostServer.ResolveChannelNameAsync to the synchronous ResolveChannelName because all awaits had been removed (CS1998). The reflection-based test in PrebuiltAppHostServerTests still pointed at the old async name and the Task<string?> unwrap, so it failed at runtime when method.Invoke returned null (no method by that name). - Test method renamed to drop Async suffix. - Reflection target updated to ResolveChannelName. - Synchronous invocation: cast Invoke result to (string?) directly, no Task unwrap, and pass [] (the production method takes no args). Verified: all 12 PrebuiltAppHostServerTests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… fix RemoteExecutor regressions Under Microsoft.DotNet.RemoteExecutor, the child process's entry assembly is the RemoteExecutor host, not Aspire.Cli.dll. The default-ctor path of IdentityChannelReader uses Assembly.GetEntryAssembly() which under that test host resolves to the wrong assembly, and the bootstrap factory then throws on missing AspireCliChannel [AssemblyMetadata]. Pin the DI registration to typeof(Program).Assembly so: - RemoteExecutor child processes read AspireCliChannel from the real Aspire.Cli.dll regardless of test host. - Symmetric with the InformationalVersion read in the same factory, which already uses typeof(Program).Assembly. Fixes the four CliSmokeTests/SdkDumpCommandTests regressions Ocean caught in PR1 final gate review of 1dc0dde. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Removes 6 "Acquisition v3" comments left behind by f1438f6 in the install scripts. The reasoning that those comments captured — that we no longer write the global channel field because channel is now baked into assembly metadata per agreed-design-v3.md §2 — is the entire purpose of the parent commit (PR1-S6, f1438f6). Keeping it in the commit message rather than as dead source comments avoids workstream-local labels (e.g. "v3") leaking into the long-lived codebase. Changes: - eng/scripts/get-aspire-cli.sh line 1024: removed 4-line comment - eng/scripts/get-aspire-cli.ps1 line 1242: removed 4-line comment - eng/scripts/get-aspire-cli-pr.sh lines 1079, 1195: removed 3-line comments (2x) - eng/scripts/get-aspire-cli-pr.ps1 lines 1301, 1399: removed 3-line comments (2x) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The reader is registered as a DI singleton (and consumed at startup by the CliExecutionContext factory), but ReadChannel() previously re-scanned Assembly.GetCustomAttributes<AssemblyMetadataAttribute>() and did a linear FirstOrDefault on every call. Cache the resolved channel string in a Lazy<string> with LazyThreadSafetyMode.ExecutionAndPublication so the metadata read happens exactly once per instance (= once per process under singleton scope). The constructor still does NOT trigger the read — only Lazy<T> construction, no .Value access. Validation (missing/empty metadata) still throws on the FIRST ReadChannel() call so DI consumers see the error eagerly when they first need the channel, preserving the deliberate "deferred error" contract from PR1-S4. Subsequent calls return the cached string. AOT-safe: Lazy<T> uses no reflection internally. No new package refs. Public surface (string ReadChannel() on IIdentityChannelReader) is unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ineData Collapse 14 single-input ParsePrNumber [Fact] tests into one [Theory] + [InlineData] block. Each row preserves the exact input/ expected pair from the original Fact, so coverage is unchanged (14 InlineData rows + 1 ParsePrNumber_RealCliInformationalVersion [Fact] retained because it asserts a range, not equality, and exercises the running CLI assembly's InformationalVersion). 25 tests still pass after the conversion. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Per the no-internal-labels directive (.squad/decisions.md), removed `// PR1-Sx:` / `// PR1-TGx:` style comments and parenthetical `(PR1-Sx)` references from test comments. These labels are workstream-internal shorthand that won't make sense to anyone reading the tests outside this workstream. WHAT/WHY explanations are preserved; only the workstream-shorthand prefix is removed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The computeCliChannel pwsh step in the build stage of azure-pipelines.yml and azure-pipelines-unofficial.yml exported $(aspireCliChannel) as an output variable, but no consumer references stageDependencies.build.Windows.outputs['computeCliChannel.aspireCliChannel']. The live consumer is the inline computeCliChannel step inside build_sign_native.yml (introduced by PR1-S2b/B6) — that template runs BEFORE the build stage, so the output variable was never reachable from where it was needed. Two-source-of-truth maintained only by a 'MUST stay in sync' comment is drift waiting to happen. Delete the dead step and update the comment in build_sign_native.yml to reflect that the inline compute is now the canonical (and only) AzDO compute for AspireCliChannel. The unrelated computeChannel step (which exports installerChannel for the prepare_installers stage) is untouched. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ld> AdditionalProperties eng/clipack/Common.projitems invokes an inner <MSBuild Targets="Publish"> on Aspire.Cli.csproj and was relying on MSBuild's global-property inheritance to carry AspireCliChannel into that inner build. Compare against RuntimeIdentifier, VersionSuffix, OfficialBuildId, and Configuration in the same item group: each is forwarded explicitly via <AdditionalProperties>. The codebase has clearly learned not to trust implicit inheritance for properties that materially shape the output. Add AspireCliChannel to the explicitly forwarded set so the inner Publish sees the channel value the AzDO pipeline computed and passed in via /p:AspireCliChannel=$(aspireCliChannel) (build_sign_native.yml), and so the contract does not silently break if a future refactor strips global property inheritance. Verified: dotnet msbuild eng/clipack/Aspire.Cli.linux-x64.csproj /p:AspireCliChannel=staging /getProperty:AspireCliChannel evaluates and returns 'staging' with no errors. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… (option-a)
The reseed call sites (5 of them — ScaffoldingService, 3 template factories,
GuestAppHostProject) write CliExecutionContext.Channel into the project's
aspire.config.json#channel. Previously this returned the raw identity-channel
value from [AssemblyMetadata("AspireCliChannel", ...)] — the literal string
"pr" for PR builds — which would never match a hive directory created by
PackagingService (which names PR hives "pr-<N>", e.g. "pr-16820").
Per option-(a) in .squad/decisions.md: the consumer-facing Channel property
now exposes the resolved hive label. The constructor still accepts the raw
identity value (kept in the new private _channel field). The new
IdentityChannel property exposes that raw value for any caller that needs
the build-time taxonomy.
- For non-PR builds, Channel == IdentityChannel == constructor input.
- For PR builds with PrNumber.HasValue, Channel returns "pr-<N>".
- IdentityChannel always returns the constructor-provided value verbatim.
The PrNumber doc-comment now refers to IdentityChannel (the build-time
identity) rather than Channel (the resolved hive label).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
.github/workflows/ci.yml passes
/p:VersionSuffix=pr.$($Env:PR_NUMBER).g$SHORT_SHA
to the build, which produces an InformationalVersion of the shape
"<base>-pr.<NUMBER>.g<SHA>+<sha>". The previous parser scanned for "-pr"
followed immediately by digits and so returned null on every PR-channel
CLI shipped from GitHub Actions — leaving CliExecutionContext.PrNumber
null on real PR builds.
Accept an optional single '.' between the "-pr" marker and the digits.
The "preview" carve-out remains: "1.2.3-preview.5" still returns null
because the character after "-pr" ('e') is neither '.' nor a digit and
the digit scan finds no digits.
Verified shapes (matches AC list):
"0.0.0-pr.5" -> 5
"0.0.0-pr12345.deadbeef" -> 12345
"0.0.0-pr.12345.gabcd1234" -> 12345
"0.0.0-pr.12345.gabcd1234+abc" -> 12345
"0.0.0-pr" -> null
"0.0.0" -> null
"1.2.3-preview.5" -> null
Span-based, allocation-free, AOT-safe (no regex).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The DI factory for CliExecutionContext previously parsed PrNumber out of AssemblyInformationalVersion unconditionally. Spec invariant: PrNumber is non-null only when the identity channel is "pr". A non-PR build whose InformationalVersion happens to contain a "-pr<digits>" segment (e.g. a custom VersionSuffix) would otherwise produce a non-null PrNumber on a stable/staging/daily channel. Skip the InformationalVersion read entirely on non-PR channels: PrNumber stays null. On the "pr" channel we still parse, and a parser miss yields null (the bootstrap remains tolerant of malformed PR informational versions, even though Fix 2 widens the parser to accept the real CI shape). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…figService (4th reader missed in S7)
PR1-S7 swept the global-channel read fallback from 3 project readers
(DotNetBasedAppHostServerProject, PrebuiltAppHostServer, NewCommand)
but missed a 4th: TemplateNuGetConfigService. gpt-5.5 caught this in
the PR1 review.
The same dead-read pattern existed at three call sites:
- PromptToCreateOrUpdateNuGetConfigAsync(string?, ...) ~L75-78
- CreateOrUpdateNuGetConfigWithoutPromptAsync(...) ~L110-113
- ResolveTemplatePackageAsync(...) ~L156-159
Each one had:
if (string.IsNullOrEmpty(channelName))
{
channelName = await configurationService.GetConfigurationAsync(
"channel", cancellationToken);
}
With the global-channel writers gone (PR1-S6 + PR1-S8/S9) these reads
were the same dead-code-becomes-contamination surface (G1) that S7
removed elsewhere — the global ~/.aspire/aspire.config.json#channel
value is no longer a channel source.
Existing fall-through logic handles "no preference":
- PromptToCreateOrUpdate / CreateOrUpdateWithoutPrompt: early-return
when channelName is unset (no NuGet.config write occurs without an
explicit --channel input). User-prompt/sidecar config writes are
optional, not derived from baked CLI identity.
- ResolveTemplatePackageAsync: falls through to the implicit/PR-hives
branch (allChannels.Where(c => c.Type is PackageChannelType.Implicit)
or all-channels-with-hives). Same shape as NewCommand post-S7.
Deviation from AC: AC said "Do NOT remove the _configurationService
injection — it's likely used for non-channel reads." Empirically there
were zero non-channel reads of configurationService in this file. The
class uses a primary constructor (not a separate field), so leaving
the unused param triggered CS9113 ("parameter is unread"), and capturing
into a `private readonly` field triggered CS9124 + CA1823. Removing the
ctor param is the only clean option in primary-ctor land — the analogous
PR1-S7 readers used traditional ctors with explicit fields, where
unused-private-field warnings are tolerated. Livingston's parallel test
edits will adapt to the new ctor signature.
Build: dotnet build src/Aspire.Cli/Aspire.Cli.csproj /p:SkipNativeBuild=true
clean (0 warnings, 0 errors).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Reinstate channel-resolution coverage for `aspire init` after the recent removal of the on-disk global-channel fallback and the matching shift to sourcing the channel from the running CLI binary's identity channel. Tests added (driving `InitCommand.ExecuteAsync` end-to-end): - InitCommand_ProjectMode_NoChannelOverride_ResolvesAgainstCliExecutionContextChannel: Theory across stable / staging / daily / pr-12345. Each row pins CliExecutionContext.Channel and registers a uniquely-sourced explicit channel; the assertion captures the package source seen by `dotnet new install` and verifies it came from the matching channel — proving the resolver picked the binary's identity channel. - InitCommand_ProjectMode_PrBuildResolvesToPrNumberedHive: End-to-end pipeline test for PR builds. Constructs CliExecutionContext with channel='pr' + PrNumber=12345 and asserts the resolver reaches the pr-12345 hive — catching joint-contract mismatches between the producer emitting 'pr' and the consumer expecting 'pr-<N>'. - InitCommand_DoesNotConsultGlobalConfigurationServiceForChannelKey: Negative-shape tripwire. Injects an IConfigurationService that throws on any GetConfigurationAsync(key='channel') / GetConfigurationFromDirectoryAsync call. Runs in project mode so the previously-vulnerable resolver path is exercised. Reproduces the regression class that surfaced when the global-channel fallback was removed but a caller silently relied on it. Existing tests that exercised the `ChannelOverride: null` path (WhenSolutionAndProjectInSameDirectory, WhenSolutionDirectoryHasNoProjectFiles, WhenSolutionExistsAndChannelIsImplicit, WhenSolutionExistsAndPrHivesPresent, WhenChannelTemplateSearchFails) were updated to override the test CliExecutionContext channel to 'default' so the resolver still matches the implicit channel by name. The resolver now always receives a non-null channel override sourced from CliExecutionContext.Channel. Per Ocean's design call in .squad/decisions/inbox/ocean-pr1-init-channel-fix.md. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The earlier fix only patched the project-mode resolver site (InitCommand.DropCSharpProjectSkeletonAsync, line 332 — TemplatePackageQuery.ChannelOverride). The single-file path (DropCSharpSingleFileSkeletonAsync, line 272) still passed channelName: null to TemplateNuGetConfigService.CreateOrUpdateNuGetConfigWithoutPromptAsync, so a developer running 'aspire init' in an empty directory got an apphost.cs with #:sdk Aspire.AppHost.Sdk@<version> but no workspace nuget.config wiring that SDK to the running CLI's identity-channel hive — leaving SDK resolution to fall back to the default feed for any non-stable build (staging, daily, pr-<N>, local). Apply Ocean's option-(a) wiring to both sites: the channel argument now defaults to CliExecutionContext.Channel, mirroring the project-mode fix. Both call sites in InitCommand pass _executionContext.Channel as their channel input — the consistent shape a future maintainer will see when reading the two paths side-by-side. Restore Livingston-10's deferred single-file test as InitCommand_SingleFileMode_NoChannelOverride_WiresNuGetConfigToCliExecutionContextChannel, a Theory across stable / staging / daily / pr-12345 mirroring the project-mode Theory. Each row pins CliExecutionContext.Channel and registers an explicit channel with a uniquely-sourced feed; the assertion reads the workspace nuget.config emitted by NuGetConfigMerger and verifies it carries the matching feed URL. Per Ocean's design call in .squad/decisions/inbox/ocean-pr1-init-channel-fix.md. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
Author
Wave 6: channel-resolution fix + coverage gapsCI failures triaged. Root cause was a real user-facing bug, not flake: an earlier sweep of global-channel readers cut the wire that What was brokenPolyglot CI was the canary (~25 EndToEnd failures with
Fix (Ocean's option-a — preserves G1 contract)
Coverage closed (this wave)
Polyglot CI script cleanup
Comment cleanupFound and scrubbed one C-001 violation that escaped the earlier sweep ( Tests: Ready for re-review. |
Remove absence-framing language that described what was removed ('fallback was removed',
'previously-vulnerable'). Rewrite to focus on current behavior: the test ensures init does
not consult the global configuration service for the channel key, exercising the
template-package resolver code path.
Commit 5b9dd62 updated GlobalSettings_MigratedFromLegacyFormat for the new migration spec (channel is baked into the binary, not stored in global config). The same spec change applies to two sibling tests that still asserted the pre-spec behavior: - GlobalMigration_PreservesAllValueTypes - GlobalMigration_HandlesCommentsAndTrailingCommas Both now assert: migrated aspire.config.json keeps non-channel content (features, packages) but drops the channel key; legacy globalsettings.json is preserved unchanged with channel intact; `aspire config get channel` returns the not-found error since the migrated global config has no channel key. Mirrors the assertion shape of GlobalSettings_MigratedFromLegacyFormat. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Two related bugs caused polyglot E2E tests (Java, TypeScript, Python,
JavaScript) and SingleFileAppHostInitDotnetRunTests to fail when the
CLI was built with AspireCliChannel=local (via build-cli-native-archives.yml)
and installed through get-aspire-cli-pr.sh --local-dir.
Root cause 1 — channel name mismatch (SingleFileAppHostInitDotnetRunTests):
The --local-dir install path creates a hive directory named
"run-$GITHUB_RUN_ID" (e.g. run-25422767716), but the CLI binary baked
with AspireCliChannel=local exposes Channel = "local". When InitCommand
called CreateOrUpdateNuGetConfigWithoutPromptAsync("local", ...) the
lookup found no channel named "local" and returned false, so no
NuGet.config was written and dotnet run --file apphost.cs could not
resolve Aspire.AppHost.Sdk.
Fix: PackagingService.GetChannelsAsync now renames the hive channel to
"local" when executionContext.IdentityChannel == "local", so the channel
name always matches what the CLI reports as its identity.
Root cause 2 — dotnet package search fails for local flat-folder sources
(polyglot aspire add):
For hive channels, PackageChannel.GetIntegrationPackagesAsync built a
temporary NuGet.config pointing at the local .nupkg folder and ran
dotnet package search against it. dotnet package search only supports
NuGet v3 API sources, not local flat folders, so it returned zero
results. Because Aspire.Hosting.JavaScript (and other newly-added
hosting integrations) ship only in the local hive during PR validation,
no integration packages were found and aspire add failed.
Fix: PackageChannel.GetIntegrationPackagesAsync now short-circuits when
PinnedVersion is set and the Aspire* mapping points at a local directory:
it enumerates .nupkg files directly (mirroring the pattern already used
in GetTemplatePackagesAsync and GetLocalHivePinnedVersion) and returns
all Aspire.Hosting.* packages with the pinned version.
Supporting changes:
- PackageChannelNames.Local constant added for the "local" channel name
- VersionHelper.IsLocalBuildChannel now recognises "local" in addition
to "pr-*" and "run-*" prefixes so NuGet.config injection and version
matching work correctly for local-channel hives
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Wave 7 introduced a rename in GetChannelsAsync: when IdentityChannel=="local" (set by build-cli-native-archives.yml via /p:AspireCliChannel=local), every hive directory was aliased to "local". This was correct for ephemeral run-<N> hives created by get-aspire-cli-pr.sh --local-dir (the polyglot E2E setup), but it also clobbered pr-<N> hives installed by get-aspire-cli-pr.ps1/sh, which are looked up by their exact name by cli-starter-validation.ps1. Fix: only apply the "local" alias when the hive name does NOT start with "pr-". PR-install hives keep their pr-<N> name; run-<N> hives are still aliased to "local" as Wave 7 intended. Regression observed in CI run 25453755247 on commit 9fe9daf: ❌ No channel found matching 'pr-16820'. Valid options are: default, stable, daily, local (jobs: Windows ARM64 + x64 Starter Validation) Adds regression test GetChannelsAsync_WhenIdentityChannelIsLocal_PrNamedHiveKeepsItsName that would have caught this before merge. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When installing from --local-dir without an explicit --hive-label and without GITHUB_RUN_ID set, both get-aspire-cli-pr.sh and .ps1 were defaulting to "run-local" as the hive directory name. Since Wave 7 (9fe9daf), PackagingService.GetChannelsAsync renames any non-pr-* hive to "local" when IdentityChannel == "local". The on-disk directory was "run-local/" but the in-memory channel name was "local", causing PrebuiltAppHostServer.TryCreateTemporaryNuGetConfigAsync to find the "local" channel and emit a PSM-restricted NuGet config. For the 7 E2E tests with requiresNugets=false the hive packages dir is empty, so `aspire-managed nuget restore` exits non-zero and `aspire new` exits 10. Aligning the default to "local" makes the on-disk dir match the identity channel name so the Wave 7 rename becomes a no-op, and is a necessary prerequisite for the follow-up C# revert in PackagingService (routing to Livingston under Ocean review). Fixes the script-side half of the Wave 9 regressions for: ConfigDiscoveryTests, DotnetToolSmokeTests, KubernetesDeployBasicApiService, KubernetesDeployWithValkey, PythonReact, StagingChannelTests, TypeScriptStarterTemplate NOTE: The C# revert of the GetChannelsAsync rename is still required; this commit alone is necessary but not sufficient. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…7 dead rename Wave 9 Part 2: two changes to fix 7 E2E regressions introduced by Wave 7. Change 1 (PrebuiltAppHostServer): In TryCreateTemporaryNuGetConfigAsync, add a guard that returns null (no PSM config) when the resolved channel name matches the CLI's own identity channel and is not a pr-* channel. This restores the pre-Wave-7 behavior where the local identity hive never triggered PSM emission. PR hives retain PSM because they represent complete, isolated package sets. Change 2 (PackagingService): Remove the in-memory rename block (added in Wave 7) that aliased non-pr hive directory names to 'local'. With the Wave 9 Part 1 script fix (hive_label='local'), the rename was already a no-op; removing it eliminates dead code and cognitive complexity. Tests: Replace Wave 7/8 rename-specific unit tests with a single test asserting channel.Name == directory name. Update PrebuiltAppHostServer test constructors to pass the new CliExecutionContext parameter. Fixes: aspire new / aspire start restore failures in CI E2E tests caused by PSM restricting Aspire* packages to the local hive and breaking NuGet dependency resolution. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Closes Ocean's Wave 9 Part 2 follow-up. Adds cross-product coverage for the guard added in ed36ba4 — the four quadrants of (channelName, IdentityChannel) where PSM either fires or skips. Mirrors the pattern the cli-channel-hive-rename-coverage SKILL prescribes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Locally-built CLIs without a build-time AspireCliChannel value previously identified as "daily", colliding with the real daily PackageChannel and triggering false-positive PSM short-circuits. Default is now "local" (matches PackageChannelNames.Local). Updates the 4-value enumerations in IdentityChannelReader, test smoke gates, and the v2 default in CliExecutionContext. Reviewers (Ocean opus-4.7 + Linus gpt-5.5) flagged this as CRITICAL drift between the post-Wave 9 "5-value taxonomy" and the as-implemented build wiring (findings C2, C3, M1, N1, N2, F1). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The post-Wave 9 PSM guard at PrebuiltAppHostServer.cs:425 used
"channelName == IdentityChannel && !channelName.StartsWith(\"pr-\")",
which silently skipped temp NuGet config emission for stable,
staging, and daily identity matches — broader than the comment
("local identity hive") claimed. With csproj default "local"
(prior commit), a stable CLI on a stable project would lose its
package-source-mapping and silently use the user's NuGet sources.
Guard now checks IdentityChannel == PackageChannelNames.Local
(constant, not literal). Two tests that codified the broader
behavior flipped to assert the narrower correct behavior.
PackagingServiceTests setup updated to pass an explicit non-local
channel so the test name stays accurate as defaults evolve.
Findings H2 + N3.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…hive label
build-cli-native-archives.yml previously hardcoded
/p:AspireCliChannel=local for ALL builds, including PR validation.
PR-installed CLIs would identify as "local" while their packages
lived in pr-<N>/ hives — the resulting identity/hive mismatch
broke PR-feed package routing.
Workflow now computes channel via job-level env, mirroring AzDO's
computeCliChannel: pull_request -> "pr"; release/ branches ->
"staging"; otherwise -> "daily". (AzDO's release-tag -> "stable"
branch intentionally omitted; GH never publishes stable.)
get-aspire-cli-pr.{sh,ps1} install_from_local_dir no longer
prefers run-$GITHUB_RUN_ID when GITHUB_RUN_ID is set. Local-dir
installs are local-dev installs; the CLI's identity is "local"
and the hive label should match. The --run-id install path
(separate function) is unchanged and still emits run-<id>.
Findings C1 + H1.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
radical
changed the title
The PR-build CLI archive contains .nupkg files whose version suffix encodes the PR identity (e.g. "13.4.0-pr.16820.g3703c5c4"). When install_from_local_dir or polyglot-validation/setup-local-cli.sh defaults to hive_label="local", PR-built packages land at ~/.aspire/hives/local/ but the CLI's CliExecutionContext.Channel resolves to "pr-<N>" and looks them up there. Auto-detect the PR identity from the .nupkg filename pattern "pr\.([0-9]+)\.[0-9a-g]+" and set hive_label="pr-<N>" when it matches; fall back to "local" otherwise. Explicit --hive-label still wins. Closes the polyglot-validation regression in CI run 25469878546 (20 failed jobs, all on Linux polyglot scenarios). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Both arms (refs/heads/main → daily, fallback → daily) produced the same value. Reviewer correctly pointed out the elseif adds no information; collapse to a single else with a comment clarifying the fallback intent. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… resolution When the CLI is built on a local-build channel (pr-*, run-*, local) and a hive-backed PackageChannel of that name is registered, NewCommand should prefer it over the Implicit (nuget.org) channel for resolving the template version when no --channel is supplied. Previously, ResolveCliTemplateVersionAsync hard-coded a fallback to the Implicit channel, regardless of CliExecutionContext.Channel. With the rest of the channel-coherence triangle (build-time identity → execution context → hive layout) now aligned, the implicit pick still routed template version resolution to nuget.org and produced the latest stable (e.g. 13.2.4). That stable version flowed into aspire.config.json's sdk.version, then into RestoreCommand as a stable-only range (>= 13.2.4). Package source mapping correctly routed it to the PR hive, but the hive only contained the corresponding prerelease (13.4.0-pr.16820.gSHA), and NuGet refuses prerelease packages for stable-only ranges, so restore failed with 'Unable to find a stable package …'. This regression manifested only after the hive-label autodetect fix (4e9ea68) correctly aligned the hive with the build channel — before that fix, the wrong hive was being consulted and the failure mode was different. Closing the channel-coherence triangle exposed this fourth axis (runtime channel selection at consumer sites). Cast-replay verified against run 25477301939 attempt 2: ConfigDiscoveryTests.RunFromParentDirectory_UsesExistingConfigNearAppHost TypeScriptStarterTemplateTests.CreateAndRunTypeScriptStarterProject Both showed the identical 'Unable to find a stable package Aspire.Hosting with version (>= 13.2.4)' restore error inside aspire new. Other call sites (AddCommand, TemplateNuGetConfigService, UpdateCommand) already include all channels when hives are present; only NewCommand had the blind Implicit pick. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…B_RUN_ID branch)
The existing AssemblyMetadataChannelTests.AspireCliChannel_AssemblyMetadata_IsOneOfExpectedValues
asserts `baked ∈ s_validChannels`. Because "daily" is also in that set, reverting the csproj
default from "local" back to "daily" would pass the smoke test silently — that is the C2 blind
spot identified in ocean-pr1-design-review.md F1 / fix-review C2.
The new CliAssembly_BakesChannel_AsLocal_MatchingCsprojDefault test pins the baked value to
exactly PackageChannelNames.Local using Assert.Equal, not set-membership. A csproj revert to
any other valid value (including "daily") will now fail the build immediately.
The existing LocalDir_DryRun_UsesLocalDirectoryWithoutGh (bash) and
LocalDir_WhatIf_UsesLocalDirectoryWithoutGh (PS) tests exercise the --local-dir happy path but
do not set GITHUB_RUN_ID in the script's environment. If someone re-introduces the removed
GITHUB_RUN_ID branch (fix-review H1, ocean-pr1-design-review.md F3), those tests would not
catch the regression because GITHUB_RUN_ID is absent.
The two new LocalDir_*_WithGitHubRunIdEnvSet_UsesLocalHiveLabel tests inject GITHUB_RUN_ID=99999
via ProcessStartInfo.Environment (not the test-process env) and assert:
- The output contains "local" (hive label from auto-detect / fallback).
- The output does NOT contain "run-99999" (the regression shape).
Tests added (3):
tests/Aspire.Cli.Tests/AssemblyMetadataChannelTests.cs
→ CliAssembly_BakesChannel_AsLocal_MatchingCsprojDefault (C2)
tests/Aspire.Acquisition.Tests/Scripts/PRScriptShellTests.cs
→ LocalDir_DryRun_WithGitHubRunIdEnvSet_UsesLocalHiveLabel (H1-sh)
tests/Aspire.Acquisition.Tests/Scripts/PRScriptPowerShellTests.cs
→ LocalDir_WhatIf_WithGitHubRunIdEnvSet_UsesLocalHiveLabel (H1-ps)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Drop the negation: `IsNullOrWhiteSpace(x) ? fallback : x` reads as 'when no input, use baked default; otherwise honor input', which matches how the comment immediately above describes the contract. Identical change applied to ScaffoldingService and the Python/Go starter templates. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The only remaining "local" string-literal channel comparison in production code lived inside IsLocalBuildChannel itself. Replace it with the PackageChannelNames.Local constant so the helper and its constant share a single source of truth. All other production sites already route through this helper or compare to PackageChannelNames.Local. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…k packages dir, not nuget.org Adds GetChannelsAsync_LocalHive_AspireMappingPointsAtLocalDirectory_NotPublicFeed to defend the regression where the 'local' hive channel's Aspire* PackageMapping must point at the local 'hives/local/packages' directory (filesystem path) and NOT at https://api.nuget.org/v3/index.json. Existing local-hive tests cover pinned version derivation and integration package enumeration, but none pinned the Mappings shape itself, so a future refactor that rebuilt the local channel on top of the public feed would silently break local-build CLI installs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… CLI
Adds AddCommand_WithLocalHive_PrefersCurrentCliVersion mirroring the existing
PR-hive equivalent. Lays down a 'hives/local/packages' directory containing
Aspire.Hosting.<cliVersion>.nupkg and Aspire.Hosting.Redis.<cliVersion>.nupkg,
then runs 'aspire add redis'. Asserts:
- the local channel is enumerated (GetPrHiveCount > 0 branch fires for the
'local' hive, not just pr-* / run-* hives),
- VersionHelper.IsLocalBuildChannel("local") + TryGetCurrentCliVersionMatch
pick the local-hive package whose version matches the current CLI version,
- the user is NOT prompted (non-interactive CLI-version match path),
- the public stable result returned via SearchPackagesAsync is ignored.
PackagingService-level coverage already exists for the 'local' channel name,
pinned version derivation, and integration package file enumeration; this is
the missing end-to-end pin that 'aspire add' on a local-channel CLI / local
hive install actually surfaces the local packages instead of falling back to
public NuGet.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The PR-route installer no longer calls Save-GlobalSettings (the channel selection happens via the local hive / nuget config layout, not the aspire CLI's global config). The .sh and .ps1 helpers were unreferenced. Acquisition tests already assert these symbols are absent from the installer source, so no test changes are needed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ease route The release-route installer no longer touches the aspire CLI's global config — channel selection happens through a different path now. Both Save-GlobalSettings and Remove-GlobalSettings (and their bash mirrors) were unreferenced in eng/scripts/. Acquisition tests in ReleaseScriptShellTests and ReleaseScriptPowerShellTests already assert these symbols are absent from the installer source, so no test changes are needed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…s, not just pr-*) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
1 participant
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.
PR1 — Channel binding (acquisition v3)
Implements the channel-binding slice of the v3 acquisition redesign per
docs/specs/acquisition/v3-prs/pr1-channel.md(in this PR).What ships
[AssemblyMetadata("AspireCliChannel")]baked intoAspire.Cliat build time. CI computes the value per build kind: PR validation →pr, release branches →staging, main →daily. Tag/release →stable(AzDO only). Local builds default tolocal.IdentityChannelReaderreads assembly metadata at startup; populatesCliExecutionContext.IdentityChannel(raw 5-value taxonomy:stable | staging | daily | pr | local) andCliExecutionContext.Channel(hive-label form: identity verbatim except PR builds becomepr-<N>).~/.aspire/aspire.config.json#channelremoved.DotNetBasedAppHostServerProject,PrebuiltAppHostServer,NewCommand.ScaffoldingService,CliTemplateFactory.*StarterTemplate,GuestAppHostProject) switched to bakedCliExecutionContext.Channel.IdentityChannel == PackageChannelNames.Local(closes the post-Wave 9 daily-on-daily silent regression).AspireCliChannelis one of the 5 valid values.Why
Pre-PR1, the install scripts wrote a "channel" key into
~/.aspire/aspire.config.json(a hive-label string likepr-12345orrun-local). Three project readers fell back to that global value when no project-local channel was set. The result was a cross-route contamination surface (G1 in the design doc): a script-route install would silently overwrite the channel that a PR-route install expected, and the parsed value was sometimes a hive label that noPackageChannelinPackagingServicerecognized. The Wave 9 PSM collision was the loud symptom; the structural fix is this PR.How it was reviewed
agreed-design-v3.md§2/§4 invariants (Ocean, claude-opus-4.7).Out of scope
PR2 (route primitives), PR3 (CLI bug fix), PR4 (lifecycle), PR5 (cleanup) — see
agreed-design-v3-pr-grouping.mdin this PR for the full ladder.🤖 Reviewed by Ocean (claude-opus-4.7) + Linus (gpt-5.5 cross-model)
Local tests: 2669 / 2649 / 0 / 20 ✅