Fix validation to support multiple webpack patterns and add comprehensive tests

Addresses critical issues identified in code review:

**Issue 1: Regex Pattern Too Restrictive**
- Original regex only matched hardcoded path.resolve() pattern
- Real dummy app uses config.outputPath, which wasn't matched at all
- Validation would silently skip without detecting actual configs

**Fix:**
- Support hardcoded path: require('path').resolve(__dirname, '../../path')
- Detect config.outputPath usage (can't validate, inform user)
- Detect variable usage (can't validate, inform user)
- Clear messaging for each pattern type

**Issue 2: Missing Path Normalization**
- String equality comparison failed on equivalent paths
- ./ssr-generated vs ssr-generated would be false mismatch
- Trailing slashes caused false positives

**Fix:**
- normalize_path() method strips leading ./ and /
- Removes trailing slashes
- Handles whitespace
- Graceful handling of nil/non-string values

**Issue 3: No Test Coverage**
- New validation methods had zero tests
- High risk of regressions

**Fix:**
- Added 20+ comprehensive test cases covering:
  - File not found scenarios
  - Hardcoded paths (matching and mismatched)
  - config.outputPath detection
  - Variable detection
  - Error handling
  - Path normalization edge cases
  - All return nil vs success vs warning paths

**Testing Results:**
- All 20 new tests passing
- 30/31 total specs passing (1 pre-existing failure unrelated)
- Covers real-world patterns including dummy app config

**Technical Details:**
- extract_webpack_output_path() now pattern-aware
- Better user messaging for unvalidatable configs
- Maintains backward compatibility
- No breaking changes

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

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

Author: justin808

lib/react_on_rails/doctor.rb

@@ -1171,20 +1171,16 @@ def validate_server_bundle_path_sync(rails_bundle_path)
       begin
         webpack_content = File.read(webpack_config_path)
 
-        # Extract the path from webpack config
-        # Look for: path: require('path').resolve(__dirname, '../../ssr-generated')
-        path_regex = %r{path:\s*require\(['"]path['"]\)\.resolve\(__dirname,\s*['"]\.\./\.\./([^'"]+)['"]\)}
-        path_match = webpack_content.match(path_regex)
-
-        unless path_match
-          checker.add_info("\n  ℹ️  Could not parse webpack server bundle path - skipping validation")
-          return
-        end
+        # Try to extract the path from webpack config
+        webpack_bundle_path = extract_webpack_output_path(webpack_content, webpack_config_path)
+
+        return unless webpack_bundle_path
 
-        webpack_bundle_path = path_match[1]
+        # Normalize and compare paths
+        normalized_webpack_path = normalize_path(webpack_bundle_path)
+        normalized_rails_path = normalize_path(rails_bundle_path)
 
-        # Compare the paths
-        if webpack_bundle_path == rails_bundle_path
+        if normalized_webpack_path == normalized_rails_path
           checker.add_success("\n  ✅ Webpack and Rails configs are in sync (both use '#{rails_bundle_path}')")
         else
           checker.add_warning(<<~MSG.strip)
@@ -1209,6 +1205,43 @@ def validate_server_bundle_path_sync(rails_bundle_path)
         checker.add_info("\n  ℹ️  Could not validate webpack config: #{e.message}")
       end
     end
+
+    # Extract output.path from webpack config, supporting multiple patterns
+    def extract_webpack_output_path(webpack_content, _webpack_config_path)
+      # Pattern 1: path: require('path').resolve(__dirname, '../../ssr-generated')
+      hardcoded_pattern = %r{path:\s*require\(['"]path['"]\)\.resolve\(__dirname,\s*['"]\.\./\.\./([^'"]+)['"]\)}
+      if (match = webpack_content.match(hardcoded_pattern))
+        return match[1]
+      end
+
+      # Pattern 2: path: config.outputPath (can't validate - runtime value)
+      if webpack_content.match?(/path:\s*config\.outputPath/)
+        checker.add_info(<<~MSG.strip)
+          \n  ℹ️  Webpack config uses config.outputPath (from shakapacker.yml)
+          Cannot validate sync with Rails config as this is resolved at build time.
+          Ensure your shakapacker.yml public_output_path matches server_bundle_output_path.
+        MSG
+        return nil
+      end
+
+      # Pattern 3: path: some_variable (can't validate)
+      if webpack_content.match?(/path:\s*[a-zA-Z_]\w*/)
+        checker.add_info("\n  ℹ️  Webpack config uses a variable for output.path - cannot validate")
+        return nil
+      end
+
+      checker.add_info("\n  ℹ️  Could not parse webpack server bundle path - skipping validation")
+      nil
+    end
+
+    # Normalize path for comparison (remove leading ./, trailing /)
+    def normalize_path(path)
+      return path unless path.is_a?(String)
+
+      normalized = path.strip
+      normalized = normalized.sub(%r{^\.?/}, "") # Remove leading ./ or /
+      normalized.sub(%r{/$}, "") # Remove trailing /
+    end
   end
   # rubocop:enable Metrics/ClassLength
 end

spec/lib/react_on_rails/doctor_spec.rb

@@ -188,4 +188,201 @@
       end
     end
   end
+
+  describe "server bundle path validation" do
+    let(:doctor) { described_class.new }
+    let(:checker) { doctor.instance_variable_get(:@checker) }
+
+    before do
+      allow(checker).to receive(:add_info)
+      allow(checker).to receive(:add_success)
+      allow(checker).to receive(:add_warning)
+    end
+
+    describe "#validate_server_bundle_path_sync" do
+      context "when webpack config file doesn't exist" do
+        before do
+          allow(File).to receive(:exist?).with("config/webpack/serverWebpackConfig.js").and_return(false)
+        end
+
+        it "adds info message and skips validation" do
+          expected_msg = "\n  ℹ️  Webpack server config not found - skipping path validation"
+          expect(checker).to receive(:add_info).with(expected_msg)
+          doctor.send(:validate_server_bundle_path_sync, "ssr-generated")
+        end
+      end
+
+      context "when webpack config uses hardcoded path" do
+        let(:webpack_content) do
+          <<~JS
+            serverWebpackConfig.output = {
+              filename: 'server-bundle.js',
+              path: require('path').resolve(__dirname, '../../ssr-generated'),
+            };
+          JS
+        end
+
+        before do
+          allow(File).to receive(:exist?).with("config/webpack/serverWebpackConfig.js").and_return(true)
+          allow(File).to receive(:read).with("config/webpack/serverWebpackConfig.js").and_return(webpack_content)
+        end
+
+        it "reports success when paths match" do
+          expected_msg = "\n  ✅ Webpack and Rails configs are in sync (both use 'ssr-generated')"
+          expect(checker).to receive(:add_success).with(expected_msg)
+          doctor.send(:validate_server_bundle_path_sync, "ssr-generated")
+        end
+
+        it "reports warning when paths don't match" do
+          expect(checker).to receive(:add_warning).with(/Configuration mismatch detected/)
+          doctor.send(:validate_server_bundle_path_sync, "server-bundles")
+        end
+
+        it "includes both paths in warning when mismatched" do
+          expect(checker).to receive(:add_warning) do |msg|
+            expect(msg).to include('server_bundle_output_path = "server-bundles"')
+            expect(msg).to include('output.path = "ssr-generated"')
+          end
+          doctor.send(:validate_server_bundle_path_sync, "server-bundles")
+        end
+      end
+
+      context "when webpack config uses config.outputPath" do
+        let(:webpack_content) do
+          <<~JS
+            serverWebpackConfig.output = {
+              filename: 'server-bundle.js',
+              path: config.outputPath,
+            };
+          JS
+        end
+
+        before do
+          allow(File).to receive(:exist?).with("config/webpack/serverWebpackConfig.js").and_return(true)
+          allow(File).to receive(:read).with("config/webpack/serverWebpackConfig.js").and_return(webpack_content)
+        end
+
+        it "reports that it cannot validate" do
+          expect(checker).to receive(:add_info).with(/Webpack config uses config\.outputPath/)
+          doctor.send(:validate_server_bundle_path_sync, "ssr-generated")
+        end
+
+        it "does not report success or warning" do
+          expect(checker).not_to receive(:add_success)
+          expect(checker).not_to receive(:add_warning)
+          doctor.send(:validate_server_bundle_path_sync, "ssr-generated")
+        end
+      end
+
+      context "when webpack config uses a variable" do
+        let(:webpack_content) do
+          <<~JS
+            const outputPath = calculatePath();
+            serverWebpackConfig.output = {
+              path: outputPath,
+            };
+          JS
+        end
+
+        before do
+          allow(File).to receive(:exist?).with("config/webpack/serverWebpackConfig.js").and_return(true)
+          allow(File).to receive(:read).with("config/webpack/serverWebpackConfig.js").and_return(webpack_content)
+        end
+
+        it "reports that it cannot validate" do
+          expect(checker).to receive(:add_info).with(/Webpack config uses a variable/)
+          doctor.send(:validate_server_bundle_path_sync, "ssr-generated")
+        end
+      end
+
+      context "when webpack config reading fails" do
+        before do
+          allow(File).to receive(:exist?).with("config/webpack/serverWebpackConfig.js").and_return(true)
+          allow(File).to receive(:read).and_raise(StandardError, "Permission denied")
+        end
+
+        it "handles error gracefully" do
+          expect(checker).to receive(:add_info).with(/Could not validate webpack config: Permission denied/)
+          doctor.send(:validate_server_bundle_path_sync, "ssr-generated")
+        end
+      end
+    end
+
+    describe "#extract_webpack_output_path" do
+      context "with hardcoded path pattern" do
+        let(:webpack_content) do
+          "path: require('path').resolve(__dirname, '../../my-bundle-dir')"
+        end
+
+        it "extracts the path" do
+          result = doctor.send(:extract_webpack_output_path, webpack_content, "config/webpack/test.js")
+          expect(result).to eq("my-bundle-dir")
+        end
+      end
+
+      context "with config.outputPath" do
+        let(:webpack_content) { "path: config.outputPath" }
+
+        it "returns nil and adds info message" do
+          expect(checker).to receive(:add_info).with(/config\.outputPath/)
+          result = doctor.send(:extract_webpack_output_path, webpack_content, "config/webpack/test.js")
+          expect(result).to be_nil
+        end
+      end
+
+      context "with variable" do
+        let(:webpack_content) { "path: myPath" }
+
+        it "returns nil and adds info message" do
+          expect(checker).to receive(:add_info).with(/variable/)
+          result = doctor.send(:extract_webpack_output_path, webpack_content, "config/webpack/test.js")
+          expect(result).to be_nil
+        end
+      end
+
+      context "with unrecognized pattern" do
+        let(:webpack_content) { "output: {}" }
+
+        it "returns nil and adds info message" do
+          expect(checker).to receive(:add_info).with(/Could not parse/)
+          result = doctor.send(:extract_webpack_output_path, webpack_content, "config/webpack/test.js")
+          expect(result).to be_nil
+        end
+      end
+    end
+
+    describe "#normalize_path" do
+      it "removes leading ./" do
+        expect(doctor.send(:normalize_path, "./ssr-generated")).to eq("ssr-generated")
+      end
+
+      it "removes leading /" do
+        expect(doctor.send(:normalize_path, "/ssr-generated")).to eq("ssr-generated")
+      end
+
+      it "removes trailing /" do
+        expect(doctor.send(:normalize_path, "ssr-generated/")).to eq("ssr-generated")
+      end
+
+      it "handles paths with both leading and trailing slashes" do
+        expect(doctor.send(:normalize_path, "./ssr-generated/")).to eq("ssr-generated")
+      end
+
+      it "strips whitespace" do
+        expect(doctor.send(:normalize_path, "  ssr-generated  ")).to eq("ssr-generated")
+      end
+
+      it "returns unchanged path if already normalized" do
+        expect(doctor.send(:normalize_path, "ssr-generated")).to eq("ssr-generated")
+      end
+
+      it "handles nil gracefully" do
+        expect(doctor.send(:normalize_path, nil)).to be_nil
+      end
+
+      it "handles non-string values gracefully" do
+        expect(doctor.send(:normalize_path, 123)).to eq(123)
+      end
+    end
+  end
 end