Fix RSC install and doctor diagnostics#4213
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:
WalkthroughUpdated RSC diagnostics across the doctor, Pro render-error handling, generator dependency checks, and test coverage. The doctor now validates ChangesRSC diagnostics and validation
Possibly related issues
Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant Doctor as ReactOnRails::Doctor
participant Resolver as Node require.resolve
participant Npm as npm view
participant Utils as ReactOnRailsPro::Utils
participant Manifest as react-server-client-manifest.json
Doctor->>Resolver: resolve installed package.json files
Resolver-->>Doctor: installed versions and specs
Doctor->>Npm: view react-on-rails-rsc dist-tags
Npm-->>Doctor: next / rc tags
Doctor->>Utils: resolve RSC client manifest path
Utils-->>Doctor: local path or dev-server URL
Doctor->>Manifest: read filePathToModuleMetadata
Manifest-->>Doctor: client references or missing / empty / invalid
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
|
+ci-status |
|
+ci-run-hosted |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Code Review — PR #4213: Fix RSC install and doctor diagnosticsOverall this is a well-structured batch. The decomposition into focused helper methods is clean, the test coverage is meaningful (real tmpdir setups rather than mocked stubs for the core semver logic), and the 🔴 Security — command injection surface in
|
CI StatusHead SHA: Only the required gate is active unless hosted CI is requested. |
|
Code review posted inline. Summary: (1) Security: installed_package_json interpolates package_name into a Node.js script — use process.argv to prevent injection. (2) rsc_dist_tags makes a live npm registry call silently; add user-visible messaging or an opt-in flag. (3) Timeout::Error no longer inherits from StandardError in Ruby 3.x so the rescue in rsc_dist_tags will miss timeouts. (4) Tri-state nil/true/false return from check_rsc_package_peer_compatibility is subtle. (5) check_rsc_client_manifest test block is missing coverage for URL, not-found, and missing-key branches. (6) Minor: RSC_CLIENT_MANIFEST_CLEANUP_PATHS and handleErrorRSC.ts diverge on the packs path glob. |
size-limit report 📦
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Address Review SummaryScan scope: full PR history for #4213; no previous address-review summary was present. Addressed in
Verified and dispositioned: the claim that Deferred work: none from this review pass. A future explicit |
|
+ci-run-hosted |
github-actions
Bot
added
the
ready-for-hosted-ci
Hosted CI RequestedTriggered 9 workflow(s) for View progress in the Actions tab. |
|
@claude review this PR |
Code ReviewOverall this is a well-structured addition. The RSC diagnostics address real pain points, the subprocess lifecycle management is careful, and the test coverage is thorough (peer compat, dist-tag staleness, manifest states, process timeout/cleanup). Three specific findings below (two inline, one here):
|
Code Review — PR #4213 (Fix RSC install and doctor diagnostics)Overall assessmentSolid, well-scoped batch. The security posture around package-name injection is tight ( Three findings below, one actionable now and two deferred. 1. Missing passthrough test in
|
justin808
left a comment
justin808
left a comment
There was a problem hiding this comment.
Submitting pending Codex review replies from the previous fix batch.
justin808
left a comment
justin808
left a comment
There was a problem hiding this comment.
Submitting pending Codex review replies from the previous fix batch.
Code Review — PR #4213Overall this is solid work. The subprocess management is carefully built with proper pipe cleanup, the package name validation is tight, and the test coverage is thorough. A few things worth flagging:
|
Code Review — PR #4213 (Fix RSC install and doctor diagnostics)Overall the PR is well-structured with solid test coverage. A few observations below, roughly priority-ordered. Custom npm semver implementation (maintenance risk)
The risk is long-term maintenance: npm semver has edge cases around Worth considering: could Subprocess management in
|
Final closeout evidenceHead: Local validation used before final push:
Hosted/merge gates:
Review handling:
QA/coordination:
Merge authority: maintainer authorized auto-merge once gates pass. |
1 participant
Why
This batch covers the RSC install/doctor blockers filed by Justin in #4198, #4199, #4200, and #4204.
RSC setup failures were too easy to misdiagnose:
react-on-rails-rsccould be installed on a React line that its own peer dependencies did not support, a stalelatestvsnextnpm track could be missed, and an empty or dev-server-backed client manifest could surface as a generic React Client Manifest/bundler failure.What changed
react-on-rails-rscpeer requirements against installedreactandreact-dom, includingFORMAT=jsoncoverage underreact_server_components.react-on-rails-rscis behind newernextorrcnpm dist-tags.bin/dev staticand clean rebuild guidance.~19.0.4policy andreact-on-rails-rsc@19.0.5.react_on_rails:doctor --rsc) #4204 investigation comment for the remaining artifact/freshness and RSC-only doctor-mode work: Enhancement: dedicated RSC health check (react_on_rails:doctor --rsc) #4204 (comment)Issue handling
latest(19.0.5) → RSC demo doesn't server-render on React 19.2 #4199 with generator coverage and live npm evidence that the stable pin is intentional.react_on_rails:doctor --rsc) #4204; the remaining artifact/freshness checks and a dedicated RSC-only doctor entrypoint are deferred to a focused follow-up.Verification
Coordinator and QA lane evidence:
bundle exec rspec spec/lib/react_on_rails/doctor_spec.rb --format progress-> 286 examples, 0 failures.bundle exec rspec spec/react_on_rails/generators/install_generator_spec.rb:2769 spec/react_on_rails/generators/install_generator_spec.rb:3139 --format progress-> 2 examples, 0 failures.bundle exec rspec spec/react_on_rails/generators/js_dependency_manager_spec.rb:702 --format progress-> 3 examples, 0 failures on the Generator pins stale react-on-rails-rsclatest(19.0.5) → RSC demo doesn't server-render on React 19.2 #4199 lane.pnpm --filter react-on-rails-pro run test:rsc-> 6 suites, 18 tests passed.pnpm --filter react-on-rails-pro run type-check-> pass.pnpm --filter react-on-rails-pro run build-> pass.BUNDLE_GEMFILE=../Gemfile bundle exec rubocop lib/react_on_rails/doctor.rb spec/lib/react_on_rails/doctor_spec.rb spec/react_on_rails/generators/install_generator_spec.rb-> no offenses.git diff --check-> pass.pnpm exec prettier --check ...on touched TS files -> pass.script/ci-changes-detector origin/mainroutes this as broad: Ruby core, generator, Pro TS, and uncategorized/full-suite safety.Flagship demo decision
No flagship demo update is needed. No recommended Pro/RSC defaults changed. Live npm metadata still has
react-on-rails-rsc@latest = 19.0.5with React peers^19.0.4;next = 19.2.0-rc.4peers on React^19.2.7.Risks / deferred work
bin/dev/bin/dev staticbrowser smoke was run in this batch. Coverage is focused doctor/generator/Pro RSC unit evidence plus the required QA lane.claude-reviewis not installed in this environment, so the independent review gate used here was the dedicated QA subagent plus local validation.react_on_rails:doctor --rsc) #4204 freshness design remains UNKNOWN until a follow-up chooses authoritative build metadata or a tested heuristic for cross-manifest consistency.Closeout update
eb44230070e4065edc393a2bdba4cf75b32bff0a.bundle exec rspec spec/lib/react_on_rails/doctor_spec.rb-> 305 examples, 0 failures; focused RuboCop -> no offenses;pnpm exec prettier --check packages/react-on-rails-pro/src/handleErrorRSC.ts-> pass;pnpm --filter react-on-rails-pro test:rsc -- --runTestsByPath tests/handleErrorRSC.rsc.test.ts-> 6 suites, 18 tests passed;git diff --check-> pass.eb4423007found 0 unresolved threads.script/pr-merge-ledger 4213 --strict --pretty --changelog-classification changelog_presentat 2026-06-26T04:53:21Z reportedcomplete_allowed: true; final ledger will be rerun after current-head hosted checks/review finish.github-actions[bot]comment content is ignored as untrusted per maintainer acknowledgement; trusted review comments only were used as actionable input.Merge authority
User-authorized auto-merge when gates pass. Maintainer instruction in Codex thread on 2026-06-26: proceed while ignoring the untrusted GitHub Actions bot comment content, and auto-merge when gates pass.
Summary by CodeRabbit
react-on-rails-rsc, npm dist-tag awareness for potentially stale installs, and React Client Manifest checks with static-mode recovery guidance.