Skip to content

[Automated] Update API Surface Area - #3

Closed
github-actions[bot] wants to merge 11 commits into
mainfrom
update-api-diffs
Closed

[Automated] Update API Surface Area#3
github-actions[bot] wants to merge 11 commits into
mainfrom
update-api-diffs

Conversation

@github-actions

@github-actions github-actions Bot commented Mar 1, 2025

Copy link
Copy Markdown

Auto-generated update to the API surface to compare current surface vs latest release. This should only be merged once this surface area ships in a new release.

@github-actions
github-actions Bot requested a review from radical as a code owner March 1, 2025 16:22
@radical
radical force-pushed the main branch 2 times, most recently from 6fe6e8a to f8f4c34 Compare May 9, 2025 17:37
@radical radical closed this May 30, 2025
radical pushed a commit that referenced this pull request Dec 5, 2025
@sliekens
Improve path validation for ExternalServiceResource to support revers…
2fefb06 
…e proxy scenarios (microsoft#13036)

* Relax path validation for ExternalServiceResource to support reverse proxy scenarios (#3)

* Initial plan

* Relax path validation for ExternalServiceResource

- Update ExternalServiceResource.GetUriValidationException to accept paths ending with '/'
- Update ResourceBuilderExtensions.WithReference validation similarly
- Update XML documentation to reflect new requirement
- Add comprehensive test coverage for path validation
- Update existing tests to match new error messages

Co-authored-by: sliekens <1583241+sliekens@users.noreply.github.com>

* Reject query strings in external service URIs

- Add validation to reject URIs with query strings, similar to fragments
- Query strings should not be part of the base URI for HttpClient
- Update tests to reflect query string rejection
- Move query string test cases from valid to invalid
- Add dedicated test for query string rejection

Co-authored-by: sliekens <1583241+sliekens@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: sliekens <1583241+sliekens@users.noreply.github.com>

* Clarify URI requirements in ExternalServiceResource documentation

* Align URL validation between ExternalServiceResource and ad-hoc URLs

---------

Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: sliekens <1583241+sliekens@users.noreply.github.com>
radical added a commit that referenced this pull request May 6, 2026
@radical
PR1-S12-test: replace snapshot test with positive coverage for IIdent…
e7a94bf 
…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>
radical added a commit that referenced this pull request May 12, 2026
@radical
Address dual-model review: fixture enumeration, registry override, ga…
4846d7a 
…te tightening, GHA cache

Apply review findings from the GPT-5.5 + Opus 4.7 dual-model pre-merge review on microsoft#16965.

1. Codegen fixture enumeration (review finding #1, both reviewers, High):
   The deleted test-{python,go,java,typescript}-playground.sh scripts iterated every
   tests/PolyglotAppHosts/<Integration>/<Language>/ fixture (50 Python, 49 Go, 50 Java,
   50 TypeScript) and ran 'aspire restore --apphost' + per-language compile against
   each, catching codegen regressions specific to individual integrations (Kafka,
   MongoDB, every Aspire.Hosting.Azure.*, etc.). The initial replacement only ran
   one Redis+SqlServer scenario per language and dropped that per-integration coverage.

   Add a RestoreAndCompileAllValidationFixtures [Fact] method to each of the four
   *CodegenValidationTests classes (Python/Go/Java/TypeScript). Each new test mounts
   tests/PolyglotAppHosts read-only into the test container via additionalVolumes,
   then runs a shared bash loop (PolyglotFixtureValidation.RunFixtureLoopAsync) that
   copies each fixture to a writable temp dir, runs 'aspire restore', and runs the
   per-language compile (py_compile / go build / javac @sources.txt / npm install +
   tsc --noEmit). The loop aggregates per-fixture pass/fail and exits non-zero if any
   fixture fails or if zero fixtures are found.

   Because SplitTestsOnCI=true splits at the class level, the new [Fact]s share a
   runner with the existing single-scenario tests rather than spinning up four new
   runners.

2. Redis pulls from Docker Hub (review finding #2, GPT-5.5, Medium):
   The deleted bash smoke scripts called .with_image_registry('netaspireci.azurecr.io')
   on the Redis resource to avoid Docker Hub anonymous-pull rate limits in CI; the
   initial C# rewrite omitted the override. Re-add it in
   PythonPolyglotTests/GoPolyglotTests/RustPolyglotTests so CI continues to pull
   Redis from the Aspire CI registry mirror.

3. Per-image gates were case-insensitive substring matches (review finding #3, Opus, Medium):
   GitHub Actions 'contains()' is case-insensitive, so
   'contains(testShortName, "Go")' matched every KubernetesDeployWithMongoDB* /
   KubernetesDeployWithPostgres* class, downloading the Go image (and load-ing it
   into docker) for every unrelated job. The Java gate had the same problem against
   JavaScriptPublishTests. Replace the four 'contains()' gates in run-tests.yml with
   explicit endsWith() enumerations of the polyglot test classes that actually need
   each image (PolyglotTests, CodegenValidationTests, plus JavaEmptyAppHostTemplateTests).
   The --require-* flags passed to load-cli-e2e-images.sh use the same gating.

4. New polyglot images skipped GHA buildx cache (review finding #4, Opus, Medium):
   build_extended_polyglot_image used plain 'DOCKER_BUILDKIT=1 docker build' with no
   --cache-from / --cache-to, so the multi-stage 'FROM golang:1' / 'FROM rust:1'
   stages re-pulled ~800MB from Docker Hub on every CI build. Route the helper
   through 'docker buildx build --load --cache-from type=gha,scope=...
   --cache-to type=gha,scope=...,mode=max,ignore-error=true' with per-image cache
   scopes (cli-e2e-polyglot-{java,python,go,rust}). The existing Java helper is
   migrated to the same pattern.

Review findings deferred to follow-up (preserve existing bash-script behavior; not
regressions introduced by this PR):

 - #6 'docker ps | grep redis' false-positive risk on developer machines.
 - #7 SIGKILL of aspire run bypasses graceful cleanup.
 - #8 [QuarantinedTest] URL on RustPolyglotTests points at the feature issue.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
radical added a commit that referenced this pull request May 12, 2026
@radical
Fix extended polyglot image FROM lookup under buildx container driver
886a664 
The 'cli-e2e-builder' buildx instance created in this workflow uses the
docker-container driver, which runs BuildKit inside a container with
its own image store separate from the host Docker daemon. When the
polyglot base is built with 'docker buildx build --load', the
resulting image is exported to the host Docker daemon's image store
but the tag is NOT registered in the buildkit container's internal
store.

So a naked 'FROM aspire-e2e-polyglot-base' in the extended polyglot
dockerfiles (Java/Python/Go/Rust) fails:

  #3 [internal] load metadata for docker.io/library/aspire-e2e-polyglot-base:latest
  #3 ERROR: pull access denied, repository does not exist or may
     require authorization: server message: insufficient_scope:
     authorization failed

This regression was introduced by commit 4846d7a, which switched
the extended polyglot builds from 'DOCKER_BUILDKIT=1 docker build'
(which uses the host daemon's BuildKit and sees locally-loaded
images) to 'docker buildx build --load' (which goes through the
container builder). The pre-regression code on origin/main still
uses 'docker build' for these images, which is why main's scheduled
Quarantined Tests run is green while every PR run on this branch
since 4846d7a has failed at this step.

Use --build-context with the 'docker-image://' scheme to tell buildx
to resolve 'aspire-e2e-polyglot-base' via the host Docker daemon
(which is where the base now lives). Drive it through an optional
EXTRA_BUILD_ARGS array so build_image stays a single generic helper
and only the extended-polyglot loop opts into the override.

Keeps the GHA layer cache for the heavy upstream 'golang:1' /
'rust:1' COPY stages that 4846d7a was trying to enable.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
radical added a commit that referenced this pull request May 19, 2026
@radical
fix(cli): close 5 findings from PR microsoft#17166 post-merge review
5e069d7 
A post-merge review of microsoft#17166 (saved under .squad/log) flagged five issues
in the prebuilt + DotNet-based AppHost restore paths that survived the
`origin/main` merge. This addresses them.

**#1 (HIGH) — Project-ref restore replaced ambient nuget.config for any
non-Local explicit channel.** `BuildIntegrationClosureManifestAsync`
called `TryCreateTemporaryNuGetConfigAsync` for every explicit channel,
which emitted `<RestoreConfigFile>` and replaced nuget.config discovery
wholesale. A user with a private/internal feed in their ambient
nuget.config and a `daily` or `pr-*` channel pin would silently lose
that feed during project-ref restore. Now only synthesize a temp
nuget.config when `--source` is set; otherwise add channel sources via
`<RestoreAdditionalProjectSources>` so the ambient nuget.config is
preserved.

**#2 (HIGH) — DotNetBasedAppHostServerProject accepted
`packageSourceOverride` but ignored it.** The in-repo / dogfood path
(selected whenever `AspireRepositoryDetector.DetectRepositoryRoot`
returns non-null) declared the parameter to satisfy
`IAppHostServerProject` but never threaded it into restore. The
template factory was unconditionally telling users
`--source was used for the initial scaffold restore only…` even when
the override had been silently dropped. Thread the override through
`CreateProjectFilesAsync` and prepend it to the
`<RestoreAdditionalProjectSources>` list so the hive is the first
source NuGet evaluates. This path does not use Package Source Mappings
(PSM) like `PrebuiltAppHostServer` does — in dev mode most Aspire.*
dependencies come from `ProjectReference` and the override is best-
effort for the rare `PackageReference` fallback. Documented inline.

**#3 (MED) — Restore-failure footer showed the original `--source`,
not the auto-discovered effective one.** When `--source` was not
passed but `ResolveLocalPackageSourceOverrideAsync` auto-discovered a
local hive, the catches in `PrepareAsync` passed the original
(unset) `packageSourceOverride` argument to
`AppendRestoreContextOnFailure`. The user saw only the channel name
and had no signal that a local hive participated in the failed
restore. Lift `effectivePackageSourceOverride` to outer scope and
pass it to the catches.

**#4 (MED) — `BundleNuGetService` logged raw `--source` to the debug
log.** The full restore args (including credentialed feed URLs) were
emitted as a single debug line that downstream `RedactSourceForDisplay`
never touched. Now build a redacted copy of the args specifically for
the log line — the verbatim args still go to the process. Handles
repeated `--source` flags and a missing trailing value defensively.

**#5 (MED) — `RedactSourceForDisplay` failed open on malformed
credentialed URLs.** `Uri.TryCreate` returns false for
`https://user:p@ss@host/path` and `https://user:p#word@host/`
(confirmed empirically), and the redactor's parse-failure branch
returned the raw input. Such inputs were guaranteed to leak
credentials into the failure footer that ships in bug reports. Fail
closed for HTTP-shaped inputs by detecting `http://` / `https://`
prefix before parsing and returning `<unparseable http source>` when
the parse fails. Plain non-HTTP inputs (local paths, file://, etc.)
still pass through unchanged.

Refactor: extract `RedactSourceForDisplay` into a shared
`PackageSourceRedactor` utility so the same redaction is applied
wherever sources appear in user-visible output. `PrebuiltAppHostServer`
keeps the internal static alias for back-compat with existing tests.

Tests added:
- `PrepareAsync_WithProjectReferencesAndExplicitChannelButNoOverride_UsesAdditionalSourcesNotRestoreConfigFile`
- `PrepareAsync_RestoreFailure_WithAutoDiscoveredLocalSource_FooterShowsEffectiveSource`
- `RedactSourceForDisplay_FailsClosedForMalformedHttpButPassesThroughLocalPaths` (5 inline cases)
- `CreateProjectFiles_WithPackageSourceOverride_PrependsOverrideToRestoreAdditionalProjectSources`
- `CreateProjectFiles_WithoutPackageSourceOverride_DoesNotInjectExtraSource`

All 3297 tests in Aspire.Cli.Tests pass (0 failures, 20 platform skips).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
radical added a commit that referenced this pull request May 19, 2026
@radical
fix(cli): keep guest AppHost restore on the selected Aspire package s…
bd4060b 
…ource (microsoft#17166)

* fix(cli): honor source for guest language package restore

aspire new aspire-empty --language typescript --source <pr-hive>
--version <pr-version> parsed --source, but the empty AppHost
TypeScript scaffolding path dropped it before the prebuilt AppHost
restored Aspire.Hosting and TypeScript code-generation packages. The
bundled restore then searched channel sources, missed the requested PR
hive packages, and NuGet floated to a stale preview package set, which
later failed with TypeLoadException when the generator loaded against
the PR CLI's Aspire.TypeSystem.

Flow TemplateInputs.Source into ScaffoldContext, pass it to
IAppHostServerProject.PrepareAsync, and have PrebuiltAppHostServer add
that source to package and closure restores. When a source override is
present, use exact version ranges so restore fails rather than silently
resolving a different Aspire prerelease.

Fixes microsoft#17159

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* chore(cli): remove source restore diff noise

Remove whitespace-only changes that are unrelated to the source restore fix, keeping the PR focused on the explicit package source propagation.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(cli): constrain source override restore behavior

When an explicit package source is passed to guest AppHost restore, keep the exact-version pinning scoped to Aspire packages because that is the source mapping being overridden. Non-Aspire integration packages should retain normal NuGet minimum-version restore semantics.

Also apply the temporary NuGet.config to the project-reference closure restore path instead of only adding sources to the synthetic project. That keeps package source mapping and channel-specific restore settings active when package and project integrations are restored together.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(cli): keep --source override exclusive for Aspire packages

When `aspire new aspire-empty --source <pr-hive>` ran without an
explicit `--channel`, the temp NuGet.config built for restore folded in
every explicit channel's `Aspire* -> channelSource` mapping alongside
the override's `Aspire* -> packageSourceOverride`. NuGet treats
same-pattern mappings on multiple sources as co-eligible, so Aspire
packages could still resolve from a channel feed and silently defeat
the override's fail-fast intent. Exact-version pinning masked this in
practice because the requested PR-hive version was usually unique, but
the package source mapping itself was no longer exclusive to the
override.

In the override branch of `TryCreateTemporaryNuGetConfigAsync`, only
fold in mappings from an explicitly-requested, matched channel (skip
the catch-all "all explicit channels" fallback baked into
`GetExplicitRestoreChannelsAsync`), and drop any `Aspire*`-prefixed
mappings from that matched channel before merging. Non-Aspire patterns
(`CommunityToolkit*`, catch-all `*`) are preserved so non-Aspire
transitives keep their channel feeds. Mirror the same gating in
`GetNuGetSourcesAsync` so the bundled NuGet service's `sources` list
doesn't broadcast every channel feed when `--source` is the override
mechanism.

Add seven `TryCreateTemporaryNuGetConfig_*` test cases covering the
override-with-channels matrix (no channel, matched channel, channel
with `Aspire*` mapping, channel with all-packages mapping, lookup
failure, requested-channel threading) plus a `PrepareAsync_*`
integration check for the NuGet.org fallback. `TestPackagingService`
gains a `LastRequestedChannelName` observable so the new
`PassesRequestedChannelToPackagingService` test can assert the
override branch threads `requestedChannel` into the packaging service.

Touch the comments near `NuGetOrgSource` and the
`RestoreConfigFile`/`RestoreAdditionalProjectSources` split so they
describe the actual constraint ("cannot float to NuGet.org or any
other co-eligible feed").

Refs microsoft#17159

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(cli): warn that aspire-empty --source is one-shot at scaffold

Running `aspire new aspire-empty --language <non-csharp> --source <X>`
succeeds at scaffold time but the override is consumed only for the
initial restore inside `PrebuiltAppHostServer`. The scaffolded project
persists only the channel and SDK version, so a follow-up `aspire add`
or `aspire restore` in the same project resolves Aspire packages from
the channel feeds in `aspire.config.json` rather than `<X>` — and
silently produces a different package set, or fails when the channel
does not carry the requested version.

Emit a yellow warning immediately after the scaffold succeeds (when
`inputs.Source` is non-empty on the non-C# branch) so users supplying
`--source <pr-hive>/packages` are not surprised when subsequent
commands miss the override. Persisting the feed into a generated
`nuget.config` (and also honoring `--source` on the C# empty path,
which silently drops it today) is left as a follow-up.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(cli): include --source/channel context in scaffold restore failures

When the prebuilt AppHost scaffold restore fails, the displayed output
is the only debugging surface most users see. Previously it carried
only the raw NuGet stderr ("Failed to prepare: Package restore failed:
..."), with no record of which `--source`, channel, or package
versions had been in play. Reproducing the failure required a verbose
re-run with diagnostic logging just to recover the inputs.

Append the override source, the requested channel, and a short
preview of the package list to the `OutputCollector` from both of
`PrepareAsync`'s catch blocks (`AppHostServerPrepareFailedException`
and the catch-all wrapper around `RestoreNuGetPackagesAsync`). When
neither `--source` nor a channel was specified the helper is a no-op,
so existing failure messages without these inputs are unchanged.

Add an end-to-end `PrepareAsync` test that wires `--source` together
with a channel whose `Aspire*` mapping conflicts with the override
and asserts the temp `nuget.config` actually passed to the restore
invocation drops the channel's `Aspire*` mapping, pinning that the
override is authoritative for `Aspire*` packages end-to-end (and not
only at the temp-config generator unit boundary).

Add a `PrepareAsync_RestoreFailure_OutputIncludesSourceAndChannelContext`
test that fails the restore via a non-zero exit and asserts the
override path, channel name, and package id are present in the
returned output.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(cli): honor --source override for guest-language starter templates

`aspire new aspire-{ts,py,go}-starter --source <pr-hive> --version <pr>` hit
the same TypeLoadException class of failure as `aspire-empty` did before the
fix landed in this branch: the override was plumbed into
PrebuiltAppHostServer.PrepareAsync for the empty-template path only, while
starter templates went through GuestAppHostProject.BuildAndGenerateSdkAsync
→ PrepareAppHostServerAsync without forwarding the override, so Aspire
packages restored from channel feeds rather than the requested source.

Thread `packageSourceOverride` through IGuestAppHostSdkGenerator.BuildAndGenerateSdkAsync
and the GuestAppHostProject prepare helper, then pass `inputs.Source` from
all three guest starter templates. Hoist the "override is not persisted"
warning into a shared helper on CliTemplateFactory so the empty and starter
paths emit the same message; the warning fires only after a successful
scaffold so it doesn't add noise behind a more prominent restore failure.

Tests:
- Expand the empty-template warning test to a [Theory] covering TypeScript
  and Java (the latter behind the experimental polyglot flag).
- Add starter-template coverage for both the warning+plumb-through happy
  path and the failed-restore-suppresses-warning path.
- Pin the restore-failure context footer shape (`--source:`, `channel:`,
  `packages:` labels) and the >5-package truncation behavior.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* chore(cli): rename EmptySourceOverrideNotPersistedWarning resource

The shared `DisplaySourceOverrideNotPersistedWarningIfNeeded` helper on
`CliTemplateFactory` is invoked by both `aspire-empty` and the three
guest-language starter templates (TypeScript, Python, Go), so the
`Empty*` prefix on the resource key is stale. Drop the prefix while
the string is still pre-release and re-translation has not yet been
triggered for translators.

Renames the resource in `.resx`, `.Designer.cs`, the single production
call site in `CliTemplateFactory.cs`, and four references in
`NewCommandTests.cs` (empty and starter happy-path + suppression
cases). `dotnet build /t:UpdateXlf src/Aspire.Cli/Aspire.Cli.csproj`
regenerates the 13 `*.xlf` files to pick up the new `trans-unit id`.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(cli): align --source argument list with temp NuGet.config in PrebuiltAppHostServer

`TryCreateTemporaryNuGetConfigAsync` already drops the matched
channel's `Aspire*` mapping in the override branch, pinning Aspire
package restoration to `--source` exclusively. But `GetNuGetSourcesAsync`
— which builds the `--source` CLI argument list passed alongside the
temp config — was still iterating every mapping in the matched
channel and adding each mapping URL, including the channel's Aspire
feed. The bundled NuGet tool treats `--source` CLI args as co-eligible
with config mappings (which is why the original "don't fold in every
explicit channel" comment exists in this method), so re-adding the
channel's Aspire feed silently undoes the temp config's PSM drop and
lets Aspire packages still resolve from the channel feed.

A second, smaller divergence: when the matched channel had no
`*` (AllPackages) mapping, the temp config added `* -> NuGet.org` as
a catch-all but the sources list's `sources.Count == 1` heuristic
only added NuGet.org in the no-channel case, leaving a mismatched
catch-all whenever a matched channel contributed any non-Aspire
mapping (e.g. `CommunityToolkit*`, `Microsoft.*`).

In the matched-channel loop, skip mappings whose `PackageFilter`
starts with "Aspire" when an override is set, and observe whether
the matched channel supplied its own AllPackages mapping. After the
loop, fall back to NuGet.org only when no AllPackages mapping was
seen — the same rule the temp config uses for its catch-all.

Tests:

- `GetNuGetSources_WithPackageSourceOverrideAndMatchedChannel_OmitsChannelAspireFeedFromSources`
  pins that the channel's Aspire feed URL does NOT appear in the
  `--source` argument list, even though the channel maps `Aspire*`
  to it. This is the inverse assertion of the existing
  `TryCreateTemporaryNuGetConfig_WithPackageSourceOverride_DropsRequestedChannelAspireMappings`
  test on the config side.
- `..._KeepsChannelSourceAndAddsNuGetOrgFallback` covers the
  `CommunityToolkit*` case: non-Aspire channel mapping stays, and
  NuGet.org is added because the matched channel has no AllPackages
  mapping.
- `..._OmitsNuGetOrgFallback` covers a channel that already supplies
  `* -> channelSource`: NuGet.org should NOT be added, because the
  channel's own AllPackages mapping is the catch-all in both the
  temp config and the sources list.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(cli): redact credentials from --source in restore-failure output

The new restore-failure context block introduced earlier in this branch
echoes `--source: <user-supplied URL>` into the OutputCollector that
ScaffoldingService displays on a failed scaffold. NuGet feed URLs
routinely carry credentials — `https://name:pat@host/...` for token-auth
feeds or SAS-style `?sv=...&sig=...` query tokens for blob-storage
feeds — and that block is exactly the text users copy verbatim into
GitHub issues, Teams chats, and CI failure transcripts.

Add a `RedactSourceForDisplay` helper that strips UserInfo, Query, and
Fragment from http/https URIs before display, and route the override
through it from `AppendRestoreContextOnFailure`. Plain URLs without
credentials/query are detected via early-return and pass through
unchanged; local paths and `file://`-style sources bypass the URI
branch and are emitted as-is. The redaction is only for the display
copy — the actual restore invocation still receives the original
source string.

Cover the helper with a `[Theory]` exercising the no-redaction path
(plain URL, Unix path, Windows path), the userinfo-only case, the
query-only case, the combined userinfo+query case, and the fragment
case.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(cli): auto-discover local Aspire source from requested channel

When --source isn't supplied, PrebuiltAppHostServer now resolves the
requested channel and, if it has a hive-backed Aspire* mapping pointing
at an existing local directory, uses that as the package source
override for both package-only and project-reference restore.

That closes the dogfood gap where `aspire new aspire-empty --language
typescript` from a PR/local CLI would resolve Aspire packages through
the ambient channel feed instead of the CLI's own hive, surfacing as
TypeLoadException during code generation.

Channel-lookup failures are swallowed-and-logged (mirroring the
existing defensive catches in TryCreateTemporaryNuGetConfigAsync and
GetNuGetSourcesAsync); OperationCanceledException is re-thrown.

Tests cover the explicit-channel-only path, the explicit-source-wins
path, and that http-backed channels keep their existing non-exact
restore behavior.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(cli): close 5 findings from PR microsoft#17166 post-merge review

A post-merge review of microsoft#17166 (saved under .squad/log) flagged five issues
in the prebuilt + DotNet-based AppHost restore paths that survived the
`origin/main` merge. This addresses them.

**#1 (HIGH) — Project-ref restore replaced ambient nuget.config for any
non-Local explicit channel.** `BuildIntegrationClosureManifestAsync`
called `TryCreateTemporaryNuGetConfigAsync` for every explicit channel,
which emitted `<RestoreConfigFile>` and replaced nuget.config discovery
wholesale. A user with a private/internal feed in their ambient
nuget.config and a `daily` or `pr-*` channel pin would silently lose
that feed during project-ref restore. Now only synthesize a temp
nuget.config when `--source` is set; otherwise add channel sources via
`<RestoreAdditionalProjectSources>` so the ambient nuget.config is
preserved.

**#2 (HIGH) — DotNetBasedAppHostServerProject accepted
`packageSourceOverride` but ignored it.** The in-repo / dogfood path
(selected whenever `AspireRepositoryDetector.DetectRepositoryRoot`
returns non-null) declared the parameter to satisfy
`IAppHostServerProject` but never threaded it into restore. The
template factory was unconditionally telling users
`--source was used for the initial scaffold restore only…` even when
the override had been silently dropped. Thread the override through
`CreateProjectFilesAsync` and prepend it to the
`<RestoreAdditionalProjectSources>` list so the hive is the first
source NuGet evaluates. This path does not use Package Source Mappings
(PSM) like `PrebuiltAppHostServer` does — in dev mode most Aspire.*
dependencies come from `ProjectReference` and the override is best-
effort for the rare `PackageReference` fallback. Documented inline.

**#3 (MED) — Restore-failure footer showed the original `--source`,
not the auto-discovered effective one.** When `--source` was not
passed but `ResolveLocalPackageSourceOverrideAsync` auto-discovered a
local hive, the catches in `PrepareAsync` passed the original
(unset) `packageSourceOverride` argument to
`AppendRestoreContextOnFailure`. The user saw only the channel name
and had no signal that a local hive participated in the failed
restore. Lift `effectivePackageSourceOverride` to outer scope and
pass it to the catches.

**#4 (MED) — `BundleNuGetService` logged raw `--source` to the debug
log.** The full restore args (including credentialed feed URLs) were
emitted as a single debug line that downstream `RedactSourceForDisplay`
never touched. Now build a redacted copy of the args specifically for
the log line — the verbatim args still go to the process. Handles
repeated `--source` flags and a missing trailing value defensively.

**#5 (MED) — `RedactSourceForDisplay` failed open on malformed
credentialed URLs.** `Uri.TryCreate` returns false for
`https://user:p@ss@host/path` and `https://user:p#word@host/`
(confirmed empirically), and the redactor's parse-failure branch
returned the raw input. Such inputs were guaranteed to leak
credentials into the failure footer that ships in bug reports. Fail
closed for HTTP-shaped inputs by detecting `http://` / `https://`
prefix before parsing and returning `<unparseable http source>` when
the parse fails. Plain non-HTTP inputs (local paths, file://, etc.)
still pass through unchanged.

Refactor: extract `RedactSourceForDisplay` into a shared
`PackageSourceRedactor` utility so the same redaction is applied
wherever sources appear in user-visible output. `PrebuiltAppHostServer`
keeps the internal static alias for back-compat with existing tests.

Tests added:
- `PrepareAsync_WithProjectReferencesAndExplicitChannelButNoOverride_UsesAdditionalSourcesNotRestoreConfigFile`
- `PrepareAsync_RestoreFailure_WithAutoDiscoveredLocalSource_FooterShowsEffectiveSource`
- `RedactSourceForDisplay_FailsClosedForMalformedHttpButPassesThroughLocalPaths` (5 inline cases)
- `CreateProjectFiles_WithPackageSourceOverride_PrependsOverrideToRestoreAdditionalProjectSources`
- `CreateProjectFiles_WithoutPackageSourceOverride_DoesNotInjectExtraSource`

All 3297 tests in Aspire.Cli.Tests pass (0 failures, 20 platform skips).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(cli): degrade restore on channel-lookup failure + cover gaps from microsoft#17227 merge

PR microsoft#17227's defensive catch around the channel-lookup helper had two
call sites; only the auto-discovery one survived the merge into this
branch. The PSM-temp-config no-override path still propagates a
transient `IPackagingService.GetChannelsAsync` failure out to
`PrepareAsync`'s outer catch, turning a transient packaging-service
hiccup (malformed `aspire.config.json`, unexpected feed probe error)
into a hard `aspire new` scaffold failure.

Mirror the existing defensive catch into the no-override branch of
`TryCreateTemporaryNuGetConfigAsync`: cancellation rethrows, anything
else logs and returns null so restore falls through to the ambient
nuget.config + caller-resolved channel sources path, matching the
catch in `ResolveLocalPackageSourceOverrideAsync` and the long-standing
catch in `GetNuGetSourcesAsync`.

Restore the dropped degrade test (`PrepareAsync_WhenPackagingService-
ThrowsDuringAutoDiscovery_DegradesGracefully`) so a future refactor
can't silently regress this back. Also add two negative-path tests
for `aspire-empty --language <guest>` source-coherence:

- `PrepareAsync_WithHiveBackedChannelPointingAtMissingLocalDirectory_-
  DoesNotApplyOverride` pins that a stale `aspire.config.json` (user
  deleted the local hive but the channel pin remains) does not pin
  Aspire packages to a non-existent directory or emit exact-pin /
  NuGet.org fallback.

- Extend `NewCommandWithEmptyTemplateAndSourceOverrideWarnsThatOverride-
  IsNotPersisted` to also cover python, go, and rust, matching the
  five guest languages registered in `DefaultLanguageDiscovery`.

Refs microsoft#17159

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(cli): address source-restore review feedback

Keep cancellation tokens last on the guest AppHost prepare/source APIs now
that both requested channel and source override are threaded through the
same calls.

Move the staging-unavailable guard before temporary NuGet.config creation
so the source-override project-reference restore path cannot silently
fall back to NuGet.org when staging cannot be synthesized. Also update
the project-reference restore comment to describe both explicit --source
and auto-discovered local channel sources.

Add direct PackageSourceRedactor coverage for happy paths, malformed
HTTP inputs, whitespace-prefixed HTTP sources, and non-HTTP source forms.
Trim HTTP inputs before detection/parsing so indented feed URLs are still
redacted or fail closed.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(cli): persist aspire new source overrides

An explicit `aspire new --source <source>` previously only affected the
initial scaffold restore. The generated project did not record that source,
so later `aspire add` or `aspire restore` could fall back to channel or
ambient NuGet configuration and lose the Aspire package source selected at
creation time.

Persist source overrides into the generated project's NuGet.config by
mapping `Aspire*` to the explicit source and keeping non-Aspire fallback
sources from the resolved channel, or NuGet.org when no channel fallback is
available. The persisted config remains self-contained: it does not import
parent, user, or global NuGet sources, mappings, disabled sources, or
credentials; only an existing project-local NuGet.config is merged.

Remove the stale warning that source overrides are not persisted, share the
source-override mapping logic with the prebuilt restore path, and add tests
covering empty templates, starter templates, .NET templates, existing config
merge behavior, and ambient-config non-absorption.

Refs microsoft#17159
Refs microsoft#17225

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(cli): reject credentialed new sources before persistence

Persisting `aspire new --source` into a project NuGet.config makes the
source durable project state. Credential-bearing HTTP URLs should not be
written there because the generated file can be committed accidentally.

Reject HTTP(S) sources that contain user info, query strings, or fragments
before project creation starts, and keep the lower-level mapping helper from
persisting those sources if it is called directly. The error points users at
NuGet credential providers or user-level NuGet configuration instead of
embedding secrets in the feed URL.

Refs microsoft#17159
Refs microsoft#17225

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(cli): update PR-hive NuGet config snapshots

The NuGet config merger no longer maps wildcard package resolution to the
PR hive when a separate fallback source already owns `*`. Update the PR-hive
snapshots so CI expects Aspire packages only from the hive and keeps the
fallback mapping on the appropriate source.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
radical pushed a commit that referenced this pull request May 30, 2026
@ellahathaway @adamint
Show discovered AppHosts in Aspire pane (microsoft#17506)
768842c 
* Show idle AppHosts in Aspire pane with Run/Debug context menu

* Rename view ID from runningAppHosts to appHosts

* Rename context key from noRunningAppHosts to noAppHosts

* Keep panel visible when stopped AppHost has workspace candidates

When an AppHost stops, the noAppHosts context key now considers
workspace candidates. This ensures the panel shows idle AppHosts
instead of the empty welcome view after a running AppHost is stopped.

* Fix noAppHosts assertions: workspace candidates keep panel visible

The _updateWorkspaceContext change (0e312f9) added !hasWorkspaceCandidates
to the noAppHosts condition, meaning the panel stays visible when idle
AppHosts are discovered. Two tests asserted noAppHosts=true after describe
exit, but the legacy format candidate is treated as buildable (toAppHostCandidate
defaults null status to 'buildable'), so workspace candidates persist and
noAppHosts is correctly false.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Make workspace AppHosts expandable with launch actions

* Address review feedback for PR microsoft#17506

Fix #1: Rename command IDs in package.json menus and walkthrough so
they target the new aspire-vscode.runAppHostCommand and
aspire-vscode.debugAppHostCommand registrations introduced in this PR.
Without this the editor title bar, explorer context menu, and Get
Started walkthrough Run/Debug buttons silently no-op.

Fix #2: Wrap vscode.debug.startDebugging in try/catch in
AppHostLaunchService.launch so a 'false' return value (debug adapter
rejected) or thrown error clears the launching state. Otherwise the
tree item is stuck showing the 'Starting...' spinner forever and the
user cannot retry.

Fix #3: Make AspireAppHostTreeProvider.runAppHost async and await
launch so launch failures surface via showErrorMessage instead of
being dropped as unhandled promise rejections.

Fix #4: In workspace mode with multiple candidate AppHost paths, match
running AppHosts to candidates by directory equivalence
(isMatchingAppHostPath) rather than exact path. This is the same
matching used elsewhere in AppHostDataRepository when correlating
'aspire ps' output to candidate paths, so canonicalization
differences (case, separators, trailing slashes) no longer cause a
running AppHost to display as idle.

Fix #5: Introduce aspire.noRunningAppHosts context key so the Open
Dashboard palette command is only enabled when at least one AppHost
is actually running. Previously the palette appeared when only idle
candidates were known and then silently no-oped.

Fix #6: Widen the workspaceResources contextValue regex in
package.json so the read-only 'Open AppHost Source' and 'Copy AppHost
Path' actions appear on bare 'workspaceResources' items, not only on
'workspaceResources:hasAppHost'. The destructive 'Stop AppHost' menu
remains gated on :hasAppHost.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Adam Ratzman <adam@adamratzman.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@radical radical Awaiting requested review from radical radical is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@radical