[codex] Reduce packaged gem files - #1110
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ 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 |
Code ReviewOverall: Clean, well-motivated change. Moving from a regex-exclusion filter to an explicit allowlist is easier to reason about, and the size reduction (294 → 75 files, ~486K → ~121K) is significant with no functional regression. CorrectnessBehavioral change in The old gemspec appended
Test CoverageThe new specs are well-structured and cover the key install assets. One minor gap: Side Effects Worth NotingThe new allowlist incidentally drops several root-level files the old regex filter left in:
Verdict: Looks good to approve once the draft is lifted. The size reduction is meaningful, the tests are solid, and the approach is simpler than what it replaces. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| } + Dir.glob("sig/**/*.rbs") | ||
|
|
||
| s.test_files = `git ls-files -- test/*`.split("\n") | ||
| s.files = `git ls-files -z CHANGELOG.md MIT-LICENSE README.md shakapacker.gemspec lib sig`.split("\x0") |
There was a problem hiding this comment.
Note for future contributors: git ls-files sig only includes tracked files. The old approach used Dir.glob("sig/**/*.rbs") which worked even for untracked files. If you add a new .rbs signature file, make sure to git add it before building the gem, otherwise it will be silently excluded from the published package.
| "lib/install/config/shakapacker.yml", | ||
| "lib/install/bin/shakapacker", | ||
| "lib/install/bin/shakapacker-dev-server" | ||
| ) |
There was a problem hiding this comment.
Consider adding "lib/install/application.js" to this list — it's the default JS entrypoint copied by shakapacker:install and isn't currently covered by the explicit spot-check (though it is implicitly covered by the "includes lib directory" test above).
Code ReviewSummaryThis PR switches gem packaging from an exclusion-list approach ( Positives
IssuesStructural: The new Minor: The Minor: install asset coverage in spec is incomplete The spec asserts six specific install files exist but misses a few others that Risk AssessmentThe PR's own risk note is accurate: the main hazard is a runtime/installer file being unintentionally excluded in a future refactor. The allowlist + explicit spec coverage for critical install assets is a good mitigation. The risk is low — the allowlist is simple and the spec is clear. Verdict: Approve after the |
| expect(rbs_files).not_to be_empty | ||
| end | ||
|
|
||
| it "does not package test files through test_files metadata" do |
There was a problem hiding this comment.
This test asserts on gemspec.test_files but is nested inside describe "s.files". Move it into its own sibling block for accurate organization:
| it "does not package test files through test_files metadata" do | |
| describe "s.test_files" do | |
| it "is empty" do | |
| expect(gemspec.test_files).to be_empty | |
| end | |
| end |
(The closing end for describe "s.files" should come just before this new block.)
Code ReviewOverviewThis PR replaces a blocklist-based Positive
Issues / SuggestionsMissing CHANGELOG entry – Per the project guidelines, gem-size reductions are a user-visible improvement and belong in the
Pre-existing "excludes" specs are now trivially true – The Rspack config templates not explicitly tested – VerdictGood, focused change. The two concrete items to address before merging are the CHANGELOG entry and optionally expanding the install-assets spec to cover the rspack config templates. |
|
|
||
| describe "s.test_files" do | ||
| it "is empty" do | ||
| expect(gemspec.test_files).to be_empty |
There was a problem hiding this comment.
test_files is deprecated since RubyGems 2.0 and always returns [] in modern environments, so this spec cannot fail in practice. It's a harmless sanity-check, but consider whether it's worth carrying the maintenance weight. If kept, a brief comment explaining why it's explicitly set to [] (to silence the deprecation warning from the previous git ls-files -- test/* assignment) would help future readers.
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![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.
Bugbot Autofix is ON, but it could not run because the branch was deleted or merged before autofix could start.
Reviewed by Cursor Bugbot for commit ce32605. Configure here.
| "lib/install/config/shakapacker.yml", | ||
| "lib/install/bin/shakapacker", | ||
| "lib/install/bin/shakapacker-dev-server" | ||
| "lib/install/bin/shakapacker", |
There was a problem hiding this comment.
Missing comma causes silent string concatenation in test
High Severity
A missing comma after "lib/install/bin/shakapacker-dev-server" on line 47 causes Ruby to implicitly concatenate it with the next string literal "lib/install/bin/shakapacker", producing the nonsensical path "lib/install/bin/shakapacker-dev-serverlib/install/bin/shakapacker". This appears to be a copy-paste error — lines 46–47 duplicate lines 48–49. Additionally, the include( opened on line 41 is never closed with ), which is a syntax error.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit ce32605. Configure here.
Greptile SummaryThis PR replaces the broad
Confidence Score: 2/5The gemspec change is correct, but the spec file guarding it cannot be loaded due to a syntax error, leaving the new allowlist unvalidated. The spec file introduced to validate the new allowlist has an unclosed parenthesis and a missing comma that cause a SyntaxError at parse time. Every test in the file is unreachable until fixed, so the stated validation command exits with a load error rather than confirming the gem contents are correct. spec/shakapacker/gemspec_spec.rb — the 'includes install assets' example block needs both a comma and a closing parenthesis before it can run. Important Files Changed
Reviews (1): Last reviewed commit: "Update spec/shakapacker/gemspec_spec.rb" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| it "includes install assets needed by shakapacker:install" do | ||
| expect(gemspec.files).to include( | ||
| "lib/install/template.rb", | ||
| "lib/install/application.js", | ||
| "lib/install/package.json", | ||
| "lib/install/config/shakapacker.yml", | ||
| "lib/install/bin/shakapacker", | ||
| "lib/install/bin/shakapacker-dev-server" | ||
| "lib/install/bin/shakapacker", | ||
| "lib/install/bin/shakapacker-dev-server", | ||
| "lib/install/config/rspack/rspack.config.js", | ||
| "lib/install/config/rspack/rspack.config.ts", | ||
| "lib/install/config/webpack/webpack.config.js", | ||
| "lib/install/config/webpack/webpack.config.ts" | ||
| end |
There was a problem hiding this comment.
"lib/install/bin/shakapacker-dev-server" on line 47 causes Ruby's implicit adjacent-literal concatenation, turning that line and the next into a single fabricated path "lib/install/bin/shakapacker-dev-serverlib/install/bin/shakapacker" that will never match a real file. Second, the opening include( on line 41 is never closed with ) before end, so Ruby raises SyntaxError: unexpected keyword_end, expecting ')' and the entire spec file fails to load. Together these defeat the stated validation step bundle exec rspec spec/shakapacker/gemspec_spec.rb.
| it "includes install assets needed by shakapacker:install" do | |
| expect(gemspec.files).to include( | |
| "lib/install/template.rb", | |
| "lib/install/application.js", | |
| "lib/install/package.json", | |
| "lib/install/config/shakapacker.yml", | |
| "lib/install/bin/shakapacker", | |
| "lib/install/bin/shakapacker-dev-server" | |
| "lib/install/bin/shakapacker", | |
| "lib/install/bin/shakapacker-dev-server", | |
| "lib/install/config/rspack/rspack.config.js", | |
| "lib/install/config/rspack/rspack.config.ts", | |
| "lib/install/config/webpack/webpack.config.js", | |
| "lib/install/config/webpack/webpack.config.ts" | |
| end | |
| it "includes install assets needed by shakapacker:install" do | |
| expect(gemspec.files).to include( | |
| "lib/install/template.rb", | |
| "lib/install/application.js", | |
| "lib/install/package.json", | |
| "lib/install/config/shakapacker.yml", | |
| "lib/install/bin/shakapacker", | |
| "lib/install/bin/shakapacker-dev-server", | |
| "lib/install/config/rspack/rspack.config.js", | |
| "lib/install/config/rspack/rspack.config.ts", | |
| "lib/install/config/webpack/webpack.config.js", | |
| "lib/install/config/webpack/webpack.config.ts" | |
| ) | |
| end |
| "lib/install/bin/shakapacker-dev-server" | ||
| "lib/install/bin/shakapacker", | ||
| "lib/install/bin/shakapacker-dev-server", |
There was a problem hiding this comment.
Bug: missing comma causes implicit string concatenation + duplicate entries
Line 47 is missing a trailing comma, so Ruby silently concatenates the two adjacent string literals into one string:
"lib/install/bin/shakapacker-dev-server" "lib/install/bin/shakapacker"
# => "lib/install/bin/shakapacker-dev-serverlib/install/bin/shakapacker"
That concatenated path doesn't exist, so the assertion is checking for a nonexistent file. Lines 48 and 49 also duplicate the two entries that should have appeared here.
| "lib/install/bin/shakapacker-dev-server" | |
| "lib/install/bin/shakapacker", | |
| "lib/install/bin/shakapacker-dev-server", | |
| "lib/install/bin/shakapacker", | |
| "lib/install/bin/shakapacker-dev-server", |
| "lib/install/config/webpack/webpack.config.ts" | ||
| end |
There was a problem hiding this comment.
Bug: missing closing ) — syntax error
The include( call opened on line 41 is never closed before end. This is a SyntaxError that prevents the entire spec file from loading, so none of the tests in this file will run.
| "lib/install/config/webpack/webpack.config.ts" | |
| end | |
| "lib/install/config/webpack/webpack.config.ts" | |
| ) | |
| end |
Review: [codex] Reduce packaged gem filesOverviewGreat improvement — dropping from 294 → 75 files and 486 K → ~121 K is a meaningful reduction. The explicit allowlist in the gemspec is simpler and safer than the old exclusion-based approach with gemspec change ✅s.files = `git ls-files -z CHANGELOG.md MIT-LICENSE README.md shakapacker.gemspec lib sig`.split("\x00")Clean and correct. Clearing spec file — two bugs that need a follow-up fix
|
Code Review: Reduce packaged gem filesOverviewThis PR replaces the broad gemspec change ✅The change from: s.files = `git ls-files -z`.split("\x0").reject { |f|
f.match(%r{^(test|spec|features|tmp|node_modules|packages|coverage|Gemfile.lock|rakelib)($|/)}) ||
f.end_with?(".gem")
} + Dir.glob("sig/**/*.rbs")to: s.files = `git ls-files -z CHANGELOG.md MIT-LICENSE README.md shakapacker.gemspec lib sig`.split("\x0")is correct and simpler. All Minor note: gemspec_spec.rb — Critical Bugs ❌The new 1. Missing comma → implicit string concatenation (line 47) "lib/install/bin/shakapacker-dev-server" # ← no comma
"lib/install/bin/shakapacker", # ← adjacent string literalIn Ruby, adjacent string literals without a separator are silently concatenated at parse time, producing the single bogus string 2. Missing closing expect(gemspec.files).to include(
...
"lib/install/config/webpack/webpack.config.ts"
end # ← closes the `it` block but `include(` is never closedThe These two bugs together mean the install-asset coverage added by this PR provides no real protection — the test either doesn't parse or tests for a nonexistent path. 3. Duplicate entries (lines 45–46 and 48–49)
CIThe Linting check returned Recommended follow-upA minimal fix in a follow-up PR: it "includes install assets needed by shakapacker:install" do
expect(gemspec.files).to include(
"lib/install/template.rb",
"lib/install/application.js",
"lib/install/package.json",
"lib/install/config/shakapacker.yml",
"lib/install/bin/shakapacker",
"lib/install/bin/shakapacker-dev-server",
"lib/install/config/rspack/rspack.config.js",
"lib/install/config/rspack/rspack.config.ts",
"lib/install/config/webpack/webpack.config.js",
"lib/install/config/webpack/webpack.config.ts"
)
end |
| it "includes install assets needed by shakapacker:install" do | ||
| expect(gemspec.files).to include( | ||
| "lib/install/template.rb", | ||
| "lib/install/application.js", | ||
| "lib/install/package.json", | ||
| "lib/install/config/shakapacker.yml", | ||
| "lib/install/bin/shakapacker", | ||
| "lib/install/bin/shakapacker-dev-server" | ||
| "lib/install/bin/shakapacker", | ||
| "lib/install/bin/shakapacker-dev-server", | ||
| "lib/install/config/rspack/rspack.config.js", | ||
| "lib/install/config/rspack/rspack.config.ts", | ||
| "lib/install/config/webpack/webpack.config.js", | ||
| "lib/install/config/webpack/webpack.config.ts" | ||
| end |
There was a problem hiding this comment.
This it block has two bugs that make it provide no real coverage:
Bug 1 — Missing closing ) for include( (line 54)
The include( opened on line 41 is never closed with ). Ruby sees the end on line 54 and raises a SyntaxError — the block never runs.
Bug 2 — Missing comma causes implicit string concatenation (line 47)
"lib/install/bin/shakapacker-dev-server" # ← no trailing comma
"lib/install/bin/shakapacker",Ruby silently concatenates adjacent string literals at parse time, producing "lib/install/bin/shakapacker-dev-servershakapacker". The test would be asserting the existence of a path that never exists.
Bug 3 — Duplicate entries
"lib/install/bin/shakapacker" and "lib/install/bin/shakapacker-dev-server" each appear twice.
| it "includes install assets needed by shakapacker:install" do | |
| expect(gemspec.files).to include( | |
| "lib/install/template.rb", | |
| "lib/install/application.js", | |
| "lib/install/package.json", | |
| "lib/install/config/shakapacker.yml", | |
| "lib/install/bin/shakapacker", | |
| "lib/install/bin/shakapacker-dev-server" | |
| "lib/install/bin/shakapacker", | |
| "lib/install/bin/shakapacker-dev-server", | |
| "lib/install/config/rspack/rspack.config.js", | |
| "lib/install/config/rspack/rspack.config.ts", | |
| "lib/install/config/webpack/webpack.config.js", | |
| "lib/install/config/webpack/webpack.config.ts" | |
| end | |
| it "includes install assets needed by shakapacker:install" do | |
| expect(gemspec.files).to include( | |
| "lib/install/template.rb", | |
| "lib/install/application.js", | |
| "lib/install/package.json", | |
| "lib/install/config/shakapacker.yml", | |
| "lib/install/bin/shakapacker", | |
| "lib/install/bin/shakapacker-dev-server", | |
| "lib/install/config/rspack/rspack.config.js", | |
| "lib/install/config/rspack/rspack.config.ts", | |
| "lib/install/config/webpack/webpack.config.js", | |
| "lib/install/config/webpack/webpack.config.ts" | |
| ) | |
| end |
## Summary PR #1110 (merged earlier today) introduced a syntax error in `spec/shakapacker/gemspec_spec.rb`. The new "includes install assets needed by shakapacker:install" test duplicated two bin entries (with the first one missing its trailing comma) and omitted the closing `)` for `include(`. The file no longer parses as Ruby. This breaks CI on every open PR against main: - **Linting** (`bundle exec rubocop`): `Lint/Syntax: unexpected token tSTRING` - **Testing** (`bundle exec rspec spec/shakapacker/*_spec.rb`): "1 error occurred outside of examples" — rspec aborts before running any test ## Fix Removed the duplicated bin lines and added the missing closing paren. \`\`\`diff "lib/install/bin/shakapacker", - "lib/install/bin/shakapacker-dev-server" - "lib/install/bin/shakapacker", "lib/install/bin/shakapacker-dev-server", "lib/install/config/rspack/rspack.config.js", ... "lib/install/config/webpack/webpack.config.ts" + ) end \`\`\` ## Test plan - [x] \`bundle exec rubocop\` passes (140 files, no offenses) - [x] \`bundle exec rspec spec/shakapacker/gemspec_spec.rb\` passes (9 examples, 0 failures) - [ ] CI green 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Low risk: test-only change that fixes a Ruby syntax error which was preventing lint/spec runs, without impacting runtime code paths. > > **Overview** > Fixes a syntax error in `spec/shakapacker/gemspec_spec.rb` that was breaking parsing/CI by removing duplicated `bin` entries in the `include(...)` list and restoring the missing closing `)` for the expectation. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 16f02ed. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
## Summary Stamp the `v10.1.0-rc.0` version header (dated May 20, 2026) on the existing Unreleased entries and add two missing post-`v10.0.0` user-visible changes. ### New entries added - `Changed`: **Slimmed the published gem from ~486K (294 files) to ~121K (75 files)** — [PR #1110](#1110) - `Fixed`: **Detected single-dot (`./...`) local-path declarations in `NodePackageVersion#find_version`** — [PR #1106](#1106) (follow-up to #1086) ### Stamp - Renamed the existing Unreleased section to `## [v10.1.0-rc.0] - May 20, 2026` and inserted a fresh empty `## [Unreleased]` above it. - Updated the version diff links at the bottom (`[Unreleased]` now compares from `v10.1.0-rc.0`; new `[v10.1.0-rc.0]` link compares from `v10.0.0`). ### Skipped (not user-visible) - #1118 (prettier formatting), #1109 / #1108 / #1094 / #1092 (docs only), #1117 (CI bump), #1113 (setup-script internal), #1111 (release task internal), #1114 (test fix), #1098 (internal prompt sync), #919 (test/lint). ## Test plan - [x] `yarn lint` passes - [ ] Reviewer confirms the version stamp + diff links are correct - [ ] After merge: run `bundle exec rake release` (no args — picks up `v10.1.0-rc.0` from CHANGELOG.md) 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Low risk documentation-only change that updates release notes and version comparison links with no runtime/code impact. > > **Overview** > **Prepares release notes for `v10.1.0-rc.0`** by moving the existing Unreleased items under a new dated `v10.1.0-rc.0` header and adding a fresh empty `## [Unreleased]` section. > > Also adds two changelog entries (gem package slimming and improved local-path detection in `NodePackageVersion#find_version`) and updates the compare links so `[Unreleased]` now diffs from `v10.1.0-rc.0` and a new `[v10.1.0-rc.0]` link compares against `v10.0.0`. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit f3cf5fa. 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** * Changelog updated for v10.1.0-rc.0 release. * **Changed** * Published package size optimized. * **Bug Fixes** * Rspack React Refresh v2 loading compatibility resolved. * Node package version resolution improved for local-path dependencies. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/shakacode/shakapacker/pull/1119?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.7 (1M context) <noreply@anthropic.com>
](https://app.coderabbit.ai/change-stack/shakacode/shakapacker/pull/1119?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack){kind=link}
PR #1110 slimmed shakapacker.gemspec to an explicit allowlist (CHANGELOG.md MIT-LICENSE README.md shakapacker.gemspec lib sig), dropping the gem-root package.json. But lib/tasks/shakapacker/check_node.rake still reads it via `Pathname.new("#{__dir__}/../../../package.json").realpath` for the engines.node range. In the installed gem, realpath raised Errno::ENOENT and the outer rescue printed the misleading "Node.js not installed. Exiting!" — most visibly during `rails assets:precompile` via shakapacker:clean → verify_install → check_node, even though Node had just run the webpack build. - shakapacker.gemspec: re-add package.json to the allowlist. - spec/shakapacker/gemspec_spec.rb: regression test for the inclusion so a future trim cannot silently re-break it. - lib/tasks/shakapacker/check_node.rake: drop .realpath and gate the engines-range check on pkg_path.exist?, so a missing file no longer masquerades as Node missing. - CHANGELOG.md: Fixed entry under [Unreleased]. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PR #1110 slimmed shakapacker.gemspec to an explicit allowlist (CHANGELOG.md MIT-LICENSE README.md shakapacker.gemspec lib sig), dropping the gem-root package.json. But lib/tasks/shakapacker/check_node.rake still reads it via `Pathname.new("#{__dir__}/../../../package.json").realpath` for the engines.node range. In the installed gem, realpath raised Errno::ENOENT and the outer rescue printed the misleading "Node.js not installed. Exiting!" — most visibly during `rails assets:precompile` via shakapacker:clean → verify_install → check_node, even though Node had just run the webpack build. - shakapacker.gemspec: re-add package.json to the allowlist. - spec/shakapacker/gemspec_spec.rb: regression test for the inclusion so a future trim cannot silently re-break it. - lib/tasks/shakapacker/check_node.rake: drop .realpath and gate the engines-range check on pkg_path.exist?, so a missing file no longer masquerades as Node missing. - CHANGELOG.md: Fixed entry under [Unreleased]. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…gem (#1120) ## Summary - **Root cause:** [PR #1110](#1110) slimmed `shakapacker.gemspec` to an explicit allowlist (`CHANGELOG.md MIT-LICENSE README.md shakapacker.gemspec lib sig`), which dropped the gem-root `package.json` from the published gem. But `lib/tasks/shakapacker/check_node.rake` still reads it via `Pathname.new("#{__dir__}/../../../package.json").realpath` for the `engines.node` range. In the installed gem `realpath` raises `Errno::ENOENT`, the outer `rescue Errno::ENOENT` swallows it, and the task prints the misleading `Node.js not installed. Exiting!` — most visibly during `bundle exec rails assets:precompile`, since `react_on_rails` enhances that task to invoke `shakapacker:clean` → `verify_install` → `check_node` (and Node had just run the webpack build seconds earlier). - **Fixes:** - `shakapacker.gemspec` — re-add `package.json` to the allowlist so the published gem contains the file `check_node.rake` reads. - `lib/tasks/shakapacker/check_node.rake` — drop `.realpath` and gate the engines-range check on `pkg_path.exist?`. If `package.json` is ever absent again, the task skips the version check rather than blaming Node. - `spec/shakapacker/gemspec_spec.rb` — regression test asserting `gemspec.files.include?("package.json")`, so a future allowlist trim cannot silently re-break this. - `CHANGELOG.md` — Fixed entry under `[Unreleased]`. - **Gem footprint:** went from 75 files (~121K) to 76 files; `package.json` is ~6.7K, so the slim-gem goal of [#1110](#1110) is essentially preserved. ## Repro (before fix) ``` $ mv package.json /tmp/ && cd spec/shakapacker/test_app $ bundle exec rake shakapacker:check_node Node.js not installed. Please download and install Node.js https://nodejs.org/en/download/ Exiting! $ node -v v22.20.0 # Node was here the whole time ``` ## Test plan - [x] Reproduced the bug on baseline (above). - [x] After fix: same scenario with `package.json` hidden → `rake shakapacker:check_node` exits 0 silently (no false Node error). - [x] Happy path unchanged: `rake shakapacker:check_node` from `spec/shakapacker/test_app` exits 0. - [x] `bundle exec rspec spec/shakapacker/gemspec_spec.rb spec/shakapacker/rake_tasks_spec.rb` → 24 examples, 0 failures. - [x] `bundle exec rubocop shakapacker.gemspec lib/tasks/shakapacker/check_node.rake spec/shakapacker/gemspec_spec.rb` → 0 offenses. - [x] `bundle exec gem build shakapacker.gemspec` → succeeds; `tar -tz` on the resulting gem confirms both `package.json` and `lib/install/package.json` ship. - [ ] CI green. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Low risk: small packaging + rake-task guard changes that only affect Node version checking and error messaging when `package.json` is missing. > > **Overview** > Fixes `shakapacker:check_node` incorrectly reporting Node as missing in the published gem by **shipping the gem-root `package.json`** again and making the task **skip the `engines.node` validation when that file isn’t present**. > > Adds a regression spec to ensure `package.json` remains included in `shakapacker.gemspec`, and documents the fix in `CHANGELOG.md`. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 20f8d4b. 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 * **Bug Fixes** * Fixed `shakapacker:check_node` task incorrectly reporting "Node.js not installed" when the gem package was missing `package.json`. * Gem packaging now includes `package.json` to ensure proper Node.js version validation. * Task now gracefully skips version range checks when `package.json` is unavailable instead of raising errors. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/shakacode/shakapacker/pull/1120?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.7 (1M context) <noreply@anthropic.com>
](https://app.coderabbit.ai/change-stack/shakacode/shakapacker/pull/1120?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack){kind=link}
## Summary Stamps the `v10.1.0-rc.1` header and collapses `v10.1.0-rc.0` per the project's RC collapse convention. - Renamed `## [v10.1.0-rc.0] - May 20, 2026` → `## [v10.1.0-rc.1] - May 21, 2026` - Moved the post-rc.0 `Unreleased` Fixed entry for PR #1032 (webpack-dev-server `static` config) into the merged `### Fixed` section - Dropped the PR #1120 entry (`shakapacker:check_node` regression fix) — it only fixes a bug introduced by PR #1110 that shipped solely in `v10.1.0-rc.0`, so users going from `v10.0.0` → `v10.1.0-rc.1` never see it - Updated `[Unreleased]` and `[v10.1.0-rc.1]` diff links ## Test plan - [ ] CI passes - [ ] Maintainer confirms the rc.0 → rc.1 collapse looks right <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Low risk documentation-only change that updates release notes and comparison links without affecting runtime code. > > **Overview** > **Updates `CHANGELOG.md` for the `v10.1.0-rc.1` release.** > > Renames the `v10.1.0-rc.0` section to `v10.1.0-rc.1` (with the new date), moves the webpack-dev-server `static` fix (PR #1032) from *Unreleased* into the release’s *Fixed* section, removes the rc.0-only `shakapacker:check_node` regression note (PR #1120), and updates the bottom compare links to point at `v10.1.0-rc.1`. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit eaf83d5. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PR #1110 (gem allowlist) and PR #1120 (added `package.json` to the allowlist to fix `shakapacker:check_node`) ship together in v10.1.0-rc.1. Update the rc.1 entry to credit both and reflect the final allowlist instead of just the PR #1110 state. The earlier "drop PR #1120" change (PR #1124) is now superseded. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
## Summary Follow-up to #1124. Per maintainer feedback ("we might just merge both entries"), update the `v10.1.0-rc.1` changelog so the PR #1110 entry reflects the **final shipped state** instead of dropping the PR #1120 fix entirely. - Credit both PRs in the link list: `[PR #1110](…), [PR #1120](…)` - Add `package.json` to the documented allowlist — that's what actually ships in v10.1.0-rc.1 thanks to PR #1120 This is a docs-only change. The released gem and tag are unaffected. If you want the GitHub release notes for v10.1.0-rc.1 to reflect this too, the release notes need to be edited on GitHub after this merges. The companion `/update-changelog` command update in #1125 already prefers this "merge into the original PR entry" approach over "drop entirely". ## Test plan - [ ] Maintainer confirms the merged entry reads correctly <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Docs-only change updating a single changelog entry; no runtime code or packaging behavior is modified. > > **Overview** > Updates the `v10.1.0-rc.1` changelog entry for the gem size reduction to reflect the final shipped state by **crediting both** [PR #1110] and [PR #1120] and by **including `package.json`** in the documented gem file allowlist. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 76d3a20. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
## Summary When collapsing prereleases (e.g., `v10.1.0-rc.0` → `v10.1.0-rc.1`), a fix for a bug introduced by a PR that only ever shipped in the prerelease train is invisible to consumers of the last stable release. Listing it under the new RC/release header just adds noise. This PR updates `.claude/commands/update-changelog.md` so the command: - **Excludes RC-only regression fixes** in the "Do NOT add entries for" list (with a forward-reference to the detailed rule) - **Adds a dedicated step** in the RC/beta collapse procedure that walks through detecting and dropping these entries, with the rc.0 → rc.1 example - **Requires surfacing dropped entries in the final summary** so the maintainer can override case-by-case (e.g., keep one for RC testers) ## Motivation Caught while stamping `v10.1.0-rc.1` in #1124. PR #1120 fixes a regression introduced by PR #1110 (gem-file allowlist), which itself only shipped in `v10.1.0-rc.0`. Users on `v10.0.0` jumping straight to `v10.1.0-rc.1` never see the bug, so PR #1120's fix is internal RC churn rather than a user-facing change. The previous version of the command would have kept it in the changelog. ## Test plan - [ ] Maintainer review of the prose - [ ] Try the next `/update-changelog rc` invocation and confirm the model now asks about (or silently drops) RC-only regression fixes <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Low risk: documentation/procedure updates only, affecting how changelog entries are curated during RC/beta version stamping. > > **Overview** > Updates the `/update-changelog` instructions to **avoid standalone changelog entries for RC-only regression fixes** (fixes that only undo a regression introduced earlier in the same prerelease train). > > Adds a dedicated prerelease-collapsing step that directs the maintainer to **merge such fixes into the original prerelease PR entry (or drop them if redundant)**, and to **explicitly list any merged/dropped items in the final summary** for review/override. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 08b4488. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…coverage-v1 * origin/main: Teach /update-changelog to drop RC-only regression fixes (#1125) Restructure supplemental package dependencies (#1131) (#1133) [codex] Add Shakapacker brand assets (#1135) [codex] Fix shakapacker config helper binstubs (#1132) Surface v10.1 supplemental packages from README and key docs (#1134) CHANGELOG: merge PR #1120 into PR #1110 entry for v10.1.0-rc.1 (#1126) # Conflicts: # lib/install/bin/diff-bundler-config # lib/install/bin/shakapacker-config # package/configExporter/cli.ts # spec/dummy/bin/shakapacker-config # spec/shakapacker/binstub_sync_spec.rb # spec/shakapacker/helper_binstubs_spec.rb
1 participant
Summary
git ls-filesgem file list with an explicit runtime/install allowlist.test_filesmetadata in the Ruby gem.Package impact
Fixes #987
Validation
bundle exec rspec spec/shakapacker/gemspec_spec.rbbundle exec rubocop shakapacker.gemspec spec/shakapacker/gemspec_spec.rbgit diff --checkgem build shakapacker.gemspecruby -rrubygems/package -e 'path = Dir["shakapacker-*.gem"].first; files = Gem::Package.new(path).spec.files; puts "size=#{File.size(path)}"; puts "file_count=#{files.length}"'Note
Medium Risk
Medium risk because it changes what ships in the published gem; missing required runtime/installer assets would only surface at install/runtime for consumers.
Overview
Switches gem packaging from a broad
git ls-files+ exclusions to an explicit allowlist (CHANGELOG.md,MIT-LICENSE,README.md, gemspec,lib,sig), and clearss.test_filesmetadata.Expands gemspec specs to assert repo-only directories/files (e.g.
package/,docs/,test/,node_modules/,spec/,Gemfile.lock) are excluded while required installer assets forshakapacker:installand.rbssignatures remain included.Reviewed by Cursor Bugbot for commit ce32605. Bugbot is set up for automated code reviews on this repo. Configure here.