Revert NuGet-client migration (#629, #647) to unblock ADO main builds - #654
Merged
Alexandre Zollinger Chohfi (azchohfi) merged 3 commits intoJul 16, 2026
Merged
Conversation
Copilot started reviewing on behalf of
Alexandre Zollinger Chohfi (azchohfi)
July 16, 2026 18:32
View session
Zach Teutsch (zateutsch)
approved these changes
Jul 16, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Reverts the NuGet-client migration and related coverage to remove prerelease NuGet.* dependencies that block ADO builds.
Changes:
- Restores the hand-rolled NuGet service.
- Removes NuGet-client packages, feeds, configuration, and tests.
- Reverts related documentation and update/cache behavior.
Reviewed changes
Copilot reviewed 35 out of 35 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
src/winapp-CLI/WinApp.Cli/WinApp.Cli.csproj |
Removes NuGet-client dependencies and AOT switch. |
src/winapp-CLI/WinApp.Cli/Services/WorkspaceSetupService.cs |
Removes NuGet configuration rooting. |
src/winapp-CLI/WinApp.Cli/Services/PackageInstallationService.cs |
Restores legacy cache/dependency handling. |
src/winapp-CLI/WinApp.Cli/Services/NugetSourceProvider.cs |
Deletes configured-source support. |
src/winapp-CLI/WinApp.Cli/Services/NugetService.Dependencies.cs |
Deletes client-based dependency resolver. |
src/winapp-CLI/WinApp.Cli/Services/NugetPackageDownloader.cs |
Deletes client-based downloader. |
src/winapp-CLI/WinApp.Cli/Services/INugetService.cs |
Restores legacy interface contract. |
src/winapp-CLI/WinApp.Cli/Services/BuildToolsService.cs |
Removes pinned-version normalization. |
src/winapp-CLI/WinApp.Cli/Helpers/HostBuilderExtensions.cs |
Removes deleted service registrations. |
src/winapp-CLI/WinApp.Cli/Commands/UpdateCommand.cs |
Restores legacy update behavior. |
src/winapp-CLI/WinApp.Cli.Tests/WorkspaceSetupServiceConfigRootTests.cs |
Deletes configuration-root tests. |
src/winapp-CLI/WinApp.Cli.Tests/WinApp.Cli.Tests.csproj |
Removes NuGet AOT test switch. |
src/winapp-CLI/WinApp.Cli.Tests/UpdateCommandTests.cs |
Deletes update-command coverage. |
src/winapp-CLI/WinApp.Cli.Tests/PackageInstallationServiceTests.cs |
Deletes installation-service coverage. |
src/winapp-CLI/WinApp.Cli.Tests/NugetServiceVersionRangeTests.cs |
Deletes range-resolution tests. |
src/winapp-CLI/WinApp.Cli.Tests/NugetServiceTests.cs |
Restores legacy NuSpec tests. |
src/winapp-CLI/WinApp.Cli.Tests/NugetServiceInstallGraphTests.cs |
Deletes graph-integrity tests. |
src/winapp-CLI/WinApp.Cli.Tests/NugetServiceFeedTests.cs |
Deletes custom-feed tests. |
src/winapp-CLI/WinApp.Cli.Tests/NugetServiceDependencyTests.cs |
Deletes dependency-resolution tests. |
src/winapp-CLI/WinApp.Cli.Tests/NugetServiceCoverageTests.cs |
Deletes migration coverage. |
src/winapp-CLI/WinApp.Cli.Tests/NugetPackageDownloaderCoverageTests.cs |
Deletes downloader coverage. |
src/winapp-CLI/WinApp.Cli.Tests/NugetFeedTestHelpers.cs |
Deletes local-feed helpers. |
src/winapp-CLI/WinApp.Cli.Tests/FakeNugetService.cs |
Restores the legacy fake. |
src/winapp-CLI/WinApp.Cli.Tests/BuildToolsServiceTests.cs |
Removes normalization regression test. |
src/winapp-CLI/WinApp.Cli.Tests/BaseCommandTests.cs |
Removes cancellation-token overload. |
src/winapp-CLI/Directory.Packages.props |
Removes prerelease NuGet pins. |
nuget.config |
Removes secondary feed and source mapping. |
docs/usage.md |
Removes private-feed documentation. |
docs/fragments/skills/winapp-cli/setup.md |
Removes private-feed guidance. |
docs/dotnet-run-support.md |
Restores AOT blocker text. |
.pipelines/release-nuget.config |
Removes upstream-feed explanation. |
.github/plugin/skills/winapp-cli/setup/SKILL.md |
Synchronizes generated setup guidance. |
.claude/skills/winapp-setup/SKILL.md |
Synchronizes Claude setup guidance. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+93
to
+98
| protected async Task<int> ParseAndInvokeWithCaptureAsync(Command command, string[] manifestArgs) | ||
| => await ParseAndInvokeWithCaptureAsync(command, manifestArgs, TestContext.CancellationToken); | ||
|
|
||
| /// <summary> | ||
| /// Overload that invokes with a caller-supplied cancellation token (e.g. a pre-cancelled one) so | ||
| /// tests can exercise a command's <see cref="OperationCanceledException"/> handling deterministically. | ||
| /// </summary> | ||
| protected async Task<int> ParseAndInvokeWithCaptureAsync(Command command, string[] manifestArgs, CancellationToken cancellationToken) | ||
| { | ||
| var parseResult = command.Parse(manifestArgs); | ||
| parseResult.InvocationConfiguration.Output = TestAnsiConsole.Profile.Out.Writer; | ||
| parseResult.InvocationConfiguration.Error = ConsoleStdErr; | ||
| return await parseResult.InvokeAsync(parseResult.InvocationConfiguration, cancellationToken: cancellationToken); | ||
| return await parseResult.InvokeAsync(parseResult.InvocationConfiguration, cancellationToken: TestContext.CancellationToken); |
| /// Returns the directory for a specific package version in the NuGet global packages cache. | ||
| /// Uses the standard NuGet layout: {cache}/{lowercase-id}/{version}/ | ||
| /// </summary> | ||
| DirectoryInfo GetNuGetPackageDir(string packageName, string version); |
Comment on lines
+51
to
+53
| // Check if already installed in NuGet global cache | ||
| var packageDir = nugetService.GetNuGetPackageDir(packageName, version); | ||
| if (packageDir.Exists) |
| // version (e.g. "1.0" vs "1.0.0") spuriously counting as an update, | ||
| // and a lower "latest" ever silently downgrading the pinned version. | ||
| if (NugetService.CompareVersions(latestVersion, package.Version) > 0) | ||
| if (latestVersion != package.Version) |
Comment on lines
78
to
82
| catch (Exception ex) | ||
| { | ||
| taskContext.AddStatusMessage($"{UiSymbols.Warning} Failed to check {package.Name}: {ex.Message}"); | ||
| // Keep current version on error, but remember the failure so the | ||
| // command exits non-zero and does not claim everything is up to date. | ||
| // Keep current version on error | ||
| updatedConfig.SetVersion(package.Name, package.Version); |
Comment on lines
+177
to
+181
| The CLI currently has NativeAOT compilation errors related to Newtonsoft.Json and NuGet.Protocol. These must be resolved before the NuGet package can include the CLI binaries. | ||
|
|
||
| **Error summary:** | ||
| - 146 trim/AOT analysis errors | ||
| - Related to reflection-heavy code in Newtonsoft.Json |
Comment on lines
+113
to
+116
| // Check if already installed in NuGet global cache | ||
| var packageDir = nugetService.GetNuGetPackageDir(packageName, version); | ||
| if (packageDir.Exists) | ||
| { |
Comment on lines
+112
to
114
| // Already installed on disk? | ||
| if (packageDir.Exists) | ||
| { |
Contributor
Build Metrics ReportBinary Sizes
Test Results✅ 3224 passed, 4 skipped out of 3228 tests in 566.1s (-114 tests, -171.5s vs. baseline) Test Coverage✅ 85.2% line coverage, 79.1% branch coverage · CLI Startup Time42ms median (x64, Updated 2026-07-16 19:03:18 UTC · commit |
…629)" This reverts the NuGet-client migration (#629) to unblock ADO main builds, which fail on the prerelease NuGet.* 7.9.0-rc packages sourced from the dnceng dotnet-tools feed (not nuget.org). See #653. Conflict resolution for coverage PRs that landed on top of #629: - FakeNugetService.cs restored to its pre-#629 form. - BaseCommandTests.cs: reverted #629 changes but kept the generic ParseAndInvokeWithCaptureAsync(..., CancellationToken) overload, which is unrelated test infra used by UI-command cancellation tests (#645). - Deleted PackageInstallationServiceTests.cs and UpdateCommandTests.cs (new in #629; later expanded by #640/#641 for the migrated code). - Deleted PackageInstallationServiceCacheMarkerTests.cs (added by #641; exercised the migrated NugetService via now-removed NugetFeedTestHelpers and INugetService.IsPackageInstalled). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: d55408ba-4e7d-41aa-95a0-f7c801632eaf
Alexandre Zollinger Chohfi (azchohfi)
force-pushed
the
azchohfi-revert-nuget-client-migration
branch
from
July 16, 2026 18:42
db896e4 to
1ca10dc
Compare
Alexandre Zollinger Chohfi (azchohfi)
deleted the
azchohfi-revert-nuget-client-migration
branch
July 16, 2026 20:39
This was referenced Jul 16, 2026
Alexandre Zollinger Chohfi (azchohfi)
added a commit
that referenced
this pull request
Jul 17, 2026
… (#671) Raise NuGet/update/install per-file coverage to >=95% (#630) Bucket 2 of the QA-gate follow-up wave — re-covers the pre-migration NuGet client that #654 restored. Coverage: UpdateCommand.cs 100%, PackageInstallationService.cs 100%, NugetService.cs 99.4% (only the unreachable InstallPackageRecursiveAsync present-key guard remains, documented via <remarks> #630). Product (behavior-preserving test seams only): - GetUserProfileDirectory seam so the %USERPROFILE% fallback is hermetically testable (eliminates a documented ceiling). - HttpGetAsync seam (default delegate = HttpClient.GetAsync) so restore/ download/version-resolution run offline against canned responses. - Normalize transitive dependency version ranges via ParseMinimumVersion before recursing, and skip the fetch when the minimum is empty (open lower-bound ranges like "(,2.0.0]"), mirroring the on-disk ResolveDependenciesAsync guard. Prevents a malformed flat-container URL that silently dropped transitive deps. No-op for exact versions. Tests: NugetServiceOfflineTests (restore/download/transitive/diamond-dedup/ version-filtering/nuspec-parsing/HTTP-error/open-lower-bound guard), PackageInstallationServiceTests, UpdateCommandTests. Debug + Release (both projects, warnings-as-errors) 0W/0E. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
4 tasks
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.
Reverts the NuGet-client migration (#629) and its stacked coverage PR (#647).
Tracks #653.
Why
After migrating
NugetServiceto the official NuGet client libraries,mainbuilds fail on ADO due to a security issue: #629 pins theNuGet.*packages to the prerelease7.9.0-rc.36120sourced from the dnceng dotnet-tools public feed (not nuget.org). The security gate flags consuming a prerelease from a secondary public feed. This was always intended as temporary — theNuGet.UseSystemTextJsonDeserializationswitch needed to keep the Native AOT publish clean isn't in a nuget.org stable yet.What this reverts
NugetPackageDownloader.cstest seams.NugetService, and removes theNuGet.*RC pins, thedotnet-toolssource +packageSourceMappingfromnuget.config, the.pipelines/release-nuget.configupstream, and theDirectory.Packages.propspins.Conflict resolution notes
Several coverage PRs merged after #629 built on top of its surface, so a plain revert left conflicts. Resolved as follows:
FakeNugetService.cs— restored to its pre-Migrate NugetService to the official NuGet client libraries #629 form.BaseCommandTests.cs— reverted Migrate NugetService to the official NuGet client libraries #629's NuGet-specific changes but kept the genericParseAndInvokeWithCaptureAsync(..., CancellationToken)overload, which is unrelated test infrastructure used by UI-command cancellation tests from test(ui): raise UI-command logic coverage to >=95% per file #645.PackageInstallationServiceTests.csandUpdateCommandTests.cs(both new in Migrate NugetService to the official NuGet client libraries #629; later expanded by Tests: non-UI CLI commands coverage to =>95% #640/test(msix): raise packaging service coverage to >=95% + fix AppX build-artifact exclusion bug #641 for the migrated code, which no longer exists).PackageInstallationServiceCacheMarkerTests.cs(added by test(msix): raise packaging service coverage to >=95% + fix AppX build-artifact exclusion bug #641; exercised the migratedNugetServiceagainst a local folder feed viaNugetFeedTestHelpers+INugetService.IsPackageInstalled, all of which are removed here).Validation
dotnet build winapp.sln -c Debug— 0 warnings / 0 errorsdotnet-tools,7.9.0, orpackageSourceMappinginnuget.config/Directory.Packages.props/.pipelines/release-nuget.config.Re-landing
This migration should be reapplied only once stable
NuGet.Protocol 7.9.0and all relatedNuGet.*packages are published on nuget.org (not on dnceng dotnet-tools or any other secondary public feed). At that point: repoint the pins to the nuget.org stable, and drop thedotnet-toolssource,packageSourceMapping, and the release-nuget.config upstream. See #653.