Skip to content

feat: add REACT_ON_RAILS_BASE_PORT for concurrent worktree port management#3142

Open
justin808 wants to merge 1 commit intomainfrom
jg/concurrent-worktree-ports
Open

feat: add REACT_ON_RAILS_BASE_PORT for concurrent worktree port management#3142
justin808 wants to merge 1 commit intomainfrom
jg/concurrent-worktree-ports

Conversation

@justin808
Copy link
Copy Markdown
Member

@justin808 justin808 commented Apr 15, 2026

Summary

  • Adds a tool-agnostic REACT_ON_RAILS_BASE_PORT env var that derives all service ports from a single base value (Rails = base+0, webpack = base+1, renderer = base+2)
  • Also recognizes CONDUCTOR_PORT as a fallback, so Conductor.build works out of the box
  • Makes all Procfiles use ${PORT:-3000} and ${RENDERER_PORT:-3800} patterns instead of hardcoded values
  • Propagates renderer port via RENDERER_PORT and REACT_RENDERER_URL env vars for automatic Pro Node Renderer configuration

This enables coding agent tools (Conductor.build, Codex, Quad Code, etc.) to run multiple worktrees concurrently without port conflicts — just set one env var and everything works.

Changes

Core logic:

  • PortSelector: new base port priority tier (checked before explicit env vars and auto-detect)
  • ServerManager.configure_ports: sets RENDERER_PORT and REACT_RENDERER_URL when base port is active

Procfile templates (all now use env var fallbacks):

  • OSS: Procfile.dev, Procfile.dev-prod-assets, Procfile.dev-static-assets
  • Pro: Procfile.dev, Procfile.prod, Procfile.static
  • Generator (pro_setup.rb): uses ${RENDERER_PORT:-3800} in generated Procfile lines

Documentation:

  • .env.example: documents base port, RENDERER_PORT, and REACT_RENDERER_URL
  • process-managers.md: new "Coding Agent / CI Integration" and "Manual Worktree Port Setup (Pro)" sections

Test plan

  • PortSelector unit tests: base port derivation, CONDUCTOR_PORT fallback, priority ordering, out-of-range fallback, nil renderer when no base port
  • RuboCop passes on all changed files
  • All pre-commit and pre-push hooks pass
  • CI passes
  • Manual test: REACT_ON_RAILS_BASE_PORT=4000 bin/dev prints correct port assignments
  • Generator tests pass (updated assertions for new Procfile pattern)

🤖 Generated with Claude Code


Note

Medium Risk
Changes dev port-selection and environment propagation used by bin/dev, so misconfiguration could break local startup or Pro SSR renderer connectivity, but it doesn’t affect production runtime.

Overview
Adds support for a workspace-level base port (REACT_ON_RAILS_BASE_PORT, with CONDUCTOR_PORT fallback) so bin/dev can deterministically assign Rails/webpack (and Pro renderer) ports without probing or per-worktree .env files.

Updates port configuration propagation to also set RENDERER_PORT and REACT_RENDERER_URL when a base port is active, and standardizes Procfiles/generator output to use env-var fallbacks like ${PORT:-3000} and ${RENDERER_PORT:-3800}. Documentation and specs are expanded to cover the new base-port workflow and renderer manual setup.

Reviewed by Cursor Bugbot for commit 37a9ae1. Bugbot is set up for automated code reviews on this repo. Configure here.

Summary by CodeRabbit

Release Notes

  • New Features

    • Port management is now environment-driven with configurable defaults for Rails, webpack, and renderer services
    • Introduced base port configuration to derive all development ports from a single value, simplifying multi-environment setup
  • Documentation

    • Added guide on port configuration for development environments and worktree setups

…ement

Add a tool-agnostic base port mechanism so coding agent tools
(Conductor.build, Codex, Quad Code, etc.) can run multiple worktrees
simultaneously without port conflicts.

When REACT_ON_RAILS_BASE_PORT (or CONDUCTOR_PORT as fallback) is set,
PortSelector derives all service ports from fixed offsets:
  Rails = base+0, webpack = base+1, renderer = base+2

ServerManager propagates the renderer port via RENDERER_PORT and
REACT_RENDERER_URL env vars so the Pro Node Renderer works automatically.

All Procfiles now use ${PORT:-3000} and ${RENDERER_PORT:-3800} patterns
instead of hardcoded values, enabling env-var-based port overrides.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 15, 2026

Walkthrough

This PR introduces configurable port assignment for React on Rails development through a REACT_ON_RAILS_BASE_PORT environment variable. The port selector now derives fixed offsets for Rails, webpack, and renderer services from this base value. When no base port is detected, it falls back to individual environment variables or auto-detection. Multiple Procfiles and templates are updated to use environment variable substitution with defaults.

Changes

