fix handled dependencies for multidir

silent/tests/testdata/vu-group-multidir-all.txt

@@ -1,7 +1,15 @@
+# Testing a grouped multi-dir configuration.
+
 dependabot update -f input.yml --local . --updater-image ghcr.io/dependabot/dependabot-updater-silent
 stderr 'created \| dependency-a \( from 1.2.3 to 1.2.5 \), dependency-b \( from 2.2.3 to 2.2.5 \)'
 pr-created foo/expected.json bar/expected.json
 
+# Testing a grouped multi-dir configuration with pre-existing PR for the group.
+# Should result in no PRs.
+
+dependabot update -f input-preexisting.yml --local . --updater-image ghcr.io/dependabot/dependabot-updater-silent
+! stdout create_pull_request
+
 -- foo/manifest.json --
 {
   "dependency-a": { "version": "1.2.3" },
@@ -76,7 +84,33 @@ job:
     api-endpoint: https://example.com/api/v3
     repo: dependabot/smoke-tests
   dependency-groups:
-    - name: first
+    - name: all
+      rules:
+        patterns:
+          - "*"
+
+-- input-preexisting.yml --
+job:
+  package-manager: "silent"
+  source:
+    directories:
+      - "/foo"
+      - "/bar"
+    provider: example
+    hostname: example.com
+    api-endpoint: https://example.com/api/v3
+    repo: dependabot/smoke-tests
+  dependency-groups:
+    - name: all
       rules:
         patterns:
           - "*"
+  existing-group-pull-requests:
+    - dependency-group-name: all
+      dependencies:
+        - dependency-name: dependency-a
+          version: 1.2.5
+        - dependency-name: dependency-b
+          version: 2.2.5
+        - dependency-name: dependency-c
+          version: 3.2.5

silent/tests/testdata/vu-group-multidir-semver.txt

@@ -0,0 +1,71 @@
+# Testing a grouped multi-dir configuration using semver rules.
+
+dependabot update -f input.yml --local . --updater-image ghcr.io/dependabot/dependabot-updater-silent
+stdout -count=2 create_pull_request
+pr-created foo/expected-1.json bar/expected-1.json
+pr-created foo/expected-2.json bar/expected-2.json
+
+# When there is the same dependency in both directories, one is a major update the other a patch,
+# and the user asked for majors in one group and minors in the other, we should create two group PRs
+# for the same dependency, one in each directory.
+
+-- foo/manifest.json --
+{
+  "dependency-a": { "version": "1.0.0" }
+}
+
+-- bar/manifest.json --
+{
+  "dependency-a": { "version": "2.0.0" }
+}
+
+-- foo/expected-1.json --
+{
+  "dependency-a": { "version": "2.0.1" }
+}
+
+-- bar/expected-1.json --
+{
+  "dependency-b": { "version": "2.0.0" }
+}
+
+-- foo/expected-2.json --
+{
+  "dependency-a": { "version": "1.0.0" }
+}
+
+-- bar/expected-2.json --
+{
+  "dependency-b": { "version": "2.0.1" }
+}
+
+-- dependency-a --
+{
+  "versions": [
+    "1.0.0",
+    "2.0.0",
+    "2.0.1"
+  ]
+}
+
+-- input.yml --
+job:
+  package-manager: "silent"
+  source:
+    directories:
+      - "/foo"
+      - "/bar"
+    provider: example
+    hostname: example.com
+    api-endpoint: https://example.com/api/v3
+    repo: dependabot/smoke-tests
+  dependency-groups:
+    - name: major
+      rules:
+        update-types:
+          - major
+    - name: minor
+      rules:
+        update-types:
+          - minor
+          - patch

updater/lib/dependabot/dependency_snapshot.rb

@@ -104,7 +104,16 @@ def job_group
     end
 
     sig { params(dependency_names: T.any(String, T::Array[String])).void }
-    def add_handled_dependencies(dependency_names)
+    def add_handled_dependencies_all_directories(dependency_names)
+      directories.each do |dir|
+        set = @handled_dependencies[dir] || Set.new
+        set += Array(dependency_names)
+        @handled_dependencies[dir] = set
+      end
+    end
+
+    sig { params(dependency_names: T.any(String, T::Array[String])).void }
+    def add_handled_dependencies_current_directory(dependency_names)
       raise "Current directory not set" if @current_directory == ""
 
       set = @handled_dependencies[@current_directory] || Set.new

updater/lib/dependabot/updater/group_update_creation.rb

@@ -148,7 +148,7 @@ def compile_updates_for(dependency, dependency_files, group) # rubocop:disable M
 
         # Consider the dependency handled so no individual PR is raised since it is in this group.
         # Even if update is not possible, etc.
-        dependency_snapshot.add_handled_dependencies(dependency.name)
+        dependency_snapshot.add_handled_dependencies_current_directory(dependency.name)
 
         if checker.up_to_date?
           log_up_to_date(dependency)
@@ -169,7 +169,7 @@ def compile_updates_for(dependency, dependency_files, group) # rubocop:disable M
           requirements_to_unlock: requirements_to_unlock
         )
       rescue Dependabot::InconsistentRegistryResponse => e
