Skip to content

only try to create pr if update operations were performed#14463

Merged
brettfo merged 1 commit into
mainfrom
dev/brettfo/nuget-empty-dependencies
Mar 16, 2026
Merged

only try to create pr if update operations were performed#14463
brettfo merged 1 commit into
mainfrom
dev/brettfo/nuget-empty-dependencies

Conversation

@brettfo
Copy link
Copy Markdown
Contributor

@brettfo brettfo commented Mar 16, 2026

The NuGet create_security_pr handler can get into a weird state if analysis updates a file on disk but no updates were possible.

This is admittedly an edge case, but can happen if a project/props/targets injects targets that MSBuild evaluates that edits a tracked file but then no security update was possible. The result is a non-empty set of files with edits but an empty set of update operations which then causes errors when attempting to generate a PR. A PR is obviously incorrect here so the fix is to not only check the edited file count before creating a PR, but also to check the update operation count.

@github-actions github-actions Bot added the L: dotnet:nuget NuGet packages via nuget or dotnet label Mar 16, 2026
@brettfo brettfo marked this pull request as ready for review March 16, 2026 20:58
@brettfo brettfo requested a review from a team as a code owner March 16, 2026 20:58
Copilot AI review requested due to automatic review settings March 16, 2026 20:58
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR prevents the NuGet create_security_pr update handler from attempting to create a pull request when files were modified on disk but no update operations were actually performed (an edge case that can occur during analysis/MSBuild evaluation).

Changes:

  • Gate PR creation on both (a) modified dependency files and (b) at least one update operation being performed.
  • Add a regression test ensuring “errant” on-disk file edits with zero update operations do not trigger PR creation.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/UpdateHandlers/CreateSecurityUpdatePullRequestHandler.cs Adds an additional guard (updateOperationsPerformed.Count > 0) before creating a PR to avoid invalid PR generation when only file edits occurred.
nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Run/UpdateHandlers/CreateSecurityUpdatePullRequestHandlerTests.cs Adds a test that simulates a file being touched during analysis while CanUpdate=false, asserting no PR creation occurs.

@brettfo brettfo force-pushed the dev/brettfo/nuget-empty-dependencies branch from 35ded2d to a66766b Compare March 16, 2026 21:38
@brettfo brettfo merged commit 003dad9 into main Mar 16, 2026
100 checks passed
@brettfo brettfo deleted the dev/brettfo/nuget-empty-dependencies branch March 16, 2026 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

L: dotnet:nuget NuGet packages via nuget or dotnet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants