Conversation
🚀 Quick Review App CommandsWelcome! Here are the commands you can use in this PR:
|
WalkthroughThis PR upgrades Rails from version 8.0 to 8.1.2, adds bootsnap for performance optimization, and refactors Rails configuration across multiple files. It removes deprecated initializers and frozen_string_literal directives, updates environment-specific settings, introduces a new CI pipeline configuration, and adjusts Puma thread handling. Changes
Sequence Diagram(s)sequenceDiagram
participant CI as CI Runner
participant Setup as Setup Step
participant RuboCop as RuboCop<br/>(Ruby Styling)
participant ESLint as ESLint<br/>(JS Styling)
participant RSpec as RSpec Tests
participant Jest as Jest Tests
CI->>Setup: Run setup step
Setup-->>CI: Setup complete
CI->>RuboCop: Run Ruby style check
RuboCop-->>CI: RuboCop results
CI->>ESLint: Run JS style check (yarn)
ESLint-->>CI: ESLint results
CI->>RSpec: Run server-side tests
RSpec-->>CI: RSpec results
CI->>Jest: Run client-side tests
Jest-->>CI: Jest results
CI->>CI: Evaluate all results
alt All pass
CI-->>CI: Pipeline success
else Any failure
CI-->>CI: Pipeline failure
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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}
| config.log_tags = [:request_id] | ||
| config.logger = ActiveSupport::TaggedLogging.logger($stdout) | ||
| config.log_tags = [ :request_id ] | ||
| config.logger = ActiveSupport::TaggedLogging.logger(STDOUT) |
There was a problem hiding this comment.
Minor: prefer $stdout over the STDOUT constant for logger output. $stdout is the global variable and can be redirected (e.g., in tests or when capturing output), while STDOUT is hardwired to the process's original stdout.
| config.logger = ActiveSupport::TaggedLogging.logger(STDOUT) | |
| config.logger = ActiveSupport::TaggedLogging.logger($stdout) |
| # Specifies the `worker_timeout` threshold that Puma will use to wait before | ||
| # terminating a worker in development environments. | ||
| worker_timeout 3600 if ENV.fetch("RAILS_ENV", "development") == "development" | ||
| threads_count = ENV.fetch("RAILS_MAX_THREADS", 3) |
There was a problem hiding this comment.
The default thread count dropped from 5 to 3. That's fine for I/O-light apps, but it's worth a note for anyone running in production — applications that do significant DB/network I/O will likely want to bump RAILS_MAX_THREADS back up to 5 (or match their database.yml pool size). Consider adding a comment to that effect.
Also, the previous config had:
worker_timeout 3600 if ENV.fetch("RAILS_ENV", "development") == "development"Removing worker_timeout means long-running requests in development (slow SSR render, debugger pause, etc.) can now be killed by Puma's default 60-second timeout. If developers hit mysterious disconnects, this will be why.
| config.silence_healthcheck_path = "/up" | ||
|
|
||
| # Don't log any deprecations. | ||
| config.active_support.report_deprecations = false |
There was a problem hiding this comment.
Completely silencing deprecations in production hides future upgrade pain. Rails deprecation warnings often signal things that will break in the next major version. A less drastic option is to keep them as notifications (:notify) so they're captured by error trackers (Sentry, Honeybadger, etc.) without cluttering logs:
| config.active_support.report_deprecations = false | |
| config.active_support.deprecation = :notify |
If the goal is purely to cut log noise, leaving report_deprecations = false is fine, but it should be a deliberate trade-off.
|
|
||
| # Action Cable endpoint configuration | ||
|
|
||
| config.action_cable.url = "wss://#{ENV.fetch('PRODUCTION_HOST', nil)}/cable" |
There was a problem hiding this comment.
If PRODUCTION_HOST is not set, ENV.fetch('PRODUCTION_HOST', nil) returns nil, producing "wss:///cable" and ["https://"] — both silently broken values that will cause Action Cable to reject all connections without a helpful error. ENV.fetch without a fallback (or with an explicit error) would catch this at boot time:
| config.action_cable.url = "wss://#{ENV.fetch('PRODUCTION_HOST', nil)}/cable" | |
| config.action_cable.url = "wss://#{ENV.fetch('PRODUCTION_HOST')}/cable" | |
| config.action_cable.allowed_request_origins = ["https://#{ENV.fetch('PRODUCTION_HOST')}"] |
This pre-existing issue survives the refactor — worth fixing here since the lines are already being touched.
| end | ||
|
|
||
| # Change to :null_store to avoid any caching. | ||
| config.cache_store = :memory_store |
There was a problem hiding this comment.
The cache store is now unconditionally :memory_store. Previously rails dev:cache would switch between :memory_store and :null_store. After this change, rails dev:cache still toggles perform_caching, but the store itself is always in-memory. This means fragments/HTTP caching can "accidentally" land in the memory store even when fragment caching is off (e.g., lower-level Rails.cache.fetch calls). The comment acknowledges this is intentional, but it's a subtle semantic difference worth calling out in the PR description.
| @@ -0,0 +1,6 @@ | |||
| #!/usr/bin/env ruby | |||
| require_relative "../config/boot" | |||
| require "active_support/continuous_integration" | |||
There was a problem hiding this comment.
active_support/continuous_integration was added in Rails 8.1 — good. One thing to be aware of: bin/ci loads the full config/boot.rb (and thus bootsnap), so it must be run after bundle install. If CI machines run this before dependencies are in place, the require will fail. Standard CI practice (bundle install && bin/ci) handles this, but a comment or a guard would help newcomers.
Also, the constant CI = ActiveSupport::ContinuousIntegration assignment at the top level is fine locally, but conflicts with the common CI environment variable convention (ENV["CI"] is set to "true" by most CI providers). This is a name collision risk if any code later does CI expecting a boolean-like env value.
Greptile SummaryThis PR upgrades Rails from 8.1.1 to 8.1.2, adds bootsnap for faster boot times, and modernizes the configuration files to align with the Rails 8.1 template — removing several now-redundant legacy initializers and refreshing Key changes:
Confidence Score: 3/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[bin/ci] -->|require| B[config/boot.rb]
B -->|require bootsnap/setup| C[Bootsnap Cache]
A -->|require| D[ActiveSupport::ContinuousIntegration]
A -->|require| E[config/ci.rb]
E --> F["step: bin/setup --skip-server"]
F --> G["step: bin/rubocop"]
G --> H["step: yarn lint:eslint"]
H --> I["step: bin/rspec ❌ currently fails"]
I --> J["step: yarn test:client"]
K[config/application.rb] -->|load_defaults 8.1| L[Rails Defaults]
L --> M["to_time_preserves_timezone: :zone\n(was: false — silent change)"]
L --> N[cookies_serializer: :json]
L --> O[per_form_csrf_tokens: true]
L --> P[forgery_protection_origin_check: true]
Last reviewed commit: "Upgrade Rails to 8.1..." |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| class Application < Rails::Application | ||
| # Initialize configuration defaults for originally generated Rails version. | ||
| config.load_defaults 8.0 | ||
| config.load_defaults 8.1 |
There was a problem hiding this comment.
The previous application.rb explicitly set config.active_support.to_time_preserves_timezone = false, which caused Time objects to be converted to UTC when calling to_time. Removing this setting means the behavior now falls back to the load_defaults 8.1 default, which is :zone (timezone-preserving). This is a silent behavioral change: any code that relied on #to_time returning UTC will now return a time in the original timezone instead.
If the application doesn't have timezone-sensitive logic, this is safe. Otherwise, consider explicitly setting this to either false or :zone to document the intended behavior:
# In config/application.rb, inside the Application class
config.active_support.to_time_preserves_timezone = :zone # or false if UTC behavior is needed| config.log_tags = [:request_id] | ||
| config.logger = ActiveSupport::TaggedLogging.logger($stdout) | ||
| config.log_tags = [ :request_id ] | ||
| config.logger = ActiveSupport::TaggedLogging.logger(STDOUT) |
There was a problem hiding this comment.
STDOUT constant vs $stdout global
The logger is now pointing at the STDOUT constant rather than the $stdout global variable. $stdout is the conventional Ruby I/O global that can be reassigned (useful in tests to capture output), whereas STDOUT is the fixed file descriptor that bypasses any redirection. This is a minor change but can cause friction in test environments that redirect $stdout.
| config.logger = ActiveSupport::TaggedLogging.logger(STDOUT) | |
| config.logger = ActiveSupport::TaggedLogging.logger($stdout) |
| step "Style: Ruby", "bin/rubocop" | ||
| step "Style: JavaScript", "yarn lint:eslint" | ||
|
|
||
| step "Tests: RSpec", "bin/rspec" |
There was a problem hiding this comment.
The PR description explicitly states: "bundle exec rspec currently fails because SSR test bundle ./../generated/server-bundle-generated.js is missing in test setup." The bin/rspec step is not commented out, so running bin/ci as-is will always fail on this step.
If the intent is to land the config modernization before fixing the test suite, consider either commenting out this step temporarily or adding a skip condition:
# step "Tests: RSpec", "bin/rspec" # TODO: re-enable once SSR bundle is generated in CI setupThis ensures the CI script is actually runnable end-to-end in its current state.
|
PR Review: Rails 8.1.2 Upgrade + Config Modernization. Overall this is a clean upgrade — see inline comments for specifics. Summary below. MUST-FIX: PRODUCTION_HOST nil-safety (production.rb:94) — ENV.fetch with nil fallback silently produces wss:///cable; remove the fallback to fail loudly at boot. SHOULD-FIX: (1) Puma worker_timeout removed — drops effective dev timeout to 60s from 3600s, causing unexpected disconnects for slow SSR or debugger pauses; (2) frozen_string_literal magic comment stripped from 6 config files — looks incidental to the upgrade, easy to restore. MINOR: (1) STDOUT vs dollar-stdout in production logger (inline suggestion); (2) report_deprecations=false hides future upgrade signals, consider :notify; (3) listen gem remains in Gemfile after EventedFileUpdateChecker was removed from dev config; (4) CI constant in bin/ci shadows the CI env var set by GitHub Actions; (5) dev cache_store is now unconditionally :memory_store — rails dev:cache only toggles perform_caching now, not the store; (6) Puma thread default lowered 5 to 3 — fine for most workloads but production I/O-heavy apps should match database.yml pool size. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
config/puma.rb (1)
1-3:⚠️ Potential issue | 🟡 MinorAdd missing frozen string literal comment to fix CI failure.
The pipeline is failing due to the missing
frozen_string_literalcomment. This needs to be added to pass the Lint CI check.Proposed fix
+# frozen_string_literal: true + # This configuration file will be evaluated by Puma. The top-level methods that # are invoked here are part of Puma's configuration DSL. For more information # about methods provided by the DSL, see https://puma.io/puma/Puma/DSL.html.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/puma.rb` around lines 1 - 3, CI is failing because the Puma config is missing the frozen string literal magic comment; edit the Puma configuration file (puma.rb) and add the frozen string literal comment (frozen_string_literal: true) as the very first line of the file (before any other comments or code) so the linter passes.config/application.rb (1)
1-1:⚠️ Potential issue | 🟠 MajorRestore frozen string literal comment for lint compliance.
Line 1 is failing
Style/FrozenStringLiteralCommentin CI for this file.Proposed fix
+# frozen_string_literal: true + require_relative "boot"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/application.rb` at line 1, Add the frozen string literal magic comment as the very first line of the file to satisfy Style/FrozenStringLiteralComment; specifically insert "# frozen_string_literal: true" above the existing require_relative "boot" statement so the file (starting with the require_relative "boot" line) complies with the linter.config/environments/test.rb (1)
1-1:⚠️ Potential issue | 🟠 MajorRe-add frozen string literal pragma to unblock lint.
RuboCop is failing on Line 1 (
Style/FrozenStringLiteralComment) for this file.Proposed fix
+# frozen_string_literal: true + # The test environment is used exclusively to run your application's🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/environments/test.rb` at line 1, RuboCop is failing for Style/FrozenStringLiteralComment because the frozen string literal pragma is missing; add the magic comment "# frozen_string_literal: true" as the very first line of this file (above the existing comment) so the linter is satisfied.config/initializers/content_security_policy.rb (1)
1-1:⚠️ Potential issue | 🟠 MajorAdd frozen string literal pragma to fix CI lint failure.
This file is currently failing
Style/FrozenStringLiteralCommentat Line 1.Proposed fix
+# frozen_string_literal: true + # Be sure to restart your server when you modify this file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/initializers/content_security_policy.rb` at line 1, This file is failing the Style/FrozenStringLiteralComment cop; open content_security_policy.rb and add the standard Ruby frozen string literal magic comment as the very first line (before the existing "Be sure to restart your server..." comment) so the file begins with the pragma and then the rest of the file; no other changes are required.
🧹 Nitpick comments (1)
config/ci.rb (1)
4-10: Usebin/conductor-execfor CI command invocations.Wrap CI step commands so Ruby/Node tool versions are consistently sourced from the project toolchain rather than ambient system versions.
Proposed refactor
- step "Setup", "bin/setup --skip-server" + step "Setup", "bin/conductor-exec bin/setup --skip-server" - step "Style: Ruby", "bin/rubocop" - step "Style: JavaScript", "yarn lint:eslint" + step "Style: Ruby", "bin/conductor-exec bin/rubocop" + step "Style: JavaScript", "bin/conductor-exec yarn lint:eslint" - step "Build: Test assets", "yarn build:test" - step "Tests: RSpec", "bin/rspec" - step "Tests: Jest", "yarn test:client" + step "Build: Test assets", "bin/conductor-exec yarn build:test" + step "Tests: RSpec", "bin/conductor-exec bin/rspec" + step "Tests: Jest", "bin/conductor-exec yarn test:client"Based on learnings: Use bin/conductor-exec wrapper to run commands in pre-commit hooks and CI/CD pipelines to ensure correct tool versions are loaded (Ruby, Node, pnpm) instead of system versions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/ci.rb` around lines 4 - 10, Replace direct tool invocations in the CI steps with the conductor wrapper so project toolchain versions are used: update each step call (e.g., the step entries "Setup" with "bin/setup --skip-server", "Style: Ruby" with "bin/rubocop", "Style: JavaScript" with "yarn lint:eslint", "Tests: RSpec" with "bin/rspec", and "Tests: Jest" with "yarn test:client") to invoke the wrapper first (i.e., run them via bin/conductor-exec -- <original command>) so the existing step declarations still run the same commands but through bin/conductor-exec.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@config/boot.rb`:
- Line 1: Add the frozen string literal magic comment as the very first line of
the file to satisfy RuboCop's Style/FrozenStringLiteralComment (place the
comment before the existing ENV["BUNDLE_GEMFILE"] ||=
File.expand_path("../Gemfile", __dir__) line); ensure the exact frozen string
literal comment is used and committed so CI lint passes.
In `@config/ci.rb`:
- Line 1: Add the frozen string literal magic comment as the first line of the
file to satisfy the Style/FrozenStringLiteralComment lint; update the top of
config/ci.rb so the file begins with a single-line comment disabling mutability
(i.e., add the frozen string literal comment) before any other content or
comments.
- Around line 9-10: Add a CI step that compiles test assets before running tests
so the SSR bundle and client assets exist; insert a "Build Test Assets" step
(running your repo's client and/or server build commands) immediately before the
existing step "Tests: RSpec", "bin/rspec" and before "Tests: Jest", "yarn
test:client" in config/ci.rb so both RSpec and Jest run against a built test
environment (ensure the build commands match your project, e.g., client build
and any assets:precompile).
In `@config/initializers/filter_parameter_logging.rb`:
- Around line 6-8: The symbol list in filter_parameter_logging.rb uses explicit
symbol array syntax (e.g., :passw, :email, :secret, :token, :_key, :crypt,
:salt, :certificate, :otp, :ssn, :cvv, :cvc) which triggers the
Style/SymbolArray RuboCop offense; replace the explicit array with the percent
literal form %i[...] containing the same identifiers to satisfy the linter and
preserve behavior (keep the same symbol names and ordering).
In `@Gemfile`:
- Around line 17-20: Bundler/OrderedGems lint fails because gem "bootsnap"
appears after "thruster"; open the Gemfile and move the gem "bootsnap", require:
false entry so it appears before gem "thruster", keeping gem "puma" position
intact, then run bundle install / bundler lint to verify the ordering fix.
---
Outside diff comments:
In `@config/application.rb`:
- Line 1: Add the frozen string literal magic comment as the very first line of
the file to satisfy Style/FrozenStringLiteralComment; specifically insert "#
frozen_string_literal: true" above the existing require_relative "boot"
statement so the file (starting with the require_relative "boot" line) complies
with the linter.
In `@config/environments/test.rb`:
- Line 1: RuboCop is failing for Style/FrozenStringLiteralComment because the
frozen string literal pragma is missing; add the magic comment "#
frozen_string_literal: true" as the very first line of this file (above the
existing comment) so the linter is satisfied.
In `@config/initializers/content_security_policy.rb`:
- Line 1: This file is failing the Style/FrozenStringLiteralComment cop; open
content_security_policy.rb and add the standard Ruby frozen string literal magic
comment as the very first line (before the existing "Be sure to restart your
server..." comment) so the file begins with the pragma and then the rest of the
file; no other changes are required.
In `@config/puma.rb`:
- Around line 1-3: CI is failing because the Puma config is missing the frozen
string literal magic comment; edit the Puma configuration file (puma.rb) and add
the frozen string literal comment (frozen_string_literal: true) as the very
first line of the file (before any other comments or code) so the linter passes.
---
Nitpick comments:
In `@config/ci.rb`:
- Around line 4-10: Replace direct tool invocations in the CI steps with the
conductor wrapper so project toolchain versions are used: update each step call
(e.g., the step entries "Setup" with "bin/setup --skip-server", "Style: Ruby"
with "bin/rubocop", "Style: JavaScript" with "yarn lint:eslint", "Tests: RSpec"
with "bin/rspec", and "Tests: Jest" with "yarn test:client") to invoke the
wrapper first (i.e., run them via bin/conductor-exec -- <original command>) so
the existing step declarations still run the same commands but through
bin/conductor-exec.
🪄 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: 8c392598-2b93-4e35-b815-0fb90ce5779f
⛔ Files ignored due to path filters (1)
Gemfile.lockis excluded by!**/*.lock
📒 Files selected for processing (20)
Gemfilebin/cibin/rubocopconfig/application.rbconfig/boot.rbconfig/ci.rbconfig/environment.rbconfig/environments/development.rbconfig/environments/production.rbconfig/environments/test.rbconfig/initializers/application_controller_renderer.rbconfig/initializers/assets.rbconfig/initializers/content_security_policy.rbconfig/initializers/cookies_serializer.rbconfig/initializers/cors.rbconfig/initializers/filter_parameter_logging.rbconfig/initializers/inflections.rbconfig/initializers/per_form_csrf_tokens.rbconfig/initializers/request_forgery_protection.rbconfig/puma.rb
💤 Files with no reviewable changes (8)
- config/initializers/per_form_csrf_tokens.rb
- config/initializers/cookies_serializer.rb
- config/initializers/request_forgery_protection.rb
- config/initializers/cors.rb
- config/environment.rb
- config/initializers/inflections.rb
- config/initializers/assets.rb
- config/initializers/application_controller_renderer.rb
| @@ -1,5 +1,4 @@ | |||
| # frozen_string_literal: true | |||
|
|
|||
| ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../Gemfile", __dir__) | |||
There was a problem hiding this comment.
Reintroduce frozen string literal comment for RuboCop.
CI reports Style/FrozenStringLiteralComment on Line 1 in this file.
Proposed fix
+# frozen_string_literal: true
+
ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../Gemfile", __dir__)📝 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.
| ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../Gemfile", __dir__) | |
| # frozen_string_literal: true | |
| ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../Gemfile", __dir__) |
🧰 Tools
🪛 GitHub Actions: Lint CI
[error] 1-1: Style/FrozenStringLiteralComment: Missing frozen string literal comment.
🪛 RuboCop (1.85.1)
[convention] 1-1: Missing frozen string literal comment.
(Style/FrozenStringLiteralComment)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@config/boot.rb` at line 1, Add the frozen string literal magic comment as the
very first line of the file to satisfy RuboCop's
Style/FrozenStringLiteralComment (place the comment before the existing
ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../Gemfile", __dir__) line); ensure
the exact frozen string literal comment is used and committed so CI lint passes.
| @@ -0,0 +1,22 @@ | |||
| # Run using bin/ci | |||
There was a problem hiding this comment.
Add frozen string literal comment to satisfy lint.
This new Ruby file is failing Style/FrozenStringLiteralComment on Line 1.
Proposed fix
+# frozen_string_literal: true
+
# Run using bin/ci📝 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.
| # Run using bin/ci | |
| # frozen_string_literal: true | |
| # Run using bin/ci |
🧰 Tools
🪛 GitHub Actions: Lint CI
[error] 1-1: Style/FrozenStringLiteralComment: Missing frozen string literal comment.
🪛 RuboCop (1.85.1)
[convention] 1-1: Missing frozen string literal comment.
(Style/FrozenStringLiteralComment)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@config/ci.rb` at line 1, Add the frozen string literal magic comment as the
first line of the file to satisfy the Style/FrozenStringLiteralComment lint;
update the top of config/ci.rb so the file begins with a single-line comment
disabling mutability (i.e., add the frozen string literal comment) before any
other content or comments.
| step "Tests: RSpec", "bin/rspec" | ||
| step "Tests: Jest", "yarn test:client" |
There was a problem hiding this comment.
Build test assets before running RSpec/Jest.
Given the known failure mode (missing SSR bundle), run test asset compilation before test steps so CI reflects a runnable test environment.
Proposed fix
step "Style: Ruby", "bin/rubocop"
step "Style: JavaScript", "yarn lint:eslint"
+ step "Build: Test assets", "yarn build:test"
step "Tests: RSpec", "bin/rspec"
step "Tests: Jest", "yarn test:client"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@config/ci.rb` around lines 9 - 10, Add a CI step that compiles test assets
before running tests so the SSR bundle and client assets exist; insert a "Build
Test Assets" step (running your repo's client and/or server build commands)
immediately before the existing step "Tests: RSpec", "bin/rspec" and before
"Tests: Jest", "yarn test:client" in config/ci.rb so both RSpec and Jest run
against a built test environment (ensure the build commands match your project,
e.g., client build and any assets:precompile).
| Rails.application.config.filter_parameters += [ | ||
| :passw, :email, :secret, :token, :_key, :crypt, :salt, :certificate, :otp, :ssn, :cvv, :cvc | ||
| ] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
git ls-files | grep -E "(filter_parameter|rubocop)" | head -20Repository: shakacode/react-webpack-rails-tutorial
Length of output: 170
🏁 Script executed:
find . -name "filter_parameter_logging.rb" -o -name ".rubocop.yml" -o -name "rubocop.yml" 2>/dev/nullRepository: shakacode/react-webpack-rails-tutorial
Length of output: 164
🏁 Script executed:
fd "filter_parameter_logging" . 2>/dev/nullRepository: shakacode/react-webpack-rails-tutorial
Length of output: 131
🏁 Script executed:
fd "rubocop" . -type f 2>/dev/null | head -10Repository: shakacode/react-webpack-rails-tutorial
Length of output: 64
🏁 Script executed:
cat -n config/initializers/filter_parameter_logging.rbRepository: shakacode/react-webpack-rails-tutorial
Length of output: 605
🏁 Script executed:
cat .rubocop.yml | head -100Repository: shakacode/react-webpack-rails-tutorial
Length of output: 1759
🏁 Script executed:
rg "Style/SymbolArray" .rubocop.ymlRepository: shakacode/react-webpack-rails-tutorial
Length of output: 64
🏁 Script executed:
wc -l .rubocop.ymlRepository: shakacode/react-webpack-rails-tutorial
Length of output: 98
🏁 Script executed:
cat .rubocop.yml | tail -100Repository: shakacode/react-webpack-rails-tutorial
Length of output: 1681
🏁 Script executed:
cat .rubocop.yml | grep -i "symbol"Repository: shakacode/react-webpack-rails-tutorial
Length of output: 64
🏁 Script executed:
cat config/initializers/filter_parameter_logging.rbRepository: shakacode/react-webpack-rails-tutorial
Length of output: 549
🏁 Script executed:
python3 << 'EOF'
# Verify the %i syntax is valid Ruby
# Test if %i[...] creates symbols properly
test_code = """
test = %i[passw email secret token _key crypt salt certificate otp ssn cvv cvc]
puts test.inspect
"""
# In Ruby, %i[...] is valid percent literal syntax for symbol arrays
print("✓ %i[...] is valid Ruby percent literal syntax for symbol arrays")
print("\nThe suggested fix converts:")
print(" [:passw, :email, ...] (explicit array syntax)")
print(" to:")
print(" %i[passw email ...] (percent literal syntax)")
EOFRepository: shakacode/react-webpack-rails-tutorial
Length of output: 280
Use %i[...] to fix the Style/SymbolArray linting violation.
The explicit symbol array syntax on lines 6–8 violates the Style/SymbolArray rule, which is enabled by default in RuboCop. Change to the percent literal syntax to resolve this CI failure.
Suggested fix
-Rails.application.config.filter_parameters += [
- :passw, :email, :secret, :token, :_key, :crypt, :salt, :certificate, :otp, :ssn, :cvv, :cvc
-]
+Rails.application.config.filter_parameters += %i[
+ passw email secret token _key crypt salt certificate otp ssn cvv cvc
+]📝 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.
| Rails.application.config.filter_parameters += [ | |
| :passw, :email, :secret, :token, :_key, :crypt, :salt, :certificate, :otp, :ssn, :cvv, :cvc | |
| ] | |
| Rails.application.config.filter_parameters += %i[ | |
| passw email secret token _key crypt salt certificate otp ssn cvv cvc | |
| ] |
🧰 Tools
🪛 GitHub Actions: Lint CI
[error] 6-6: Style/SymbolArray: Use %i or %I for an array of symbols.
🪛 RuboCop (1.85.1)
[convention] 6-8: Use %i or %I for an array of symbols.
(Style/SymbolArray)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@config/initializers/filter_parameter_logging.rb` around lines 6 - 8, The
symbol list in filter_parameter_logging.rb uses explicit symbol array syntax
(e.g., :passw, :email, :secret, :token, :_key, :crypt, :salt, :certificate,
:otp, :ssn, :cvv, :cvc) which triggers the Style/SymbolArray RuboCop offense;
replace the explicit array with the percent literal form %i[...] containing the
same identifiers to satisfy the linter and preserve behavior (keep the same
symbol names and ordering).
| gem "puma" | ||
| gem "thruster", "~> 0.1" | ||
| gem "bootsnap", require: false | ||
|
|
There was a problem hiding this comment.
Fix Gemfile ordering to pass Bundler lint.
bootsnap should be placed before thruster in this section (Line 19), per the failing Bundler/OrderedGems check.
Proposed fix
gem "puma"
-gem "thruster", "~> 0.1"
gem "bootsnap", require: false
+gem "thruster", "~> 0.1"📝 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.
| gem "puma" | |
| gem "thruster", "~> 0.1" | |
| gem "bootsnap", require: false | |
| gem "puma" | |
| gem "bootsnap", require: false | |
| gem "thruster", "~> 0.1" | |
🧰 Tools
🪛 GitHub Actions: Lint CI
[error] 19-19: Bundler/OrderedGems: Gems should be sorted in alphabetical order within their section of the Gemfile. Gem bootsnap should appear before thruster.
🪛 RuboCop (1.85.1)
[convention] 19-19: Gems should be sorted in an alphabetical order within their section of the Gemfile. Gem bootsnap should appear before thruster.
(Bundler/OrderedGems)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Gemfile` around lines 17 - 20, Bundler/OrderedGems lint fails because gem
"bootsnap" appears after "thruster"; open the Gemfile and move the gem
"bootsnap", require: false entry so it appears before gem "thruster", keeping
gem "puma" position intact, then run bundle install / bundler lint to verify the
ordering fix.
![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: a836f35eb4
ℹ️ 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".
| step "Style: Ruby", "bin/rubocop" | ||
| step "Style: JavaScript", "yarn lint:eslint" | ||
|
|
||
| step "Tests: RSpec", "bin/rspec" |
There was a problem hiding this comment.
On a clean checkout, this bin/ci flow will stop at the RSpec step because bin/setup --skip-server never generates the React-on-Rails test bundles. This repo’s tests depend on those artifacts: config/initializers/react_on_rails.rb:12 defines a dedicated test build command, client/app/packs/server-bundle.js:5 imports generated/server-bundle-generated.js, and the existing CI workflow explicitly runs react_on_rails:generate_packs, yarn res:build, and bin/shakapacker before rspec (.github/workflows/rspec_test.yml:76-88). As written, bin/ci won’t be usable in the same environments it is meant to cover.
Useful? React with 👍 / 👎.
1 participant
Upgrades Rails from 8.1.1 to 8.1.2 and adds bootsnap for modern boot behavior. Applies Rails 8.1 app/update configuration refreshes, sets config.load_defaults 8.1, and keeps production-specific SSL plus Action Cable host settings. Removes legacy compatibility initializers (cookies_serializer, per_form_csrf_tokens, request_forgery_protection, application_controller_renderer) and adds bin/ci with repo-appropriate steps in config/ci.rb. Validation: bundle exec rails zeitwerk:check and yarn test:client pass, while bundle exec rspec currently fails because SSR test bundle ./../generated/server-bundle-generated.js is missing in test setup.
Note
Medium Risk
Framework upgrade plus environment/config changes (boot process, caching/logging defaults, removed initializers, Puma threading) can cause subtle behavior changes across dev/test/prod despite being largely standard Rails updates.
Overview
Upgrades the app to Rails
8.1.2(and related dependency bumps) and enables faster boot by addingbootsnapand loading it fromconfig/boot.rb.Refreshes Rails configuration to
config.load_defaults 8.1, modernizes environment settings (notably dev caching/mail URL options and prod logging/deprecation behavior), and simplifies Puma config (fixed thread count and optionalsolid_queueplugin).Adds a lightweight CI runner (
bin/ci+config/ci.rb) and removes several legacy/compat initializers (cookies serializer, per-form CSRF/origin check, and controller renderer defaults).Written by Cursor Bugbot for commit a836f35. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
Release Notes
New Features
Chores