Forward-port multi-URL rolling-deploy seeding to main - #4782
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
WalkthroughRolling-deploy documentation now defines release-time renderer cache seeding and readiness ordering for promoted images. HTTP adapter configuration is standardized on plural previous URLs, with corresponding implementation, type signature, and specification updates. ChangesRolling deploy cache seeding
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 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 |
Fixes slow SSR after a rolling deploy under the staging-to-production promotion model, where the promoted image was pre-seeded with staging's previous bundle instead of production's draining bundle. Key changes: - Gem: renamed config.rolling_deploy_previous_url to config.rolling_deploy_previous_urls. The built-in HTTP adapter accepts a single URL, a comma-separated string, or an Array. Discovery collects publishable artifact identities across endpoints: duplicate legacy hashes are omitted when provenance is ambiguous, and v2 payloads are accepted only after identity verification. Fetch retries allowed origins in configured order, with graceful per-endpoint failure. Seeding from staging + production lets a promoted image carry both environments' then-live bundles. - Docs: new "Promotion deploys need a boot seed" guidance (build-time vs boot seed, the two-pending-promotions case, readiness-gate-on-completion), and renderer-topology awareness (renderer-before-Rails ordering applies only to the separate-workloads topology; same-workload deploys atomically) cross-linked with container-deployment.md. - Domain records: react_on_rails_pro/CONTEXT.md, ADR 0001, CONTEXT-MAP. Breaking change (RC-only): the singular rolling_deploy_previous_url was introduced during the 17.0.0 RC cycle and never shipped in a stable release, so the rename is a free change within 17.0.0 rather than a break for any released version. Rename any config.rolling_deploy_previous_url and the ROLLING_DEPLOY_PREVIOUS_URL build arg to their plural forms. Targeted at release/17.0.0 (rc phase, development mode); forward-port to main via git cherry-pick -x. (cherry picked from commit cb79ee7)
Greptile SummaryForward-ports the plural multi-origin rolling-deploy configuration and promotion guidance.
Confidence Score: 5/5The PR appears safe to merge with no concrete changed-code defect identified. The executable changes consistently remove the temporary singular configuration surface across implementation, signatures, tests, and documentation while preserving the existing guarded multi-origin discovery and fetch behavior. Important Files Changed
Sequence DiagramsequenceDiagram
participant Seed as Build/Boot Seed
participant A as Previous Origin A
participant B as Previous Origin B
participant Renderer as New Node Renderer
Seed->>A: GET /manifest
Seed->>B: GET /manifest
A-->>Seed: Artifact IDs
B-->>Seed: Artifact IDs
loop Each eligible artifact
Seed->>A: GET /bundles/:id
alt Missing, failed, or rejected payload
Seed->>B: GET /bundles/:id
B-->>Seed: Verified bundle and companions
else Verified response
A-->>Seed: Verified bundle and companions
end
Seed->>Renderer: Stage cache entry
end
Reviews (1): Last reviewed commit: "Multi-URL rolling-deploy adapter + promo..." | Re-trigger Greptile |
6f7bdcf to
fac2944
Compare
|
+ci-status |
CI StatusHead SHA: Only the required gate is active unless hosted CI is requested. |
|
+ci-run-hosted |
Code ReviewOverviewClean forward-port of #4544's source changes to Code quality
Test coverage
Minor observations (non-blocking)
SecurityNo new attack surface — the change is a config-surface rename plus documentation; the existing v2 identity verification, origin-ordering, and HTTPS/loopback token-safety logic are unchanged. Overall: solid, well-tested, mechanical rename with thorough documentation. No functional bugs found. |
Hosted CI RequestedTriggered 9 workflow(s) for View progress in the Actions tab. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fac2944574
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@docs/pro/rolling-deploy-adapters.md`:
- Around line 154-160: Define a supported execution location for
react_on_rails_pro:pre_seed_renderer_cache that is compatible with the Node
renderer entrypoint: either document a combined Ruby/Bundler runtime and
command, or move the boot seed to a Ruby-capable release job, sidecar, or sync
path. Update docs/pro/rolling-deploy-adapters.md lines 154-160,
react_on_rails_pro/CONTEXT.md lines 37-40, and
react_on_rails_pro/docs/adr/0001-boot-seed-rolling-deploy-bundles.md lines 9-12
consistently, keeping the renderer container entrypoint runnable.
In `@docs/pro/rolling-deploy-custom-adapters.md`:
- Around line 121-122: Update the rolling_deploy_previous_urls documentation to
remove the claim that promoted images are “born ready” to serve the production
draining bundle. Explain that including both build and promotion environments
improves build-time fallback warmth, while the target environment’s draining
bundle is resolved during release-time boot seeding and may be stale or
incomplete in the built image.
In `@llms-full-pro.txt`:
- Around line 3901-3903: Revise the “Multi-source seeding” description to avoid
claiming promoted images are promotion-ready or “born ready.” State that passing
both endpoints warms the currently advertised production bundle and reduces
fallback risk, while clarifying that boot seeding remains required for
promotion-time correctness.
🪄 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 Plus
Run ID: 691c439f-35b9-4f7b-be96-8156ab70f9fd
📒 Files selected for processing (13)
CONTEXT-MAP.mddocs/oss/building-features/node-renderer/container-deployment.mddocs/pro/rolling-deploy-adapters.mddocs/pro/rolling-deploy-custom-adapters.mdllms-full-pro.txtllms-full.txtreact_on_rails_pro/CONTEXT.mdreact_on_rails_pro/docs/adr/0001-boot-seed-rolling-deploy-bundles.mdreact_on_rails_pro/lib/react_on_rails_pro/configuration.rbreact_on_rails_pro/lib/react_on_rails_pro/rolling_deploy_adapters/http.rbreact_on_rails_pro/sig/react_on_rails_pro/configuration.rbsreact_on_rails_pro/spec/react_on_rails_pro/configuration_spec.rbreact_on_rails_pro/spec/react_on_rails_pro/rolling_deploy_adapters/http_spec.rb
ReviewForward-port of #4544 to Correctness
Notes
Minor / non-blocking
No blocking issues found — code changes are minimal, well-tested, and the doc updates are internally consistent. |
Review feedback dispositionCurrent head:
All four review threads have a disposition and are resolved. Local docs/generation/link/license checks and an independent exact-head review are green. Hosted CI and reviewer checks are restarting for this head. |
Review SummaryReviewed the diff ( Overview
Code quality
Potential issue found
Other notes
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@react_on_rails_pro/CONTEXT.md`:
- Line 39: Update the documented pre-seed renderer cache command in CONTEXT.md
to invoke the task with bundle exec rake instead of bare rake, preserving the
existing task name and arguments.
🪄 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 Plus
Run ID: 543eb7a3-9e59-4ca8-8fd2-5d69209905ea
📒 Files selected for processing (13)
CONTEXT-MAP.mddocs/oss/building-features/node-renderer/container-deployment.mddocs/pro/rolling-deploy-adapters.mddocs/pro/rolling-deploy-custom-adapters.mdllms-full-pro.txtllms-full.txtreact_on_rails_pro/CONTEXT.mdreact_on_rails_pro/docs/adr/0001-boot-seed-rolling-deploy-bundles.mdreact_on_rails_pro/lib/react_on_rails_pro/configuration.rbreact_on_rails_pro/lib/react_on_rails_pro/rolling_deploy_adapters/http.rbreact_on_rails_pro/sig/react_on_rails_pro/configuration.rbsreact_on_rails_pro/spec/react_on_rails_pro/configuration_spec.rbreact_on_rails_pro/spec/react_on_rails_pro/rolling_deploy_adapters/http_spec.rb
🚧 Files skipped from review as they are similar to previous changes (11)
- llms-full.txt
- docs/oss/building-features/node-renderer/container-deployment.md
- react_on_rails_pro/docs/adr/0001-boot-seed-rolling-deploy-bundles.md
- react_on_rails_pro/spec/react_on_rails_pro/configuration_spec.rb
- CONTEXT-MAP.md
- react_on_rails_pro/sig/react_on_rails_pro/configuration.rbs
- docs/pro/rolling-deploy-custom-adapters.md
- react_on_rails_pro/spec/react_on_rails_pro/rolling_deploy_adapters/http_spec.rb
- react_on_rails_pro/lib/react_on_rails_pro/rolling_deploy_adapters/http.rb
- docs/pro/rolling-deploy-adapters.md
- llms-full-pro.txt
There was a problem hiding this comment.
Pull request overview
Forward-ports the Pro rolling-deploy “multi previous URLs” work to main, including configuration/API changes, updated documentation on promotion/boot seeding, and regenerated LLM artifacts. This aims to prevent slow SSR after staging→production image promotion by enabling multi-origin seeding and documenting a release-time boot seed.
Changes:
- Updates Pro rolling-deploy config and adapter behavior around
rolling_deploy_previous_urls(including parsing and usage in the built-in HTTP adapter). - Refreshes Pro + OSS docs to explain promotion-model pitfalls, boot seeding, multi-source seeding, and renderer topology/ordering.
- Updates Pro specs and RBS signatures, adds Pro context/ADR material, and regenerates
llms-full*.txt.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| react_on_rails_pro/spec/react_on_rails_pro/rolling_deploy_adapters/http_spec.rb | Updates adapter specs to use plural config and removes stubs for the singular config. |
| react_on_rails_pro/spec/react_on_rails_pro/configuration_spec.rb | Adjusts configuration spec expectations to match the plural-only configuration. |
| react_on_rails_pro/sig/react_on_rails_pro/configuration.rbs | Updates RBS for the plural config type/signature changes. |
| react_on_rails_pro/lib/react_on_rails_pro/rolling_deploy_adapters/http.rb | Switches adapter configuration reading to the plural-only config and updates related comments. |
| react_on_rails_pro/lib/react_on_rails_pro/configuration.rb | Removes the singular config from defaults/accessors/initializer and associated validation. |
| react_on_rails_pro/docs/adr/0001-boot-seed-rolling-deploy-bundles.md | Adds ADR documenting boot seeding rationale/constraints under promotion deploys. |
| react_on_rails_pro/CONTEXT.md | Adds Pro rolling-deploy terminology and relationship mapping (build-time vs boot seed, promotion model). |
| llms-full.txt | Regenerated OSS LLM artifact content reflecting updated container-deployment guidance. |
| llms-full-pro.txt | Regenerated Pro LLM artifact content reflecting updated rolling-deploy documentation. |
| docs/pro/rolling-deploy-custom-adapters.md | Documents promotion/boot-seed gap and multi-source seeding guidance. |
| docs/pro/rolling-deploy-adapters.md | Documents promotion/boot-seed need, updated adapter configuration, and ordering/topology clarification. |
| docs/oss/building-features/node-renderer/container-deployment.md | Clarifies “separate workloads” version-drift risk mitigation via rolling-deploy adapter and ordering. |
| CONTEXT-MAP.md | Adds Pro rolling-deploy context entry and a brief Rolling Deploy → RSC relationship note. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
react_on_rails_pro/lib/react_on_rails_pro/configuration.rb:223
- Removing the
rolling_deploy_previous_url:initializer keyword (and the associated accessor) is a user-facing breaking change: apps upgrading from earlier Pro versions that configuredconfig.rolling_deploy_previous_url(still documented historically in CHANGELOG.md) will now hitArgumentError: unknown keyword/NoMethodError.
If this incompatibility is intentional, it should be explicitly called out in upgrade/breaking-change documentation (and/or a deprecation period preserved via an alias) so upgrades fail with a clear migration path rather than a surprising runtime error.
def initialize(renderer_url: nil, renderer_password: nil, license_token: nil, # rubocop:disable Metrics/AbcSize
server_renderer: nil,
renderer_use_fallback_exec_js: nil, prerender_caching: nil,
renderer_http_pool_size: nil, renderer_http_pool_timeout: nil,
renderer_http_pool_warn_timeout: nil, renderer_http_keep_alive_timeout: nil,
tracing: nil,
dependency_globs: nil, excluded_dependency_globs: nil, rendering_returns_promises: nil,
remote_bundle_cache_adapter: nil, rolling_deploy_adapter: nil,
rolling_deploy_token: nil, rolling_deploy_previous_urls: nil,
rolling_deploy_mount_path: nil,
Late review dispositionCurrent head:
The three late threads are resolved. Exact-head local generation/link/license checks and an independent review are green; hosted CI and current-head reviewer checks are restarting. |
ReviewReviewed the diff ( OverviewForward-ports the source changes from #4544 to Code quality
Risk / compatibility noteThis is a breaking rename (no deprecation path for Nothing blockingNo correctness, security, or performance issues found. The core logic (multi-origin discovery, provenance/identity checks, ordered retry) is unchanged from what's already hardened on |
…t-policy * origin/main: (33 commits) Fix ci-required base ref for PR merge-ref checkouts (#4819) Honor response charset and reject non-2xx HTTP-served SSR bundles (#4817) [Pro] Redact RSC render-error metadata on the fetched (client-navigation) payload path (#4821) Forward-port the 17.0.1 changelog section to main (#4814) Handle selector metacharacters in renderComponent DOM IDs (#4808) [Pro] Prevent caching RSC renders with errors (#4804) Agents: trust Copilot review identities (#4807) Agents: bind fleet closeout to generated pack (#4805) Docs: ADR 0002 — Skills-in-package over MCP for agent-native DX (#4735) Scope GitHub release commands to the origin repository (#4803) Forward-port OSS npm license metadata fix (#4794) Add golden-output gate for the serverWebpackConfig generator template (#4790) Cover the rspack CSS SSR generator fixes and de-duplicate the loader path (#4788) Configure agent workflow repo policy (#4785) Forward-port gh include mixed framing from #4684 (#4784) Release: enforce one-change forward-port closeout (#4783) Forward-port multi-URL rolling-deploy seeding to main (#4782) Docs: clarify React 18 streaming without RSC (#4780) Docs: forward-port v17 upgrade and generator gate guidance (#4781) Record the final React on Rails 17.0.0 changelog (#4742) ... # Conflicts: # AGENTS.md # internal/contributor-info/release-train-runbook.md
Summary
Forward-port the source changes from #4544 to
mainas their own focused PR.The change fixes slow SSR during staging-to-production image promotion. A promoted image can now seed rolling-deploy bundles from multiple previous-deployment endpoints, while a release-time boot seed resolves the target environment's actually draining bundle.
Implementation
rolling_deploy_previous_urlconfiguration to pluralrolling_deploy_previous_urls.main-branch safety hardening:The commit retains
cherry-pick -xprovenance from source squash commitcb79ee7229a6dddc5bc460109a4185230dce4c9a.Forward-port scope
CHANGELOG.mdis intentionally unchanged here. The final 17.0.0 changelog, including this feature, landed separately in Record the final React on Rails 17.0.0 changelog #4742.mainwere reconciled instead of duplicated.Pull Request checklist
Update CHANGELOG file— already landed in Record the final React on Rails 17.0.0 changelog #4742Verification
git diff --checkMERGE_READYThe CI change detector recommends the full hosted suite because this includes Pro Ruby runtime code. Optimized hosted CI will be requested for the final head before merge.
Summary by CodeRabbit
rolling_deploy_previous_urls(singular option removed).