Skip to content

Forward-port multi-URL rolling-deploy seeding to main - #4782

Merged
justin808 merged 3 commits into
mainfrom
jg-codex/forward-port-4544
Jul 24, 2026
Merged

Forward-port multi-URL rolling-deploy seeding to main#4782
justin808 merged 3 commits into
mainfrom
jg-codex/forward-port-4544

Conversation

@justin808

@justin808 justin808 commented Jul 24, 2026

Copy link
Copy Markdown
Member

Summary

Forward-port the source changes from #4544 to main as 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

  • Rename the RC-only singular rolling_deploy_previous_url configuration to plural rolling_deploy_previous_urls.
  • Accept one URL, a comma-delimited string, or an array of URLs.
  • Preserve the newer main-branch safety hardening:
    • omit ambiguous multi-origin legacy hashes;
    • verify v2 payload identity before accepting a response;
    • retry only eligible origins in configured order;
    • degrade gracefully when an individual endpoint fails.
  • Document build-time multi-source seeding, release-time boot seeding, deployment ordering, and renderer topology.
  • Add the Pro context map and ADR, and regenerate the LLM documentation artifacts.

The commit retains cherry-pick -x provenance from source squash commit cb79ee7229a6dddc5bc460109a4185230dce4c9a.

Forward-port scope

Pull Request checklist

Verification

  • Focused Pro configuration and HTTP adapter specs: 125 examples, 0 failures
  • Pro CI-equivalent RuboCop
  • RBS validation
  • Pro license-header validation
  • Prettier
  • Documentation sidebar and full link checks
  • LLM documentation generation check and 7-test harness
  • Pre-push changed-file RuboCop and online Markdown link checks
  • git diff --check
  • Independent maker-distinct exact-head review: MERGE_READY

The 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

  • New Features
    • Added release-time (boot) seeding for the Node Renderer cache during rolling deployments.
    • Added multi-source seeding support using multiple prior deployment URLs.
  • Changes
    • Standardized rolling-deploy configuration on rolling_deploy_previous_urls (singular option removed).
    • Improved deploy stability by gating renderer readiness on boot-seed completion to avoid deploy stalls.
  • Documentation
    • Expanded guidance on promotion behavior, separate-workloads version drift mitigation, and deployment ordering.
    • Updated adapter setup and rolling-deploy correctness notes, including multi-source discovery and RSC draining considerations.

Copilot AI review requested due to automatic review settings July 24, 2026 15:01
@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5f5268d0-a7bd-486f-b962-9bde92561281

📥 Commits

Reviewing files that changed from the base of the PR and between 69635a2 and 5f0742f.

📒 Files selected for processing (4)
  • docs/pro/rolling-deploy-custom-adapters.md
  • llms-full-pro.txt
  • react_on_rails_pro/CONTEXT.md
  • react_on_rails_pro/docs/adr/0001-boot-seed-rolling-deploy-bundles.md
🚧 Files skipped from review as they are similar to previous changes (4)
  • react_on_rails_pro/docs/adr/0001-boot-seed-rolling-deploy-bundles.md
  • docs/pro/rolling-deploy-custom-adapters.md
  • react_on_rails_pro/CONTEXT.md
  • llms-full-pro.txt

Walkthrough

Rolling-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.

Changes

Rolling deploy cache seeding

Layer / File(s) Summary
Boot-seed deployment contract
react_on_rails_pro/CONTEXT.md, react_on_rails_pro/docs/adr/..., docs/pro/rolling-deploy-adapters.md, docs/pro/rolling-deploy-custom-adapters.md, llms-full-pro.txt
Documents environment-specific draining bundles, readiness-gated boot seeding, renderer-before-Rails ordering, and fallback behavior for promotions.
Multi-source seeding guidance
docs/pro/rolling-deploy-adapters.md, docs/pro/rolling-deploy-custom-adapters.md, llms-full-pro.txt, CONTEXT-MAP.md
Describes multiple previous endpoints, provenance and eligibility rules, ordered fetching, and build-time versus boot-seed behavior.
Plural previous-URL configuration contract
react_on_rails_pro/lib/react_on_rails_pro/configuration.rb, react_on_rails_pro/sig/.../configuration.rbs, react_on_rails_pro/spec/react_on_rails_pro/configuration_spec.rb
Removes the singular configuration API and ambiguity validation, and updates plural initializer, type signatures, and configuration coverage.
HTTP adapter integration and validation
react_on_rails_pro/lib/react_on_rails_pro/rolling_deploy_adapters/http.rb, react_on_rails_pro/spec/react_on_rails_pro/...
Reads plural previous URLs directly and updates adapter-related specifications.
Separate-workload deployment guidance
docs/oss/building-features/node-renderer/container-deployment.md, llms-full.txt
Clarifies bundle-version drift and the rolling-deploy adapter mitigation for separate workloads.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Suggested labels: full-ci

Suggested reviewers: copilot

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: forwarding multi-URL rolling-deploy seeding into main.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch jg-codex/forward-port-4544

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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-apps

greptile-apps Bot commented Jul 24, 2026

Copy link
Copy Markdown

Greptile Summary

Forward-ports the plural multi-origin rolling-deploy configuration and promotion guidance.

  • Removes the RC-only singular rolling_deploy_previous_url configuration surface in favor of rolling_deploy_previous_urls.
  • Updates the HTTP adapter, type signatures, and specs for ordered multi-origin discovery and fallback.
  • Documents build-time multi-source seeding, release-time boot seeding, deployment ordering, and renderer topology.
  • Adds Pro context and ADR material and regenerates the LLM documentation artifacts.

Confidence Score: 5/5

The 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

Filename Overview
react_on_rails_pro/lib/react_on_rails_pro/configuration.rb Removes the intentionally RC-only singular previous-URL configuration API and retains the plural configuration and HTTP-adapter validation.
react_on_rails_pro/lib/react_on_rails_pro/rolling_deploy_adapters/http.rb Resolves discovery inputs exclusively from the plural setting while preserving ordered multi-origin provenance and verified fallback behavior.
react_on_rails_pro/sig/react_on_rails_pro/configuration.rbs Aligns the public RBS configuration surface with the plural URL setting.
react_on_rails_pro/spec/react_on_rails_pro/configuration_spec.rb Updates configuration coverage to reflect removal of the temporary singular compatibility surface.
react_on_rails_pro/spec/react_on_rails_pro/rolling_deploy_adapters/http_spec.rb Covers plural arrays, comma-delimited values, origin ordering, provenance filtering, identity checks, and endpoint-failure fallback.
docs/pro/rolling-deploy-adapters.md Documents promotion-time boot seeding and renderer-before-Rails deployment ordering.
docs/pro/rolling-deploy-custom-adapters.md Documents multi-source build seeding and its relationship to boot-time correctness.

Sequence Diagram

sequenceDiagram
  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
Loading

Reviews (1): Last reviewed commit: "Multi-URL rolling-deploy adapter + promo..." | Re-trigger Greptile

@justin808
justin808 force-pushed the jg-codex/forward-port-4544 branch from 6f7bdcf to fac2944 Compare July 24, 2026 15:03
@justin808

Copy link
Copy Markdown
Member Author

+ci-status

@github-actions

Copy link
Copy Markdown
Contributor

CI Status

Head SHA: fac2944574aa
Changed files: 13
Docs-only heuristic (matches ci-changes-detector metadata paths): no
ready-for-hosted-ci label: absent
force-full-hosted-ci label: absent
Current hosted-CI waiver: not present for this SHA
Automatic release-target hosted mode: inactive
Observed exact-head coverage: modes[missing=9]; successful=0, pending=0, failed=0, missing=9

Only the required gate is active unless hosted CI is requested.

@justin808

Copy link
Copy Markdown
Member Author

+ci-run-hosted

@claude

claude Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Code Review

Overview

Clean forward-port of #4544's source changes to main. The core change replaces the RC-only singular rolling_deploy_previous_url config with the plural rolling_deploy_previous_urls (already supporting a single URL, comma-delimited string, or array), removes the now-dead singular/plural coexistence and its "don't configure both" validator, and updates docs/specs/RBS accordingly. Most of the diff (322 additions / 81 deletions across 13 files) is documentation (CONTEXT-MAP.md, new react_on_rails_pro/CONTEXT.md, new ADR, docs/pro/*, and the regenerated llms-full*.txt mirrors) plus renaming a config option and its tests.

Code quality

  • react_on_rails_pro/lib/react_on_rails_pro/configuration.rb: removal of DEFAULT_ROLLING_DEPLOY_PREVIOUS_URL, the custom rolling_deploy_previous_urls reader/attr_writer pair, the rolling_deploy_previous_url constructor kwarg, and validate_rolling_deploy_previous_urls is consistent — rolling_deploy_previous_urls is now a plain attr_accessor, and I couldn't find any remaining reference to the singular attribute anywhere in the codebase (grepped .rb/.rbs/.md/.txt).
  • react_on_rails_pro/lib/react_on_rails_pro/rolling_deploy_adapters/http.rb: configured_previous_url (singular helper) is deleted and configured_previous_urls now reads straight from config.rolling_deploy_previous_urls via Array(...), which correctly normalizes a nil, String, or Array value. The v2 provenance/identity-check hardening (multi-origin ambiguous-hash handling, ordered retry across eligible origins, payload identity verification) is untouched by this PR, as the description claims.
  • sig/react_on_rails_pro/configuration.rbs: the accessor type changed from String? | Array[String] to (String | Array[String])? — semantically equivalent (both include nil), just tighter RBS style.
  • Doc cross-references check out: the new #promotion-deploys-need-a-release-time-boot-seed and #multi-source-seeding anchors match their headings, and the relative link from docs/oss/.../container-deployment.md to ../../../pro/rolling-deploy-adapters.md resolves correctly.

Test coverage

  • configuration_spec.rb and http_spec.rb were updated in lockstep: every instance_double(ReactOnRailsPro::Configuration, rolling_deploy_previous_url: nil, ...) stub was removed (since instance_double would fail on a verifying double if the real class no longer defines that method), and the singular→plural fallback / "both configured raises" tests were replaced with a single "accepts plural previous URLs" test. This looks complete for the rename.

