install-deps: isolate failures so one bad dep never blocks the rest

lib/dev/deps/brew_integration.rb

@@ -44,17 +44,24 @@ def initialize(repository:, cache:, taps: [], project_dir: nil)
 
       # Install all brew dependencies. Registers taps on first call.
       #
+      # Tap registration stays outside the per-dep isolation: every install
+      # is predetermined to fail for the same root cause, so it surfaces as
+      # one integration-level failure instead of N per-dep echoes.
+      #
       # @param dependencies [Array<Dependency>] brew deps to install
+      # @raise [TapRegistrationError] if a tap cannot be registered
+      # @raise [PartialInstallError] if any dep fails; the rest were attempted
       sig { params(dependencies: T::Array[Dependency]).void }
       def install_all(dependencies)
         ensure_taps_registered
-        dependencies.each do |dep|
+        failures = collect_failures(dependencies) do |dep|
           if dep.metadata["cask"]
             install_cask(dep)
           else
             install_formula(dep)
           end
         end
+        raise PartialInstallError, failures if failures.any?
       end
 
       private

lib/dev/deps/cmake_integration.rb

@@ -58,13 +58,21 @@ def initialize(repository:, cache:, project_root:)
       # Install all cmake dependencies: fetch sources, run post_install hooks,
       # generate deps.cmake and deps.targets.cmake.
       #
+      # On any per-dep failure the batch artifacts are NOT rewritten: a file
+      # generated from a partial set would silently drop the failed deps'
+      # variables. The previous files stay in place — stale but consistent,
+      # the same philosophy as the install stamp.
+      #
       # @param dependencies [Array<Dependency>] cmake deps to install
+      # @raise [PartialInstallError] if any dep fails; the rest were attempted
       sig { params(dependencies: T::Array[Dependency]).void }
       def install_all(dependencies)
-        dependencies.each do |dep|
+        failures = collect_failures(dependencies) do |dep|
           fetch_dep(dep)
           run_post_install(dep)
         end
+        raise PartialInstallError, failures if failures.any?
+
         write_deps_cmake(dependencies)
         write_targets_cmake(dependencies)
       end

lib/dev/deps/gh_integration.rb

@@ -70,9 +70,11 @@ def initialize(repository:, cache:, project_root: nil)
       # Install all gh dependencies.
       #
       # @param dependencies [Array<Dependency>] gh deps to install
+      # @raise [PartialInstallError] if any dep fails; the rest were attempted
       sig { params(dependencies: T::Array[Dependency]).void }
       def install_all(dependencies)
-        dependencies.each { |dep| install(dep) }
+        failures = collect_failures(dependencies) { |dep| install(dep) }
+        raise PartialInstallError, failures if failures.any?
       end
 
       private

lib/dev/deps/installer.rb

@@ -2,6 +2,7 @@
 # frozen_string_literal: true
 
 require "sorbet-runtime"
+require_relative "integration"
 
 module Dev
   module Deps
@@ -12,6 +13,28 @@ module Deps
     class Installer
       extend T::Sig
 
