Restore shadowed thread-locals; stop mutating report_on_exception

Three scope helpers in StdoutRouter hard-reset thread state on exit
instead of restoring the value they replaced:

* Capture#run set Thread.current.report_on_exception = false and never
  restored it, permanently disabling exception reporting on any thread
  that ran a capture.
* Capture#run reset :cliui_current_capture to nil, so a nested capture
  left the still-active outer capture invisible to current_capture
  consumers such as in_alternate_screen.
* with_id reset :cliui_output_id to nil, so a nested ID scope stripped
  the outer scope's output labelling for the rest of the outer block.

Thread-locals are now set and restored through one helper:
StdoutRouter.with_thread_locals sets values for the duration of a block
and restores whatever they shadowed. That also removes the clobbering
window the hand-rolled save/restore pairs had: a method-level ensure
runs even when something raises before the save, nulling the enclosing
scope's values.

Capture#run no longer touches report_on_exception at all. The 2019
suppression (fec0004) existed to silence Ruby's thread-death report
when a spinner task raised; SpinGroup now runs tasks through WorkQueue,
which rescues errors in-thread, so no report fires there either way,
and the respond_to? guard is dead at required_ruby_version >= 3.2.
Restoring the flag rather than dropping it produced the same visible
change (the report reappears for callers that run a Capture in a thread
of their own and let it die), so drop it: cli-ui stops writing a
thread-wide diagnostic flag it doesn't own. A subprocess test pins the
now-deliberate thread-death report.

Co-authored-by: River <river@shopify.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: 3 people

lib/cli/ui/stdout_router.rb

@@ -209,36 +209,27 @@ 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
+          StdoutRouter.with_thread_locals({ cliui_current_capture: self }) do
+            self.class.with_stdin_masked do
+              StdoutRouter.with_thread_locals({
+                no_cliui_frame_inset: !@with_frame_inset,
+                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,
+              }) do
+                @block.call
               end
-              print_captured_output # suppress writing to terminal by default
             end
-
-            @block.call
           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
@@ -314,19 +305,26 @@ class << self
         #: io_like?
         attr_accessor :duplicate_output_to
 
+        # Sets thread-local values for the duration of the block, restoring whatever they shadowed.
+        #: [T] (Hash[Symbol, untyped] values) { -> T } -> T
+        def with_thread_locals(values, &block)
+          previous = values.keys.to_h { |key| [key, Thread.current[key]] }
+          values.each { |key, value| Thread.current[key] = value }
+          yield
+        ensure
+          previous&.each { |key, value| Thread.current[key] = value }
+        end
+
         #: [T] (on_streams: Array[io_like]) { (String id) -> T } -> T
         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
+          with_thread_locals({ cliui_output_id: { id: id, streams: streams } }) do
+            yield(id)
+          end
         end
 
         #: -> Hash[Symbol, (String | io_like)]?

test/cli/ui/stdout_router_test.rb

@@ -24,6 +24,136 @@ 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_with_thread_locals_restores_previous_values
+        Thread.current[:cliui_test_local] = :outer
+        StdoutRouter.with_thread_locals({ cliui_test_local: :inner }) do
+          assert_equal(:inner, Thread.current[:cliui_test_local])
+        end
+        assert_equal(:outer, Thread.current[:cliui_test_local])
+
+        assert_raises(RuntimeError) do
+          StdoutRouter.with_thread_locals({ cliui_test_local: :inner }) { raise('boom') }
+        end
+        assert_equal(:outer, Thread.current[:cliui_test_local])
+      ensure
+        Thread.current[:cliui_test_local] = nil
+      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_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'