Minor observations (non-blocking)

  • Breaking change for early adopters, by design: since rolling_deploy_previous_url (singular) is removed outright rather than deprecated, anyone who already configured it against a published 17.0.0.rc.x gem will get a NoMethodError-free silent no-op... actually worse, since the constructor kwarg is gone entirely, rolling_deploy_previous_url: in a configure block will now raise NoMethodError on the missing writer. The PR description frames this as acceptable because the option is "RC-only," which is a reasonable call for a pre-1.0 feature, but it's worth being explicit in the PR/changelog that this is a breaking rename for anyone who adopted the RC config already (current gem version is 17.0.0.rc.6).
  • CHANGELOG.md still references config.rolling_deploy_previous_url in the historical entry for PR feat: scaffold built-in HTTP rolling-deploy adapter (#3240 part 1) #3379 — that's fine as a historical record of what that PR shipped at the time, not something this PR needs to touch.
  • Cosmetic: in http_spec.rb's "previous_url scheme validation" describe block, the local variable is still named previous_url (singular) even though it's fed into the plural rolling_deploy_previous_urls: stub — harmless, just a naming leftover from the pre-rename test.

Security

No 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.

@github-actions

github-actions Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Hosted CI Requested

Triggered 9 workflow(s) for fac2944574aa.
Skipped 0 workflow(s) with equivalent exact-head coverage.
Mode: optimized hosted CI (path-selected by script/ci-changes-detector).
Added ready-for-hosted-ci, so future commits will keep running optimized hosted CI until +ci-stop-hosted is used.

View progress in the Actions tab.

@github-actions github-actions Bot added the ready-for-hosted-ci Run optimized hosted GitHub CI for this PR label Jul 24, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread react_on_rails_pro/lib/react_on_rails_pro/configuration.rb

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 242592b and 6f7bdcf.

📒 Files selected for processing (13)
  • CONTEXT-MAP.md
  • docs/oss/building-features/node-renderer/container-deployment.md
  • docs/pro/rolling-deploy-adapters.md
  • docs/pro/rolling-deploy-custom-adapters.md
  • llms-full-pro.txt
  • llms-full.txt
  • react_on_rails_pro/CONTEXT.md
  • react_on_rails_pro/docs/adr/0001-boot-seed-rolling-deploy-bundles.md
  • react_on_rails_pro/lib/react_on_rails_pro/configuration.rb
  • react_on_rails_pro/lib/react_on_rails_pro/rolling_deploy_adapters/http.rb
  • react_on_rails_pro/sig/react_on_rails_pro/configuration.rbs
  • react_on_rails_pro/spec/react_on_rails_pro/configuration_spec.rb
  • react_on_rails_pro/spec/react_on_rails_pro/rolling_deploy_adapters/http_spec.rb

Comment thread docs/pro/rolling-deploy-adapters.md Outdated
Comment thread docs/pro/rolling-deploy-custom-adapters.md Outdated
Comment thread llms-full-pro.txt Outdated
@claude

claude Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Review

Forward-port of #4544 to main. Scope is clean: config rename (rolling_deploy_previous_url → plural-only rolling_deploy_previous_urls), the multi-source/boot-seed docs, and a new CONTEXT.md/ADR. Verified the diff (gh pr diff 4782) against the checked-out tree.

Correctness

  • Clean rename, no stragglers. Grepped the whole tree for the singular rolling_deploy_previous_url; the only remaining hit is the historical CHANGELOG.md entry for PR feat: scaffold built-in HTTP rolling-deploy adapter (#3240 part 1) #3379, which is correct to leave as-is (it documents what shipped at that point in time).
  • No dangling references. configured_previous_url and validate_rolling_deploy_previous_urls were removed from http.rb/configuration.rb and I confirmed there are no remaining callers of either.
  • rolling_deploy_previous_urls accessor simplified correctly. Replacing the custom fallback reader (@rolling_deploy_previous_urls.nil? ? rolling_deploy_previous_url : ...) with a plain attr_accessor is safe now that the singular knob is gone — no behavior change for the plural path, since configured_previous_urls in http.rb already normalizes both a single string and an array via Array(...).
  • RBS matches the Ruby. attr_accessor rolling_deploy_previous_urls: (String | Array[String])? correctly reflects the accessor now that the singular attr and its bespoke reader are gone.
  • Doc anchors check out. Spot-checked the new cross-links (#architecture-options, #promotion-deploys-need-a-release-time-boot-seed, #multi-source-seeding) against the actual headings in container-deployment.md and rolling-deploy-adapters.md — they resolve.
  • llms-full.txt/llms-full-pro.txt regeneration matches the source docs — diffed identically to the corresponding docs/ changes.

Notes

  • This is a breaking change for anyone who adopted the singular rolling_deploy_previous_url off the RC branch — worth confirming (per the PR description) that it never shipped in a stable release, since there's no deprecation path here, just removal. The PR description says this is RC-only, which the CHANGELOG (per Record the final React on Rails 17.0.0 changelog #4742, already landed) seems to confirm.
  • CHANGELOG.md is intentionally untouched here and already contains the multi-source/boot-seed entry from Record the final React on Rails 17.0.0 changelog #4742 — confirmed no duplicate or conflicting entry was introduced.
  • Test coverage for the rename looks complete: configuration_spec.rb drops the singular/plural conflict test and replaces it with a plural-only acceptance test; http_spec.rb systematically swaps every rolling_deploy_previous_url: stub to rolling_deploy_previous_urls:. I didn't spot any leftover instance_double stubs still using the old singular key.

Minor / non-blocking

  • The new react_on_rails_pro/CONTEXT.md and ADR are thorough (arguably long for a single-config-rename PR), but per the project's context-map conventions this seems to be the expected format for Pro subsystem documentation, so no change requested.

No blocking issues found — code changes are minimal, well-tested, and the doc updates are internally consistent.

@justin808

Copy link
Copy Markdown
Member Author

Review feedback disposition

Current head: 69635a2e547d9a524ced127620f4b13bc1791d7a

  • Fixed the boot-seed execution contract: the task is Rails/Rake; combined and Node-only deployment shapes now require a Ruby-capable step, shared cache volume or completed copy/sync, and gated renderer start/readiness.
  • Fixed the build-time wording: multi-source seeding is fallback warmth only; release-time seeding resolves the current draining bundle.
  • Regenerated and verified llms-full-pro.txt.
  • Skipped the singular-accessor request as not applicable: Pro rolling deploy: multi-URL previous_urls + promotion boot-seed docs #4544 intentionally replaced the RC-only API before stable; rc.12 and stable v17.0.0 are plural-only.

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.

Comment thread react_on_rails_pro/docs/adr/0001-boot-seed-rolling-deploy-bundles.md Outdated
@claude

claude Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Review Summary

Reviewed the diff (gh pr diff 4782, 13 files, +344/-81) — this forward-ports the multi-URL rolling-deploy seeding work from release/17.0.0 (originally #4544) to main, plus new context/ADR docs.

Overview

  • Renames Pro's rolling_deploy_previous_url (singular) config to rolling_deploy_previous_urls (plural-only), dropping the backward-compat fallback and the "don't configure both" validation.
  • Updates Configuration, the HTTP rolling-deploy adapter, RBS sigs, and specs consistently — verified via grep that no singular rolling_deploy_previous_url references remain anywhere in code, generators, or RBS (only a historical CHANGELOG.md entry, which is correctly left alone).
  • Adds substantial documentation: promotion-vs-build-time seeding tradeoffs, a "boot seed" release-time step, deploy-ordering requirements, a new react_on_rails_pro/CONTEXT.md, and ADR 0001-boot-seed-rolling-deploy-bundles.md. llms-full*.txt artifacts are regenerated to match.

Code quality

  • The config rename is clean and complete: attr_writer/custom reader for the plural→singular fallback, the dual-config validator, and all singular keyword args are removed together — no orphaned code paths.
  • configured_previous_urls in http.rb is simplified correctly (removed configured_previous_url singular helper, which had no other callers).
  • Since this config was introduced only on the pre-release branch (never shipped in a stable release per the PR description), removing backward compatibility here is reasonable rather than a breaking change for existing users.
  • Spec updates are thorough — every instance_double in http_spec.rb and the two configuration_spec.rb cases were updated in lockstep with the rename (dual-config-error test replaced with a plural-acceptance test).

Potential issue found

  • Left an inline comment on react_on_rails_pro/docs/adr/0001-boot-seed-rolling-deploy-bundles.md:6 — it names the private client hichee-staging as a Control Plane upstream: example. This repo's own internal/ docs explicitly treat shakacode/hichee as private and instruct against exposing it publicly, but this ADR lands in the public docs tree. Suggest genericizing the example.

Other notes

  • No functional/runtime logic changed outside the config rename — the "safety hardening" (ambiguous multi-origin hash omission, v2 identity verification, ordered retry, per-endpoint graceful degradation) was already on main; this PR is additive documentation plus the rename.
  • CHANGELOG is intentionally untouched here per the PR description (already landed in Record the final React on Rails 17.0.0 changelog #4742) — consistent with the diff.
  • No security concerns in the code itself (URL validation, token handling, HTTPS gating all pre-existing and untouched).

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 6f7bdcf and 69635a2.

📒 Files selected for processing (13)
  • CONTEXT-MAP.md
  • docs/oss/building-features/node-renderer/container-deployment.md
  • docs/pro/rolling-deploy-adapters.md
  • docs/pro/rolling-deploy-custom-adapters.md
  • llms-full-pro.txt
  • llms-full.txt
  • react_on_rails_pro/CONTEXT.md
  • react_on_rails_pro/docs/adr/0001-boot-seed-rolling-deploy-bundles.md
  • react_on_rails_pro/lib/react_on_rails_pro/configuration.rb
  • react_on_rails_pro/lib/react_on_rails_pro/rolling_deploy_adapters/http.rb
  • react_on_rails_pro/sig/react_on_rails_pro/configuration.rbs
  • react_on_rails_pro/spec/react_on_rails_pro/configuration_spec.rb
  • react_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

Comment thread react_on_rails_pro/CONTEXT.md Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread react_on_rails_pro/lib/react_on_rails_pro/configuration.rb
Copilot AI review requested due to automatic review settings July 24, 2026 15:21

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 configured config.rolling_deploy_previous_url (still documented historically in CHANGELOG.md) will now hit ArgumentError: 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,

Copilot AI review requested due to automatic review settings July 24, 2026 15:27
@justin808

Copy link
Copy Markdown
Member Author

Late review disposition

Current head: 5f0742f4069c1dcf93263fb89e8cb60dd1b3f07e

  • Fixed the public/private boundary: removed the private deployment identifier and retained only generic Control Plane upstream: terminology.
  • Fixed all affected runnable seed commands to use bundle exec rake; regenerated and verified llms-full-pro.txt.
  • Skipped the repeated singular-config suggestion as not applicable after verifying that RC12 and stable v17.0.0 are already plural-only.

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.

@claude

claude Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Review

Reviewed the diff (gh pr diff 4782) plus the current state of the touched files in this checkout.

Overview

Forward-ports the source changes from #4544 to main: renames the RC-only rolling_deploy_previous_url (singular) config to rolling_deploy_previous_urls (plural-only), removes the singular/plural fallback and the "don't configure both" validation, and adds substantial documentation covering multi-source build-time seeding and the new release-time ("boot") seed for staging→production image promotion. CHANGELOG.md is intentionally untouched (already landed via #4742).

Code quality

  • The configuration.rb change is a clean removal: DEFAULT_ROLLING_DEPLOY_PREVIOUS_URL, the singular attr_writer+custom fallback reader, the constructor keyword, and validate_rolling_deploy_previous_urls are all removed together, and rolling_deploy_previous_urls becomes a plain attr_accessor. No stray references to the singular knob remain anywhere in lib/, sig/, docs, or the generated llms-full*.txt artifacts (verified via repo-wide grep).
  • http.rb's configured_previous_url (singular-first) helper is removed along with its only caller; configured_previous_urls now reads directly off config.rolling_deploy_previous_urls — straightforward and correct (Array(nil) => [], Array("single") => ["single"], Array([...]) => [...]).
  • RBS signature (configuration.rbs) and specs (configuration_spec.rb, http_spec.rb) were updated consistently — no leftover instance_double stubs for the removed singular attribute.
  • Doc cross-references (#promotion-deploys-need-a-release-time-boot-seed, #multi-source-seeding, etc.) all resolve to real headings in the target files, and the referenced rake react_on_rails_pro:pre_seed_renderer_cache task already exists in react_on_rails_pro/lib/react_on_rails_pro/pre_seed_renderer_cache.rb — this PR is documentation-only for that part, matching the stated forward-port scope.

Risk / compatibility note

This is a breaking rename (no deprecation path for rolling_deploy_previous_url), but per the PR description the singular form was RC-only and never in a public release, so removing rather than deprecating it is reasonable — just flagging it explicitly since it's a public config API change.

Nothing blocking

No 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 main — this PR only renames the config surface and expands docs. Test coverage for the rename looks complete (constructor, validation-removal, and adapter normalization specs all updated).

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

@justin808
justin808 added this pull request to the merge queue Jul 24, 2026
Merged via the queue into main with commit 46865c2 Jul 24, 2026
57 checks passed
@justin808
justin808 deleted the jg-codex/forward-port-4544 branch July 24, 2026 16:10
justin808 added a commit that referenced this pull request Jul 31, 2026
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-hosted-ci Run optimized hosted GitHub CI for this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants