Address PR review feedback for #15068

- bun: expose error_context on NoChangeError so observability picks it up

- updater: emit "unknown" instead of empty-string for nil boolean metric tags via bool_tag helper

- npm_and_yarn: detect fallback traces by stable fingerprint to avoid false positives from dep names containing "audit"

- yarn lockfile updater: extract yarn berry audit-fix fallback into helper methods to drop Metrics/AbcSize + Metrics/MethodLength rubocop disables

- pnpm lockfile updater: extract fallback logic into maybe_run_pnpm_fallback to drop Metrics/AbcSize rubocop disable

- pnpm lockfile updater: add load-order require for dependabot/npm_and_yarn/file_updater

Author: robaiken

bun/lib/dependabot/bun/file_updater.rb

@@ -20,6 +20,9 @@ class FileUpdater < Dependabot::FileUpdaters::Base
       class NoChangeError < StandardError
         extend T::Sig
 
+        sig { returns(T::Hash[Symbol, T.untyped]) }
+        attr_reader :error_context
+
         sig { params(message: String, error_context: T::Hash[Symbol, T.untyped]).void }
         def initialize(message:, error_context:)
           super(message)

npm_and_yarn/lib/dependabot/npm_and_yarn/file_updater.rb

@@ -241,15 +241,24 @@ def error_context(updated_files:)
           .returns([T::Boolean, T.nilable(T::Boolean)])
       end
       def compute_fallback_state(traces)
-        fallback_traces = traces.select do |t|
-          t.command.include?("audit") || t.command.include?("--depth Infinity")
-        end
+        fallback_traces = traces.select { |t| fallback_trace?(t) }
         attempted = fallback_traces.any?
         succeeded =
           (fallback_traces.any? { |t| t.content_changed_after == true } if attempted)
         [attempted, succeeded]
       end
 
+      # Detect whether a trace represents a fallback invocation (audit-fix
+      # or pnpm deep update). Match against the stable `fingerprint` (or
+      # `command` when no fingerprint is set) rather than the raw command
+      # so that user-controlled dependency names cannot false-positive
+      # this check.
+      sig { params(trace: CommandTrace).returns(T::Boolean) }
+      def fallback_trace?(trace)
+        key = trace.fingerprint || trace.command
+        key.include?("audit") || key.include?("--depth Infinity")
+      end
+
       sig { returns(T::Array[CommandTrace]) }
       def aggregated_command_traces
         traces = T.let([], T::Array[CommandTrace])

npm_and_yarn/lib/dependabot/npm_and_yarn/file_updater/pnpm_lockfile_updater.rb

@@ -1,6 +1,7 @@
 # typed: strict
 # frozen_string_literal: true
 
+require "dependabot/npm_and_yarn/file_updater"
 require "dependabot/shared_helpers/command_trace"
 require "dependabot/npm_and_yarn/helpers"
 require "dependabot/npm_and_yarn/package/registry_finder"
@@ -151,7 +152,7 @@ def updated_pnpm_lock_content(pnpm_lock, updated_pnpm_workspace_content: nil)
           )
             .returns(String)
         end
-        def run_pnpm_update(pnpm_lock:, updated_pnpm_workspace_content: nil) # rubocop:disable Metrics/AbcSize
+        def run_pnpm_update(pnpm_lock:, updated_pnpm_workspace_content: nil)
           # Set dependency files and credentials for automatic env variable injection
           Helpers.dependency_files = dependency_files
           Helpers.credentials = credentials
@@ -160,36 +161,66 @@ def run_pnpm_update(pnpm_lock:, updated_pnpm_workspace_content: nil) # rubocop:d
             File.write(".npmrc", npmrc_content(pnpm_lock))
 
             SharedHelpers.with_git_configured(credentials: credentials) do
