Make polyglot integration restores consistent and cache-safe - #19763
Draft
David Negstad (danegsta) wants to merge 4 commits into
Draft
Make polyglot integration restores consistent and cache-safe#19763David Negstad (danegsta) wants to merge 4 commits into
David Negstad (danegsta) wants to merge 4 commits into
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2dd71101-249d-428b-81e7-9b247b9b3a2a
David Negstad (danegsta)
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
Unifies polyglot integration restore behavior across generated AppHost projects, including source resolution, closure generation, cache layout, and runtime configuration.
Changes:
- Adds shared C# project, restore-source, closure, and environment helpers.
- Updates prebuilt and repository-mode AppHost generation.
- Expands restore fingerprinting and project-reference tests.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
tests/Aspire.Cli.Tests/Projects/PrebuiltAppHostServerTests.cs |
Tests restore configuration, closure outputs, and cache normalization. |
src/Aspire.Cli/Utils/CliPathHelper.cs |
Adds path and staging-cache helpers. |
src/Aspire.Cli/Projects/PrebuiltAppHostServer.cs |
Adopts shared restore and closure infrastructure. |
src/Aspire.Cli/Projects/IntegrationRestoreSourceResolver.cs |
Centralizes channel and source resolution. |
src/Aspire.Cli/Projects/IntegrationClosureEnvironment.cs |
Centralizes runtime integration environment variables. |
src/Aspire.Cli/Projects/IntegrationClosureBuilder.cs |
Centralizes closure project generation and manifest reading. |
src/Aspire.Cli/Projects/DotNetBasedAppHostServerProject.cs |
Uses the shared C# project model. |
src/Aspire.Cli/Projects/CSharpProjectFile.cs |
Introduces a generated C# project model. |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: d38799a7-a99c-48c5-b45b-9d6594dd1830
Contributor
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 19763Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 19763" |
This comment has been minimized.
This comment has been minimized.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Suppressed comments (1)
src/Aspire.Cli/Projects/PrebuiltAppHostServer.cs:750
- This only checks
PackageSourceMappings, but the no-channel path returns all explicit channel URLs inAdditionalSourceswhile leavingPackageSourceMappingsnull. If one of those channels contains a SAS/user-info URL, it is written into the long-livedIntegrationRestore.csprojasRestoreAdditionalProjectSources, and restore-cache skipping remains enabled. Include all effective sources in the credential check and keep the generated project/config ephemeral (or reject the ambiguous no-channel case) so credentials are never persisted.
var hasCredentialBearingRestoreSource = restoreSources.PackageSourceMappings?.Any(
static mapping => PackageSourceOverrideMappings.HasCredentialMaterial(mapping.Source)) == true;
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: d38799a7-a99c-48c5-b45b-9d6594dd1830
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: d38799a7-a99c-48c5-b45b-9d6594dd1830
Contributor
Tests selector (audit mode)The full test matrix and all jobs still run in audit mode. The tests and jobs below are what selective CI would run under enforcement. 2 / 102 test projects · 4 jobs, from 12 changed files. Selected test projects (2 / 102)
Selected jobs (4)
How these were chosen — grouped by what changed📦 affected project 🧪 🧪 🧪 Job reasons
Selection computed for commit |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
src/Aspire.Cli/Projects/DotNetBasedAppHostServerProject.cs:164
- This set now stores project paths, but
OrdinalIgnoreCasecollapses distinct paths such as/src/Foo/Foo.csprojand/src/foo/foo.csprojon case-sensitive Linux/macOS filesystems, silently omitting one validProjectReference. Use ordinal path identity here; the restore graph uses the same rule specifically to avoid dropping distinct paths (PrebuiltAppHostServer.cs:378-380).
var addedProjects = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
Comment on lines
+153
to
+165
| var identityAttribute = | ||
| string.Equals(itemName, "author", StringComparison.OrdinalIgnoreCase) || | ||
| string.Equals(itemName, "repository", StringComparison.OrdinalIgnoreCase) | ||
| ? "name" | ||
| : string.Equals(itemName, "certificate", StringComparison.OrdinalIgnoreCase) | ||
| ? "fingerprint" | ||
| : string.Equals(itemName, "fileCert", StringComparison.OrdinalIgnoreCase) || | ||
| string.Equals(itemName, "storeCert", StringComparison.OrdinalIgnoreCase) | ||
| ? "packageSource" | ||
| : null; | ||
|
|
||
| return identityAttribute is null || | ||
| string.Equals(GetAttributeValue(first, identityAttribute), GetAttributeValue(second, identityAttribute), StringComparison.OrdinalIgnoreCase); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Polyglot AppHosts can load .NET integrations from both NuGet packages and project references. Those paths need to honor the same package channels, source mappings, central package management isolation, output layout, and closure-manifest contract; otherwise a generated restore can select the wrong feed, inherit repository package versions, or produce an incomplete runtime closure.
Generated integration projects now share one C# project model, restore-source resolver, closure builder, cache layout, and runtime environment setup. This makes the existing prebuilt AppHost path consistent across package and project-reference integrations, including explicit channels, source overrides, staging-feed cache isolation, and project-reference assembly discovery.
Fixes # (issue)
Checklist
<remarks />and<code />elements on your triple slash comments?