Cohort / File(s) Summary
Core Port Selection Logic
react_on_rails/lib/react_on_rails/dev/port_selector.rb, react_on_rails/lib/react_on_rails/dev/server_manager.rb
Added base port detection (REACT_ON_RAILS_BASE_PORT, CONDUCTOR_PORT) with deterministic offset derivation (+0/+1/+2). select_ports now returns a third :renderer entry; configure_ports conditionally sets RENDERER_PORT and REACT_RENDERER_URL env vars when renderer port is present.
Generator & Template Updates
react_on_rails/lib/generators/react_on_rails/pro_setup.rb, react_on_rails/lib/generators/react_on_rails/templates/base/base/.env.example
Updated Procfile.dev template to use ${RENDERER_PORT:-3800} shell expansion; added .env.example documentation and placeholders for REACT_ON_RAILS_BASE_PORT, RENDERER_PORT, and REACT_RENDERER_URL with manual worktree setup guidance.
Documentation
docs/oss/building-features/process-managers.md
Added sections describing base port configuration, fixed port offsets, automatic env var setup via bin/dev, port resolution priority chain, and manual worktree port setup example.
Test Fixtures - Core
react_on_rails/spec/dummy/Procfile.dev, Procfile.dev-prod-assets, Procfile.dev-static-assets, Procfile.dev.no.turbolinks
Updated Rails process commands to use ${PORT:-<default>} instead of fixed port values, enabling environment-driven port configuration.
Test Fixtures - Pro
react_on_rails_pro/spec/dummy/Procfile.dev, Procfile.prod, Procfile.static, react_on_rails_pro/spec/execjs-compatible-dummy/Procfile.dev, Procfile.dev-static
Updated Rails and node-renderer process commands to use ${PORT:-<default>} and ${RENDERER_PORT:-<default>} for environment-driven port assignment.
Test Specifications
react_on_rails/spec/react_on_rails/dev/port_selector_spec.rb, install_generator_spec.rb, pro_generator_examples.rb
Added test contexts for REACT_ON_RAILS_BASE_PORT and CONDUCTOR_PORT precedence, deterministic offset derivation, fallback behavior, and updated assertions for shell default expansion syntax in Procfile expectations.

Sequence Diagram(s)

sequenceDiagram
    participant dev as bin/dev
    participant ps as PortSelector
    participant sm as ServerManager
    participant svc as Services<br/>(Rails, Webpack,<br/>Node-Renderer)

    dev->>ps: select_ports()
    
    alt REACT_ON_RAILS_BASE_PORT detected
        ps->>ps: Validate base_port<br/>(1-65535)
        ps->>ps: Derive rails=base<br/>webpack=base+1<br/>renderer=base+2
        ps-->>dev: {rails, webpack,<br/>renderer}
    else CONDUCTOR_PORT detected
        ps->>ps: Derive from<br/>CONDUCTOR_PORT
        ps-->>dev: {rails, webpack,<br/>renderer}
    else Manual ENV vars or<br/>auto-detect
        alt PORT env var set
            ps->>ps: Use PORT
        else
            ps->>ps: Auto-detect free port
        end
        alt SHAKAPACKER_DEV_SERVER_PORT set
            ps->>ps: Use it for webpack
        else
            ps->>ps: Auto-detect free port
        end
        ps->>ps: renderer=nil
        ps-->>dev: {rails, webpack, nil}
    end
    
    dev->>sm: configure_ports(selected)
    
    alt selected[:renderer] present
        sm->>sm: Set ENV[RENDERER_PORT]
        sm->>sm: Set ENV[REACT_RENDERER_URL]
        sm-->>svc: Env vars ready
    else renderer absent
        sm-->>svc: No renderer config
    end
    
    svc->>svc: Bind to assigned ports
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

  • #2494: Modifies dev port handling and ServerManager's configure_ports logic, with overlapping concerns around port configuration and environment variable setup.

Poem

🐰 A base port springs forth from the env,
With offsets that perfectly bend,
Rails, webpack, and renderer align,
All dancing to \$\{PORT:-3000\}'s design,
Dev workflows now seamless, my friend! 🌱

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title accurately describes the main change: introducing REACT_ON_RAILS_BASE_PORT for concurrent worktree port management, which is the central feature across all modified files.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch jg/concurrent-worktree-ports

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

MAX_ATTEMPTS = 100
DEFAULT_RAILS_PORT = 3000
DEFAULT_WEBPACK_PORT = 3035
DEFAULT_RENDERER_PORT = 3800
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The DEFAULT_RENDERER_PORT constant is defined here but never actually referenced in code. The Procfile templates and generator both hardcode 3800 directly (e.g. ${RENDERER_PORT:-3800}). Either use this constant somewhere or remove it to avoid misleading readers into thinking it's the authoritative source.

Suggested change
DEFAULT_RENDERER_PORT = 3800
DEFAULT_RENDERER_PORT = 3800 # used as Procfile fallback only; see ${RENDERER_PORT:-3800}

Alternatively, if the constant remains, the generator template could reference it — though that requires ERB templating in the Procfile, which may not be worth the complexity.

Comment on lines +98 to +104
def base_port
BASE_PORT_ENV_VARS.each do |var|
val = ENV[var]&.to_i
return val if val&.between?(1, 65_535)
end
nil
end
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential port overflow bug. base_port validates that the base value is in 1..65535, but doesn't ensure the derived ports (base + 1, base + 2) also stay within that range. A base port of 65534 would derive a renderer port of 65536, which is an invalid TCP port.

Suggested change
def base_port
BASE_PORT_ENV_VARS.each do |var|
val = ENV[var]&.to_i
return val if val&.between?(1, 65_535)
end
nil
end
def base_port
BASE_PORT_ENV_VARS.each do |var|
val = ENV[var]&.to_i
max_base = 65_535 - BASE_PORT_RENDERER_OFFSET
return val if val&.between?(1, max_base)
end
nil
end

