Skip to content

Fix combined_version to prefer higher version when merging duplicate dependencies#15507

Open
thavaahariharangit with Copilot wants to merge 11 commits into
mainfrom
copilot/dependabot-alert-fix
Open

Fix combined_version to prefer higher version when merging duplicate dependencies#15507
thavaahariharangit with Copilot wants to merge 11 commits into
mainfrom
copilot/dependabot-alert-fix

Conversation

Copilot AI commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

What are you trying to accomplish?

When the same dependency appears in multiple files (e.g., requirements.txt pins urllib3==2.7.0 and constraints.txt pins urllib3==2.5.0), DependencySet#combined_version always kept the lower version. This caused the dependency graph snapshot to report the vulnerable version even after the user updated their manifest, preventing alerts from transitioning to "fixed".

The fix swaps the version preference so the higher version wins when both entries have the same top-level status:

# Before: always kept lower
elsif version_class.new(new_dep.version) > version_class.new(old_dep.version)
  old_dep.version
else
  new_dep.version
end

# After: keeps higher
elsif version_class.new(new_dep.version) > version_class.new(old_dep.version)
  new_dep.version
else
  old_dep.version
end

Anything you want to highlight for special attention from reviewers?

  • The higher version is semantically correct: it represents what pip actually resolves/installs when both a requirement and a constraint exist for the same package.
  • The existing precedence rules (prefer non-nil over nil, prefer top-level over transitive, prefer valid semver over git SHA) are unchanged. Only the final tiebreaker between two valid versions with the same top-level status is affected.
  • This is shared common/ infrastructure, so it impacts multiple ecosystems. In this PR we updated affected expectations in Python, Maven, Gradle, GitHub Actions, and npm/yarn lockfile/parser/resolver specs.
  • Blast radius note: npm/yarn file updater behavior changed in certain workspace scenarios because lockfile selection depends on merged dependency versions; specs were updated to assert the current behavior.
  • Follow-up parity fix included: Bun now mirrors npm/yarn’s :all_versions flattening and subdependency lockfile filtering logic so duplicate-version lockfiles are handled consistently.

How will you know you've accomplished your goal?

  • common/spec/dependabot/file_parsers/base/dependency_set_spec.rb passes with the higher-version behavior.
  • Python constraint scenario reports the higher effective version in combined dependency snapshots.
  • Updated ecosystem specs pass for the changed semantics (Python, Maven, Gradle, GitHub Actions, npm_and_yarn, and metadata finder).

Checklist

  • I have run relevant tests for the changed files and updated expectations.
  • I have written clear and descriptive commit messages.
  • I have updated this PR description to reflect the actual blast radius of the shared change.

Review follow-ups

  • Clarified PR blast radius: this shared DependencySet change required expectation updates across Python, Maven, Gradle, GitHub Actions, and npm/yarn specs.
  • Clarified that npm/yarn workspace file updater behavior changed in specific scenarios due lockfile selection depending on merged dependency versions.
  • Added Bun parity fixes to mirror npm/yarn behavior:
    • flatten nested :all_versions metadata in bun/helpers.rb
    • evaluate all candidate versions when filtering subdependency lockfiles in bun/sub_dependency_files_filterer.rb
  • Updated a stale npm spec description so it matches the asserted behavior.

Copilot AI self-assigned this Jul 6, 2026
Copilot AI review requested due to automatic review settings July 6, 2026 12:53
Copilot AI removed the request for review from Copilot July 6, 2026 12:53
…dependencies

When the same dependency appears in multiple files (e.g., requirements.txt and
constraints.txt) with different pinned versions, DependencySet.combined_version
now picks the higher version instead of the lower one.

This fixes a bug where the dependency graph job would report a vulnerable
version from a constraint file even though the main requirements file had
been updated to a non-vulnerable version, preventing Dependabot alerts from
transitioning to "fixed".

Closes github/dependabot-updates#13931

Co-authored-by: thavaahariharangit <164553783+thavaahariharangit@users.noreply.github.com>
Copilot AI requested review from Copilot and removed request for Copilot July 6, 2026 13:08
With the combined_version fix preferring higher versions, update the test
expectation from "2.0.0" (constraint version) to "2.4.1" (requirements.txt
version) when both files pin the same dependency at different versions.

Co-authored-by: thavaahariharangit <164553783+thavaahariharangit@users.noreply.github.com>
Copilot AI requested review from Copilot and removed request for Copilot July 6, 2026 13:10
Copilot AI changed the title [WIP] Fix Dependabot alert not transitioning to fixed status Fix combined_version to prefer higher version when merging duplicate dependencies Jul 6, 2026
Copilot AI requested a review from thavaahariharangit July 6, 2026 13:11
Copilot AI review requested due to automatic review settings July 7, 2026 09:47
@thavaahariharangit thavaahariharangit marked this pull request as ready for review July 7, 2026 09:47
@thavaahariharangit thavaahariharangit requested a review from a team as a code owner July 7, 2026 09:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This pull request updates Dependabot’s shared DependencySet merging behavior so that when duplicate dependencies with the same top-level status are merged, the higher valid version is retained in the combined dependency’s version field. This primarily affects snapshot/reporting scenarios (e.g., dependency graph “fixed” transitions) rather than per-file update behavior.

Changes:

  • Adjust DependencySet#combined_version to prefer the higher version when both versions are valid and have equal precedence.
  • Update the Python file parser spec to expect the combined dependency version to reflect the higher pinned requirement when both requirement and constraint entries exist.
  • Clarify the Python helper test description/comment around parsing constraint files.
Show a summary per file
File Description
common/lib/dependabot/file_parsers/base/dependency_set.rb Changes the final tiebreaker in combined_version so the higher version wins when merging duplicates.
python/spec/dependabot/python/file_parser_spec.rb Updates expectations for the combined dependency version when both requirements and constraints specify versions.
python/helpers/test/test_parse_requirements.py Updates test docstring/comment wording around constraint-file parsing behavior.

Review details

  • Files reviewed: 3/3 changed files
  • Comments generated: 1
  • Review effort level: Low

Comment thread common/lib/dependabot/file_parsers/base/dependency_set.rb

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review details

  • Files reviewed: 14/14 changed files
  • Comments generated: 3
  • Review effort level: Medium

Comment thread npm_and_yarn/spec/dependabot/npm_and_yarn/file_updater_spec.rb
Comment thread npm_and_yarn/spec/dependabot/npm_and_yarn/file_updater_spec.rb
Copilot AI review requested due to automatic review settings July 7, 2026 13:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review details

  • Files reviewed: 17/17 changed files
  • Comments generated: 1
  • Review effort level: Medium

Comment thread npm_and_yarn/spec/dependabot/npm_and_yarn/file_updater_spec.rb Outdated
Copilot AI review requested due to automatic review settings July 7, 2026 13:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review details

  • Files reviewed: 18/18 changed files
  • Comments generated: 0 new
  • Review effort level: Low

Copilot AI requested review from Copilot and removed request for Copilot July 7, 2026 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

L: github:actions GitHub Actions L: java:gradle Maven packages via Gradle L: java:maven Maven packages via Maven L: javascript L: python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants