ci: consolidate Ruby setup and Bundler caching#3354
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:
WalkthroughAcross eight CI workflow files and one Gemfile, this PR replaces manual Ruby gem caching (via ChangesRuby bundler-cache migration
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Code ReviewOverviewThis PR is a solid, well-motivated refactor. Replacing ad-hoc One potential regression and a couple of minor gaps to address before merge: 1. Ordering bug in
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Greptile SummaryThis PR replaces ad-hoc
Confidence Score: 4/5The migration is correct and beneficial across 8 of the 9 workflow files; only benchmark.yml has a step-ordering issue that will cause cold-cache runs to fail. In benchmark.yml the two bundler-cache: true steps execute before libyaml-dev is installed on the runner. On a cache miss, bundler tries to compile psych 5.x (present in the dummy app Gemfile.lock) without the required native library, crashing the build. All other workflows handle this ordering correctly. .github/workflows/benchmark.yml — the libyaml-dev install step needs to move above the bundler-cache steps. Important Files Changed
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
.github/workflows/pro-test-package-and-gem.yml (1)
291-294: ⚡ Quick winNarrow
BUNDLE_DISABLE_CHECKSUM_VALIDATIONscope.Line 293 sets checksum validation bypass for the full job. Prefer step-level env on the Ruby setup step so this exception does not leak to unrelated commands.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/pro-test-package-and-gem.yml around lines 291 - 294, The job-level env currently sets BUNDLE_DISABLE_CHECKSUM_VALIDATION: 'true' which broadens checksum bypass; remove that key from the job env block and instead add BUNDLE_DISABLE_CHECKSUM_VALIDATION: 'true' to only the Ruby/bundler setup step (the step that installs Ruby/gems or runs bundle install) so the exception is limited to bundler setup; ensure other steps no longer inherit the variable and confirm the Ruby setup step still uses the same string value..github/workflows/pro-lint.yml (1)
90-93: ⚡ Quick winScope checksum bypass to the Ruby setup step instead of the whole job.
Line 92 currently applies
BUNDLE_DISABLE_CHECKSUM_VALIDATIONto every step in the job. Keep this override narrowly scoped to theruby/setup-rubystep(s) to reduce blast radius.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/pro-lint.yml around lines 90 - 93, The job-level env currently sets BUNDLE_DISABLE_CHECKSUM_VALIDATION which affects every step; instead remove that env from the top-level env block and add BUNDLE_DISABLE_CHECKSUM_VALIDATION: 'true' to only the ruby/setup-ruby step definition (the step using uses: ruby/setup-ruby), leaving REACT_ON_RAILS_PRO_LICENSE at job level unchanged; ensure the ruby/setup-ruby step's env contains the BUNDLE_DISABLE_CHECKSUM_VALIDATION entry so only Ruby setup is scoped to bypass checksum validation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/benchmark.yml:
- Around line 209-215: The workflow currently uses the tag-pinned action
reference ruby/setup-ruby@v1 in the "Setup Ruby", "Install Core dummy app Ruby
gems", and "Install Pro dummy app Ruby gems" steps; replace each occurrence of
ruby/setup-ruby@v1 with ruby/setup-ruby@<full_commit_sha> (the full immutable
commit SHA for the action) to pin the action to a specific commit and improve
supply-chain integrity, preserving the existing with: keys (ruby-version,
bundler, working-directory, bundler-cache, cache-version) exactly as they are.
In @.github/workflows/integration-tests.yml:
- Around line 124-131: Replace the mutable action ref ruby/setup-ruby@v1 with
immutable full commit SHAs for every uses: entry in the workflow (the "Setup
Ruby and install dummy app gems" step and the two other ruby/setup-ruby uses
mentioned in the review); locate each uses: line that references ruby/setup-ruby
and update it to ruby/setup-ruby@<full-commit-sha> (use the exact commit SHA
from the ruby/setup-ruby repository at the desired release), keeping the
existing with: keys (ruby-version, bundler, working-directory, bundler-cache,
cache-version) unchanged so the steps behavior is identical but pinned to an
immutable SHA.
In @.github/workflows/lint-js-and-ruby.yml:
- Around line 97-113: Replace the mutable actions in the two Ruby setup steps so
they are pinned to immutable commit SHAs: locate the steps named "Setup Ruby and
install package gems" and "Install dummy app gems" that currently use
`ruby/setup-ruby@v1` and change each `uses: ruby/setup-ruby@v1` to the
corresponding `uses: ruby/setup-ruby@<commit-sha>` (use the latest stable commit
SHA from the ruby/setup-ruby repository) to ensure reproducible CI runs.
In @.github/workflows/precompile-check.yml:
- Around line 87-93: The workflow step "Setup Ruby" currently uses the mutable
action reference "ruby/setup-ruby@v1"; replace that with a specific immutable
commit SHA for ruby/setup-ruby to avoid unexpected changes (i.e., pin the action
instead of using `@v1`). Locate the step by its name "Setup Ruby" or the uses
string "ruby/setup-ruby@v1" and update the uses field to the
repo@{full-commit-sha} value from the ruby/setup-ruby releases/tags, then commit
the updated workflow.
In @.github/workflows/pro-integration-tests.yml:
- Around line 99-105: Replace the floating tag ruby/setup-ruby@v1 with an
immutable commit SHA: locate each occurrence of the action usage (the step
titled "Setup Ruby and install Pro dummy app gems" which currently uses
ruby/setup-ruby@v1, and the other two occurrences that match that same uses
string) and change the reference to ruby/setup-ruby@<full-commit-sha> (obtain
the latest stable commit SHA from the ruby/setup-ruby repository and use that
exact 40-char SHA) so the workflow uses a pinned immutable version; update all
three occurrences consistently.
In @.github/workflows/pro-lint.yml:
- Around line 98-112: Replace the floating ruby/setup-ruby@v1 references in the
workflow steps named "Setup Ruby and install Pro package gems" and "Install Pro
dummy app gems" with a specific commit SHA of the ruby/setup-ruby action; locate
the two uses: the with-blocks that include ruby-version, bundler and
working-directory, fetch the canonical commit SHA from the ruby/setup-ruby repo
(or your org's approved mirror) and update both uses to
ruby/setup-ruby@<commit-sha> so both steps are pinned to the same immutable
revision.
---
Nitpick comments:
In @.github/workflows/pro-lint.yml:
- Around line 90-93: The job-level env currently sets
BUNDLE_DISABLE_CHECKSUM_VALIDATION which affects every step; instead remove that
env from the top-level env block and add BUNDLE_DISABLE_CHECKSUM_VALIDATION:
'true' to only the ruby/setup-ruby step definition (the step using uses:
ruby/setup-ruby), leaving REACT_ON_RAILS_PRO_LICENSE at job level unchanged;
ensure the ruby/setup-ruby step's env contains the
BUNDLE_DISABLE_CHECKSUM_VALIDATION entry so only Ruby setup is scoped to bypass
checksum validation.
In @.github/workflows/pro-test-package-and-gem.yml:
- Around line 291-294: The job-level env currently sets
BUNDLE_DISABLE_CHECKSUM_VALIDATION: 'true' which broadens checksum bypass;
remove that key from the job env block and instead add
BUNDLE_DISABLE_CHECKSUM_VALIDATION: 'true' to only the Ruby/bundler setup step
(the step that installs Ruby/gems or runs bundle install) so the exception is
limited to bundler setup; ensure other steps no longer inherit the variable and
confirm the Ruby setup step still uses the same string value.
🪄 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: 4dc4b01e-1c5e-4d1b-8fe0-584aa1a57bfd
⛔ Files ignored due to path filters (1)
react_on_rails/Gemfile.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
.github/workflows/benchmark.yml.github/workflows/examples.yml.github/workflows/gem-tests.yml.github/workflows/integration-tests.yml.github/workflows/lint-js-and-ruby.yml.github/workflows/precompile-check.yml.github/workflows/pro-integration-tests.yml.github/workflows/pro-lint.yml.github/workflows/pro-test-package-and-gem.yml
Code Review: ci: replace manual gem caching with ruby/setup-ruby bundler-cacheOverviewThis is a well-motivated cleanup that replaces ad-hoc Positives
Issues1. Missing
|
justin808
force-pushed
the
jg/2224-bundler-cache-ci
branch
from
7eacce3 to
8ff6cd2
Compare
Review Round 2 — Address & Fix CIPushed CI failures fixed
Review comments addressed
Deferred
Watching CI now. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Missing
BUNDLE_DISABLE_CHECKSUM_VALIDATIONfor Pro dummy app gems- Added the checksum validation opt-out to the affected Pro dummy app workflow job environments before bundler-cache runs.
Or push these changes by commenting:
@cursor push 20a5e74604
Preview (20a5e74604)
diff --git a/.github/workflows/benchmark.yml b/.github/workflows/benchmark.yml
--- a/.github/workflows/benchmark.yml
+++ b/.github/workflows/benchmark.yml
@@ -144,6 +144,7 @@
env:
SECRET_KEY_BASE: 'dummy-secret-key-for-ci-testing-not-used-in-production'
REACT_ON_RAILS_PRO_LICENSE: ${{ secrets.REACT_ON_RAILS_PRO_LICENSE_V2 }}
+ BUNDLE_DISABLE_CHECKSUM_VALIDATION: 'true'
steps:
# ============================================
diff --git a/.github/workflows/pro-test-package-and-gem.yml b/.github/workflows/pro-test-package-and-gem.yml
--- a/.github/workflows/pro-test-package-and-gem.yml
+++ b/.github/workflows/pro-test-package-and-gem.yml
@@ -91,6 +91,7 @@
runs-on: ubuntu-22.04
env:
REACT_ON_RAILS_PRO_LICENSE: ${{ secrets.REACT_ON_RAILS_PRO_LICENSE_V2 }}
+ BUNDLE_DISABLE_CHECKSUM_VALIDATION: 'true'
steps:
- uses: actions/checkout@v4
with:You can send follow-ups to the cloud agent here.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/benchmark.yml:
- Around line 203-204: The workflow step named "Fix dependency for libyaml-dev"
installs libyaml-dev without refreshing apt metadata; modify that step so it
runs an apt update (e.g., run "sudo apt-get update" or "sudo apt update")
immediately before "sudo apt install libyaml-dev -y" to ensure the package index
is current and prevent intermittent install failures.
In @.github/workflows/integration-tests.yml:
- Line 133: The workflow currently references actions/setup-node@v4 in the two
“Setup Node” steps; update both occurrences to pin the action to a full commit
SHA (e.g., actions/setup-node@<full-commit-sha>) so the CI is reproducible and
supply-chain hardened, ensuring you use the same SHA for both steps and replace
the two instances of actions/setup-node@v4 accordingly.
In @.github/workflows/lint-js-and-ruby.yml:
- Line 114: Replace the mutable reference "uses: actions/setup-node@v4" with an
immutable commit SHA (e.g., "uses: actions/setup-node@<commit-sha>") wherever it
appears; obtain the latest stable commit SHA from the actions/setup-node
repository tags or releases, update the workflow step to use that exact SHA, and
apply the same change to every workflow that currently references
"actions/setup-node@v4" so all usages are pinned to the chosen immutable commit.
In @.github/workflows/precompile-check.yml:
- Around line 85-86: The CI step named "Fix dependency for libyaml-dev" runs a
blocking interactive apt command; change the step to perform a non-interactive
install (e.g., use apt-get install -y and/or set DEBIAN_FRONTEND=noninteractive)
so the workflow cannot hang on prompts—update that run command accordingly to
use a non-interactive install invocation.
🪄 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: 2b7c395e-a14a-47c9-a2a0-6edcf78c2b72
⛔ Files ignored due to path filters (1)
react_on_rails/Gemfile.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
.github/workflows/benchmark.yml.github/workflows/examples.yml.github/workflows/gem-tests.yml.github/workflows/integration-tests.yml.github/workflows/lint-js-and-ruby.yml.github/workflows/precompile-check.yml.github/workflows/pro-integration-tests.yml.github/workflows/pro-lint.yml.github/workflows/pro-test-package-and-gem.ymlreact_on_rails/rakelib/run_rspec.rake
Code Review: CI Ruby Caching Migration (#3354)This is a well-motivated refactoring that fixes real bugs (cache path mismatches in Bugs / Issues1. All Pro workflows are missing Every open-source workflow that uses 2.
3. The job calls Improvements4. The comment is good. The root cause is that 5. Static Minor: the benchmark workflow uses a hardcoded What's done well
|
Review address summary (latest batch)Triaged 4 new coderabbit threads since the last reply batch. Fixed (1)
Deferred (3)
All addressed threads have been marked resolved. |
Address review feedback on the bundler-cache CI migration (#3354). - benchmark.yml: When both RUN_CORE and RUN_PRO are true, the Pro setup-ruby step's bundler-cache wrote BUNDLE_GEMFILE to $GITHUB_ENV, leaking the Pro dummy Gemfile into subsequent Core bundle-exec calls (bin/prod-assets, bin/prod, benchmarks/bench.rb). Pin BUNDLE_GEMFILE to the Core dummy Gemfile via absolute path on the affected Core steps. Fixes a real correctness bug flagged by cursor[bot]. - pro-lint.yml: Switch BUNDLE_GEMFILE overrides on Lint Ruby and Validate RBS type signatures from relative `Gemfile` to absolute `${{ github.workspace }}/react_on_rails_pro/Gemfile` so they're robust to changes in defaults.run.working-directory; document why. - pro-test-package-and-gem.yml: Embed matrix.ruby-version in cache-version (belt-and-suspenders for future matrix expansion). - Gemfile.development_dependencies: Warn future readers not to reintroduce the `if ENV["DISABLE_TURBOLINKS"]` conditional, which would re-break `bundle install --frozen`. - benchmark.yml: Minor — document the START_POINT_HASH_ARG empty-string edge case for the bencher retry substitution; revert printf format string from double to single quotes to drop backtick escapes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
After the second ruby/setup-ruby call in lint-js-and-ruby.yml (build job) and integration-tests.yml (dummy-app-integration-tests job), $GITHUB_ENV's BUNDLE_GEMFILE points at the dummy app Gemfile. The dummy Gemfile shares dev dependencies via eval_gemfile, so today these steps still resolve rubocop/rbs/rspec — but pin the package Gemfile explicitly for parity with pro-lint.yml and benchmark.yml, and to stay robust if the Gemfiles ever diverge. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Apply the remaining optional polish items from PR #3354 review. - Rename "Fix dependency for libyaml-dev" to "Install libyaml-dev (required for psych v5 native extensions)" across all 13 occurrences (8 workflows). The previous name read like a fix for a broken dependency rather than a prerequisite install. - Align pro-integration-tests.yml job-level checksum comment with the more accurate wording already used in benchmark.yml and pro-test-package-and-gem.yml ("Pro gem server does not publish checksums…"), so contributors understand why job-level scope is required. - Add an inline comment in lint-js-and-ruby.yml above the `generate file system-based packs` step explaining the deliberate absence of a BUNDLE_GEMFILE pin (the dummy app Gemfile leaked into $GITHUB_ENV by the previous setup-ruby step is intentionally reused). - Switch the `gh issue list --jq` filter at benchmark.yml:787 from double to single quotes to match the other jq filters in this file. - Document in AGENTS.md "Never" that conditional gem declarations like `gem "turbolinks" if ENV["DISABLE_TURBOLINKS"].nil?` must not be reintroduced in react_on_rails/Gemfile.development_dependencies; the conditional inclusion breaks `bundle install --frozen` in CI. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
justin808
force-pushed
the
jg/2224-bundler-cache-ci
branch
from
32a1141 to
bbf71aa
Compare
|
Address-review checkpoint at 2026-05-28T08:28:50Z. Scan window: feedback after the prior checkpoint at 2026-05-28T05:04:26Z. Mattered
Optional
Skipped
The branch was rebased onto Future full-PR |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 4 total unresolved issues (including 3 from previous reviews).
Bugbot Autofix is ON. A cloud agent has been kicked off to fix the reported issue.
Reviewed by Cursor Bugbot for commit bbf71aa. Configure here.
| echo "🚀 Starting Pro production server..." | ||
| cd react_on_rails_pro/spec/dummy | ||
|
|
||
| $SERVER_CMD & |
There was a problem hiding this comment.
Pro benchmark Prepare/Start steps missing BUNDLE_GEMFILE pinning
Medium Severity
The Core benchmark steps all pin BUNDLE_GEMFILE for "Prepare Core production assets", "Start Core production server", and "Execute Core benchmark suite", with a comment explaining that bin/prod-assets, bin/prod, and bench.rb shell out through bundle exec. The Pro equivalents only pin BUNDLE_GEMFILE for "Execute Pro benchmark suite" but omit it for "Prepare Pro production assets" and "Start Pro production server", even though those steps also invoke bin/prod-assets and bin/prod which shell out via bundle exec. Since setup-bundle now internally calls ruby/setup-ruby@v1 (which may persist BUNDLE_GEMFILE across steps), the Pro Prepare and Start steps could resolve against the wrong bundle if the environment retains a stale value from the preceding Core setup-bundle invocation.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit bbf71aa. Configure here.
|
test |
Code Review: CI Ruby Setup ConsolidationOverviewThis PR replaces manual Strengths
Issues / Concerns1. Behavioral change: non-frozen runs now save the gem cacheOld Suggestion: Suppress the cache write when 2. libyaml-dev is a documentation-only contract in setup-bundleThe Suggestion: Add an 3. Double ruby/setup-ruby invocation in most jobsJobs that call 4. BUNDLE_FROZEN env interaction with ruby/setup-ruby
Minor Points
SummaryThe consolidation is correct and the root-cause fix is sound. The main concern worth addressing before merge is the non-frozen cache write change (issue 1) — the old behaviour was an explicit safety guard and losing it silently widens the risk surface for minimum-dependency legs. Everything else is minor or an improvement. |
| description: Cache and install Ruby gems for a Bundler project. | ||
| description: > | ||
| Setup Ruby, cache, and install gems for a Bundler project. On Linux, | ||
| ensure libyaml-dev is already present or install it before calling this action. |
There was a problem hiding this comment.
The libyaml-dev prerequisite is stated in the description but is not enforced — setup-bundle itself never installs it. Callers using setup-ruby first get it implicitly; callers that use setup-bundle directly (e.g. the Playwright workflow) do not.
Consider adding an install-libyaml input (default 'true') that mirrors the one in setup-ruby, so any direct caller of setup-bundle is protected automatically:
| ensure libyaml-dev is already present or install it before calling this action. | |
| Setup Ruby, cache, and install gems for a Bundler project. On Linux, | |
| libyaml-dev is installed automatically (set install-libyaml: 'false' to skip | |
| if the caller already ran setup-ruby). |
| ruby-version: ${{ inputs.ruby-version }} | ||
| working-directory: ${{ inputs.working-directory }} | ||
| bundler: ${{ inputs.bundler-version }} | ||
| bundler-cache: true |
There was a problem hiding this comment.
Behavioral change from old implementation: the previous setup-bundle explicitly skipped the cache-save step when frozen == 'false':
if: inputs.frozen == 'true' && steps.restore-ruby-gems-cache.outputs.cache-hit != 'true'ruby/setup-ruby with bundler-cache: true always writes the cache regardless of BUNDLE_FROZEN. The minimum-dependency legs that set frozen: 'false' will now write gem caches after every non-frozen bundle install. The cache keys are Gemfile.lock-hash-based so cross-contamination is unlikely, but the old guard made it structurally impossible.
Consider making bundler-cache conditional on the frozen input:
| bundler-cache: true | |
| bundler-cache: ${{ inputs.frozen }} |
(ruby/setup-ruby accepts 'true'/'false' strings for bundler-cache, so this maps directly.)
| with: | ||
| ruby-version: ${{ inputs.ruby-version }} | ||
| bundler: ${{ inputs.bundler-version }} | ||
| bundler-cache: false |
There was a problem hiding this comment.
Explicit bundler-cache: false here is correct and important — this action is intentionally for Ruby-only setup (used before setup-bundle). Without this explicit false, ruby/setup-ruby defaults to no caching, but being explicit prevents confusion if the default ever changes upstream.
| with: | ||
| ruby-version: 3 | ||
| bundler: 2.5.9 | ||
| ruby-version: '3.4' |
There was a problem hiding this comment.
Pinning to '3.4' (was 3, which resolved to the latest Ruby 3.x patch) is more stable for CI but will silently stay on 3.4 after Ruby 3.5 ships. Worth tracking this alongside the next Ruby minor upgrade.
|
View the full report in the job summary. |
|
Closing this as superseded and splitting the still-relevant work into two focused PRs. While rebasing onto
What
The original branch |
…3482) ## Summary Declares the `turbolinks` development dependency **unconditionally** in `react_on_rails/Gemfile.development_dependencies`, replacing the `gem "turbolinks" if ENV["DISABLE_TURBOLINKS"]...` conditional. The conditional resolved to a dependency set that differs from `Gemfile.lock` whenever `DISABLE_TURBOLINKS` is set — the lock file is generated without the env var, but `bundle install` runs with it — which breaks `bundle install --frozen` in CI. `main` currently works around this by forcing `BUNDLE_FROZEN=false` in the `dummy_no_turbolinks` rake task. This PR removes the root cause so the workaround is no longer needed. ## Why this is safe `DISABLE_TURBOLINKS` already controls Turbolinks at the **application layer**, not via gem inclusion: ```erb <%# react_on_rails/spec/dummy/app/assets/javascripts/application_non_webpack.js.erb %> <% if ENV["DISABLE_TURBOLINKS"].blank? %> <% require_asset "turbolinks" %> <% end %> ``` So with the gem always present, `DISABLE_TURBOLINKS=TRUE` still disables Turbolinks (the asset is not required). The `rake run_rspec:dummy_no_turbolinks` coverage is unchanged — it still exercises the no-Turbolinks code path. `Gemfile.lock` is unaffected: it already pins `turbolinks (5.2.1)` because lock resolution runs without `DISABLE_TURBOLINKS`. ## Changes - `react_on_rails/Gemfile.development_dependencies` — declare `gem "turbolinks"` unconditionally (+ explanatory comment). - `react_on_rails/rakelib/run_rspec.rake` — drop the now-unnecessary `BUNDLE_FROZEN=false` workaround from `dummy_no_turbolinks`. - `AGENTS.md` — document the footgun so the conditional declaration is not reintroduced. ## Context Split out of #3354 as a focused, standalone fix. The broader Bundler-cache consolidation from that branch was superseded by #3429 (extract `setup-bundle`) and #3448 (independent benchmark suites) already on `main`; this is the one root-cause reliability fix from #3354 that `main` does not yet have. ## Test plan - [x] `bundle exec rubocop rakelib/run_rspec.rake Gemfile.development_dependencies` — no offenses - [x] `npx prettier --check AGENTS.md` — passes - [x] Confirmed `Gemfile.lock` already contains `turbolinks (5.2.1)`; no lock change - [x] Verified the app-layer `require_asset` guard handles `DISABLE_TURBOLINKS` independently of gem presence - [ ] CI `rake run_rspec:dummy_no_turbolinks` (and frozen installs) pass on the branch Related to #2224 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Dev/test dependency and CI wiring only; no production gem behavior change, and no-Turbolinks coverage stays via the existing asset guard. > > **Overview** > Fixes **CI `bundle install --frozen` failures** when `DISABLE_TURBOLINKS` is set by always declaring **`turbolinks`** in `react_on_rails/Gemfile.development_dependencies`, instead of omitting it based on that env var. > > **`DISABLE_TURBOLINKS`** still turns Turbolinks off at runtime via the dummy app’s **`require_asset`** guard; only gem resolution changes. The **`dummy_no_turbolinks`** rake task no longer sets **`BUNDLE_FROZEN=false`**. > > **AGENTS.md** and **CONTRIBUTING.md** document the rule so conditional turbolinks declarations are not reintroduced. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit b8a214e. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Clarified that certain dev-only assets remain bundled while runtime toggles disable them, to keep dependency sets consistent with the lockfile. * **Chores** * Made development/test workflows more reliable by aligning gem declarations with the lockfile and adjusting test-run behavior to avoid frozen-bundle failures in CI. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/shakacode/react_on_rails/pull/3482?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
](https://app.coderabbit.ai/change-stack/shakacode/react_on_rails/pull/3482?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack){kind=link}
## Summary Reworks the shared `setup-bundle` composite action to delegate Ruby setup **and** gem caching to `ruby/setup-ruby@v1` with `bundler-cache: true`, replacing the manual `actions/cache` restore/save logic plus the `bundle check || bundle install` step that #3429 extracted. Adds a companion `setup-ruby` action and rolls both out across the OSS, Pro, integration, examples, precompile, Playwright, and lint workflows. This is the caching-mechanism half of the original #3354 work, re-derived cleanly on top of current `main`. (The other salvageable piece — the unconditional-turbolinks frozen-install fix — is split into #3482. The rest of #3354 was superseded by #3429 and #3448 and is dropped.) ## What changes - **`.github/actions/setup-bundle`** — instead of `actions/cache/restore` → `bundle check || bundle install` → `actions/cache/save`, it now calls `ruby/setup-ruby@v1` with `bundler-cache: true`, which installs Ruby, restores/saves the gem cache, and runs `bundle install` in one step. Inputs simplified accordingly (`bundle-path`/`install-args` dropped; `cache-version` passthrough added; `bundler-version` uses ruby/setup-ruby's selector values — `Gemfile.lock`/`default`/`latest`/`none`/`x.y.z`). - **`.github/actions/setup-ruby`** (new) — Ruby runtime + Linux `libyaml-dev`, no gems. Used by jobs that need Ruby before their first bundle install (e.g. `script/convert`, `script/check-gemfile-lock-platforms`). - **Workflows** — `examples`, `gem-tests`, `integration-tests`, `playwright`, `precompile-check`, `pro-integration-tests`, `pro-test-package-and-gem`, and `lint-js-and-ruby` migrated to the `setup-ruby` + `setup-bundle` pattern. ## Why `ruby/setup-ruby`'s built-in `bundler-cache` is the upstream-recommended caching path and removes the bespoke cache key / restore-keys / save-condition logic the action currently maintains by hand. Less custom CI code to keep correct. ## Tradeoff / discussion This **reverses the caching mechanism** that #3429 extracted (manual `actions/cache`). #3429 was a refactor of pre-existing logic, not necessarily a deliberate rejection of `bundler-cache: true`, so this is offered as the follow-on improvement #3354 originally proposed — but it's worth a maintainer call. `benchmark.yml` is intentionally **not** restructured: its `setup-benchmark-runner` (added by #3448) still installs Ruby for `foreman`, and the subsequent `setup-bundle` call now handles each dummy app's gems via bundler-cache. That means `ruby/setup-ruby` runs twice in benchmark jobs (once for the runner tools, once for the dummy bundle) — functional and matches the approved #3354 pattern, but flagging it in case reviewers prefer a deeper benchmark refactor. ## Test plan - [x] `actionlint` (with `SHELLCHECK_OPTS='-S warning'`) — clean - [x] Ruby `YAML.load_file` parses all changed action/workflow files - [x] Verified every `setup-bundle` call site passes only supported inputs (`working-directory`/`ruby-version`/`frozen`); no `bundle-path`/`install-args`/`lockfile` anywhere - [x] Verified every `setup-ruby` call site passes only supported inputs (`ruby-version`/`bundler-version`) - [x] Trailing-newline check on all changed files - [ ] Full GitHub Actions run is green (the double `ruby/setup-ruby` invocation in benchmark + bundler-cache behavior should be confirmed in CI) Related to #2224 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Touches how every major workflow installs and caches gems; behavior shifts from custom cache keys to upstream bundler-cache, though frozen/minimum-dependency semantics are preserved intentionally. > > **Overview** > **Centralizes Ruby/Bundler setup in two composite actions** and rolls them through OSS, Pro, integration, examples, lint, precompile, and Playwright workflows. > > `setup-bundle` no longer uses hand-rolled `actions/cache` restore/save plus `bundle check || bundle install`. It now calls `ruby/setup-ruby@v1` with **`bundler-cache: true`**, passing `BUNDLE_FROZEN`, jobs/retry env vars, and optional **`cache-version`**. Inputs **`bundle-path`** and **`install-args`** are removed; **`bundler-version`** aligns with upstream selectors (default **`Gemfile.lock`**). > > A new **`setup-ruby`** action installs Linux **`libyaml-dev`** when needed and runs `ruby/setup-ruby` with **`bundler-cache: false`** for jobs that need Ruby/Bundler before the first Gemfile-scoped install (e.g. **`script/convert`**, platform checks). Workflows switch from inline `ruby/setup-ruby` to **`setup-ruby`** then **`setup-bundle`**, with **`bundler-version`** instead of **`bundler`**. > > Minor workflow hardening: quoted **`$GITHUB_OUTPUT`** / **`$GITHUB_ENV`**, grouped detect-changes output writes, explicit **`BUNDLE_GEMFILE`** for package lint/RBS steps, and comments documenting **`frozen: false`** on minimum-dependency matrix legs. Per-job **`libyaml-dev`** apt steps are dropped where **`setup-ruby`** covers them. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 037489a. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Adopted a unified, repo-local Ruby setup for consistent Ruby/bundler across CI. * Simplified bundler inputs and caching behaviour, adding a cache-version toggle and clarifying frozen/bundler defaults. * Improved CI output and environment variable writing for more reliable workflow execution. * Added optional libyaml installation and standardized Ruby versions (including 3.4 in some jobs). <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/shakacode/react_on_rails/pull/3483?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
](https://app.coderabbit.ai/change-stack/shakacode/react_on_rails/pull/3483?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack){kind=link}
2 participants
Summary
.github/actions/setup-bundle, which now callsruby/setup-ruby@v1withbundler-cache: trueinstead of maintaining customactions/cacherestore/save logic in each workflow..github/actions/setup-rubyfor jobs that need Ruby runtime setup before a separate Bundler install step, including the sharedlibyaml-devprerequisite.bundle execcalls must run against a package or dummy app.turbolinksstays unconditional so Bundler lock resolution is stable, Pro checksum validation remains enabled, and benchmark runs use the configuredBENCHMARK_PORTconsistently.Review and CI updates
origin/mainand force-pushed the branch atbbf71aa3cc429dae277783105a599218f494b126.BUNDLE_GEMFILEtoreact_on_rails_pro/spec/dummy/Gemfilefor the Pro benchmark step, matching the Core benchmark behavior becausebench.rbshells out throughbundle exec.BUNDLE_GEMFILEoverride fromrun_rspec:all_dummy, so nested dummy appbundle execcalls can use the dummy Gemfile and lockfile.3.4pin in the lint bootstrap step,BUNDLE_JOBS: '4'preservation insetup-bundle, explicitbundler-cache: falseinsetup-ruby, Bencher retry indentation cleanup, and asetup-bundledescription note for the Linuxlibyaml-devprerequisite.inspect_pr_checks.pyreports no failing checks at the time this description was updated.Test plan
git rebase origin/maingit diff --check origin/main...HEAD && git diff --checkSHELLCHECK_OPTS='-S warning' actionlint -colorruby -e 'require "yaml"; ARGV.each { |f| YAML.load_file(f); puts "parsed #{f}" }' .github/actions/setup-bundle/action.yml .github/actions/setup-ruby/action.yml .github/workflows/benchmark.yml .github/workflows/examples.yml .github/workflows/gem-tests.yml .github/workflows/integration-tests.yml .github/workflows/lint-js-and-ruby.yml .github/workflows/pro-integration-tests.yml .github/workflows/pro-test-package-and-gem.ymlpnpm start format.listDifferent(cd react_on_rails && bundle exec rubocop)react_on_rails/rakelib/run_rspec.rakeand Markdown link check onAGENTS.mdbbf71aa3cc429dae277783105a599218f494b126Local note
pnpm run lintcurrently fails in existingpackages/react-on-rails-pro-node-renderer/src/**TypeScript files that are outside this PR diff. I did not modify Pro package source files as part of this workflow-focused PR.Fixes #2224
Note
Cursor Bugbot is generating a summary for commit bbf71aa. Configure here.