Comment on lines 36 to +47
def select_ports
bp = base_port
if bp
ports = {
rails: bp + BASE_PORT_RAILS_OFFSET,
webpack: bp + BASE_PORT_WEBPACK_OFFSET,
renderer: bp + BASE_PORT_RENDERER_OFFSET
}
puts "Base port #{bp} detected. Using Rails :#{ports[:rails]}, " \
"webpack :#{ports[:webpack]}, renderer :#{ports[:renderer]}"
return ports
end
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Misleading log when PORT is already set. select_ports unconditionally prints "Using Rails :4000" when a base port is active, but configure_ports in server_manager.rb uses ENV["PORT"] ||= — so if PORT is already set in the environment (e.g. PORT=3000), the actual Rails process will run on 3000, not 4000.

In practice this may not matter for the target coding-agent use-case (where PORT wouldn't be pre-set), but it's a confusing invariant violation: the log claims one port while the process uses another.

Consider either:

  1. Having select_ports check if the base-port would actually win (i.e. check that ENV["PORT"] is not already set before printing), or
  2. Moving the log message into configure_ports after the ||= assignments, so it reflects the ports that were actually applied.

Comment on lines 840 to +843
ENV["PORT"] ||= selected[:rails].to_s
ENV["SHAKAPACKER_DEV_SERVER_PORT"] ||= selected[:webpack].to_s
if selected[:renderer]
ENV["RENDERER_PORT"] ||= selected[:renderer].to_s
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

REACT_RENDERER_URL not auto-derived from an explicit RENDERER_PORT. When a base port is active, both RENDERER_PORT and REACT_RENDERER_URL are set automatically — great. But if someone manually sets RENDERER_PORT=3801 (e.g. for a non-base-port worktree), they also have to manually set REACT_RENDERER_URL=http://localhost:3801. The docs cover this in the "Manual Worktree Port Setup" section, but it's an easy mistake to miss.

A small improvement would be to derive REACT_RENDERER_URL from RENDERER_PORT when the latter is already set and the former is not:

Suggested change
ENV["PORT"] ||= selected[:rails].to_s
ENV["SHAKAPACKER_DEV_SERVER_PORT"] ||= selected[:webpack].to_s
if selected[:renderer]
ENV["RENDERER_PORT"] ||= selected[:renderer].to_s
if selected[:renderer]
ENV["RENDERER_PORT"] ||= selected[:renderer].to_s
ENV["REACT_RENDERER_URL"] ||= "http://localhost:#{selected[:renderer]}"
elsif ENV["RENDERER_PORT"] && !ENV["REACT_RENDERER_URL"]
ENV["REACT_RENDERER_URL"] = "http://localhost:#{ENV["RENDERER_PORT"]}"
end

This is a nice-to-have and can stay for a follow-up, but worth flagging.

@claude
Copy link
Copy Markdown
Contributor

claude bot commented Apr 15, 2026

Code Review

Overview

This PR adds a REACT_ON_RAILS_BASE_PORT env var (with a CONDUCTOR_PORT fallback) that lets coding-agent tools assign all service ports from a single value — Rails = base+0, webpack = base+1, Pro renderer = base+2. All Procfiles are standardized to use ${PORT:-N} patterns and generator output is updated to match. The design is clean and the scope is well-contained.

What Works Well

  • Priority chain is sensible — base port > explicit per-service vars > auto-detect. The BASE_PORT_ENV_VARS list makes it easy to add new tool-specific vars later.
  • No probing when base port is set — deterministic and fast, which is exactly what CI/agent environments need.
  • Test coverage is solid — all priority tiers, edge cases (out-of-range base, both vars set), and the renderer-nil path are covered. The around blocks cleanly save/restore env state.
  • Procfile standardization is a good cleanup${PORT:-3000} should have been the pattern all along; this PR makes it consistent across OSS and Pro fixtures.
  • configure_ports uses ||= correctly — won't stomp on env vars a user already set intentionally.

Issues

Bug (medium) — derived ports can overflow 65535.
base_port validates the raw env var is in 1..65535 but doesn't check that base + 2 (the renderer offset) also stays in range. REACT_ON_RAILS_BASE_PORT=65534 would produce a renderer port of 65536, which is invalid. The fix is to cap the valid base range at 65535 - BASE_PORT_RENDERER_OFFSET. See inline comment.

Misleading log when PORT is already in the environment.
select_ports prints "Base port 4000 detected. Using Rails :4000, webpack :4001, renderer :4002" unconditionally when a base port is found, but configure_ports uses ||= — so a pre-existing PORT=3000 would still be honoured and Rails would actually run on 3000. The log then contradicts reality. Harmless in the primary coding-agent use case (where PORT wouldn't be pre-set), but worth tightening. See inline comment.

Minor — DEFAULT_RENDERER_PORT constant is unused.
It's defined at the top of PortSelector but never referenced in code. The Procfiles and generator both hardcode 3800 inline. Either use the constant or remove it. See inline comment.

Nice-to-have — REACT_RENDERER_URL not auto-derived from explicit RENDERER_PORT.
Manual-worktree users who set RENDERER_PORT=3801 also have to remember to set REACT_RENDERER_URL=http://localhost:3801. A one-liner in configure_ports could derive it automatically. The docs cover it, but easy to miss. See inline comment (can defer to follow-up).

Test Coverage Gap

There's no test for the scenario where REACT_ON_RAILS_BASE_PORT is set and PORT is already in the environment. Given the misleading-log issue above, a test that asserts configure_ports honours the pre-existing PORT (or alternatively overrides it — whichever the intended behavior is) would prevent regressions once the behavior is clarified.

Docs

The new process-managers.md section and .env.example comments are clear and accurate. The offset table is a nice touch. One small nit: the "Priority chain" callout in the docs says "base port > explicit per-service env vars" — worth noting that this is the select_ports priority, not the final configure_ports priority (which still lets pre-existing env vars win via ||=).

Summary

The approach is sound and the feature is useful. The port-overflow bug is the one concrete correctness issue to fix before merging. The log/priority conflict is worth at least a decision: either document that base port always wins (and change ||= to = in configure_ports) or document that pre-existing env vars take final precedence (and fix the log). Everything else can be a follow-up.

🤖 Generated with Claude Code

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 37a9ae17d4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +843 to +844
ENV["RENDERER_PORT"] ||= selected[:renderer].to_s
ENV["REACT_RENDERER_URL"] ||= "http://localhost:#{selected[:renderer]}"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve renderer env vars for unbundled process managers

When a base port is used, this code sets RENDERER_PORT/REACT_RENDERER_URL, but those values are lost in the common run_process_outside_bundle path: Bundler.with_unbundled_env resets runtime env and ProcessManager::ENV_KEYS_TO_PRESERVE currently re-injects only PORT and SHAKAPACKER_DEV_SERVER_PORT (react_on_rails/lib/react_on_rails/dev/process_manager.rb). As a result, globally installed Overmind/Foreman can still start the renderer on the default 3800, so concurrent Pro worktrees continue to conflict despite a base port being set.

Useful? React with 👍 / 👎.

def base_port
BASE_PORT_ENV_VARS.each do |var|
val = ENV[var]&.to_i
return val if val&.between?(1, 65_535)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Validate base port against derived offset range

The base-port validator accepts any value up to 65535, but select_ports derives webpack/renderer with +1 and +2; for inputs like 65535, those derived ports become invalid (65536/65537) and downstream processes fail to bind. The base check should constrain input to the maximum safe value for all offsets (for current offsets, <= 65533) or validate each derived port before returning.

Useful? React with 👍 / 👎.

@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Apr 15, 2026

Greptile Summary

This PR introduces REACT_ON_RAILS_BASE_PORT (with CONDUCTOR_PORT as a fallback) so coding-agent tools can run multiple git worktrees concurrently by setting a single env var that derives all service ports via fixed offsets (Rails = base+0, webpack = base+1, renderer = base+2). Procfile templates across OSS and Pro are updated to use ${PORT:-NNNN} patterns, and ServerManager.configure_ports is extended to propagate RENDERER_PORT / REACT_RENDERER_URL automatically when a base port is active.

Confidence Score: 5/5

Safe to merge; all findings are P2 edge cases that don't affect the primary use case.

All three findings are P2: the 65534/65536 port boundary is a theoretical edge case, the production-assets mode gap is a minor scope limitation, and the ||= vs documented priority discrepancy requires both REACT_ON_RAILS_BASE_PORT and an explicit PORT to be set simultaneously. None of these block the core concurrent-worktree use case. Test coverage is solid, Procfile templates are correct, and the generator change is well-tested.

port_selector.rb (upper bound of base_port validation) and server_manager.rb (run_production_like bypass and ||= priority semantics)

Important Files Changed

Filename Overview
react_on_rails/lib/react_on_rails/dev/port_selector.rb New PortSelector class that adds base-port detection from REACT_ON_RAILS_BASE_PORT / CONDUCTOR_PORT with fixed offset derivation; base port upper bound should be 65_533 not 65_535 to keep all derived ports valid
react_on_rails/lib/react_on_rails/dev/server_manager.rb configure_ports now propagates RENDERER_PORT and REACT_RENDERER_URL from base-port selection, but run_production_like bypasses configure_ports entirely, so REACT_ON_RAILS_BASE_PORT is ignored in that mode; also
react_on_rails/spec/react_on_rails/dev/port_selector_spec.rb Comprehensive unit tests covering base port derivation, CONDUCTOR_PORT fallback, priority ordering, out-of-range fallback, and existing explicit-port flows; no coverage for base=65534 edge case
react_on_rails/lib/generators/react_on_rails/templates/base/base/.env.example Adds REACT_ON_RAILS_BASE_PORT, RENDERER_PORT, and REACT_RENDERER_URL documentation; clear and accurate
react_on_rails/lib/generators/react_on_rails/pro_setup.rb Generated Procfile.dev line for node-renderer now uses ${RENDERER_PORT:-3800}, correctly picking up the env var set by configure_ports when base port is active
docs/oss/building-features/process-managers.md Adds Coding Agent / CI Integration and Manual Worktree Port Setup sections; offset table, priority chain, and .env examples are clear and accurate

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["bin/dev starts"] --> B["configure_ports called"]
    B --> C["PortSelector.select_ports"]
    C --> D{"base_port set?\nREACT_ON_RAILS_BASE_PORT\nor CONDUCTOR_PORT"}
    D -- Yes --> E["rails = base+0\nwebpack = base+1\nrenderer = base+2\nPrint message"]
    D -- No --> F{"Explicit PORT /\nSHAKAPACKER_DEV_SERVER_PORT?"}
    F -- Yes --> G["Use explicit values\nrenderer = nil"]
    F -- No --> H["Auto-detect free ports\nfrom defaults (3000, 3035)\nrenderer = nil"]
    E --> I["configure_ports:\nENV PORT ||= rails\nENV SDSP ||= webpack\nENV RENDERER_PORT ||= renderer\nENV REACT_RENDERER_URL ||= url"]
    G --> I
    H --> I
    I --> J["Procfile.dev launched\nrails -p ${PORT:-3000}\nnode-renderer RENDERER_PORT=${RENDERER_PORT:-3800}"]
    K["bin/dev prod / production-assets"] --> L["run_production_like\ncalls find_available_port directly\n⚠️ base port ignored"]
Loading

Comments Outside Diff (2)

  1. react_on_rails/lib/react_on_rails/dev/server_manager.rb, line 838-848 (link)

    P2 ||= lets pre-existing PORT silently override base-port assignment

    When REACT_ON_RAILS_BASE_PORT=4000 and PORT=3000 are both set, PortSelector.select_ports prints "Base port 4000 detected. Using Rails :4000, webpack :4001…", but the subsequent ENV["PORT"] ||= selected[:rails].to_s is a no-op because PORT is already "3000". The Procfile's ${PORT:-3000} then runs Rails on 3000 while webpack starts on 4001 — an inconsistent, mixed-port setup. The documented priority ("base port > explicit per-service env vars") is not enforced at the assignment step.

    If the intent is that an explicit PORT in the environment should always take precedence, the PortSelector print message should be suppressed or qualified when it detects that per-service vars are already set. If base port should truly win (as documented), the assignment needs ENV["PORT"] = selected[:rails].to_s (no ||=) when base_port is active.

  2. react_on_rails/lib/react_on_rails/dev/server_manager.rb, line 595-603 (link)

    P2 REACT_ON_RAILS_BASE_PORT silently ignored in production-assets mode

    run_production_like calls PortSelector.find_available_port directly (line 603) instead of configure_ports, so REACT_ON_RAILS_BASE_PORT / CONDUCTOR_PORT and the resulting RENDERER_PORT / REACT_RENDERER_URL env vars are never applied when a user runs bin/dev prod or bin/dev production-assets. The base-port feature works for HMR and static modes but is silently skipped here. At minimum, the documentation or a code comment should note this limitation; ideally configure_ports is called (or its base-port logic extracted) for consistency.

Reviews (1): Last reviewed commit: "feat: add REACT_ON_RAILS_BASE_PORT for c..." | Re-trigger Greptile

def base_port
BASE_PORT_ENV_VARS.each do |var|
val = ENV[var]&.to_i
return val if val&.between?(1, 65_535)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Derived port can exceed 65535 with near-max base

The validation only guards the base value, but the renderer port is derived at base + 2. A base of 65534 passes the between?(1, 65_535) check, yet produces a renderer port of 65536, which is outside the valid TCP range and will fail at bind time. The upper bound should be 65_533 (i.e., 65_535 - BASE_PORT_RENDERER_OFFSET) to ensure all three derived ports stay valid.

Suggested change
return val if val&.between?(1, 65_535)
return val if val&.between?(1, 65_533)

Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 3 potential issues.

Fix All in Cursor

Bugbot Autofix is ON, but it could not run because on-demand usage is turned off. To enable Bugbot Autofix, turn on on-demand usage and set a spend limit in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 37a9ae1. Configure here.

if selected[:renderer]
ENV["RENDERER_PORT"] ||= selected[:renderer].to_s
ENV["REACT_RENDERER_URL"] ||= "http://localhost:#{selected[:renderer]}"
end
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Base-derived ports don't override pre-existing env vars

High Severity

The documentation states the priority chain is "base port > explicit per-service env vars > auto-detect," but configure_ports uses ||= for all env var assignments including the newly added RENDERER_PORT and REACT_RENDERER_URL. This means if any of these env vars are already set (e.g., from a .env file), the base-derived values are silently ignored. When a base port is active, these assignments need to be unconditional (=) to enforce the documented priority.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 37a9ae1. Configure here.

return val if val&.between?(1, 65_535)
end
nil
end
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Base port validation allows derived ports beyond 65535

Low Severity

The base_port method validates that the base value is between 1 and 65535, but derived ports (base + 1 for webpack, base + 2 for renderer) can exceed the valid port range. A base port of 65534 or 65535 would produce invalid port numbers that cause service startup failures. The upper bound check needs to account for the maximum offset (BASE_PORT_RENDERER_OFFSET).

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 37a9ae1. Configure here.

MAX_ATTEMPTS = 100
DEFAULT_RAILS_PORT = 3000
DEFAULT_WEBPACK_PORT = 3035
DEFAULT_RENDERER_PORT = 3800
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused constant DEFAULT_RENDERER_PORT is dead code

Low Severity

DEFAULT_RENDERER_PORT is defined but never referenced anywhere in the codebase. The renderer port is only derived from the base port offset and the hardcoded 3800 default lives in the Procfile shell expressions (${RENDERER_PORT:-3800}), so this constant serves no purpose.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 37a9ae1. Configure here.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@react_on_rails/lib/react_on_rails/dev/port_selector.rb`:
- Around line 98-102: The base_port method currently allows ENV values up to
65_535 but then derives additional ports as base+1 and base+2; update the
validation in base_port (and any use of BASE_PORT_ENV_VARS) to ensure the chosen
base plus the highest offset (2) stays ≤ 65_535 (i.e., accept bases in the range
1..65_533), so change the conditional that returns val to check val between 1
and (65_535 - 2) and reject/skip values outside that safe range.

In `@react_on_rails/lib/react_on_rails/dev/server_manager.rb`:
- Around line 842-845: The current code may set ENV["REACT_RENDERER_URL"] using
selected[:renderer] even when an existing ENV["RENDERER_PORT"] was preserved;
update the logic so REACT_RENDERER_URL is constructed from the effective port
value (the ENV["RENDERER_PORT"] after the ||= assignment) rather than from
selected[:renderer]; locate the block that references selected[:renderer] in
server_manager.rb (the if selected[:renderer] branch) and change the URL
construction to use ENV["RENDERER_PORT"] (or a local port variable assigned from
ENV["RENDERER_PORT"]) so the URL always matches the actual RENDERER_PORT in use.

In `@react_on_rails/spec/react_on_rails/dev/port_selector_spec.rb`:
- Around line 10-15: Wrap each around hook's example.run in a begin...ensure so
ENV cleanup always runs (e.g., change the around do |example| block that sets
ENV["REACT_ON_RAILS_BASE_PORT"] to use begin; example.run; ensure ... end), and
in the "CONDUCTOR_PORT only" and "out-of-range base port" contexts explicitly
unset or restore both ENV["REACT_ON_RAILS_BASE_PORT"] and ENV["CONDUCTOR_PORT"]
(capture old values, set the test values, run example, then in ensure restore
originals or delete the keys when nil) so base_port's lookup order cannot be
influenced by ambient env vars.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e7ac5b86-7952-413d-89b3-278d22119277

📥 Commits

Reviewing files that changed from the base of the PR and between de5b53a and 37a9ae1.

📒 Files selected for processing (17)
  • docs/oss/building-features/process-managers.md
  • react_on_rails/lib/generators/react_on_rails/pro_setup.rb
  • react_on_rails/lib/generators/react_on_rails/templates/base/base/.env.example
  • react_on_rails/lib/react_on_rails/dev/port_selector.rb
  • react_on_rails/lib/react_on_rails/dev/server_manager.rb
  • react_on_rails/spec/dummy/Procfile.dev
  • react_on_rails/spec/dummy/Procfile.dev-prod-assets
  • react_on_rails/spec/dummy/Procfile.dev-static-assets
  • react_on_rails/spec/dummy/Procfile.dev.no.turbolinks
  • react_on_rails/spec/react_on_rails/dev/port_selector_spec.rb
  • react_on_rails/spec/react_on_rails/generators/install_generator_spec.rb
  • react_on_rails/spec/react_on_rails/support/shared_examples/pro_generator_examples.rb
  • react_on_rails_pro/spec/dummy/Procfile.dev
  • react_on_rails_pro/spec/dummy/Procfile.prod
  • react_on_rails_pro/spec/dummy/Procfile.static
  • react_on_rails_pro/spec/execjs-compatible-dummy/Procfile.dev
  • react_on_rails_pro/spec/execjs-compatible-dummy/Procfile.dev-static

Comment on lines +98 to +102
def base_port
BASE_PORT_ENV_VARS.each do |var|
val = ENV[var]&.to_i
return val if val&.between?(1, 65_535)
end
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Validate base port against the highest derived offset.
Line 101 allows values up to 65_535, but this selector also derives webpack/renderer as base+1/base+2. A base like 65_534 yields invalid ports (65_536+).

💡 Suggested fix
         def base_port
+          max_base_port = 65_535 - BASE_PORT_RENDERER_OFFSET
           BASE_PORT_ENV_VARS.each do |var|
             val = ENV[var]&.to_i
-            return val if val&.between?(1, 65_535)
+            return val if val&.between?(1, max_base_port)
           end
           nil
         end
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def base_port
BASE_PORT_ENV_VARS.each do |var|
val = ENV[var]&.to_i
return val if val&.between?(1, 65_535)
end
def base_port
max_base_port = 65_535 - BASE_PORT_RENDERER_OFFSET
BASE_PORT_ENV_VARS.each do |var|
val = ENV[var]&.to_i
return val if val&.between?(1, max_base_port)
end
nil
end
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@react_on_rails/lib/react_on_rails/dev/port_selector.rb` around lines 98 -
102, The base_port method currently allows ENV values up to 65_535 but then
derives additional ports as base+1 and base+2; update the validation in
base_port (and any use of BASE_PORT_ENV_VARS) to ensure the chosen base plus the
highest offset (2) stays ≤ 65_535 (i.e., accept bases in the range 1..65_533),
so change the conditional that returns val to check val between 1 and (65_535 -
2) and reject/skip values outside that safe range.

Comment on lines +842 to +845
if selected[:renderer]
ENV["RENDERER_PORT"] ||= selected[:renderer].to_s
ENV["REACT_RENDERER_URL"] ||= "http://localhost:#{selected[:renderer]}"
end
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Keep REACT_RENDERER_URL in sync with the effective RENDERER_PORT.
At Line 843, an existing RENDERER_PORT is preserved, but Line 844 still builds URL from selected[:renderer]. That can point Rails to a different port than the renderer process.

💡 Suggested fix
           if selected[:renderer]
             ENV["RENDERER_PORT"] ||= selected[:renderer].to_s
-            ENV["REACT_RENDERER_URL"] ||= "http://localhost:#{selected[:renderer]}"
+            ENV["REACT_RENDERER_URL"] ||= "http://localhost:#{ENV['RENDERER_PORT']}"
           end
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if selected[:renderer]
ENV["RENDERER_PORT"] ||= selected[:renderer].to_s
ENV["REACT_RENDERER_URL"] ||= "http://localhost:#{selected[:renderer]}"
end
if selected[:renderer]
ENV["RENDERER_PORT"] ||= selected[:renderer].to_s
ENV["REACT_RENDERER_URL"] ||= "http://localhost:#{ENV['RENDERER_PORT']}"
end
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@react_on_rails/lib/react_on_rails/dev/server_manager.rb` around lines 842 -
845, The current code may set ENV["REACT_RENDERER_URL"] using
selected[:renderer] even when an existing ENV["RENDERER_PORT"] was preserved;
update the logic so REACT_RENDERER_URL is constructed from the effective port
value (the ENV["RENDERER_PORT"] after the ||= assignment) rather than from
selected[:renderer]; locate the block that references selected[:renderer] in
server_manager.rb (the if selected[:renderer] branch) and change the URL
construction to use ENV["RENDERER_PORT"] (or a local port variable assigned from
ENV["RENDERER_PORT"]) so the URL always matches the actual RENDERER_PORT in use.

Comment on lines +10 to +15
around do |example|
old = ENV.fetch("REACT_ON_RAILS_BASE_PORT", nil)
ENV["REACT_ON_RAILS_BASE_PORT"] = "5000"
example.run
ENV["REACT_ON_RAILS_BASE_PORT"] = old
end
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, find and examine the actual file
fd -t f port_selector_spec.rb

Repository: shakacode/react_on_rails

Length of output: 128


🏁 Script executed:

# Read the file to see the actual implementation
cat -n react_on_rails/spec/react_on_rails/dev/port_selector_spec.rb | head -100

Repository: shakacode/react_on_rails

Length of output: 4107


🏁 Script executed:

# Check if there are other test files in this directory to understand patterns
ls -la react_on_rails/spec/react_on_rails/dev/

Repository: shakacode/react_on_rails

Length of output: 623


🏁 Script executed:

# Search for other uses of around hooks in the codebase to see patterns
rg "around do \|example\|" --type rb react_on_rails/spec/ | head -20

Repository: shakacode/react_on_rails

Length of output: 94


🏁 Script executed:

# Also check if there's any documentation or comments in the codebase about env var testing
rg "ENV\[" react_on_rails/spec/react_on_rails/dev/port_selector_spec.rb -A 2 -B 2 | head -50

Repository: shakacode/react_on_rails

Length of output: 1457


🏁 Script executed:

# Look at the port_selector implementation to understand the precedence
fd -t f port_selector.rb | head -5

Repository: shakacode/react_on_rails

Length of output: 122


🏁 Script executed:

# Read the port_selector implementation to understand precedence
cat -n react_on_rails/lib/react_on_rails/dev/port_selector.rb | head -80

Repository: shakacode/react_on_rails

Length of output: 3920


🏁 Script executed:

# Also search for other around hooks using ENV in the codebase to see if there's a pattern
rg "around do" -A 6 react_on_rails/spec/ | grep -A 6 "ENV\["

Repository: shakacode/react_on_rails

Length of output: 11726


🏁 Script executed:

# Let's check if RSpec has any special handling by looking at actual test failures
# First, let me verify the base_port_env_vars order and how the method checks them
cat -n react_on_rails/lib/react_on_rails/dev/port_selector.rb | sed -n '36,50p'

Repository: shakacode/react_on_rails

Length of output: 722


🏁 Script executed:

# Let's also check the base_port method to confirm precedence
rg "def base_port" -A 10 react_on_rails/lib/react_on_rails/dev/port_selector.rb

Repository: shakacode/react_on_rails

Length of output: 375


🏁 Script executed:

# Check if there are any RSpec documentation comments or notes about around hooks
rg "around" react_on_rails/spec/ -B 2 -A 10 | grep -i "ensure\|exception\|fail" | head -20

Repository: shakacode/react_on_rails

Length of output: 784


🏁 Script executed:

# Let's examine more of the git_utils_spec to see if ensure is actually used there
cat -n react_on_rails/spec/react_on_rails/git_utils_spec.rb | grep -A 8 "around do"

Repository: shakacode/react_on_rails

Length of output: 3126


🏁 Script executed:

# Let me check the exact context of the CONDUCTOR_PORT test more carefully
cat -n react_on_rails/spec/react_on_rails/dev/port_selector_spec.rb | sed -n '42,56p'

Repository: shakacode/react_on_rails

Length of output: 680


🏁 Script executed:

# And check the render_options_spec where ensure IS used
rg "around do" -A 10 react_on_rails/spec/react_on_rails/react_component/render_options_spec.rb | head -20

Repository: shakacode/react_on_rails

Length of output: 519


🏁 Script executed:

# Check if there are any test runs or CI configs that might show expected test patterns
fd -t f "\.github\|\.circleci\|travis\|rakefile" | head -10

Repository: shakacode/react_on_rails

Length of output: 50


🏁 Script executed:

# Let me check if there's any RSpec documentation or comments in the codebase about exception handling
rg "example.run" -B 2 -A 2 react_on_rails/spec/ | grep -A 5 -B 5 "ensure\|begin"

Repository: shakacode/react_on_rails

Length of output: 4132


🏁 Script executed:

# Let me also check the git_utils tests more carefully - do they actually use ensure anywhere?
cat -n react_on_rails/spec/react_on_rails/git_utils_spec.rb | head -50

Repository: shakacode/react_on_rails

Length of output: 2245


🏁 Script executed:

# Let's also verify one more thing: what happens if a test in port_selector_spec fails?
# Check if there are any test execution logs or CI patterns
cat -n react_on_rails/spec/react_on_rails/dev/port_selector_spec.rb | tail -50

Repository: shakacode/react_on_rails

Length of output: 1902


Use ensure blocks in around hooks and explicitly unset higher-priority env vars in isolated test contexts.

The around hooks lack exception safety—if example.run raises, cleanup is skipped, leaking ENV state. Additionally, the "CONDUCTOR_PORT only" context (line 42) and "out-of-range base port" context (line 75) don't explicitly unset REACT_ON_RAILS_BASE_PORT and CONDUCTOR_PORT respectively. Since base_port checks REACT_ON_RAILS_BASE_PORT first, an ambient env var can silently change test behavior, causing flakiness.

Suggested fix pattern
 context "when CONDUCTOR_PORT is set (without REACT_ON_RAILS_BASE_PORT)" do
   around do |example|
-    old = ENV.fetch("CONDUCTOR_PORT", nil)
-    ENV["CONDUCTOR_PORT"] = "6000"
-    example.run
-    ENV["CONDUCTOR_PORT"] = old
+    old_base = ENV.fetch("REACT_ON_RAILS_BASE_PORT", nil)
+    old_conductor = ENV.fetch("CONDUCTOR_PORT", nil)
+    ENV["REACT_ON_RAILS_BASE_PORT"] = nil
+    ENV["CONDUCTOR_PORT"] = "6000"
+    begin
+      example.run
+    ensure
+      ENV["REACT_ON_RAILS_BASE_PORT"] = old_base
+      ENV["CONDUCTOR_PORT"] = old_conductor
+    end
   end

Apply the same begin ... ensure pattern to the other added around hooks (lines 10-15, 59-67, 76-81). For line 59-67 (both vars set), ensure both are restored.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
around do |example|
old = ENV.fetch("REACT_ON_RAILS_BASE_PORT", nil)
ENV["REACT_ON_RAILS_BASE_PORT"] = "5000"
example.run
ENV["REACT_ON_RAILS_BASE_PORT"] = old
end
around do |example|
old = ENV.fetch("REACT_ON_RAILS_BASE_PORT", nil)
ENV["REACT_ON_RAILS_BASE_PORT"] = "5000"
begin
example.run
ensure
ENV["REACT_ON_RAILS_BASE_PORT"] = old
end
end
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@react_on_rails/spec/react_on_rails/dev/port_selector_spec.rb` around lines 10
- 15, Wrap each around hook's example.run in a begin...ensure so ENV cleanup
always runs (e.g., change the around do |example| block that sets
ENV["REACT_ON_RAILS_BASE_PORT"] to use begin; example.run; ensure ... end), and
in the "CONDUCTOR_PORT only" and "out-of-range base port" contexts explicitly
unset or restore both ENV["REACT_ON_RAILS_BASE_PORT"] and ENV["CONDUCTOR_PORT"]
(capture old values, set the test values, run example, then in ensure restore
originals or delete the keys when nil) so base_port's lookup order cannot be
influenced by ambient env vars.

ihabadham added a commit that referenced this pull request Apr 15, 2026
## Summary

- Applies `prettier --write` to five tracked docs files that are
currently out of compliance on `main`.
- No content changes — only whitespace/table-alignment fixes produced by
Prettier.

## Why this is needed

The formatting drift landed via a docs-only merge to `main`. The
`lint-js-and-ruby.yml` workflow is conditionally skipped for docs-only
commits on `main` (see the job condition at
[`.github/workflows/lint-js-and-ruby.yml:79-87`](https://github.com/shakacode/react_on_rails/blob/main/.github/workflows/lint-js-and-ruby.yml#L79-L87)),
so the check never ran on merge. It surfaces on every subsequent
code-touching PR because the lint job runs for those (e.g.
[#3151](#3151),
[#3148](#3148),
[#3142](#3142),
[#3097](#3097)).

Merging this unblocks the `build` → `Check formatting` step on those
PRs.

## Test plan

- [x] `pnpm start format.listDifferent` passes for the five files
- [x] Diff is purely whitespace — no link targets, code blocks, or prose
changed

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

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Documentation**
* Improved formatting across API reference, core concepts, migration,
and RSC docs: refined table layouts, adjusted column alignment, and
standardized emphasis for clearer warnings.
* Updated link text and phrasing for several guides for clearer
navigation.
* Reformatted code examples and JSX/Route snippets for more compact,
consistent presentation and improved readability.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant