Fix: Universal process manager bundler context fallback

lib/react_on_rails/dev/process_manager.rb

@@ -1,14 +1,15 @@
 # frozen_string_literal: true
 
+require "timeout"
+
 module ReactOnRails
   module Dev
     class ProcessManager
       class << self
+        # Check if a process is available and usable in the current execution context
+        # This accounts for bundler context where system commands might be intercepted
         def installed?(process)
-          IO.popen([process, "-v"], &:close)
-          true
-        rescue Errno::ENOENT
-          false
+          installed_in_current_context?(process)
         end
 
         def ensure_procfile(procfile)
@@ -31,20 +32,111 @@ def run_with_process_manager(procfile)
           # Clean up stale files before starting
           FileManager.cleanup_stale_files
 
-          if installed?("overmind")
-            system("overmind", "start", "-f", procfile)
-          elsif installed?("foreman")
-            system("foreman", "start", "-f", procfile)
+          # Try process managers in order of preference
+          return if run_process_if_available("overmind", ["start", "-f", procfile])
+          return if run_process_if_available("foreman", ["start", "-f", procfile])
+
+          show_process_manager_installation_help
+          exit 1
+        end
+
+        private
+
+        # Check if a process is actually usable in the current execution context
+        # This is important for commands that might be intercepted by bundler
+        def installed_in_current_context?(process)
+          # Try to execute the process with version flags to see if it works
+          # Use system() because that's how we'll actually call it later
+          version_flags_for(process).any? do |flag|
+            # Add timeout to prevent hanging on version checks
+            Timeout.timeout(5) do
+              system(process, flag, out: File::NULL, err: File::NULL)
+            end
+          end
+        rescue Errno::ENOENT, Timeout::Error
+          false
+        end
+
+        # Get appropriate version flags for different processes
+        def version_flags_for(process)
+          case process
+          when "overmind"
+            ["--version"]
+          when "foreman"
+            ["--version", "-v"]
           else
-            warn <<~MSG
-              NOTICE:
-              For this script to run, you need either 'overmind' or 'foreman' installed on your machine. Please try this script after installing one of them.
-            MSG
-            exit 1
+            ["--version", "-v", "-V"]
           end
         end
 
-        private
+        # Try to run a process if it's available, with intelligent fallback strategy
+        # Returns true if process was found and executed, false if not available
+        def run_process_if_available(process, args)
+          # First attempt: try in current context (works for bundled processes)
+          return system(process, *args) if installed?(process)
+
+          # Second attempt: try in system context (works for system-installed processes)
+          return run_process_outside_bundle(process, args) if process_available_in_system?(process)
+
+          # Process not available in either context
+          false
+        end
+
+        # Run a process outside of bundler context using Bundler.with_unbundled_env
+        # This allows using system-installed processes even when they're not in the Gemfile
+        def run_process_outside_bundle(process, args)
+          if defined?(Bundler)
+            Bundler.with_unbundled_env do
+              system(process, *args)
+            end
+          else
+            # Fallback if Bundler is not available
+            system(process, *args)
+          end
+        end
+
+        # Check if a process is available system-wide (outside bundle context)
+        def process_available_in_system?(process)
+          return false unless defined?(Bundler)
+
+          Bundler.with_unbundled_env do
+            # Try version flags to check if process exists outside bundler context
+            version_flags_for(process).any? do |flag|
+              # Add timeout to prevent hanging on version checks
+              Timeout.timeout(5) do
+                system(process, flag, out: File::NULL, err: File::NULL)
+              end
+            end
+          end
+        rescue Errno::ENOENT, Timeout::Error
+          false
+        end
+
+        # Improved error message with helpful guidance
+        def show_process_manager_installation_help
+          warn <<~MSG
+            ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+            ERROR: Process Manager Not Found
+            ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+            To run the React on Rails development server, you need either 'overmind' or
+            'foreman' installed on your system.
+
+            RECOMMENDED INSTALLATION:
+
+              macOS:    brew install overmind
+              Linux:    gem install foreman
+
+            IMPORTANT:
+            DO NOT add foreman to your Gemfile. Install it globally on your system.
+
+            For more information about why foreman should not be in your Gemfile, see:
+            https://github.com/shakacode/react_on_rails/blob/master/docs/javascript/foreman-issues.md
+
+            After installation, try running this script again.
+            ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+          MSG
+        end
 
         def valid_procfile_path?(procfile)
           # Reject paths with shell metacharacters

spec/react_on_rails/dev/process_manager_spec.rb

@@ -18,15 +18,42 @@
   end
 
   describe ".installed?" do
-    it "returns true when process is available" do
-      allow(IO).to receive(:popen).with(["overmind", "-v"]).and_return("Some version info")
+    it "returns true when process is available in current context" do
+      expect(Timeout).to receive(:timeout).with(5).and_yield
+      expect_any_instance_of(Kernel).to receive(:system)
+        .with("overmind", "--version", out: File::NULL, err: File::NULL).and_return(true)
       expect(described_class).to be_installed("overmind")
     end
 
-    it "returns false when process is not available" do
-      allow(IO).to receive(:popen).with(["nonexistent", "-v"]).and_raise(Errno::ENOENT)
+    it "returns false when process is not available in current context" do
+      expect(Timeout).to receive(:timeout).with(5).and_raise(Errno::ENOENT)
       expect(described_class.installed?("nonexistent")).to be false
     end
+
+    it "returns false when all version flags fail" do
+      expect(Timeout).to receive(:timeout).with(5).exactly(3).times.and_yield
+      expect_any_instance_of(Kernel).to receive(:system)
+        .with("failing_process", "--version", out: File::NULL, err: File::NULL).and_return(false)
+      expect_any_instance_of(Kernel).to receive(:system)
+        .with("failing_process", "-v", out: File::NULL, err: File::NULL).and_return(false)
+      expect_any_instance_of(Kernel).to receive(:system)
+        .with("failing_process", "-V", out: File::NULL, err: File::NULL).and_return(false)
+      expect(described_class.installed?("failing_process")).to be false
+    end
+
+    it "returns true when second version flag succeeds" do
+      expect(Timeout).to receive(:timeout).with(5).twice.and_yield
+      expect_any_instance_of(Kernel).to receive(:system)
+        .with("foreman", "--version", out: File::NULL, err: File::NULL).and_return(false)
+      expect_any_instance_of(Kernel).to receive(:system)
+        .with("foreman", "-v", out: File::NULL, err: File::NULL).and_return(true)
+      expect(described_class.installed?("foreman")).to be true
+    end
+
+    it "returns false when version check times out" do
+      expect(Timeout).to receive(:timeout).with(5).and_raise(Timeout::Error)
+      expect(described_class.installed?("hanging_process")).to be false
+    end
   end
 
   describe ".ensure_procfile" do
@@ -50,33 +77,149 @@
     end
 
     it "uses overmind when available" do
-      allow(described_class).to receive(:installed?).with("overmind").and_return(true)
-      expect_any_instance_of(Kernel).to receive(:system).with("overmind", "start", "-f", "Procfile.dev")
+      expect(described_class).to receive(:run_process_if_available)
+        .with("overmind", ["start", "-f", "Procfile.dev"]).and_return(true)
 
       described_class.run_with_process_manager("Procfile.dev")
     end
 
-    it "uses foreman when overmind not available" do
-      allow(described_class).to receive(:installed?).with("overmind").and_return(false)
-      allow(described_class).to receive(:installed?).with("foreman").and_return(true)
-      expect_any_instance_of(Kernel).to receive(:system).with("foreman", "start", "-f", "Procfile.dev")
+    it "uses foreman when overmind not available and foreman is available" do
+      expect(described_class).to receive(:run_process_if_available)
+        .with("overmind", ["start", "-f", "Procfile.dev"]).and_return(false)
+      expect(described_class).to receive(:run_process_if_available)
+        .with("foreman", ["start", "-f", "Procfile.dev"]).and_return(true)
 
       described_class.run_with_process_manager("Procfile.dev")
     end
 
     it "exits with error when no process manager available" do
-      allow(described_class).to receive(:installed?).with("overmind").and_return(false)
-      allow(described_class).to receive(:installed?).with("foreman").and_return(false)
+      expect(described_class).to receive(:run_process_if_available)
+        .with("overmind", ["start", "-f", "Procfile.dev"]).and_return(false)
+      expect(described_class).to receive(:run_process_if_available)
+        .with("foreman", ["start", "-f", "Procfile.dev"]).and_return(false)
+      expect(described_class).to receive(:show_process_manager_installation_help)
       expect_any_instance_of(Kernel).to receive(:exit).with(1)
 
       described_class.run_with_process_manager("Procfile.dev")
     end
 
     it "cleans up stale files before starting" do
-      allow(described_class).to receive(:installed?).with("overmind").and_return(true)
+      allow(described_class).to receive(:run_process_if_available).and_return(true)
       expect(ReactOnRails::Dev::FileManager).to receive(:cleanup_stale_files)
 
       described_class.run_with_process_manager("Procfile.dev")
     end
   end