+      # Aggregate of every failure across the whole install run, raised after
+      # all integrations were attempted. Entries are ordered by wave (build
+      # first), so root causes appear before derivative failures. Raising
+      # keeps the caller contract unchanged: a failed run exits non-zero and
+      # never writes the installed stamp.
+      class InstallFailedError < StandardError
+        extend T::Sig
+
+        # @return [Array<String>] one human-readable line per failure
+        sig { returns(T::Array[String]) }
+        attr_reader :entries
+
+        # @param entries [Array<String>]
+        sig { params(entries: T::Array[String]).void }
+        def initialize(entries)
+          @entries = entries
+          super("#{entries.size} dependency install failure(s) — " \
+            "installs are idempotent, fix the causes and rerun:\n" \
+            "#{entries.map { |entry| "  #{entry}" }.join("\n")}")
+        end
+      end
+
       # @param lockfile [Lockfile] lockfile reader
       # @param integrations [Hash{Symbol => Integration}] integration type → integration
       sig { params(lockfile: Lockfile, integrations: T::Hash[Symbol, T.untyped]).void }
@@ -41,6 +64,8 @@ def initialize(lockfile:, integrations:)
       # @param env [String, nil] environment name for filtering (nil = no filtering)
       # @param host [String, nil] host OS name for filtering (nil = no filtering)
       # @return [void]
+      # @raise [InstallFailedError] if any integration reported failures; every
+      #   integration was still attempted (failure isolation)
       sig { params(env: T.nilable(String), host: T.nilable(String)).void }
       def install(env: nil, host: nil)
         all_deps = @lockfile.read
@@ -49,8 +74,8 @@ def install(env: nil, host: nil)
 
         build_deps, other_deps = all_deps.partition { |d| d.group == :build }
 
-        dispatch(build_deps)
-        dispatch(other_deps)
+        failures = dispatch(build_deps) + dispatch(other_deps)
+        raise InstallFailedError, failures if failures.any?
       end
 
       private
@@ -62,12 +87,29 @@ def install(env: nil, host: nil)
       # integration generating batch artifacts (deps.cmake) would overwrite
       # its own output with each partial group.
       #
+      # A failing integration never blocks the others: the manifest declares
+      # no cross-integration edges, so the correct failure policy for the
+      # (degenerate) dependency DAG is attempt-all. Failures are collected —
+      # per-dep entries when the integration isolated them
+      # (PartialInstallError), one entry when it failed whole (e.g. a tap
+      # registration preamble) — and reported by the caller's aggregate.
+      #
       # @param deps [Array<Dependency>] dependencies to install
-      # @return [void]
-      sig { params(deps: T::Array[Dependency]).void }
+      # @return [Array<String>] one entry per failure, in dispatch order
+      sig { params(deps: T::Array[Dependency]).returns(T::Array[String]) }
       def dispatch(deps)
-        deps.group_by { |dep| @integrations[dep.integration] }.each do |integration, typed_deps|
-          integration&.install_all(typed_deps)
+        deps.group_by { |dep| @integrations[dep.integration] }.flat_map do |integration, typed_deps|
+          next [] unless integration
+
+          label = typed_deps.map(&:integration).uniq.join("+")
+          begin
+            integration.install_all(typed_deps)
+            []
+          rescue Integration::PartialInstallError => e
+            e.failures.map { |name, error| "#{label}: #{name} — #{error.message}" }
+          rescue StandardError => e
+            ["#{label}: #{e.message}"]
+          end
         end
       end
 

lib/dev/deps/integration.rb

@@ -18,6 +18,27 @@ module Deps
     class Integration
       extend T::Sig
 
+      # Aggregate of per-dep failures within one install_all call: the loop
+      # attempts every dep (one bad formula must not block an unrelated
+      # engine download), collects what failed, and raises this at the end
+      # so the run still fails loudly. The Installer flattens the failures
+      # into its cross-integration report.
+      class PartialInstallError < StandardError
+        extend T::Sig
+
+        # @return [Array<[String, StandardError]>] failed dep name → its error
+        sig { returns(T::Array[[String, StandardError]]) }
+        attr_reader :failures
+
+        # @param failures [Array<[String, StandardError]>]
+        sig { params(failures: T::Array[[String, StandardError]]).void }
+        def initialize(failures)
+          @failures = failures
+          lines = failures.map { |name, error| "  #{name}: #{error.message}" }
+          super("#{failures.size} dep install(s) failed:\n#{lines.join("\n")}")
+        end
+      end
+
       # @param repository [Repository, nil] source adapter for this integration type
       # @param cache      [Cache, nil]      shared download cache
       sig { params(repository: T.nilable(Repository), cache: T.nilable(Cache)).void }
@@ -36,6 +57,30 @@ def install_all(dependencies)
 
       private
 
+      # Attempt the block for every dep, isolating failures so one bad dep
+      # never blocks the rest of the batch. Callers raise PartialInstallError
+      # themselves (after skipping any batch post-processing that requires a
+      # fully-successful set).
+      #
+      # @param dependencies [Array<Dependency>]
+      # @yieldparam dep [Dependency] the dep to install
+      # @return [Array<[String, StandardError]>] failed dep name → its error
+      sig do
+        params(
+          dependencies: T::Array[Dependency],
+          blk: T.proc.params(dep: Dependency).void,
+        ).returns(T::Array[[String, StandardError]])
+      end
+      def collect_failures(dependencies, &blk)
+        failures = T.let([], T::Array[[String, StandardError]])
+        dependencies.each do |dep|
+          blk.call(dep)
+        rescue StandardError => e
+          failures << [dep.name, e]
+        end
+        failures
+      end
+
       sig { returns(T.nilable(Repository)) }
       attr_reader :repository
 

test/dev/deps/brew_integration_test.rb

@@ -100,10 +100,48 @@ class Dev::Deps::BrewIntegrationTest < Minitest::Test
          .returns(["", "Error: No available formula", failed_status])
 
     When "installing all"
-    integration.install_all(deps)
+    error = assert_raises(Dev::Deps::Integration::PartialInstallError) do
+      integration.install_all(deps)
+    end
 
-    Then
-    raises Dev::Deps::BrewIntegration::InstallError
+    Then "the per-dep failure is the brew install error"
+    error.failures[0][1].is_a?(Dev::Deps::BrewIntegration::InstallError)
+
+    Cleanup
+    FileUtils.rm_rf(dir)
+  end
+
+  test "install_all attempts the remaining formulae when one fails, then raises the aggregate" do
+    Given "two brew dependencies, the first of which fails to install"
+    dir = Dir.mktmpdir("dev-brew-int-test-")
+    cache = Dev::Deps::Cache.new(cache_dir: dir)
+    integration = Dev::Deps::BrewIntegration.new(repository: Dev::Deps::BrewRepository.new, cache: cache)
+    deps = [
+      Dev::Deps::Dependency.new(name: "bad_formula", integration: :brew, group: :build,
+        version: "1.0.0", hash: nil, metadata: {}),
+      Dev::Deps::Dependency.new(name: "good_formula", integration: :brew, group: :build,
+        version: "2.0.0", hash: nil, metadata: {}),
+    ]
+    integration.stubs(:brew_installed?).returns(false)
+    Open3.stubs(:capture3)
+         .with("brew", "install", "bad_formula")
+         .returns(["", "Error: No available formula", stub(success?: false)])
+    Open3.expects(:capture3)
+         .with("brew", "install", "good_formula")
+         .returns(["", "", stub(success?: true)])
+
+    When "installing all and capturing the aggregate error"
+    error = nil
+    begin
+      integration.install_all(deps)
+    rescue StandardError => e
+      error = e
+    end
+
+    Then "good_formula was still installed (Mocha-verified) and the aggregate lists bad_formula"
+    error.is_a?(Dev::Deps::Integration::PartialInstallError)
+    error.failures.map(&:first) == ["bad_formula"]
+    error.failures[0][1].is_a?(Dev::Deps::BrewIntegration::InstallError)
 
     Cleanup
     FileUtils.rm_rf(dir)

test/dev/deps/cmake_integration_test.rb

@@ -135,10 +135,12 @@ def prepopulate_dep(root, name)
     integration.stubs(:system).returns(false)
 
     When "installing all"
-    integration.install_all(deps)
+    error = assert_raises(Dev::Deps::Integration::PartialInstallError) do
+      integration.install_all(deps)
+    end
 
-    Then
-    raises Dev::Deps::CmakeIntegration::GitCloneError
+    Then "the per-dep failure is the clone error"
+    error.failures[0][1].is_a?(Dev::Deps::CmakeIntegration::GitCloneError)
 
     Cleanup
     FileUtils.rm_rf(dir)
@@ -165,10 +167,12 @@ def prepopulate_dep(root, name)
                .returns(false)
 
     When "installing all"
-    integration.install_all(deps)
+    error = assert_raises(Dev::Deps::Integration::PartialInstallError) do
+      integration.install_all(deps)
+    end
 
-    Then
-    raises Dev::Deps::CmakeIntegration::GitCheckoutError
+    Then "the per-dep failure is the checkout error"
+    error.failures[0][1].is_a?(Dev::Deps::CmakeIntegration::GitCheckoutError)
 
     Cleanup
     FileUtils.rm_rf(dir)
@@ -192,10 +196,12 @@ def prepopulate_dep(root, name)
                .returns(false)
 
     When "installing all"
-    integration.install_all(deps)
+    error = assert_raises(Dev::Deps::Integration::PartialInstallError) do
+      integration.install_all(deps)
+    end
 
-    Then
-    raises Dev::Deps::CmakeIntegration::DownloadError
+    Then "the per-dep failure is the download error"
+    error.failures[0][1].is_a?(Dev::Deps::CmakeIntegration::DownloadError)
 
     Cleanup
     FileUtils.rm_rf(dir)
@@ -219,10 +225,12 @@ def prepopulate_dep(root, name)
     integration.stubs(:system).with { |cmd, *_| cmd == "tar" }.returns(false)
 
     When "installing all"
-    integration.install_all(deps)
+    error = assert_raises(Dev::Deps::Integration::PartialInstallError) do
+      integration.install_all(deps)
+    end
 
-    Then
-    raises Dev::Deps::CmakeIntegration::ExtractError
+    Then "the per-dep failure is the extract error"
+    error.failures[0][1].is_a?(Dev::Deps::CmakeIntegration::ExtractError)
 
     Cleanup
     FileUtils.rm_rf(dir)
@@ -415,4 +423,55 @@ def prepopulate_dep(root, name)
     { "url" => "https://example.com/b.tar.gz" } | false        | false
     {}                                          | true         | false
   end
+
+  test "install_all leaves batch artifacts untouched when any dep fails, still attempting the rest" do
+    Given "two git deps (first fails to clone) and pre-existing batch artifacts"
+    dir = Dir.mktmpdir("dev-cmake-int-test-")
+    cache = Dev::Deps::Cache.new(cache_dir: File.join(dir, "cache"))
+    integration = Dev::Deps::CmakeIntegration.new(
+      repository: Dev::Deps::GitRepository.new, cache: cache, project_root: dir,
+    )
+    previous_deps_cmake = "# previous consistent deps.cmake\n"
+    previous_targets_cmake = "# previous consistent deps.targets.cmake\n"
+    File.write(File.join(dir, "deps.cmake"), previous_deps_cmake)
+    File.write(File.join(dir, "deps.targets.cmake"), previous_targets_cmake)
+    deps = [
+      Dev::Deps::Dependency.new(
+        name: "bad_repo", integration: :cmake, group: :app,
+        version: "abc123", hash: nil,
+        metadata: { "repo" => "https://example.com/bad_repo" },
+      ),
+      Dev::Deps::Dependency.new(
+        name: "good_repo", integration: :cmake, group: :test,
+        version: "def456", hash: nil,
+        metadata: { "repo" => "https://example.com/good_repo" },
+      ),
+    ]
+    integration.stubs(:system)
+               .with("git", "clone", "--no-checkout", "-q", "https://example.com/bad_repo", anything)
+               .returns(false)
+    integration.expects(:system)
+               .with("git", "clone", "--no-checkout", "-q", "https://example.com/good_repo", anything)
+               .returns(true)
+    integration.stubs(:system)
+               .with("git", "-c", "advice.detachedHead=false", "checkout", anything, chdir: anything)
+               .returns(true)
+
+    When "installing all and capturing the aggregate error"
+    error = nil
+    begin
+      integration.install_all(deps)
+    rescue StandardError => e
+      error = e
+    end
+
+    Then "good_repo was still cloned (Mocha-verified), artifacts are stale-but-consistent"
+    error.is_a?(Dev::Deps::Integration::PartialInstallError)
+    error.failures.map(&:first) == ["bad_repo"]
+    File.read(File.join(dir, "deps.cmake")) == previous_deps_cmake
+    File.read(File.join(dir, "deps.targets.cmake")) == previous_targets_cmake
+
+    Cleanup
+    FileUtils.rm_rf(dir)
+  end
 end