Restore shadowed routing state; settle worker failures

lib/cli/ui/stdout_router.rb

@@ -209,36 +209,35 @@ def run
 
           StdoutRouter.assert_enabled!
 
-          Thread.current[:cliui_current_capture] = self
-
-          prev_frame_inset = Thread.current[:no_cliui_frame_inset]
-          prev_hook = Thread.current[:cliui_output_hook]
-
-          if Thread.current.respond_to?(:report_on_exception)
-            Thread.current.report_on_exception = false
-          end
-
-          self.class.with_stdin_masked do
-            Thread.current[:no_cliui_frame_inset] = !@with_frame_inset
-            Thread.current[:cliui_output_hook] = ->(data, stream) do
-              stream = :stdout if @merged_output
-              case stream
-              when :stdout
-                @out.write(data)
-                @duplicate_output_to.write(data)
-              when :stderr
-                @err.write(data)
-              else raise
+          previous_capture = Thread.current[:cliui_current_capture]
+          begin
+            Thread.current[:cliui_current_capture] = self
+            self.class.with_stdin_masked do
+              previous_frame_inset = Thread.current[:no_cliui_frame_inset]
+              previous_hook = Thread.current[:cliui_output_hook]
+              begin
+                Thread.current[:no_cliui_frame_inset] = !@with_frame_inset
+                Thread.current[:cliui_output_hook] = ->(data, stream) do
+                  stream = :stdout if @merged_output
+                  case stream
+                  when :stdout
+                    @out.write(data)
+                    @duplicate_output_to.write(data)
+                  when :stderr
+                    @err.write(data)
+                  else raise
+                  end
+                  print_captured_output # suppress writing to terminal by default
+                end
+                @block.call
+              ensure
+                Thread.current[:cliui_output_hook] = previous_hook
+                Thread.current[:no_cliui_frame_inset] = previous_frame_inset
               end
-              print_captured_output # suppress writing to terminal by default
             end
-
-            @block.call
+          ensure
+            Thread.current[:cliui_current_capture] = previous_capture
           end
-        ensure
-          Thread.current[:cliui_output_hook] = prev_hook
-          Thread.current[:no_cliui_frame_inset] = prev_frame_inset
-          Thread.current[:cliui_current_capture] = nil
         end
 
         #: -> String
@@ -318,15 +317,17 @@ class << self
         def with_id(on_streams:, &block)
           require 'securerandom'
           id = format('%05d', rand(10**5))
-          Thread.current[:cliui_output_id] = {
-            id: id,
-            streams: on_streams.map do |stream|
-              stream #: as io_like
-            end,
-          }
-          yield(id)
-        ensure
-          Thread.current[:cliui_output_id] = nil
+          streams = on_streams.map do |stream|
+            stream #: as io_like
+          end
+
+          previous_id = Thread.current[:cliui_output_id]
+          begin
+            Thread.current[:cliui_output_id] = { id: id, streams: streams }
+            yield(id)
+          ensure
+            Thread.current[:cliui_output_id] = previous_id
+          end
         end
 
         #: -> Hash[Symbol, (String | io_like)]?

lib/cli/ui/work_queue.rb

@@ -128,9 +128,9 @@ def start_worker
             rescue Interrupt => e
               future.fail(e)
               raise # Always re-raise interrupts to terminate the worker
-            rescue StandardError => e
+            rescue Exception => e # rubocop:disable Lint/RescueException
               future.fail(e)
-              # Don't re-raise standard errors - allow worker to continue
+              # The future carries the error to callers; allow the worker to continue
             end
           end
         rescue Interrupt

test/cli/ui/spinner/spin_group_test.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 require 'test_helper'
+require 'timeout'
 
 module CLI
   module UI
@@ -36,6 +37,22 @@ def test_spin_group_auto_debrief_false
           assert_equal('', err)
         end
 
+        def test_spin_group_non_standard_error_does_not_hang_or_report_thread_death
+          _out, err = capture_io do
+            CLI::UI::StdoutRouter.ensure_activated
+
+            sg = SpinGroup.new(auto_debrief: false)
+            sg.add('s') { raise NotImplementedError, 'not implemented' }
+
+            error = Timeout.timeout(10) do
+              assert_raises(NotImplementedError) { sg.wait }
+            end
+            assert_equal('not implemented', error.message)
+          end
+
+          assert_equal('', err)
+        end
+
         def test_spin_group_success_debrief
           capture_io do
             CLI::UI::StdoutRouter.ensure_activated

test/cli/ui/stdout_router_test.rb

@@ -24,6 +24,139 @@ def test_current_id
         end
       end
 