-              original_content = File.read(pnpm_lock.name)
-
-              if updated_pnpm_workspace_content
-                File.write("pnpm-workspace.yaml", updated_pnpm_workspace_content["pnpm-workspace.yaml"])
-              else
-                run_pnpm_update_packages
-                write_final_package_json_files
-              end
-
-              run_pnpm_install
-
-              updated_content = File.read(pnpm_lock.name)
-              @command_traces.last&.content_changed_after = updated_content != original_content
-              if updated_content == original_content && Dependabot::Experiments.enabled?(:enable_audit_fix_fallback)
-                run_pnpm_deep_update_fallback
-                updated_content = File.read(pnpm_lock.name)
-                @command_traces.last&.content_changed_after = updated_content != original_content
-              end
-
-              if updated_content == original_content && Dependabot::Experiments.enabled?(:enable_audit_fix_fallback)
-                run_pnpm_audit_fix_fallback(pnpm_lock, original_content)
-                updated_content = File.read(pnpm_lock.name)
-                @command_traces.last&.content_changed_after = updated_content != original_content
-              end
-
-              updated_content
+              run_pnpm_update_with_fallbacks(pnpm_lock, updated_pnpm_workspace_content)
             end
           end
         end
 
+        sig do
+          params(
+            pnpm_lock: Dependabot::DependencyFile,
+            updated_pnpm_workspace_content: T.nilable(T::Hash[String, T.nilable(String)])
+          ).returns(String)
+        end
+        def run_pnpm_update_with_fallbacks(pnpm_lock, updated_pnpm_workspace_content)
+          original_content = File.read(pnpm_lock.name)
+
+          if updated_pnpm_workspace_content
+            File.write("pnpm-workspace.yaml", updated_pnpm_workspace_content["pnpm-workspace.yaml"])
+          else
+            run_pnpm_update_packages
+            write_final_package_json_files
+          end
+
+          run_pnpm_install
+          updated_content = note_content_change(pnpm_lock, original_content)
+
+          updated_content = maybe_run_pnpm_fallback(pnpm_lock, original_content, updated_content) do
+            run_pnpm_deep_update_fallback
+          end
+
+          maybe_run_pnpm_fallback(pnpm_lock, original_content, updated_content) do
+            run_pnpm_audit_fix_fallback(pnpm_lock, original_content)
+          end
+        end
+
+        # Re-reads the lockfile, sets `content_changed_after` on the most
+        # recent CommandTrace, and returns the current lockfile content.
+        sig { params(pnpm_lock: Dependabot::DependencyFile, original_content: String).returns(String) }
+        def note_content_change(pnpm_lock, original_content)
+          updated_content = File.read(pnpm_lock.name)
+          @command_traces.last&.content_changed_after = updated_content != original_content
+          updated_content
+        end
+
+        # Runs the supplied fallback only if the lockfile is still
+        # unchanged and the audit-fix experiment is enabled.
+        sig do
+          params(
+            pnpm_lock: Dependabot::DependencyFile,
+            original_content: String,
+            updated_content: String,
+            block: T.proc.void
+          ).returns(String)
+        end
+        def maybe_run_pnpm_fallback(pnpm_lock, original_content, updated_content, &block)
+          return updated_content unless updated_content == original_content
+          return updated_content unless Dependabot::Experiments.enabled?(:enable_audit_fix_fallback)
+
+          block.call # rubocop:disable Performance/RedundantBlockCall
+          note_content_change(pnpm_lock, original_content)
+        end
+
         sig { returns(T.nilable(String)) }
         def run_pnpm_update_packages
           dependency_updates = dependencies.map do |d|

npm_and_yarn/lib/dependabot/npm_and_yarn/file_updater/yarn_lockfile_updater.rb

@@ -263,19 +263,31 @@ def requirements_changed?(dependency_name)
         end
 
         sig { params(yarn_lock: Dependabot::DependencyFile).returns(T::Hash[String, String]) }
-        # rubocop:disable Metrics/AbcSize
-        # rubocop:disable Metrics/MethodLength
         def run_yarn_berry_subdependency_updater(yarn_lock:)
           dep = T.must(sub_dependencies.first)
-          update = "#{dep.name}@#{dep.version}"
+          original_content = File.read(yarn_lock.name)
+
+          run_yarn_berry_subdep_commands(dep)
+
+          updated_content = File.read(yarn_lock.name)
+          @command_traces.last&.content_changed_after = updated_content != original_content
+
+          if updated_content == original_content && Dependabot::Experiments.enabled?(:enable_audit_fix_fallback)
+            updated_content = run_yarn_berry_audit_fix_fallback(yarn_lock, dep, original_content)
+          end
 
