Skip to content

[ci] Generate binlog for 'build workloads' step - #1

Closed
radical wants to merge 1 commit into
mainfrom
workloads-binlog
Closed

[ci] Generate binlog for 'build workloads' step#1
radical wants to merge 1 commit into
mainfrom
workloads-binlog

Conversation

@radical

@radical radical commented Jan 22, 2024

Copy link
Copy Markdown
Owner

No description provided.

@radical radical closed this Jan 22, 2024
@github-staff github-staff deleted a comment May 27, 2024
@github-staff github-staff deleted a comment May 27, 2024
radical added a commit that referenced this pull request Apr 24, 2026
@radical
Address PR review feedback
313be3f 
- Precompile regex patterns during config load instead of on every match;
  invalid patterns log warnings and disable the rule (comments #2-3)
- Add note to doc examples clarifying they are snippets, not standalone
  configs (comment #1)
- Dispose JsonDocument with 'using' in 3 test methods (comments #4-6)
- Guard artifact extraction against zip-slip and symlink attacks by
  skipping symlinks and verifying resolved paths stay within trxDir
  (comment #7)
- Cap test_pattern_matched_tests output to 50 entries and drop unused
  testProject field to avoid GH Actions size limits (comment #8)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
radical added a commit that referenced this pull request Apr 27, 2026
@radical
feat(ci): add test failure retry support to auto-rerun workflow (micr…
59ee662 
…osoft#16446)

* feat(ci): add test failure retry patterns with config validation and pattern matching

Add eng/test-retry-patterns.json with initial transient failure patterns
(ECONNRESET, DNS failures, SSL errors, timeouts, Windows 0xC0000142).

Add pattern matching functions to auto-rerun-transient-ci-failures.js:
- loadRetryPatternsConfig: reads and validates JSON config
- validateRetryPatternsConfig: schema validation + regex compilation
- extractFailedTestsFromTrx: regex-based TRX XML parsing
- matchesRetryPattern: string/regex matching with case-insensitive support
- matchTestFailurePatterns: AND-within/OR-across rule matching
- matchJobLogPattern: job name + log text pattern matching

Add 30 tests in Infrastructure.Tests covering:
- Config JSON structure and schema validation (C#)
- Regex compilation validation via Node.js harness (V8 engine)
- Pattern matching: substring, regex, AND/OR logic, disabled rules
- TRX parsing: failed test extraction, output cap, XML entity decoding
- Validation edge cases: unknown props, wrong version, missing reason

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

* feat(ci): add test failure retry support to auto-rerun workflow

Extend the auto-rerun-transient-ci-failures workflow to detect transient
test failures in addition to infrastructure failures. Two new matching
paths:

1. Job log pattern matching: analyzeFailedJobs now accepts an optional
   retryPatternsConfig and runs a 3rd classification pass using
   matchJobLogPattern for test-execution-failure jobs.

2. TRX-based pattern matching: After job classification, the YAML
   workflow downloads the All-TestResults artifact, extracts .trx files,
   and matches failed test output against testFailurePatterns from
   eng/test-retry-patterns.json. When matches are found, all skipped
   test-execution-failure jobs are promoted to retryable.

New exported JS functions:
- hasTestExecutionFailureStep: checks if a job has test execution steps
- analyzeTrxFiles: parses TRX contents, matches against patterns, dedupes
- promoteTestExecutionFailureJobs: pure function to move jobs to retryable
- selectTestResultsArtifact: picks newest non-expired artifact under cap

Safety rails:
- Existing maxRetryableJobs cap (default 5) applies to promoted jobs
- 3-attempt budget shared with infrastructure retries
- Artifact download failures are non-fatal
- 100MB artifact size cap, 200 TRX file limit, 50MB per-file limit

Updated summary and PR comment formatting to distinguish infrastructure
retries from test-pattern retries.

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

* docs(ci): rewrite auto-rerun CI docs with practical how-to guide

Restructure the documentation from a behavior contract into a
user-facing guide that explains:

- How the rerun system works at a glance (flow diagram)
- The four analysis passes and what each does
- When it triggers (automatic vs manual)
- How to add/modify test failure retry patterns in
  eng/test-retry-patterns.json with worked examples
- Rule field reference tables for both pattern types
- Matching semantics (AND/OR, substring vs regex, dedup)
- Tips for writing good patterns
- How to verify with dry run
- Safety rails summarized in a table
- Architecture and file layout
- How to run the tests

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

* feat(ci): add MCR rate limiting patterns to test-retry-patterns.json

Add patterns to detect transient MCR (mcr.microsoft.com) rate limiting
failures that cause both test and infrastructure CI failures:

testFailurePatterns:
- MCR 403 Forbidden (regex scoped to mcr.microsoft.com)
- MCR 'The request is blocked' HTML response (regex scoped)
- CONTAINER1016 (.NET SDK container publish failure)
- 'pull access denied for mcr.microsoft.com' (Docker pull denial)

jobFailurePatterns:
- MCR 403 Forbidden (regex scoped to mcr.microsoft.com)
- MCR 'The request is blocked' HTML response (regex scoped)

Regex patterns use [\s\S]{0,500} to require mcr.microsoft.com within
500 chars of the error text, preventing false matches on non-MCR 403s.

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

* fix: decodeXmlEntities double-decoding and sanitize markdown in PR comments

- Fix decodeXmlEntities replacement ordering: move &amp; decode to last
  position to prevent double-decoding of &amp;quot; and &amp;apos;
- Add sanitizeMarkdown helper to escape backticks/pipes in test names
  rendered in PR comment markdown (defense-in-depth)
- Add test covering double-encoded XML entity decoding

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

* Address PR review feedback

- Precompile regex patterns during config load instead of on every match;
  invalid patterns log warnings and disable the rule (comments #2-3)
- Add note to doc examples clarifying they are snippets, not standalone
  configs (comment #1)
- Dispose JsonDocument with 'using' in 3 test methods (comments #4-6)
- Guard artifact extraction against zip-slip and symlink attacks by
  skipping symlinks and verifying resolved paths stay within trxDir
  (comment #7)
- Cap test_pattern_matched_tests output to 50 entries and drop unused
  testProject field to avoid GH Actions size limits (comment #8)

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

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@radical radical mentioned this pull request May 1, 2026
Closed
radical added a commit that referenced this pull request May 5, 2026
@radical
Add v3 PR grouping plan
177a591 
Top-level plan that carves agreed-design-v3.md into 4 mandatory PRs plus
1 optional follow-up. Lists which design changes (#1-#16) each PR carries
and shows their dependencies. Per-PR design details (acceptance criteria,
test list, file inventory) to be fleshed out in follow-up sections.

Co-authored-by: Copilot <223556219+Copilot@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 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 26, 2026
@mitchdenny
Use xUnit test method name for CLI E2E recording filenames (microsoft…
38d3b34 
…#17484)

Resolves bug #1 of the two root causes that left CLI E2E recordings
tagged as Unknown in the PR recording-comment.

CliE2ETestHelpers.CreateTestTerminal / CreateDockerTestTerminal /
CreatePodmanDockerTestTerminal (and the shared
Hex1bTestHelpers.CreateTestTerminal) use [CallerMemberName] to pick the
.cast filename. When a public [Fact] is a thin wrapper that delegates
into a private helper — e.g. DashboardRunWithAgentMcpListTracesReturnsNoTraces
=> DashboardRunWithAgentMcpCore in DashboardRunTests, or the *Core
helper in AgentMcpLogsTests — [CallerMemberName] captures the helper.
The .cast file ends up named after the helper, the TRX has no entry
for that name, and the recording-comment workflow's lookup falls
through to Unknown on every PR.

Fix: prefer TestContext.Current?.TestCase?.TestMethodName when running
inside a live xUnit test context, fall back to the [CallerMemberName]
default otherwise. The public API surface is unchanged (no caller
passes testName explicitly), so the recording filenames quietly flip
from 'DashboardRunWithAgentMcpCore' to the public test name with no
test-side edits required.

The companion workflow-side fix for the second root cause (jq splitting
on '.' inside theory parameters before stripping the param suffix) ships
in a follow-up commit on the same PR.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
radical added a commit that referenced this pull request May 27, 2026
@radical
fix(homebrew): post-review cleanup — align SHA URL, drop dead --artif…
82d4b94 
…act-version, default prepare to LiveArchives

Three related cleanups falling out of PR review:

1. generate-cask.sh's BASE_URL still pointed at ci.dot.net even
   though the cask template now embeds a github.com release URL.
   The two were only consistent because every CI caller passes
   --archive-root (which short-circuits the URL fetch). Any local
   invocation without --archive-root would have written a cask
   whose SHA was computed from ci.dot.net bytes while the cask URL
   pointed at github.com.

2. --artifact-version is now dead. The template no longer references
   ${ARTIFACT_VERSION} (the URL is parameterized solely on
   #{version}), and after fix #1 above BASE_URL no longer uses it
   either. Drop the parameter from generate-cask.sh,
   prepare-cask-artifact.sh, prepare-homebrew-cask.yml, and the two
   AzDO pipeline callers. The aspireArtifactVersion pipeline variable
   stays because WinGet still uses it.

3. Flip prepare-cask-artifact.sh's default from LiveRelease to
   LiveArchives. Every prod CI caller already passed
   skipUrlValidation=true → LiveArchives. Prepare time means "the
   cask URL points at a v#{version} release that hasn't been
   published yet" by construction, so LiveArchives is the only
   correct semantics. Drop the now-redundant skipUrlValidation
   parameter from prepare-homebrew-cask.yml and the explicit
   --validation-mode LiveArchives from the GH Actions caller.
   LiveRelease is preserved as an opt-in for local dev and for the
   Bash_PrepareHomebrewCask_FailedVerification_UninstallsCask test
   (which depends on brew install/uninstall behavior); the test now
   requests LiveRelease explicitly. LiveRelease validation in CI
   still runs unchanged via HomebrewValidateJob, which calls
   validate-cask-artifact.sh directly.

Also:

 * Replace fragile cross-file line-number references with the
   stable `audit_args` block name.
 * Add a `Homebrew Val:` row to the RELEASE SUMMARY box in
   release-publish-nuget.yml so a release manager can see at a
   glance whether SkipHomebrewValidation is set.

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

No reviews

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