+      def test_nested_with_id_restores_outer_id
+        StdoutRouter.with_id(on_streams: [$stdout]) do |outer_id|
+          StdoutRouter.with_id(on_streams: [$stdout]) do |inner_id|
+            assert_equal(inner_id, StdoutRouter.current_id&.fetch(:id))
+          end
+          assert_equal(outer_id, StdoutRouter.current_id&.fetch(:id))
+        end
+        assert_nil(StdoutRouter.current_id)
+      end
+
+      def test_nested_with_id_restores_outer_id_when_inner_raises
+        StdoutRouter.with_id(on_streams: [$stdout]) do |outer_id|
+          assert_raises(RuntimeError) do
+            StdoutRouter.with_id(on_streams: [$stdout]) { raise('inner') }
+          end
+          assert_equal(outer_id, StdoutRouter.current_id&.fetch(:id))
+        end
+        assert_nil(StdoutRouter.current_id)
+      end
+
+      def test_capture_leaves_report_on_exception_untouched
+        capture_io do
+          StdoutRouter.with_enabled do
+            prev = Thread.current.report_on_exception
+            begin
+              [true, false].each do |value|
+                Thread.current.report_on_exception = value
+                during = nil
+                StdoutRouter::Capture.new { during = Thread.current.report_on_exception }.run
+                assert_equal(value, during)
+                assert_equal(value, Thread.current.report_on_exception)
+              end
+            ensure
+              Thread.current.report_on_exception = prev
+            end
+          end
+        end
+      end
+
+      def test_capture_failure_in_a_thread_still_reports_thread_death
+        script = <<~RUBY
+          require 'cli/ui'
+          CLI::UI::StdoutRouter.enable
+          thread = Thread.new { CLI::UI::StdoutRouter::Capture.new { raise('boom') }.run }
+          begin
+            thread.join
+          rescue RuntimeError
+            nil
+          end
+        RUBY
+
+        lib = File.expand_path('../../../lib', __dir__)
+        stdout, stderr, _ = Open3.capture3(RbConfig.ruby, '-I', lib, '-e', script)
+
+        assert_match(/terminated with exception/, stderr, "stdout:\n#{stdout}\nstderr:\n#{stderr}")
+      end
+
+      def test_nested_capture_restores_outer_capture
+        capture_io do
+          StdoutRouter.with_enabled do
+            inner_current = nil
+            restored_current = nil
+            inner = StdoutRouter::Capture.new do
+              inner_current = StdoutRouter::Capture.current_capture
+            end
+            outer = StdoutRouter::Capture.new do
+              inner.run
+              restored_current = StdoutRouter::Capture.current_capture
+            end
+            outer.run
+            assert_same(inner, inner_current)
+            assert_same(outer, restored_current)
+            assert_nil(StdoutRouter::Capture.current_capture)
+          end
+        end
+      end
+
+      def test_nested_capture_restores_outer_capture_and_hook_when_inner_raises
+        capture_io do
+          StdoutRouter.with_enabled do
+            restored_current = nil
+            inner = StdoutRouter::Capture.new { raise('inner') }
+            outer = StdoutRouter::Capture.new do
+              assert_raises(RuntimeError) { inner.run }
+              restored_current = StdoutRouter::Capture.current_capture
+              puts('after inner')
+            end
+            outer.run
+            assert_same(outer, restored_current)
+            assert_includes(outer.stdout, 'after inner')
+            assert_nil(StdoutRouter::Capture.current_capture)
+          end
+        end
+      end
+
+      def test_nested_capture_can_enter_alternate_screen
+        capture_io do
+          StdoutRouter.with_enabled do
+            entered_alternate_screen = false
+            outer = StdoutRouter::Capture.new do
+              StdoutRouter::Capture.new {}.run
+              StdoutRouter::Capture.in_alternate_screen do
+                entered_alternate_screen = true
+              end
+            end
+
+            outer.run
+
+            assert(entered_alternate_screen)
+          end
+        end
+      end
+
+      def test_capture_restores_frame_inset_when_nested
+        capture_io do
+          StdoutRouter.with_enabled do
+            inset_during_inner = nil
+            inset_after_inner = nil
+            inner = StdoutRouter::Capture.new(with_frame_inset: true) do
+              inset_during_inner = Thread.current[:no_cliui_frame_inset]
+            end
+            outer = StdoutRouter::Capture.new(with_frame_inset: false) do
+              inner.run
+              inset_after_inner = Thread.current[:no_cliui_frame_inset]
+            end
+            outer.run
+            assert_equal(false, inset_during_inner)
+            assert_equal(true, inset_after_inner)
+            assert_nil(Thread.current[:no_cliui_frame_inset])
+          end
+        end
+      end
+
       def test_frame_can_autoload_after_router_is_enabled
         script = <<~RUBY
           require 'stringio'

test/cli/ui/work_queue_test.rb

@@ -65,6 +65,19 @@ def test_future_error
         assert_raises(StandardError, 'Test error') { future.value }
       end
 
+      def test_future_non_standard_error_completes_and_worker_continues
+        @work_queue = WorkQueue.new(1)
+        failed = @work_queue.enqueue { raise NotImplementedError, 'not implemented' }
+        replacement = @work_queue.enqueue { :ran }
+
+        @work_queue.wait
+
+        assert(failed.completed?)
+        error = assert_raises(NotImplementedError) { failed.value }
+        assert_equal('not implemented', error.message)
+        assert_equal(:ran, replacement.value)
+      end
+
       def test_max_concurrent
         max_concurrent = 2
         @work_queue = WorkQueue.new(max_concurrent)