+          { yarn_lock.name => updated_content }
+        end
+
+        sig { params(dep: Dependabot::Dependency).void }
+        def run_yarn_berry_subdep_commands(dep)
+          update = "#{dep.name}@#{dep.version}"
           commands = [
             ["add #{update} #{yarn_berry_args}".strip, "add <update> #{yarn_berry_args}".strip],
             ["dedupe #{dep.name} #{yarn_berry_args}".strip, "dedupe <dep_name> #{yarn_berry_args}".strip],
             ["remove #{dep.name} #{yarn_berry_args}".strip, "remove <dep_name> #{yarn_berry_args}".strip]
           ]
 
-          original_content = File.read(yarn_lock.name)
           fingerprint_batch = commands.map { |_cmd, fp| fp }.join(" && ")
           CommandTrace.record(
             traces: @command_traces,
@@ -285,33 +297,35 @@ def run_yarn_berry_subdependency_updater(yarn_lock:)
           ) do
             Helpers.run_yarn_commands(*commands)
           end
+        end
 
-          updated_content = File.read(yarn_lock.name)
-          @command_traces.last&.content_changed_after = updated_content != original_content
-          if updated_content == original_content && Dependabot::Experiments.enabled?(:enable_audit_fix_fallback)
-            begin
-              CommandTrace.record(
-                traces: @command_traces,
-                package_manager: "yarn",
-                command: "npm audit --fix --mode update-lockfile",
-                fingerprint: "npm audit --fix --mode update-lockfile"
-              ) do
-                NativeHelpers.run_yarn_audit_fix_command
-              end
-              dep.metadata[:audit_fix_used] = true
-            rescue SharedHelpers::HelperSubprocessFailed
-              Dependabot.logger.info(
-                "yarn npm audit --fix failed or partially fixed — continuing with any changes made"
-              )
+        sig do
+          params(
+            yarn_lock: Dependabot::DependencyFile,
+            dep: Dependabot::Dependency,
+            original_content: String
+          ).returns(String)
+        end
+        def run_yarn_berry_audit_fix_fallback(yarn_lock, dep, original_content)
+          begin
+            CommandTrace.record(
+              traces: @command_traces,
+              package_manager: "yarn",
+              command: "npm audit --fix --mode update-lockfile",
+              fingerprint: "npm audit --fix --mode update-lockfile"
+            ) do
+              NativeHelpers.run_yarn_audit_fix_command
             end
-            updated_content = File.read(yarn_lock.name)
-            @command_traces.last&.content_changed_after = updated_content != original_content
+            dep.metadata[:audit_fix_used] = true
+          rescue SharedHelpers::HelperSubprocessFailed
+            Dependabot.logger.info(
+              "yarn npm audit --fix failed or partially fixed — continuing with any changes made"
+            )
           end
-
-          { yarn_lock.name => updated_content }
+          updated_content = File.read(yarn_lock.name)
+          @command_traces.last&.content_changed_after = updated_content != original_content
+          updated_content
         end
-        # rubocop:enable Metrics/MethodLength
-        # rubocop:enable Metrics/AbcSize
 
         sig { returns(String) }
         def yarn_berry_args

updater/lib/dependabot/updater/error_handler.rb

@@ -196,13 +196,26 @@ def maybe_emit_no_change_metric(error_details)
           tags: {
             package_manager: job.package_manager,
             reason: detail[:reason]&.to_s || "unknown",
-            commands_succeeded: detail[:commands_succeeded].to_s,
-            fallback_attempted: detail[:fallback_attempted].to_s,
-            fallback_succeeded: detail[:fallback_succeeded].to_s
+            commands_succeeded: bool_tag(detail[:commands_succeeded]),
+            fallback_attempted: bool_tag(detail[:fallback_attempted]),
+            fallback_succeeded: bool_tag(detail[:fallback_succeeded])
           }
         )
       end
 
+      # Convert a tri-state boolean (true / false / nil) into a low-
+      # cardinality string suitable for a metric tag value. Empty strings
+      # are treated as invalid tag values by many backends, so we surface
+      # `"unknown"` instead of the default `nil.to_s == ""`.
+      sig { params(value: T.untyped).returns(String) }
+      def bool_tag(value)
+        case value
+        when true then "true"
+        when false then "false"
+        else "unknown"
+        end
+      end
+
       # Emit one info line per command trace + truncated stdout/stderr at
       # debug level. Stays a no-op if the error detail does not include
       # trace data (e.g. older payloads).