-        dependency_snapshot.add_handled_dependencies(dependency)
+        dependency_snapshot.add_handled_dependencies_current_directory(dependency)
         error_handler.log_dependency_error(
           dependency: dependency,
           error: e,
@@ -180,7 +180,7 @@ def compile_updates_for(dependency, dependency_files, group) # rubocop:disable M
       rescue StandardError => e
         # If there was an error we might not be able to determine if the dependency is in this
         # group due to semver grouping, so we consider it handled to avoid raising an individual PR.
-        dependency_snapshot.add_handled_dependencies(dependency.name)
+        dependency_snapshot.add_handled_dependencies_current_directory(dependency.name)
         error_handler.handle_dependency_error(error: e, dependency: dependency, dependency_group: group)
         [] # return an empty set
       end
@@ -344,7 +344,7 @@ def deduce_updated_dependency(dependency, original_dependency)
         Dependabot.logger.info(
           "Skipping #{dependency.name} as it has already been updated to #{dependency.version}"
         )
-        dependency_snapshot.handled_dependencies << dependency.name
+        dependency_snapshot.add_handled_dependencies_current_directory(dependency.name)
 
         Dependabot::Dependency.new(
           name: dependency.name,

updater/lib/dependabot/updater/operations/group_update_all_versions.rb

@@ -93,12 +93,12 @@ def run_grouped_dependency_updates # rubocop:disable Metrics/AbcSize
                 "Deferring creation of a new pull request. The existing pull request will update in a separate job."
               )
               # add the dependencies in the group so individual updates don't try to update them
-              dependency_snapshot.add_handled_dependencies(
+              dependency_snapshot.add_handled_dependencies_all_directories(
                 dependencies_in_existing_pr_for_group(group).map { |d| d["dependency-name"] }
               )
               # also add dependencies that might be in the group, as a rebase would add them;
               # this avoids individual PR creation that immediately is superseded by a group PR supersede
-              dependency_snapshot.add_handled_dependencies(group.dependencies.map(&:name))
+              dependency_snapshot.add_handled_dependencies_all_directories(group.dependencies.map(&:name))
               next
             end
 
@@ -109,10 +109,10 @@ def run_grouped_dependency_updates # rubocop:disable Metrics/AbcSize
             result = run_update_for(group)
             if result
               # Add the actual updated dependencies to the handled list so they don't get updated individually.
-              dependency_snapshot.add_handled_dependencies(result.updated_dependencies.map(&:name))
+              dependency_snapshot.add_handled_dependencies_all_directories(result.updated_dependencies.map(&:name))
             else
               # The update failed, add the suspected dependencies to the handled list so they don't update individually.
-              dependency_snapshot.add_handled_dependencies(group.dependencies.map(&:name))
+              dependency_snapshot.add_handled_dependencies_all_directories(group.dependencies.map(&:name))
             end
           end
         end

updater/lib/dependabot/updater/operations/refresh_group_update_pull_request.rb

@@ -93,7 +93,7 @@ def perform # rubocop:disable Metrics/AbcSize
             dependency_snapshot.groups.each do |group|
               next unless group.name != dependency_snapshot.job_group.name && pr_exists_for_dependency_group?(group)
 
-              dependency_snapshot.add_handled_dependencies(
+              dependency_snapshot.add_handled_dependencies_all_directories(
                 dependencies_in_existing_pr_for_group(group).map { |d| d["dependency-name"] }
               )
             end

updater/spec/dependabot/dependency_snapshot_spec.rb

@@ -15,25 +15,14 @@
 RSpec.describe Dependabot::DependencySnapshot do
   include DependencyFileHelpers
 
-  let(:source) do
-    Dependabot::Source.new(
-      provider: "github",
-      repo: "dependabot-fixtures/dependabot-test-ruby-package",
-      directory: "/",
-      branch: nil,
-      api_endpoint: "https://api.github.com/",
-      hostname: "github.com"
-    )
-  end
-
   let(:directory) { "/" }
   let(:directories) { nil }
 
   let(:source) do
     Dependabot::Source.new(
       provider: "github",
       repo: "dependabot-fixtures/dependabot-test-ruby-package",
-      directory: "/",
+      directory: directory,
       directories: directories
     )
   end
@@ -58,12 +47,12 @@
       Dependabot::DependencyFile.new(
         name: "Gemfile",
         content: fixture("bundler/original/Gemfile"),
-        directory: "/"
+        directory: directory
       ),
       Dependabot::DependencyFile.new(
         name: "Gemfile.lock",
         content: fixture("bundler/original/Gemfile.lock"),
-        directory: "/"
+        directory: directory
       )
     ]
   end
@@ -84,7 +73,7 @@
     "mock-sha"
   end
 
-  describe "::add_handled_dependencies" do
+  describe "::add_handled_dependencies_current_directory" do
     subject(:create_dependency_snapshot) do
       described_class.create_from_job_definition(
         job: job,
@@ -99,18 +88,39 @@
       }
     end
 
+    it "handles dependencies" do
+      snapshot = create_dependency_snapshot
+      snapshot.add_handled_dependencies_current_directory(%w(a b))
+      expect(snapshot.handled_dependencies).to eq(Set.new(%w(a b)))
+    end
+
     context "when there are multiple directories" do
+      let(:directory) { nil }
       let(:directories) { %w(/foo /bar) }
+      let(:dependency_files) do
+        [
+          Dependabot::DependencyFile.new(
+            name: "Gemfile",
+            content: fixture("bundler/original/Gemfile"),
+            directory: "/foo"
+          ),
+          Dependabot::DependencyFile.new(
+            name: "Gemfile",
+            content: fixture("bundler/original/Gemfile"),
+            directory: "/bar"
+          )
+        ]
+      end
 
       it "handles dependencies per directory" do
         snapshot = create_dependency_snapshot
         snapshot.current_directory = "/foo"
-        snapshot.add_handled_dependencies(%w(a b))
+        snapshot.add_handled_dependencies_current_directory(%w(a b))
         expect(snapshot.handled_dependencies).to eq(Set.new(%w(a b)))
 
         snapshot.current_directory = "/bar"
         expect(snapshot.handled_dependencies).to eq(Set.new)
-        snapshot.add_handled_dependencies(%w(c d))
+        snapshot.add_handled_dependencies_current_directory(%w(c d))
         expect(snapshot.handled_dependencies).to eq(Set.new(%w(c d)))
 
         snapshot.current_directory = "/foo"
@@ -119,6 +129,57 @@
     end
   end
 
+  describe "::add_handled_dependencies_all_directories" do
+    subject(:create_dependency_snapshot) do
+      described_class.create_from_job_definition(
+        job: job,
+        job_definition: job_definition
+      )
+    end
+
+    let(:job_definition) do
+      {
+        "base_commit_sha" => base_commit_sha,
+        "base64_dependency_files" => encode_dependency_files(dependency_files)
+      }
+    end
+
+    it "handles dependencies" do
+      snapshot = create_dependency_snapshot
+      snapshot.add_handled_dependencies_all_directories(%w(a b))
+      expect(snapshot.handled_dependencies).to eq(Set.new(%w(a b)))
+    end
+
+    context "when there are multiple directories" do
+      let(:directory) { nil }
+      let(:directories) { %w(/foo /bar) }
+      let(:dependency_files) do
+        [
+          Dependabot::DependencyFile.new(
+            name: "Gemfile",
+            content: fixture("bundler/original/Gemfile"),
+            directory: "/foo"
+          ),
+          Dependabot::DependencyFile.new(
+            name: "Gemfile",
+            content: fixture("bundler/original/Gemfile"),
+            directory: "/bar"
+          )
+        ]
+      end
+
+      it "handles dependencies for all directories" do
+        snapshot = create_dependency_snapshot
+        snapshot.current_directory = "/foo"
+        snapshot.add_handled_dependencies_all_directories(%w(a b))
+
+        expect(snapshot.handled_dependencies).to eq(Set.new(%w(a b)))
+        snapshot.current_directory = "/bar"
+        expect(snapshot.handled_dependencies).to eq(Set.new(%w(a b)))
+      end
+    end
+  end
+
   describe "::create_from_job_definition" do
     subject(:create_dependency_snapshot) do
       described_class.create_from_job_definition(
@@ -173,7 +234,9 @@
 
         expect(snapshot.ungrouped_dependencies.length).to eql(2)
 
-        snapshot.add_handled_dependencies(group.dependencies.find { |d| d.name == "dummy-pkg-a" }.name)
+        snapshot.add_handled_dependencies_current_directory(
+          group.dependencies.find { |d| d.name == "dummy-pkg-a" }.name
+        )
         expect(snapshot.ungrouped_dependencies.first.name).to eql("dummy-pkg-b")
 
         Dependabot::Experiments.reset!