Harden dev cleanup review fixes

justin808 · justin808 · commit 3d328f8b4b04 · 2026-05-04T00:17:39.000-10:00
diff --git a/react_on_rails/lib/react_on_rails/dev/file_manager.rb b/react_on_rails/lib/react_on_rails/dev/file_manager.rb
@@ -84,8 +84,9 @@ def process_running?(pid)
         def socket_active?(socket_path)
           return false unless File.exist?(socket_path)
 
-          socket = Socket.new(Socket::AF_UNIX, Socket::SOCK_STREAM, 0)
+          socket = nil
           begin
+            socket = Socket.new(Socket::AF_UNIX, Socket::SOCK_STREAM, 0)
             socket.connect_nonblock(Socket.sockaddr_un(socket_path))
             true
           rescue IO::WaitWritable
@@ -99,10 +100,10 @@ def socket_active?(socket_path)
             socket.getsockopt(Socket::SOL_SOCKET, Socket::SO_ERROR).int.zero?
           rescue Errno::EISCONN
             true
-          rescue SystemCallError, IOError
+          rescue ArgumentError, SystemCallError, IOError
             false
           ensure
-            socket.close
+            socket&.close
           end
         end
 
diff --git a/react_on_rails/lib/react_on_rails/dev/server_manager.rb b/react_on_rails/lib/react_on_rails/dev/server_manager.rb
@@ -68,13 +68,26 @@ def kill_processes
         # #base_port_hash so no "Base port detected" banner prints during a kill.
         def killable_ports
           base = PortSelector.base_port_hash
-          return pro_renderer_active? ? [3000, 3001, 3800] : [3000, 3001] unless base
+          return default_killable_ports unless base
 
           ports = [base[:rails], base[:webpack]]
           ports << base[:renderer] if pro_renderer_active?
           ports
         end
 
+        def default_killable_ports
+          ports = [3000, 3001]
+          ports << configured_renderer_port_for_kill if pro_renderer_active?
+          ports
+        end
+
+        def configured_renderer_port_for_kill
+          raw_port = ENV.fetch("RENDERER_PORT", nil)
+          return raw_port.strip.to_i if valid_port_string?(raw_port)
+
+          3800
+        end
+
         def development_processes
           {
             "rails" => "Rails server",
diff --git a/react_on_rails/spec/react_on_rails/dev/file_manager_spec.rb b/react_on_rails/spec/react_on_rails/dev/file_manager_spec.rb
@@ -36,6 +36,24 @@ def write_server_pid(contents)
       server&.close
     end
 
+    it "removes an overmind socket file whose path is too long for a UNIX socket address" do
+      FileUtils.mkdir_p("tmp/sockets")
+      socket_file = "tmp/sockets/overmind-#{'a' * 100}.sock"
+      File.write(socket_file, "copied socket")
+
+      expect(described_class.cleanup_stale_files).to be true
+      expect(File).not_to exist(socket_file)
+    end
+
+    it "removes an overmind socket file when socket allocation fails" do
+      FileUtils.mkdir_p("tmp/sockets")
+      File.write("tmp/sockets/overmind.sock", "copied socket")
+      allow(Socket).to receive(:new).and_raise(Errno::EMFILE)
+
+      expect(described_class.cleanup_stale_files).to be true
+      expect(File).not_to exist("tmp/sockets/overmind.sock")
+    end
+
     it "leaves non-overmind sockets in tmp/sockets/ alone even when inactive" do
       FileUtils.mkdir_p("tmp/sockets")
       File.write("tmp/sockets/puma.sock", "stale puma socket")
diff --git a/react_on_rails/spec/react_on_rails/dev/server_manager_spec.rb b/react_on_rails/spec/react_on_rails/dev/server_manager_spec.rb
@@ -1196,6 +1196,24 @@ def mock_port_selector_defaults
 
       described_class.kill_processes
     end
+
+    it "targets the configured renderer port when Pro renderer support is active without a base port" do
+      ENV["RENDERER_PORT"] = "3900"
+      allow(ReactOnRails::Dev::PortSelector).to receive(:base_port_hash).and_return(nil)
+      allow(described_class).to receive(:pro_renderer_active?).and_return(true)
+      # No pattern-based processes so kill_port_processes runs.
+      allow(Open3).to receive(:capture2).with("pgrep", any_args).and_return(["", nil])
+
+      allow(Open3).to receive(:capture2).with("lsof", "-ti", ":3000", err: File::NULL).and_return(["", nil])
+      allow(Open3).to receive(:capture2).with("lsof", "-ti", ":3001", err: File::NULL).and_return(["", nil])
+      expect(Open3).not_to receive(:capture2).with("lsof", "-ti", ":3800", err: File::NULL)
+      allow(Open3).to receive(:capture2).with("lsof", "-ti", ":3900", err: File::NULL).and_return(["3901", nil])
+
+      allow(Process).to receive(:pid).and_return(9999)
+      expect(Process).to receive(:kill).with("TERM", 3901)
+
+      described_class.kill_processes
+    end
   end
 
   describe ".find_port_pids" do
