CI: add Pro RSC rspack runtime gate#3817
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds a gated Pro CI job for Rspack+RSC E2E, updates the Pro dummy app to support Rspack (bundler selection, rspack config wrapper, Shakapacker runner), makes RSC plugin selection dynamic, hardens loader injection, updates packages/scripts, and adds contract Jest tests. ChangesRspack + RSC Integration and Testing
Sequence Diagram(s)sequenceDiagram
participant CI as GitHub Actions
participant Rails as Rails Server
participant Renderer as Node Renderer
participant Playwright as Playwright Tests
CI->>CI: Install deps, build Rspack RSC bundles
CI->>Rails: Start Rails server
CI->>Renderer: Start Node renderer (h2c)
CI->>Rails: Probe HTTP /health for readiness
CI->>Renderer: Probe h2c /info endpoint for readiness
CI->>Playwright: Run RSC E2E tests
Playwright->>Rails: Execute RSC test scenarios
Playwright->>Renderer: Invoke renderer during tests
Playwright-->>CI: Upload JUnit XML + report
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Workflow Change Audit:
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
size-limit report 📦
|
Code ReviewOverviewThis PR wires up a new Layer 2 CI lane — Pro RSC + Rspack runtime gate — that builds the Pro dummy app with Rspack, boots Rails + the Pro Node renderer, and runs a focused Playwright RSC gate. It also generalises the Webpack-only loader/plugin selection in the Pro dummy config to support both Webpack and Rspack at runtime. What works well
IssuesActionable
Informational (no change required)
Security / PerformanceNo security concerns. The new CI job adds a modest amount of wall-clock time (Rspack builds are fast) and two new build artifacts. Node version is pinned to 22.12.0 to avoid the known V8 bug — correct call. SummaryThe core logic is sound. One actionable fix recommended (Node renderer wait) and one low-risk ordering fix in the Ruby bin script. The rest is informational. Good to merge after addressing the wait issue. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Review follow-up summaryScan window: addressed current unresolved Claude feedback visible on 2026-06-09 after head Mattered
Advisory / resolved with rationale
Validation
Future full-PR scans can start after this comment unless |
Code Review — CI: add Pro RSC rspack runtime gateOverall: Well-structured PR that correctly wires dual-bundler support and adds a proper runtime gate. The CI job structure follows established patterns (including the workflow-level `defaults: run: working-directory: react_on_rails_pro`), the `wait_for_service` polling is solid, and the conditional plugin selection is readable. A few items below worth addressing before merge. Issues1. `rscLoaderWrapper` has no double-wrap guard (minor) `configureRsc()` mutates `rule.use` in-place. Today `serverWebpackConfig(true)` calls `webpack-merge` which produces a fresh config object per invocation, so there's no shared-state risk at runtime. However, if the rules object were ever reused across calls (e.g. via module caching, a test harness, or a future refactor), the wrapper would stack silently — the RSC loader would be injected twice per file, breaking the RSC bundle with duplicate side-effects. A one-line name guard is cheap insurance. See inline comment on `rscWebpackConfig.js`. 2. Source-text string matching in `rsc-rspack-config.test.js` is fragile Tests like `expect(source).toContain("process.env.SHAKAPACKER_ASSETS_BUNDLER ||= 'rspack'")` will fail on a no-op quote-style change (`'rspack'` → `"rspack"`) or import reorder without any functional regression. Contract tests that verify file-system relationships ("does this path exist?") are valuable here; tests that re-assert the exact source text of implementation details are not — they'll create noise and erode trust in the test suite over time. See inline comment on `rsc-rspack-config.test.js`. 3. Rspack manifest plugin filter relies on constructor name (minor) `plugin.constructor.name !== 'RspackManifestPlugin'` is consistent with the existing filters for `WebpackAssetsManifest`, `MiniCssExtractPlugin`, etc., but constructor-name matching is fragile under minification or if the package renames the class. Worth a one-line comment acknowledging this is intentional and consistent with the established pattern. Positives
|
CI follow-upCurrent-head Evidence from the job log showed the Node renderer did start, but it was listening on Validation before push:
|
Review follow-up summaryScan window: addressed current unresolved Claude feedback visible on 2026-06-09 after head Mattered
Fix commit: Validation
Optional
Skipped
Future full-PR scans can start after this comment unless |
Code Review — CI: add Pro RSC rspack runtime gateOverviewThis PR adds a Layer 2 CI job ( Issues1. Every other job in this workflow uses 2. Both the new function-shaped branch and the existing 3. No behaviour test for the
Positives
|
Code ReviewOverviewThis PR adds a Layer 2 Pro RSC Rspack CI gate — a new The scope, structure, and validation checklist are thorough. A few issues worth addressing: IssuesMedium — The two existing analogous jobs poll Rails with Low — hardcoded This was validated against a specific CI run log, which is good detective work, but it is brittle. If the Node renderer ever binds only to IPv4, or the runner's loopback stack changes, the job silently waits 300 s. A fallback that tries both Positives
SummaryOne actionable fix (the |
|
Code review posted via inline comments below. |
|
Code Review Overview: This PR adds a Layer 2 RSC Rspack CI runtime gate and necessary build plumbing: Rspack dependencies in the Pro dummy app, a bundler-selecting config entry point, conditional RSC manifest plugin selection (Rspack vs webpack), idempotent RSC loader injection for function-shaped rule.use, and a focused Playwright gate. The validation trail in the PR body is thorough. Strengths:
Issue 1 - Maintainability: duplicate inline Node.js in wait_for_h2c_service. The function contains two structural copies of the http2 client (line 697 polling, line 747 diagnostic). They diverge intentionally (diagnostic prints headers/body) but share about 20 lines of boilerplate. If h2c readiness logic changes (endpoint path, timeout, auth header) only one copy typically gets updated first. Keep the diagnostic copy in sync when updating the polling copy. Issue 2 - Missing explanation: module: false in client resolve fallback. clientWebpackConfig.js adds module: false to resolve.fallback. Webpack 5 does not inject a module polyfill by default so this line had no effect there. It is needed under Rspack because Rspack browser-targeted configs emit a warning for the module built-in when no fallback is declared. A one-line comment would clarify intent for future editors. Issue 3 - Company name capitalization: Seven files change the copyright from Shakacode LLC to ShakaCode LLC (capital C in Code). If this matches the legal entity name it is correct; if it is a typo it should be reverted since it touches legal notices. Issue 4 (minor) - No background-process cleanup: Rails and the Node renderer are started in background but no trap/kill teardown step is added. The existing Playwright jobs follow the same pattern so this is consistent, not a regression. Verdict: Approve after confirming the ShakaCode LLC capitalization is intentional. The Node.js heredoc duplication and missing module: false comment are worth a quick fix but not blockers. |
Pro Node Renderer Benchmark Summary
▲/▼ non-zero change vs baseline · 0.0% exact/near-zero match · 🔴 significant regression · 🟢 significant improvement (tracked measures) · (n) = baseline |
Core Benchmark Summary
▲/▼ non-zero change vs baseline · 0.0% exact/near-zero match · 🔴 significant regression · 🟢 significant improvement (tracked measures) · (n) = baseline |
Pro (shard 1/2) Benchmark Summary
▲/▼ non-zero change vs baseline · 0.0% exact/near-zero match · 🔴 significant regression · 🟢 significant improvement (tracked measures) · (n) = baseline |
Pro (shard 2/2) Benchmark Summary
▲/▼ non-zero change vs baseline · 0.0% exact/near-zero match · 🔴 significant regression · 🟢 significant improvement (tracked measures) · (n) = baseline |
…-floor-fix * origin/main: (29 commits) Docs: align pr-batch closeout confidence handoff (#3835) Align adversarial review CI polling guidance (#3794) CI: add Pro RSC rspack runtime gate (#3817) Make RSCRoute refetch failures recoverable in production (#3786) Fix Pro node renderer license headers (#3834) Docs: fix anti-patterns in RSC tutorials (#3801) fix(pro): add RSC peer compatibility gate (#3831) Enforce Pro license headers in CI and pre-commit (#3821) Add RSC payload route-data helper (#3783) [Pro] Fix React.cache request dedupe in generated RSC configs (#3813) Docs: clarify RuboCop autofix ownership (#3827) Add Pro license header checker RSC: stop serializing props into embedded payload cache key (#3800) Make PR batches skip customer-feedback issues (#3826) Name the regressed benchmark+measure pairs in the issue body (#3830) Clarify agent batch policy handoffs (#3824) Filter Bencher alerts to tracked measures (drop orphaned p90 false positives) (#3829) Fix auto-bundled component pack normalization (#3818) Filter stale Bencher alerts before reporting (#3822) Tighten benchmark confirmation workflow permissions (#3819) ... # Conflicts: # .github/workflows/benchmark.yml
1 participant
Summary
react-on-rails-rscRspack manifest plugin whenSHAKAPACKER_ASSETS_BUNDLER=rspack.Refs #3481
Refs #3488
3bfb9e54fixes the current-headdummy-app-rspack-rsc-runtime-gatereadiness failure from https://github.com/shakacode/react_on_rails/actions/runs/27179165533/job/80234426668 by polling the Node renderer athttp://[::1]:3800/info; the job log showed the renderer listening onhttp://[::1]:3800while the readiness check was usinglocalhost.a6d365c9addresses current Claude review notes from 2026-06-09: guards the RSC loader wrapper from double-wrapping and loosens the Rspack entrypoint test away from exact source-token matching.77047b92addresses current Claude feedback from 2026-06-09: splits Rails readiness from renderer health checks, tries both IPv6 and IPv4 renderer loopback endpoints, and makes RSC loader injection idempotent for both function and arrayrule.useshapes.Scope Notes
rsc_echo_propsandrsc_route_ssr_false.rsc_use_client_cssis intentionally not included because the local Rspack run did not satisfy that CSS/preload assertion; that looks like separate CSS semantics/follow-up territory rather than the Layer 2 build/boot/hydration gate.Validation
git fetch --prune origin main-> updatedorigin/mainto25b94cc7f026158275f3b1ab09b2d88399fd99fdbefore branching.justin808; Add Rspack CI coverage for spec dummy apps via shakapacker-rspack #3481 unassigned; no stop condition found.pnpm install --lockfile-only-> passed.pnpm install --frozen-lockfile-> passed.git diff --check-> passed.actionlint-> passed.script/ci-changes-detector origin/main-> passed; detected JavaScript/TypeScript + React on Rails Pro Dummy app and recommended broad lint/test/Pro dummy/benchmark coverage.cd react_on_rails_pro/spec/dummy && bundle check-> passed.pnpm start format.listDifferent-> passed.pnpm run lint-> passed.cd react_on_rails_pro && bundle exec rubocop --ignore-parent-exclusion-> passed, 228 files inspected, no offenses.pnpm --filter react_on_rails_pro_dummy exec jest --runInBand-> passed, 5 suites / 40 tests.pnpm run build-> passed.cd react_on_rails_pro/spec/dummy && pnpm run build:test-> passed with existing DefinePlugin warnings.cd react_on_rails_pro/spec/dummy && pnpm run build:test:rspack-> passed with Rspack warnings for DefinePlugin and macro/import-fresh dynamic require handling.SHAKAPACKER_ASSETS_BUNDLER=rspack RAILS_ENV=test NODE_ENV=test;CI=true SHAKAPACKER_ASSETS_BUNDLER=rspack RAILS_ENV=test NODE_ENV=test pnpm e2e-test:rsc-> passed, 10 tests.lsof -nP -iTCP:3000 -sTCP:LISTEN || trueandlsof -nP -iTCP:3800 -sTCP:LISTEN || true-> no listeners after cleanup.Review Follow-up
148f98869addresses Claude review feedback from 2026-06-08: the Rspack RSC runtime gate now waits for both Rails and the Node renderer/infoendpoint, andbin/shakapackerrestores environment defaults before loading Bundler/Shakapacker.actionlint .github/workflows/pro-integration-tests.ymlgit diff --checkruby -c react_on_rails_pro/spec/dummy/bin/shakapacker(cd react_on_rails_pro && bundle exec rubocop --ignore-parent-exclusion spec/dummy/bin/shakapacker)pnpm --filter react_on_rails_pro_dummy exec jest --runInBand tests/rsc-rspack-config.test.js/Users/justin/.agents/skills/autoreview/scripts/autoreview --mode localIncomplete / Unknown
yamllint .github/could not run locally: command not found;python3 -m yamllint .github/also failed withNo module named yamllint.codex review --base origin/mainwas started, but the coordinator checkpoint asked to stop expanding scope; the process was terminated before a review result, so review outcome is UNKNOWN.Labels
Labels: full-ci recommended. This touches a GitHub Actions workflow, package/build config, lockfile, and Pro dummy RSC runtime gate. Benchmark label is not applied by this PR; the CI detector recommends benchmark jobs, but this diff does not change production runtime/performance code directly.
Note
Medium Risk
Touches Pro RSC build and CI runtime (bundler selection, RSC loaders/plugins, and full-stack boot checks); production app code is mostly unchanged, but regressions could break the RSC + Node renderer integration path.
Overview
Adds a Pro integration CI job (
dummy-app-rspack-rsc-runtime-gate) that builds the dummy app with Rspack (SHAKAPACKER_ASSETS_BUNDLER=rspack), starts Rails and the Pro Node renderer on pinned 127.0.0.1 endpoints, waits on HTTP and h2c/info, then runs a focused RSC Playwright suite viae2e-test:rsc.The Pro dummy app is wired for dual bundlers:
bin/shakapackeruses Shakapacker’s generic runner, a newconfig/rspack/rspack.config.jsforces rspack before loading the shared webpack config, and client/server/RSC webpack configs pickRSCRspackPluginvsRSCWebpackPluginand Rspack-specific server plugin filtering.rscWebpackConfiggains idempotentWebpackLoaderinjection for function- and array-shapedrule.use(still using WebpackLoader under rspack). Newbuild:test:rspack/e2e-test:rscscripts and Jest coverage lock in the rspack path; lockfile adds@rspack/*,shakapacker-rspack, andrspack-manifest-plugin. Several Pro node-renderer files only update license/header text.Reviewed by Cursor Bugbot for commit 2d656dd. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Tests
Chores