dependabot / dependabot-core

🤖 Dependabot's core logic for creating update PRs.
https://docs.github.com/en/code-security/dependabot
MIT License
4.69k stars 1.01k forks source link

Dependabot Groups include Unexpected Dependencies when Updating Existing PR #10487

Open kahagerman opened 2 months ago

kahagerman commented 2 months ago

Is there an existing issue for this?

Package ecosystem

pnpm (applies to all)

Package manager version

pnpm=8.9.2 (likely applies to all)

Language version

node=20

Manifest location and content before the Dependabot update

N/A

dependabot.yml content

(Edited down from full monorepo config:)

---
version: 2
updates:
  - directory: /web
    commit-message:
      prefix: Web
    package-ecosystem: npm
    open-pull-requests-limit: 3
    schedule:
      interval: weekly
      day: monday
      time: '05:00'
      timezone: America/Toronto
    versioning-strategy: increase-if-necessary
    allow:
      - dependency-type: all
    groups:
      major:
        update-types: [major]
      minor:
        patterns:
          - '*' # everything else - minor, patch, etc.

Updated dependency

N/A

What you expected to see, versus what you actually saw

When manually updating a PR via dependabot (i.e. by running @dependabot recreate, or similar commands), dependencies which should not be included in the PR are added to it.

From the documentation for groups:

Dependabot creates groups in the order they appear in your dependabot.yml file. If a dependency update could belong to more than one group, it is only assigned to the first group it matches with.

This implies that the following config snippet:

    groups:
      major:
        update-types: [major]
      minor:
        patterns:
          - '*' # everything else - minor, patch, & indirect dependencies

Should not include "major" semver updates in the minor group.

However, this filtering is not applied when a dependabot run is based on an existing pull request:

Before:

Screenshot 2024-08-22 at 4 00 28 PM

After running @dependabot recreate:

Screenshot 2024-08-22 at 3 57 44 PM

(Note: I have no idea why this shows "with 19 updates", but this is unrelated.)

Native package manager behavior

N/A

Images of the diff or a link to the PR, issue, or logs

Full dependabot run logs

Here are some relevant specifics:

Smallest manifest that reproduces the issue

---
version: 2
updates:
  - directory: /
    package-ecosystem: npm
    groups:
      major:
        update-types: [major]
      minor:
        patterns:
          - '*' # everything else - minor, patch, etc.

Debugging Help / Recommendation:

When deciding which updates may be included, this job should not only check that the dependency is applicable to the current PR's group, but also verify that it is not applicable to any previous group.

If a given dependency is applicable to a previous group, that group will pick up the dependency when it is re-evaluated.

This would make the behaviour consistent between an "inital" run (when no PRs exist), and an "update" run (when one or more PRs already exist).

ilyakonrad commented 2 weeks ago

I think I have a similar issue. We have two repos with identical dependabot setup. The setup creates two groups: one with all non-major Angular-related updates, and another one with all of the rest non-major dependency updates.

version: 2
updates:
- package-ecosystem: npm
  directory: "/"
  schedule:
    interval: weekly
    time: '03:00'
  open-pull-requests-limit: 10
  groups:
    angular:
      patterns:
      - "@angular*"
      update-types:
      - "minor"
      - "patch"
    other:
      update-types:
      - "minor"
      - "patch"
  ignore:
    - dependency-name: "@angular*"
      update-types: ["version-update:semver-major"]

It worked well until recently: image

I have a suspicion that it started when I forgot to merge the updates once or twice in one of the repos. Because I never forget to merge it in the other one and we don't have any issues there. Again, the setup is identical.