fix: use env-var-driven ports in Procfile templates to support multiple worktrees#2539
fix: use env-var-driven ports in Procfile templates to support multiple worktrees#2539
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds automatic dev port selection: new ReactOnRails::Dev::PortSelector probes and returns Rails/webpack port pairs (honoring ENV overrides), ServerManager applies them to ENV before starting, Procfile templates and generator use env-driven ports with a new Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer (bin/dev)
participant SM as ServerManager
participant PS as PortSelector
participant Rails as Rails server
participant Webpack as webpack-dev-server
Dev->>SM: start dev
SM->>PS: select_ports()
PS-->>SM: { rails_port, webpack_port }
SM->>SM: set ENV["PORT"], ENV["SHAKAPACKER_DEV_SERVER_PORT"]
SM->>Rails: launch (reads ENV["PORT"])
SM->>Webpack: launch (reads ENV["SHAKAPACKER_DEV_SERVER_PORT"])
Rails-->>Dev: server bound
Webpack-->>Dev: dev-server bound
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 60573e791f
ℹ️ 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".
Greptile SummaryThis PR replaces hardcoded port numbers in all Procfile templates with POSIX
Confidence Score: 4/5
Sequence DiagramsequenceDiagram
participant Dev as Developer
participant Env as .env file
participant FM as Foreman / Overmind
participant Rails as Rails server
participant WDS as shakapacker-dev-server
Dev->>Env: Copy .env.example → .env<br/>(set PORT=3001, SHAKAPACKER_DEV_SERVER_PORT=3036)
Dev->>FM: bin/dev (reads Procfile.dev)
FM->>Env: Auto-load .env variables
FM->>Rails: bundle exec rails s -p ${PORT:-3000}<br/>resolves to 3001
FM->>WDS: bin/shakapacker-dev-server<br/>picks up SHAKAPACKER_DEV_SERVER_PORT=3036
Rails-->>Dev: Listening on :3001
WDS-->>Dev: Listening on :3036
Last reviewed commit: 60573e7 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
react_on_rails/spec/react_on_rails/support/shared_examples/base_generator_examples.rb (1)
26-28: Consider adding test coverage forProcfile.dev-prod-assets.
Procfile.dev-prod-assetsuses${PORT:-3001}(a different default), but there's no corresponding test assertion. For completeness, you could add:it "uses env-var-driven port in Procfile.dev-prod-assets" do assert_file "Procfile.dev-prod-assets", /\$\{PORT:-3001\}/ 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/support/shared_examples/base_generator_examples.rb` around lines 26 - 28, Add a parallel test to the existing spec that asserts Procfile.dev-prod-assets uses the env-var-driven port default ${PORT:-3001}; specifically, add an example similar to the existing it "uses env-var-driven port in Procfile.dev-static-assets" that calls assert_file "Procfile.dev-prod-assets", /\$\{PORT:-3001\}/ so the generator spec covers both Procfile.dev-static-assets and Procfile.dev-prod-assets.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@react_on_rails/spec/react_on_rails/support/shared_examples/base_generator_examples.rb`:
- Around line 26-28: Add a parallel test to the existing spec that asserts
Procfile.dev-prod-assets uses the env-var-driven port default ${PORT:-3001};
specifically, add an example similar to the existing it "uses env-var-driven
port in Procfile.dev-static-assets" that calls assert_file
"Procfile.dev-prod-assets", /\$\{PORT:-3001\}/ so the generator spec covers both
Procfile.dev-static-assets and Procfile.dev-prod-assets.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: af442d57-4e15-4abf-9f68-5e7f785fea48
📒 Files selected for processing (7)
docs/building-features/process-managers.mdreact_on_rails/lib/generators/react_on_rails/base_generator.rbreact_on_rails/lib/generators/react_on_rails/templates/base/base/.env.examplereact_on_rails/lib/generators/react_on_rails/templates/base/base/Procfile.devreact_on_rails/lib/generators/react_on_rails/templates/base/base/Procfile.dev-prod-assetsreact_on_rails/lib/generators/react_on_rails/templates/base/base/Procfile.dev-static-assetsreact_on_rails/spec/react_on_rails/support/shared_examples/base_generator_examples.rb
Review SummaryThe approach is sound — using Bugs1. URL is displayed before ports are configured (affects UX correctness) In both 2.
Code Quality3. 4. Design / Documentation5.
6. Documentation overstates auto-detection ( The docs say |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
react_on_rails/lib/react_on_rails/dev/server_manager.rb (1)
616-618:⚠️ Potential issue | 🟡 MinorConfigure ports before printing the access URL.
Line 617 and Line 650 print the URL before Line 643 and Line 658 select/update
ENV["PORT"], so the banner can show a stale/default port when auto-shifting occurs.🧭 Suggested ordering fix
def run_static_development(procfile, verbose: false, route: nil, skip_database_check: false) - print_procfile_info(procfile, route: route) - # Check database setup before starting exit 1 unless DatabaseChecker.check_database(skip: skip_database_check) # Check required services before starting exit 1 unless ServiceChecker.check_services + + configure_ports + print_procfile_info(procfile, route: route) @@ - configure_ports PackGenerator.generate(verbose: verbose)def run_development(procfile, verbose: false, route: nil, skip_database_check: false) - print_procfile_info(procfile, route: route) - # Check database setup before starting exit 1 unless DatabaseChecker.check_database(skip: skip_database_check) # Check required services before starting exit 1 unless ServiceChecker.check_services + + configure_ports + print_procfile_info(procfile, route: route) - configure_ports PackGenerator.generate(verbose: verbose)Also applies to: 643-643, 649-651, 658-658
🤖 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 616 - 618, The banner URL is printed by print_procfile_info before the code selects/updates ENV["PORT"], so it can show a stale port; in run_static_development (and the other affected call sites) move the port-selection/configuration logic (the code that selects/updates ENV["PORT"]) to run before print_procfile_info so the printed access URL uses the updated port value; update all analogous places (the other calls that print the URL at lines noted) to ensure ENV["PORT"] is configured prior to calling print_procfile_info.
🧹 Nitpick comments (1)
react_on_rails/spec/react_on_rails/dev/server_manager_spec.rb (1)
101-104: Restore prior ENV values instead of always deleting.Lines 101-104 can mutate global test process state when
PORT/SHAKAPACKER_DEV_SERVER_PORTwere already set before this context.♻️ Suggested test isolation improvement
- after do - ENV.delete("PORT") - ENV.delete("SHAKAPACKER_DEV_SERVER_PORT") - end + around do |example| + old_port = ENV.fetch("PORT", nil) + old_webpack_port = ENV.fetch("SHAKAPACKER_DEV_SERVER_PORT", nil) + example.run + ENV["PORT"] = old_port + ENV["SHAKAPACKER_DEV_SERVER_PORT"] = old_webpack_port + 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/server_manager_spec.rb` around lines 101 - 104, The teardown currently deletes ENV["PORT"] and ENV["SHAKAPACKER_DEV_SERVER_PORT"], which can wipe pre-existing environment values; instead capture and store the original values at the start of the example/context (e.g. prev_port and prev_shakapacker_port in the surrounding before/let) and in the after hook restore them (ENV["PORT"] = prev_port if prev_port; ENV.delete("PORT") if prev_port.nil?, same for "SHAKAPACKER_DEV_SERVER_PORT") so the after hook restores prior ENV state rather than always deleting; update the spec's after block that references ENV.delete("PORT") / ENV.delete("SHAKAPACKER_DEV_SERVER_PORT") accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/building-features/process-managers.md`:
- Around line 343-345: The fenced code block containing the text "Default ports
in use. Using Rails :3001, webpack :3036" lacks a language tag and triggers
MD040; update that fenced block by adding a language identifier (e.g., "text")
after the opening triple backticks so the block becomes ```text ... ```,
ensuring the linter passes while preserving the existing content.
In `@react_on_rails/lib/react_on_rails/dev/server_manager.rb`:
- Around line 692-696: The configure_ports method currently lets
PortSelector.select_ports propagate NoPortAvailable; catch NoPortAvailable
inside configure_ports, emit a clear user-facing error (e.g., using STDERR.puts
or the existing logger/say helper) that explains no free ports were found and
lists the attempted ports if available, then terminate with a non-zero exit
(exit 1) so the CLI exits cleanly; keep the normal behavior (setting ENV["PORT"]
and ENV["SHAKAPACKER_DEV_SERVER_PORT"]) when select_ports succeeds and reference
PortSelector.select_ports, configure_ports, and the NoPortAvailable exception in
your change.
---
Outside diff comments:
In `@react_on_rails/lib/react_on_rails/dev/server_manager.rb`:
- Around line 616-618: The banner URL is printed by print_procfile_info before
the code selects/updates ENV["PORT"], so it can show a stale port; in
run_static_development (and the other affected call sites) move the
port-selection/configuration logic (the code that selects/updates ENV["PORT"])
to run before print_procfile_info so the printed access URL uses the updated
port value; update all analogous places (the other calls that print the URL at
lines noted) to ensure ENV["PORT"] is configured prior to calling
print_procfile_info.
---
Nitpick comments:
In `@react_on_rails/spec/react_on_rails/dev/server_manager_spec.rb`:
- Around line 101-104: The teardown currently deletes ENV["PORT"] and
ENV["SHAKAPACKER_DEV_SERVER_PORT"], which can wipe pre-existing environment
values; instead capture and store the original values at the start of the
example/context (e.g. prev_port and prev_shakapacker_port in the surrounding
before/let) and in the after hook restore them (ENV["PORT"] = prev_port if
prev_port; ENV.delete("PORT") if prev_port.nil?, same for
"SHAKAPACKER_DEV_SERVER_PORT") so the after hook restores prior ENV state rather
than always deleting; update the spec's after block that references
ENV.delete("PORT") / ENV.delete("SHAKAPACKER_DEV_SERVER_PORT") accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f2c26a46-af15-4e4d-a80c-f45c0a76e014
📒 Files selected for processing (6)
docs/building-features/process-managers.mdreact_on_rails/lib/react_on_rails/dev.rbreact_on_rails/lib/react_on_rails/dev/port_selector.rbreact_on_rails/lib/react_on_rails/dev/server_manager.rbreact_on_rails/spec/react_on_rails/dev/port_selector_spec.rbreact_on_rails/spec/react_on_rails/dev/server_manager_spec.rb
PR Review: env-var-driven ports for worktree supportOverall this is a well-thought-out feature with good test coverage. The approach of using env-var-driven Procfile defaults and delegating port discovery to Critical: The generator copies Significant:
Significant:
Minor: redundant Minor: TOCTOU race in |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/spec/react_on_rails/dev/server_manager_spec.rb`:
- Around line 306-307: The tests for .procfile_port leak ENV["PORT"] because
only an after hook deletes it; add a before hook (or an around that
saves/restores ENV["PORT"]) to ensure PORT is cleared before each example so
examples that assume "PORT is unset" aren't order-dependent—specifically update
the hooks around the .procfile_port examples (replace or augment the existing
after { ENV.delete("PORT") } with before { ENV.delete("PORT") } or an around
that saves original_port = ENV["PORT"]; ENV.delete("PORT"); example.run;
ENV["PORT"] = original_port) to guarantee a clean environment for each test.
- Around line 101-104: The test currently only deletes ENV["PORT"] and
ENV["SHAKAPACKER_DEV_SERVER_PORT"] in the after do block, which allows leaked
env vars to affect the example; add a before do that clears ENV["PORT"] and
ENV["SHAKAPACKER_DEV_SERVER_PORT"] as well so each example starts with a clean
environment. Update the same context/spec where the existing after do is defined
(referencing the after do that calls ENV.delete for those keys) to include a
matching before do that calls ENV.delete for the same keys before running the
example.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c2a67669-5fe2-4d21-80ac-be208206b89f
📒 Files selected for processing (2)
react_on_rails/spec/react_on_rails/dev/port_selector_spec.rbreact_on_rails/spec/react_on_rails/dev/server_manager_spec.rb
🚧 Files skipped from review as they are similar to previous changes (1)
- react_on_rails/spec/react_on_rails/dev/port_selector_spec.rb
There was a problem hiding this comment.
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 48-54: The current explicit_rails_port and explicit_webpack_port
helpers only check for positive integers, letting values > 65,535 slip through;
update both methods to validate the TCP port range by accepting the parsed
integer only if it falls within 1..65_535 (e.g., replace the positive? check
with a range/between? check) so they return the port when valid and nil
otherwise.
- Around line 22-29: The current logic in port selection returns a fixed default
for the unset side which can still collide or produce identical rails/webpack
ports; change the single-ENV branches in the port selection logic so that when
only rails_port is set you anchor rails_port and call a new helper (e.g.,
find_available_port) to probe from DEFAULT_WEBPACK_PORT upward (skipping the
anchored rails_port) and vice versa when only webpack_port is set probe for
rails starting at DEFAULT_RAILS_PORT (skipping the anchored webpack_port);
implement find_available_port(start_port, exclude:) that uses port_available?
and raises NoPortAvailable if it cannot find a free port within a reasonable
range so the returned hash { rails: ..., webpack: ... } never collides or
returns identical ports.
In `@react_on_rails/lib/react_on_rails/dev/server_manager.rb`:
- Around line 623-625: The startup prints a stale URL because print_server_info
still falls back to 3000 instead of using the port chosen by configure_ports;
update the call site to pass the resolved port into print_server_info (or modify
print_server_info signature to read ENV["PORT"] after configure_ports) so both
print_procfile_info and print_server_info use the same port—refer to
configure_ports, print_procfile_info, and print_server_info and change the call
to pass the selected port (e.g., port: ENV["PORT"] or the local port variable)
so the banner URLs match.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c563bcc2-aa4b-4679-b025-f05ed814c083
📒 Files selected for processing (7)
docs/building-features/process-managers.mdreact_on_rails/lib/generators/react_on_rails/base_generator.rbreact_on_rails/lib/generators/react_on_rails/templates/base/base/.env.examplereact_on_rails/lib/react_on_rails/dev/port_selector.rbreact_on_rails/lib/react_on_rails/dev/server_manager.rbreact_on_rails/spec/react_on_rails/dev/server_manager_spec.rbreact_on_rails/spec/react_on_rails/support/shared_examples/base_generator_examples.rb
✅ Files skipped from review due to trivial changes (1)
- react_on_rails/lib/generators/react_on_rails/templates/base/base/.env.example
🚧 Files skipped from review as they are similar to previous changes (4)
- react_on_rails/spec/react_on_rails/dev/server_manager_spec.rb
- react_on_rails/spec/react_on_rails/support/shared_examples/base_generator_examples.rb
- docs/building-features/process-managers.md
- react_on_rails/lib/generators/react_on_rails/base_generator.rb
There was a problem hiding this comment.
🧹 Nitpick comments (1)
react_on_rails/lib/react_on_rails/dev/port_selector.rb (1)
15-18: Keepselect_portsdocs aligned with actual probing behavior.The comment on Line 17 says probing happens only when both env vars are unset, but Lines 26-33 also probe when only one side is set.
✏️ Suggested doc fix
- # Only probes when both are unset (i.e. user hasn't configured them). + # Probes when one or both ports are unset (i.e. incomplete/no user configuration).🤖 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 15 - 18, The docstring for select_ports is inaccurate: it says probing occurs only when both ENV['PORT'] and ENV['SHAKAPACKER_DEV_SERVER_PORT'] are unset, but the implementation (see select_ports) also probes for a free port for whichever side is unset while respecting the other env var; update the comment to state that the method returns { rails: Integer, webpack: Integer }, respects any set ENV['PORT'] or ENV['SHAKAPACKER_DEV_SERVER_PORT'], and probes only for the unset side(s) (i.e., will probe one side if the other is configured, or both if neither is set).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@react_on_rails/lib/react_on_rails/dev/port_selector.rb`:
- Around line 15-18: The docstring for select_ports is inaccurate: it says
probing occurs only when both ENV['PORT'] and ENV['SHAKAPACKER_DEV_SERVER_PORT']
are unset, but the implementation (see select_ports) also probes for a free port
for whichever side is unset while respecting the other env var; update the
comment to state that the method returns { rails: Integer, webpack: Integer },
respects any set ENV['PORT'] or ENV['SHAKAPACKER_DEV_SERVER_PORT'], and probes
only for the unset side(s) (i.e., will probe one side if the other is
configured, or both if neither is set).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e9a5aefe-ebb1-4419-bcda-bf08798377c6
📒 Files selected for processing (4)
react_on_rails/lib/react_on_rails/dev/port_selector.rbreact_on_rails/lib/react_on_rails/dev/server_manager.rbreact_on_rails/spec/react_on_rails/dev/port_selector_spec.rbreact_on_rails/spec/react_on_rails/dev/server_manager_spec.rb
🚧 Files skipped from review as they are similar to previous changes (2)
- react_on_rails/spec/react_on_rails/dev/server_manager_spec.rb
- react_on_rails/spec/react_on_rails/dev/port_selector_spec.rb
PR Review:
|
…upport * origin/master: (38 commits) fix: use env-var-driven ports in Procfile templates to support multiple worktrees (#2539) Fix prettier formatting in auto-bundling doc Docs: Clarify .client/.server suffixes vs use client RSC directive (#2406) Warn against using .server/.client variants with RSC features Docs: move internal-only docs out of published docs trees (#2414) Fix crash when HTTPX::ErrorResponse is returned in get_form_body_for_file (#2532) Skip flaky external URLs in lychee checks (#2547) Update mise docs: prefer shell-level shims over conductor-exec (#2537) Document compression middleware compatibility with streaming SSR (#2544) Fix duplicate node-renderer message reporting in render failures (#2531) Fix private_output_path not configured on fresh Shakapacker installs (#2289) Bump the npm-security group across 1 directory with 3 updates (#2387) docs: use https links (#2518) Consolidate changelog to keep only rc6 prerelease (#2533) Fix CSS module class name divergence with rspack SSR (#2489) Bump version to 16.4.0.rc.6 Add BugBot license scanning config (#2515) Fix buildVM promise cleanup ordering (#2483) (#2484) Fix streaming SSR hangs and silent error absorption in RSC payload injection (#2407) Ensure lefthook uses mise tools in non-interactive shells (#2512) ... # Conflicts: # CHANGELOG.md
Add entries for user-visible changes since v16.4.0.rc.6: - #2539: env-var-driven ports in Procfile templates - #2417: rspack generator config path fix - #2419: precompile hook load-based execution fix - #2577: create-react-on-rails-app validation improvements - #2416: StreamResponse status fallback fix (Pro) - #2566: empty-string license plan mismatch fix (Pro) - Updated #2561 entry to include #2568 contributor credit Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
## Summary - Add changelog entries for 6 user-visible PRs merged since v16.4.0.rc.6 that were missing from `[Unreleased]` - Update existing #2561 entry to include #2568 contributor credit ### New entries added | Section | PR | Description | |---|---|---| | Added | #2539 | Environment-variable-driven ports in Procfile templates | | Fixed | #2417 | Rspack generator config path fix | | Fixed | #2419 | Precompile hook load-based execution fix | | Fixed | #2577 | `create-react-on-rails-app` validation improvements | | Pro Fixed | #2416 | StreamResponse status fallback fix | | Pro Fixed | #2566 | Empty-string license plan mismatch fix | ### Skipped PRs (not user-visible) Docs (#2406, #2414, #2479, #2494, #2518, #2537, #2544), CI/internal (#2533, #2547, #2555, #2557, #2558, #2564), dependabot (#2387, #2541), dev dependencies (#2559, #2569, #2573). ## Test plan - [ ] Verify changelog formatting matches existing entries - [ ] Verify all user-visible PRs since v16.4.0.rc.6 are covered 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Documentation-only changelog updates with no runtime or build behavior changes. > > **Overview** > Updates `CHANGELOG.md`’s **[Unreleased]** section to include previously missing user-facing entries: Procfile templates now support env-driven ports, several generator/`bin/dev` precompile-hook and rspack-path fixes are documented, and `create-react-on-rails-app` validation improvements are noted. > > Also adds two Pro fix entries (StreamResponse status fallback and license plan empty-string validation) and updates the existing `bin/dev` precompile-hook entry to credit an additional PR/contributor. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit e75d2b5. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
## Summary - Fix `ProcessManager#run_process_outside_bundle` losing `PORT` and `SHAKAPACKER_DEV_SERVER_PORT` env vars when running foreman/overmind inside `Bundler.with_unbundled_env` ## Problem When foreman is system-installed (recommended by foreman docs — "Don't Bundle Foreman"), `ProcessManager` runs it inside `Bundler.with_unbundled_env`. This restores the pre-Bundler env snapshot, wiping any env vars set **after** `Bundler.setup` — including `PORT` and `SHAKAPACKER_DEV_SERVER_PORT` set by `PortSelector.configure_ports`. **Result:** Auto-detected ports from `PortSelector` never reach foreman's child processes: - Foreman injects its default `PORT=5000`, overriding the selected port - Webpack dev server falls back to hardcoded `3035` from `shakapacker.yml` This breaks the zero-config automatic port selection added in #2539. ## Root Cause `Bundler.with_unbundled_env` restores a snapshot of `ENV` captured **before** `Bundler.setup` ran. Any env var set at runtime after that point is `nil` inside the block. This is [documented Bundler behavior](https://bundler.io/man/bundle-exec.1.html) and a [known issue](rails/spring#669) across the ecosystem. ## Fix Capture the port env vars **before** entering the unbundled block and pass them explicitly to `system()` as an env hash — using Ruby's `Kernel#system(env, command, *args)` signature. This is the same pattern used by: - **Rails' `bundle_command`** in railties ([8.0](https://github.com/rails/rails/blob/v8.0.2/railties/lib/rails/generators/app_base.rb#L657), [8.1](https://github.com/rails/rails/blob/v8.1.2/railties/lib/rails/generators/bundle_helper.rb)) - **Spring's** process spawning with `Process.spawn(env_hash, ...)` - **Puma's** `BundlePruner` (captures `GEM_HOME`/`BUNDLE_GEMFILE` before `with_unbundled_env`) - **This codebase's own `PackGenerator`** (`run_via_bundle_exec` passes env hash to `system()` inside `with_unbundled_context`) ## Test plan Tested manually with a Rails app where foreman is system-installed (not in Gemfile): - [x] Occupied ports 3000/3035 with dummy listeners - [x] Ran `bin/dev` — `PortSelector` detected conflict, selected 3001/3036 - [x] **Before fix:** Rails bound to :5000 (foreman default), webpack crashed on :3035 - [x] **After fix:** Rails bound to :3001, webpack to :3036 — both correct 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Development environment variables for the app server PORT and the dev-server port are now preserved when launching local development processes, so custom port settings persist across restarts. * **Tests** * Updated tests to accept an environment/options hash when asserting the launched process, and to verify that the relevant port variables are included only when set. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2 participants
Summary
${PORT:-N}instead of hardcoded ports, so developers can runbin/devin multiple git worktrees simultaneously without port conflictsPortSelectorclass that auto-detects free ports when defaults (3000/3035) are occupied — no.envfile needed for the common casePortSelectorintoServerManagersobin/devsetsPORTandSHAKAPACKER_DEV_SERVER_PORTbefore foreman/overmind starts.env.exampletemplate (generated byrails g react_on_rails:install) documenting manual port overrides for users who prefer explicit controlPORT=5000into child processes whenENV["PORT"]is unset, overriding the${PORT:-N}fallbacks in Procfiles. NowPORTis always set before foreman startsHow it works
Zero-config (auto-detection): When
bin/devstarts,PortSelector.select_portsprobes 3000 and 3035. If either is occupied (checked on both IPv4 and IPv6 loopback), it scans upward for the next free port. The selected ports are set asPORTandSHAKAPACKER_DEV_SERVER_PORTbefore foreman launches.Manual override: If
PORTorSHAKAPACKER_DEV_SERVER_PORTare already set (via.envor shell), auto-detection is skipped for that port. Foreman and Overmind both read.envautomatically. Shakapacker already supportsSHAKAPACKER_DEV_SERVER_PORTon both the Ruby proxy and JS webpack-dev-server sides.# optional worktree 2 .env (only needed if you want specific ports) PORT=3001 SHAKAPACKER_DEV_SERVER_PORT=3036Test Plan
base_generator_commonexamples assert${PORT:-3000}and.env.example)Procfile.devcontains${PORT:-3000}instead of hardcoded3000.env.exampleis present with port documentationbin/devin two worktrees with different.envfiles starts without port conflictsSummary by CodeRabbit
New Features
Documentation
Tests