Optimize process manager performance and robustness

This commit improves the process manager implementation with several optimizations:

Performance improvements:
- Eliminate redundant system calls in run_process_if_available
- Streamlined control flow with early returns
- More efficient process availability checking

Robustness enhancements:
- Support multiple version flags per process (--version, -v, -V)
- Process-specific version flag mapping for better compatibility
- Improved fallback strategy that avoids double execution

Code quality:
- Cleaner, more readable method structure
- Better separation of concerns
- Enhanced test coverage for version flag handling
- RuboCop compliant code style

The implementation is now more efficient and robust while maintaining
full backward compatibility and comprehensive bundler context handling.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>

Author: AbanoubGhadban

lib/react_on_rails/dev/process_manager.rb

@@ -30,48 +30,51 @@ def run_with_process_manager(procfile)
           # Clean up stale files before starting
           FileManager.cleanup_stale_files
 
-          if process_available?("overmind")
-            run_process("overmind", ["start", "-f", procfile])
-          elsif process_available?("foreman")
-            run_process("foreman", ["start", "-f", procfile])
-          else
-            show_process_manager_installation_help
-            exit 1
-          end
+          # 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 a simple flag to see if it works
+          # 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
-          system(process, "--version", out: File::NULL, err: File::NULL)
+          version_flags_for(process).any? do |flag|
+            system(process, flag, out: File::NULL, err: File::NULL)
+          end
         rescue Errno::ENOENT
           false
         end
 
-        # Check if a process is available in either current context or system-wide
-        def process_available?(process)
-          installed?(process) || process_available_in_system?(process)
+        # Get appropriate version flags for different processes
+        def version_flags_for(process)
+          case process
+          when "overmind"
+            ["--version"]
+          when "foreman"
+            ["--version", "-v"]
+          else
+            ["--version", "-v", "-V"]
+          end
         end
 
-        # Try to run a process with intelligent fallback strategy
-        # First attempt: within current context (for processes that are in the current bundle)
-        # Fallback: outside bundler context (for system-installed processes)
-        def run_process(process, args)
-          success = if installed?(process)
-                      # Process works in current context - use it directly
-                      system(process, *args)
-                    else
-                      false
-                    end
-
-          # If current context failed or process not available, try system process
-          return if success
-
-          run_process_outside_bundle(process, args)
+        # 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
@@ -89,13 +92,13 @@ def run_process_outside_bundle(process, args)
 
         # Check if a process is available system-wide (outside bundle context)
         def process_available_in_system?(process)
-          if defined?(Bundler)
-            Bundler.with_unbundled_env do
-              # Use system() directly to check if process exists outside bundler context
-              system(process, "--version", out: File::NULL, err: File::NULL)
+          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|
+              system(process, flag, out: File::NULL, err: File::NULL)
             end
-          else
-            false
           end
         rescue Errno::ENOENT
           false

spec/react_on_rails/dev/process_manager_spec.rb

@@ -30,11 +30,23 @@
       expect(described_class.installed?("nonexistent")).to be false
     end
 
-    it "returns false when process returns false" do
+    it "returns false when all version flags fail" do
       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_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
   end
 
   describe ".ensure_procfile" do
@@ -58,88 +70,65 @@
     end
 
     it "uses overmind when available" do
-      allow(described_class).to receive(:process_available?).with("overmind").and_return(true)
-      expect(described_class).to receive(:run_process).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 and foreman is available" do
-      allow(described_class).to receive(:process_available?).with("overmind").and_return(false)
-      allow(described_class).to receive(:process_available?).with("foreman").and_return(true)
-      expect(described_class).to receive(:run_process).with("foreman", ["start", "-f", "Procfile.dev"])
+      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(:process_available?).with("overmind").and_return(false)
-      allow(described_class).to receive(:process_available?).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(:process_available?).with("overmind").and_return(true)
-      allow(described_class).to receive(:run_process)
+      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 ".process_available?" do
-    it "returns true when process is available in current context" do
+  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)
-      allow(described_class).to receive(:process_available_in_system?).with("foreman").and_return(false)
+      expect_any_instance_of(Kernel).to receive(:system).with("foreman", "start", "-f", "Procfile.dev").and_return(true)
 
-      expect(described_class.send(:process_available?, "foreman")).to be true
+      result = described_class.send(:run_process_if_available, "foreman", ["start", "-f", "Procfile.dev"])
+      expect(result).to be true
     end
 
-    it "returns true when process is available system-wide" do
+    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)
 
-      expect(described_class.send(:process_available?, "foreman")).to be true
-    end
-
-    it "returns false when process is not available anywhere" 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(false)
-
-      expect(described_class.send(:process_available?, "foreman")).to be false
-    end
-  end
-
-  describe ".run_process" do
-    before do
-      allow_any_instance_of(Kernel).to receive(:system).and_return(true)
-    end
-
-    it "tries current context first when process works there" 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)
-      expect(described_class).not_to receive(:run_process_outside_bundle)
-
-      described_class.send(:run_process, "foreman", ["start", "-f", "Procfile.dev"])
-    end
-
-    it "falls back to system process when current context fails" 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(false)
-      expect(described_class).to receive(:run_process_outside_bundle).with("foreman", ["start", "-f", "Procfile.dev"])
-
-      described_class.send(:run_process, "foreman", ["start", "-f", "Procfile.dev"])
+      result = described_class.send(:run_process_if_available, "foreman", ["start", "-f", "Procfile.dev"])
+      expect(result).to be true
     end
 
-    it "uses system process directly when not available in current context" do
-      allow(described_class).to receive(:installed?).with("overmind").and_return(false)
-      expect(described_class).to receive(:run_process_outside_bundle).with("overmind", ["start", "-f", "Procfile.dev"])
+    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)
 
-      described_class.send(:run_process, "overmind", ["start", "-f", "Procfile.dev"])
+      result = described_class.send(:run_process_if_available, "nonexistent", ["start"])
+      expect(result).to be false
     end
   end
 
@@ -162,7 +151,7 @@
   end
 
   describe ".process_available_in_system?" do
-    it "checks process availability outside bundle context" 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
@@ -178,14 +167,30 @@
       expect(described_class.send(:process_available_in_system?, "foreman")).to be false
     end
 
-    it "returns false when process fails outside bundle context" do
+    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_any_instance_of(Kernel).to receive(:system)
-        .with("overmind", "--version", out: File::NULL, err: File::NULL).and_return(false)
+        .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
+  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
 
-      expect(described_class.send(:process_available_in_system?, "overmind")).to be false
+    it "returns generic flags for unknown processes" do
+      expect(described_class.send(:version_flags_for, "unknown")).to eq(["--version", "-v", "-V"])
     end
   end