+
+  describe ".run_process_if_available" do
+    it "returns true and runs process when available in current context" do
+      allow(described_class).to receive(:installed?).with("foreman").and_return(true)
+      expect_any_instance_of(Kernel).to receive(:system).with("foreman", "start", "-f", "Procfile.dev").and_return(true)
+
+      result = described_class.send(:run_process_if_available, "foreman", ["start", "-f", "Procfile.dev"])
+      expect(result).to be true
+    end
+
+    it "tries system context when not available in current context" do
+      allow(described_class).to receive(:installed?).with("foreman").and_return(false)
+      allow(described_class).to receive(:process_available_in_system?).with("foreman").and_return(true)
+      expect(described_class).to receive(:run_process_outside_bundle)
+        .with("foreman", ["start", "-f", "Procfile.dev"]).and_return(true)
+
+      result = described_class.send(:run_process_if_available, "foreman", ["start", "-f", "Procfile.dev"])
+      expect(result).to be true
+    end
+
+    it "returns false when process not available anywhere" do
+      allow(described_class).to receive(:installed?).with("nonexistent").and_return(false)
+      allow(described_class).to receive(:process_available_in_system?).with("nonexistent").and_return(false)
+
+      result = described_class.send(:run_process_if_available, "nonexistent", ["start"])
+      expect(result).to be false
+    end
+  end
+
+  describe ".run_process_outside_bundle" do
+    it "uses Bundler.with_unbundled_env when Bundler is available" do
+      bundler_double = class_double(Bundler)
+      stub_const("Bundler", bundler_double)
+      expect(bundler_double).to receive(:with_unbundled_env).and_yield
+      expect_any_instance_of(Kernel).to receive(:system).with("foreman", "start", "-f", "Procfile.dev")
+
+      described_class.send(:run_process_outside_bundle, "foreman", ["start", "-f", "Procfile.dev"])
+    end
+
+    it "falls back to direct system call when Bundler is not available" do
+      hide_const("Bundler")
+      expect_any_instance_of(Kernel).to receive(:system).with("foreman", "start", "-f", "Procfile.dev")
+
+      described_class.send(:run_process_outside_bundle, "foreman", ["start", "-f", "Procfile.dev"])
+    end
+  end
+
+  describe ".process_available_in_system?" do
+    it "checks process availability outside bundle context with version flags" do
+      bundler_double = class_double(Bundler)
+      stub_const("Bundler", bundler_double)
+      expect(bundler_double).to receive(:with_unbundled_env).and_yield
+      expect(Timeout).to receive(:timeout).with(5).and_yield
+      expect_any_instance_of(Kernel).to receive(:system)
+        .with("foreman", "--version", out: File::NULL, err: File::NULL).and_return(true)
+
+      expect(described_class.send(:process_available_in_system?, "foreman")).to be true
+    end
+
+    it "returns false when Bundler is not available" do
+      hide_const("Bundler")
+
+      expect(described_class.send(:process_available_in_system?, "foreman")).to be false
+    end
+
+    it "tries multiple version flags before failing" do
+      bundler_double = class_double(Bundler)
+      stub_const("Bundler", bundler_double)
+      expect(bundler_double).to receive(:with_unbundled_env).and_yield
+      expect(Timeout).to receive(:timeout).with(5).twice.and_yield
+      expect_any_instance_of(Kernel).to receive(:system)
+        .with("foreman", "--version", out: File::NULL, err: File::NULL).and_return(false)
+      expect_any_instance_of(Kernel).to receive(:system)
+        .with("foreman", "-v", out: File::NULL, err: File::NULL).and_return(true)
+
+      expect(described_class.send(:process_available_in_system?, "foreman")).to be true
+    end
+
+    it "returns false when version check times out in system context" do
+      bundler_double = class_double(Bundler)
+      stub_const("Bundler", bundler_double)
+      expect(bundler_double).to receive(:with_unbundled_env).and_yield
+      expect(Timeout).to receive(:timeout).with(5).and_raise(Timeout::Error)
+
+      expect(described_class.send(:process_available_in_system?, "hanging_process")).to be false
+    end
+  end
+
+  describe ".version_flags_for" do
+    it "returns specific flags for overmind" do
+      expect(described_class.send(:version_flags_for, "overmind")).to eq(["--version"])
+    end
+
+    it "returns multiple flags for foreman" do
+      expect(described_class.send(:version_flags_for, "foreman")).to eq(["--version", "-v"])
+    end
+
+    it "returns generic flags for unknown processes" do
+      expect(described_class.send(:version_flags_for, "unknown")).to eq(["--version", "-v", "-V"])
+    end
+  end
+
+  describe ".show_process_manager_installation_help" do
+    it "displays helpful error message with installation instructions" do
+      expect { described_class.send(:show_process_manager_installation_help) }
+        .to output(/ERROR: Process Manager Not Found/).to_stderr
+      expect { described_class.send(:show_process_manager_installation_help) }
+        .to output(/DO NOT add foreman to your Gemfile/).to_stderr
+      expect { described_class.send(:show_process_manager_installation_help) }
+        .to output(/foreman-issues\.md/).to_stderr
+    end
+  end
 end