refactor: unify renderer cache staging with mode: :copy / :symlink#3167
Conversation
…l(mode:) Collapse the copy-based PreSeedRendererCache and the symlink-based PrepareNodeRenderBundles into a single entry point with a `mode:` keyword argument. Both modes produce the identical <cache>/<hash>/<hash>.js layout; only the file operation differs (FileUtils.cp vs relative symlink). Changes: - ReactOnRailsPro::PreSeedRendererCache.call now accepts mode: :copy (default, for Docker/image builds) or mode: :symlink (same-filesystem workflows). Rejects unknown modes with ArgumentError. - mode: :copy now raises a clear error when neither RENDERER_SERVER_BUNDLE_CACHE_PATH nor RENDERER_BUNDLE_PATH is set in non-dev/test environments. The Node renderer's default lookup can differ from the Ruby side (falling back to /tmp when cwd is outside the app tree), so silent fallback is unsafe for production-like deploys and was a silent-misconfig footgun. - `react_on_rails_pro:pre_seed_renderer_cache` rake task accepts MODE=copy (default) or MODE=symlink. - ReactOnRailsPro::PrepareNodeRenderBundles is now a thin deprecated shim that emits a once-per-process warning and delegates to PreSeedRendererCache.call(mode: :symlink). Public class and the `pre_stage_bundle_for_node_renderer` rake task remain for backward compatibility. - AssetsPrecompile.call invokes the unified API with mode: :symlink after precompile (no behavior change for existing users). - react_on_rails:doctor scans common deploy-script locations (Procfile*, Dockerfile, bin/*) for references to the deprecated pre_stage_bundle_for_node_renderer task and surfaces a migration warning. - Docs: docs/pro/node-renderer.md describes the unified API, both modes, the non-dev env-var requirement for copy mode, and the deprecation story. - CHANGELOG: new Changed entry describing the unification. - Tests: mode validation, raise-in-non-dev guard (including symlink opt-out), deprecation warning on the shim, and a doctor check test. Context: stacked on PR #3124 (jg/3122-preseed-renderer-cache). Refs: #3122 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 refactor. The unified API is simpler to reason about than two parallel classes sharing 90% of their logic, the deprecation shims preserve backward compatibility correctly, and the env-var guard for copy mode is a genuine foot-gun protection. The test coverage is solid. A few issues worth addressing before merge: Correctness / Edge CasesNo atomicity on partial failure. If a copy or symlink fails midway through staging (e.g. disk full after the first bundle file is written), the cache directory is left in a partially-staged state. The renderer could pick up an incomplete cache directory on startup. This was true for the previous classes too, so it's not a regression — but now that both modes share this code path it's worth a note. Wrapping the staging loop in a begin/rescue that tears down the partially-written
Test Coverage GapThe Minor / Nits
Documentation / ChangelogBoth look good. The Dockerfile snippet now correctly places |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e97260dc38
ℹ️ 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".
Greptile SummaryThis PR collapses All three prior review concerns (MODE case-normalization, Confidence Score: 5/5Safe to merge; no P0/P1 issues found, and all prior review feedback has been addressed. All three previous review concerns (mutex for deprecation guard, MODE env-var case normalization, ENV cleanup in ensure) are resolved. The remaining comments are P2 style/test-coverage suggestions: a missing Dockerfile branch test in the doctor spec, a misleading rescue message in make_relative_symlink, and a silent large-file skip in the doctor check. None affect correctness or runtime behavior. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[["rake pre_seed_renderer_cache\nMODE=copy|symlink"]] --> B{Normalize & validate MODE}
B -- invalid --> Z1[abort with valid modes list]
B -- :copy / :symlink --> C[PreSeedRendererCache.call mode:]
D[["rake pre_stage_bundle_for_node_renderer\n(deprecated)"]] -->|warn + delegate| C
E[[PrepareNodeRenderBundles.call\n deprecated shim]] -->|mutex-guarded warn + delegate| C
F[[AssetsPrecompile.call]] -->|mode: :symlink\nif node_renderer?| C
C --> G{enforce_cache_dir_env_var!}
G -- mode==:copy AND no env var AND not dev/test --> Z2[raise ReactOnRailsPro::Error]
G -- passes --> H[resolve_cache_dir]
H --> I[RendererCacheHelpers.bundle_sources]
I --> J{for each bundle src + hash}
J --> K[stage_bundle src → bundle_dir/hash.js]
K -- :copy --> K1[FileUtils.cp]
K -- :symlink --> K2[make_relative_symlink\nrealpath both sides]
J --> L[stage_assets → bundle_dir]
L -- :copy --> L1[FileUtils.cp each asset]
L -- :symlink --> L2[make_relative_symlink each asset]
L -- missing required RSC asset --> Z3[raise ReactOnRailsPro::Error]
L -- missing optional asset --> L3[warn to stderr]
style Z1 fill:#f88,stroke:#c00
style Z2 fill:#f88,stroke:#c00
style Z3 fill:#f88,stroke:#c00
Reviews (2): Last reviewed commit: "fix: address PR #3167 review feedback (r..." | Re-trigger Greptile |
- Doctor check now tailors the suggested migration command per matched file type: Dockerfile entries get the bare 'rake pre_seed_renderer_cache' (MODE=copy is the default); Procfile/bin entries get 'MODE=symlink'. - The copy-mode env-var error now mentions 'MODE=symlink' as an escape hatch for CI users who don't need an immutable artifact. - make_relative_symlink rescues Errno::ENOENT from Pathname#realpath and surfaces a clearer ReactOnRailsPro::Error when a bundle/asset vanished between existence check and staging (e.g. webpack output rotating mid-stage, dangling symlinks in the source tree). - PrepareNodeRenderBundles.reset_deprecation_warned! is now private_class_method; specs invoke it via send. - Rake task now downcases ENV['MODE'] and validates against PreSeedRendererCache::VALID_MODES before to_sym, accepting 'MODE=Copy', 'MODE=COPY', etc. Unknown values abort with a clear error listing valid modes. - Added a :symlink-mode spec that exercises asset symlinking (previously only covered via the PrepareNodeRenderBundles shim). - env-var-bypass spec now uses ensure block with a local tmpdir variable to guarantee cleanup even if Dir.mktmpdir or an intermediate assertion raises. Declined: thread-safety wrapper around @deprecation_warned. This code runs only from rake tasks and AssetsPrecompile.call (single-threaded contexts); a race would produce at most a harmless duplicate warning, not a correctness issue. Consistent with PR A precedent for @renderer_bundle_path_deprecation_warned. 27 dummy specs pass; rubocop clean on all changed files. Refs: #3122, #3167 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Addressed review feedback in 14f4a45Fixed (8 items):
Declined with rationale (1 item):
27 dummy specs pass (was 26, +1 new symlink-assets test); 163 doctor specs pass (same pre-existing unrelated failure in `auto_fix_versions`); rubocop clean. I (Claude Code) addressed the feedback on behalf of @justin808. |
Code ReviewOverall: solid refactor with good test coverage and a clean deprecation story. One genuine bug in the doctor check; the rest are minor concerns. What this does well
Issues1. Bug: doctor path scan uses
2.
3. The rescue message says "could not resolve real path for symlink source" but the rescue covers both 4. Minor - worst case is a duplicate deprecation warning. Acceptable for a shim. Minor observations
Generated with Claude Code |
Introduces a pluggable adapter protocol so applications can eliminate
the 410→retry cold-start round-trip for previously-deployed bundle
hashes during rolling deploys — not just the current hash that
PreSeedRendererCache already handles.
Protocol (parallel to remote_bundle_cache_adapter):
module MyAdapter
def self.previous_bundle_hashes
def self.fetch(bundle_hash)
def self.upload(bundle_hash, server_bundle:, rsc_bundle: nil, assets:)
end
ReactOnRailsPro.configure do |config|
config.rolling_deploy_adapter = MyAdapter
end
Integration points:
- PreSeedRendererCache.call invokes RollingDeployCacheStager after
staging the current hash(es). Seeds each previous hash's bundle +
companion assets (loadable-stats.json, RSC manifests) into the
same <cache>/<hash>/... layout so hydration stays consistent with
the deployed asset pipeline for that hash.
- AssetsPrecompile.call auto-invokes adapter.upload(current_hash, ...)
after precompile in production-like environments.
- PREVIOUS_BUNDLE_HASHES env var (comma-separated) overrides
previous_bundle_hashes discovery for CI/testing.
- Config validation checks the adapter responds to all three required
methods at configure time.
Error handling (all degrade gracefully — runtime 410-retry remains
the fallback for any seeding failure):
- Adapter not configured: no-op.
- previous_bundle_hashes raises: warn, skip previous-hash seeding.
- fetch returns nil or raises: warn, skip that hash.
- upload raises in precompile: warn, don't fail precompile.
- Hash matching current hash: deduped.
Doctor probes:
- Verifies protocol conformance.
- Probes previous_bundle_hashes with a 3s timeout; reports latency and
hash count.
- Warns on empty list ("upload side has never run").
- Surfaces resolved renderer cache dir + hash subdirs present.
- Echoes PREVIOUS_BUNDLE_HASHES env if set without an adapter.
- Never calls fetch or upload (side effects).
Docs: new docs/pro/rolling-deploy-adapters.md with protocol spec and
three reference implementations (S3, Control Plane, Filesystem).
docs/pro/node-renderer.md points at the new page.
Tests: 7 new specs for RollingDeployCacheStager covering all
invocation paths plus 5 new doctor specs.
Refs: #3122, #3167 (PR B stacked base)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
MUST-FIX (2):
- **Nil adapter + PREVIOUS_BUNDLE_HASHES bug** (greptile P1, claude):
when the env override was set without config.rolling_deploy_adapter,
the stager proceeded past the early-return guard and crashed with
NoMethodError at adapter.fetch. Now warns and returns instead.
Doctor promotes the corresponding check from :info to :warning and
explains that both env and adapter are required.
- **RSC previous-bundle staging path** (codex P2): the renderer routes
RSC requests by rsc_bundle_hash, which differs from server_bundle_hash
and is a separate <cache>/<hash>/<hash>.js entry. The original
protocol merged both bundles under a single hash's directory using
basenames, so the RSC bundle never landed where the renderer looks.
Fix: protocol refactor — each hash is now one bundle's cache entry.
fetch(hash) → { bundle: path, assets: [paths] }
upload(hash, bundle:, assets:)
previous_bundle_hashes → flat list including server AND rsc hashes
AssetsPrecompile.publish_current_bundle_if_configured now calls
upload twice when RSC is enabled (once per hash). Stager stages each
hash at <cache>/<hash>/<hash>.js with its companion assets.
SHOULD-FIX (4):
- **Timeout on adapter.fetch** (claude): wrap with Timeout.timeout
(default 30s) so a hung external store can't block pre-seeding or
assets:precompile indefinitely.
- **Relative symlinks** (greptile P2): stage_file in :symlink mode now
uses the same Pathname#realpath + relative_path_from pattern as
PreSeedRendererCache.make_relative_symlink, matching current-hash
staging and surviving cache-dir moves.
- **Remove stray module_function** (claude): was paired with
private_class_method (an unusual combination that leaks private
instance methods to any include-r). Switched to `def self.` for all
methods, private_class_method for helpers.
- **respond_to? instead of methods.include?** (claude): more idiomatic
duck-typing in both Configuration.validate_rolling_deploy_adapter
and Doctor.report_adapter_protocol.
DOC POLISH (3):
- Control Plane reference impl: switched from backtick/system shell
interpolation to Open3.capture2e with array-form args to avoid
shell injection via env-var contents. Returns both server AND RSC
hashes via previous_bundle_hashes.
- Both S3 and Control Plane examples: lazy env-var accessor methods
instead of constants evaluated at require time, so KeyError can't
fire at class-load in environments that don't configure the adapter.
- S3 reference impl: explicit note about the read-modify-write race
in update_manifest!; documents serializing deploys or using
If-Match/ETag as strict-safety alternatives.
- Updated protocol docstrings + all reference impls (S3, Control Plane,
Filesystem) to the new fetch/upload signatures.
Tests: 13 rolling-deploy specs now (was 7) — adds nil-adapter+env
warning, fetch timeout, relative-symlink assertion. Doctor spec
updated for the warn-vs-info change. 38 Pro specs pass, 167/168
doctor specs pass (1 pre-existing unrelated failure).
Refs: #3122, #3167 (PR B), #3173 (this PR)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- pre_seed_renderer_cache: rescue Errno::ENOENT inside make_relative_symlink and raise ReactOnRailsPro::Error so a dangling symlink or race between File.exist? and realpath surfaces with a clear message instead of a raw system error. - prepare_node_renderer_bundles: guard the check-then-set on @deprecation_warned with a Mutex so concurrent callers (multi-worker Puma boots of the shim) still emit exactly one warning per process. - doctor: cap File.read in check_deprecated_renderer_cache_task at 1 MiB and skip unusually large deploy scripts defensively. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Review feedback addressedAll 11 review threads addressed and resolved. Summary: Addressed in new commit
|
|
To use Codex here, create an environment for this repo. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1f9e79681f
ℹ️ 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".
Code ReviewOverall: Clean, well-structured refactoring. The unified Bug:
|
|
Mattered:
Skipped:
Validation:
Run context: this action used the selected items from the latest triage after the previous summary checkpoint at 2026-04-23T04:12:40Z. Future full-PR scans should start after this comment unless |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0752feb936
ℹ️ 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".
Introduces a pluggable adapter protocol so applications can eliminate
the 410→retry cold-start round-trip for previously-deployed bundle
hashes during rolling deploys — not just the current hash that
PreSeedRendererCache already handles.
Protocol (parallel to remote_bundle_cache_adapter):
module MyAdapter
def self.previous_bundle_hashes
def self.fetch(bundle_hash)
def self.upload(bundle_hash, server_bundle:, rsc_bundle: nil, assets:)
end
ReactOnRailsPro.configure do |config|
config.rolling_deploy_adapter = MyAdapter
end
Integration points:
- PreSeedRendererCache.call invokes RollingDeployCacheStager after
staging the current hash(es). Seeds each previous hash's bundle +
companion assets (loadable-stats.json, RSC manifests) into the
same <cache>/<hash>/... layout so hydration stays consistent with
the deployed asset pipeline for that hash.
- AssetsPrecompile.call auto-invokes adapter.upload(current_hash, ...)
after precompile in production-like environments.
- PREVIOUS_BUNDLE_HASHES env var (comma-separated) overrides
previous_bundle_hashes discovery for CI/testing.
- Config validation checks the adapter responds to all three required
methods at configure time.
Error handling (all degrade gracefully — runtime 410-retry remains
the fallback for any seeding failure):
- Adapter not configured: no-op.
- previous_bundle_hashes raises: warn, skip previous-hash seeding.
- fetch returns nil or raises: warn, skip that hash.
- upload raises in precompile: warn, don't fail precompile.
- Hash matching current hash: deduped.
Doctor probes:
- Verifies protocol conformance.
- Probes previous_bundle_hashes with a 3s timeout; reports latency and
hash count.
- Warns on empty list ("upload side has never run").
- Surfaces resolved renderer cache dir + hash subdirs present.
- Echoes PREVIOUS_BUNDLE_HASHES env if set without an adapter.
- Never calls fetch or upload (side effects).
Docs: new docs/pro/rolling-deploy-adapters.md with protocol spec and
three reference implementations (S3, Control Plane, Filesystem).
docs/pro/node-renderer.md points at the new page.
Tests: 7 new specs for RollingDeployCacheStager covering all
invocation paths plus 5 new doctor specs.
Refs: #3122, #3167 (PR B stacked base)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
MUST-FIX (2):
- **Nil adapter + PREVIOUS_BUNDLE_HASHES bug** (greptile P1, claude):
when the env override was set without config.rolling_deploy_adapter,
the stager proceeded past the early-return guard and crashed with
NoMethodError at adapter.fetch. Now warns and returns instead.
Doctor promotes the corresponding check from :info to :warning and
explains that both env and adapter are required.
- **RSC previous-bundle staging path** (codex P2): the renderer routes
RSC requests by rsc_bundle_hash, which differs from server_bundle_hash
and is a separate <cache>/<hash>/<hash>.js entry. The original
protocol merged both bundles under a single hash's directory using
basenames, so the RSC bundle never landed where the renderer looks.
Fix: protocol refactor — each hash is now one bundle's cache entry.
fetch(hash) → { bundle: path, assets: [paths] }
upload(hash, bundle:, assets:)
previous_bundle_hashes → flat list including server AND rsc hashes
AssetsPrecompile.publish_current_bundle_if_configured now calls
upload twice when RSC is enabled (once per hash). Stager stages each
hash at <cache>/<hash>/<hash>.js with its companion assets.
SHOULD-FIX (4):
- **Timeout on adapter.fetch** (claude): wrap with Timeout.timeout
(default 30s) so a hung external store can't block pre-seeding or
assets:precompile indefinitely.
- **Relative symlinks** (greptile P2): stage_file in :symlink mode now
uses the same Pathname#realpath + relative_path_from pattern as
PreSeedRendererCache.make_relative_symlink, matching current-hash
staging and surviving cache-dir moves.
- **Remove stray module_function** (claude): was paired with
private_class_method (an unusual combination that leaks private
instance methods to any include-r). Switched to `def self.` for all
methods, private_class_method for helpers.
- **respond_to? instead of methods.include?** (claude): more idiomatic
duck-typing in both Configuration.validate_rolling_deploy_adapter
and Doctor.report_adapter_protocol.
DOC POLISH (3):
- Control Plane reference impl: switched from backtick/system shell
interpolation to Open3.capture2e with array-form args to avoid
shell injection via env-var contents. Returns both server AND RSC
hashes via previous_bundle_hashes.
- Both S3 and Control Plane examples: lazy env-var accessor methods
instead of constants evaluated at require time, so KeyError can't
fire at class-load in environments that don't configure the adapter.
- S3 reference impl: explicit note about the read-modify-write race
in update_manifest!; documents serializing deploys or using
If-Match/ETag as strict-safety alternatives.
- Updated protocol docstrings + all reference impls (S3, Control Plane,
Filesystem) to the new fetch/upload signatures.
Tests: 13 rolling-deploy specs now (was 7) — adds nil-adapter+env
warning, fetch timeout, relative-symlink assertion. Doctor spec
updated for the warn-vs-info change. 38 Pro specs pass, 167/168
doctor specs pass (1 pre-existing unrelated failure).
Refs: #3122, #3167 (PR B), #3173 (this PR)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Mattered:
Review follow-up:
Validation:
CI as of this checkpoint:
Future full-PR scans should start after this comment unless |
|
CI update after the previous checkpoint:
Future full-PR scans should start after this comment unless |
e25fd1f
into
jg/3122-preseed-renderer-cache
Introduces a pluggable adapter protocol so applications can eliminate
the 410→retry cold-start round-trip for previously-deployed bundle
hashes during rolling deploys — not just the current hash that
PreSeedRendererCache already handles.
Protocol (parallel to remote_bundle_cache_adapter):
module MyAdapter
def self.previous_bundle_hashes
def self.fetch(bundle_hash)
def self.upload(bundle_hash, server_bundle:, rsc_bundle: nil, assets:)
end
ReactOnRailsPro.configure do |config|
config.rolling_deploy_adapter = MyAdapter
end
Integration points:
- PreSeedRendererCache.call invokes RollingDeployCacheStager after
staging the current hash(es). Seeds each previous hash's bundle +
companion assets (loadable-stats.json, RSC manifests) into the
same <cache>/<hash>/... layout so hydration stays consistent with
the deployed asset pipeline for that hash.
- AssetsPrecompile.call auto-invokes adapter.upload(current_hash, ...)
after precompile in production-like environments.
- PREVIOUS_BUNDLE_HASHES env var (comma-separated) overrides
previous_bundle_hashes discovery for CI/testing.
- Config validation checks the adapter responds to all three required
methods at configure time.
Error handling (all degrade gracefully — runtime 410-retry remains
the fallback for any seeding failure):
- Adapter not configured: no-op.
- previous_bundle_hashes raises: warn, skip previous-hash seeding.
- fetch returns nil or raises: warn, skip that hash.
- upload raises in precompile: warn, don't fail precompile.
- Hash matching current hash: deduped.
Doctor probes:
- Verifies protocol conformance.
- Probes previous_bundle_hashes with a 3s timeout; reports latency and
hash count.
- Warns on empty list ("upload side has never run").
- Surfaces resolved renderer cache dir + hash subdirs present.
- Echoes PREVIOUS_BUNDLE_HASHES env if set without an adapter.
- Never calls fetch or upload (side effects).
Docs: new docs/pro/rolling-deploy-adapters.md with protocol spec and
three reference implementations (S3, Control Plane, Filesystem).
docs/pro/node-renderer.md points at the new page.
Tests: 7 new specs for RollingDeployCacheStager covering all
invocation paths plus 5 new doctor specs.
Refs: #3122, #3167 (PR B stacked base)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
MUST-FIX (2):
- **Nil adapter + PREVIOUS_BUNDLE_HASHES bug** (greptile P1, claude):
when the env override was set without config.rolling_deploy_adapter,
the stager proceeded past the early-return guard and crashed with
NoMethodError at adapter.fetch. Now warns and returns instead.
Doctor promotes the corresponding check from :info to :warning and
explains that both env and adapter are required.
- **RSC previous-bundle staging path** (codex P2): the renderer routes
RSC requests by rsc_bundle_hash, which differs from server_bundle_hash
and is a separate <cache>/<hash>/<hash>.js entry. The original
protocol merged both bundles under a single hash's directory using
basenames, so the RSC bundle never landed where the renderer looks.
Fix: protocol refactor — each hash is now one bundle's cache entry.
fetch(hash) → { bundle: path, assets: [paths] }
upload(hash, bundle:, assets:)
previous_bundle_hashes → flat list including server AND rsc hashes
AssetsPrecompile.publish_current_bundle_if_configured now calls
upload twice when RSC is enabled (once per hash). Stager stages each
hash at <cache>/<hash>/<hash>.js with its companion assets.
SHOULD-FIX (4):
- **Timeout on adapter.fetch** (claude): wrap with Timeout.timeout
(default 30s) so a hung external store can't block pre-seeding or
assets:precompile indefinitely.
- **Relative symlinks** (greptile P2): stage_file in :symlink mode now
uses the same Pathname#realpath + relative_path_from pattern as
PreSeedRendererCache.make_relative_symlink, matching current-hash
staging and surviving cache-dir moves.
- **Remove stray module_function** (claude): was paired with
private_class_method (an unusual combination that leaks private
instance methods to any include-r). Switched to `def self.` for all
methods, private_class_method for helpers.
- **respond_to? instead of methods.include?** (claude): more idiomatic
duck-typing in both Configuration.validate_rolling_deploy_adapter
and Doctor.report_adapter_protocol.
DOC POLISH (3):
- Control Plane reference impl: switched from backtick/system shell
interpolation to Open3.capture2e with array-form args to avoid
shell injection via env-var contents. Returns both server AND RSC
hashes via previous_bundle_hashes.
- Both S3 and Control Plane examples: lazy env-var accessor methods
instead of constants evaluated at require time, so KeyError can't
fire at class-load in environments that don't configure the adapter.
- S3 reference impl: explicit note about the read-modify-write race
in update_manifest!; documents serializing deploys or using
If-Match/ETag as strict-safety alternatives.
- Updated protocol docstrings + all reference impls (S3, Control Plane,
Filesystem) to the new fetch/upload signatures.
Tests: 13 rolling-deploy specs now (was 7) — adds nil-adapter+env
warning, fetch timeout, relative-symlink assertion. Doctor spec
updated for the warn-vs-info change. 38 Pro specs pass, 167/168
doctor specs pass (1 pre-existing unrelated failure).
Refs: #3122, #3167 (PR B), #3173 (this PR)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…3167) ## Summary Collapses `ReactOnRailsPro::PreSeedRendererCache` (copy) and `ReactOnRailsPro::PrepareNodeRenderBundles` (symlink) into a single entry point: `PreSeedRendererCache.call(mode: :copy | :symlink)`. Both modes produce the same \`<cache>/<bundleHash>/<bundleHash>.js\` layout — only the file operation differs. Stacked on #3124 (PR A). Target base is \`jg/3122-preseed-renderer-cache\`; will be rebased onto \`master\` once PR A merges. Refs #3122. ## Changes - **Unified API**: \`PreSeedRendererCache.call(mode: :copy)\` for Docker/image builds (default); \`(mode: :symlink)\` for same-filesystem workflows (local dev, CI, Heroku-style same-dyno deploys, bundle-caching restores). Unknown modes → \`ArgumentError\`. - **Non-dev env-var guard (copy mode only)**: When neither \`RENDERER_SERVER_BUNDLE_CACHE_PATH\` nor \`RENDERER_BUNDLE_PATH\` is set in a non-dev/test environment, \`mode: :copy\` raises a clear error. Motivation: the Node renderer's default cache-path resolution can differ from the Ruby side (falling back to \`/tmp\` when \`cwd\` is outside the app tree), so silent fallback is a misconfig footgun that pre-seeds bundles into a directory the renderer never reads. - **Rake task**: \`react_on_rails_pro:pre_seed_renderer_cache\` accepts \`MODE=copy\` (default) or \`MODE=symlink\`. - **Deprecations (soft)**: \`PrepareNodeRenderBundles\` and the \`pre_stage_bundle_for_node_renderer\` rake task remain as thin shims emitting once-per-process deprecation warnings; both delegate to the unified API with \`mode: :symlink\`. No behavior change for existing users. - **Auto-invocation**: \`AssetsPrecompile.call\` now calls \`PreSeedRendererCache.call(mode: :symlink)\` after precompile (previously \`PrepareNodeRenderBundles.call\` — same effect). - **Doctor check**: \`react_on_rails:doctor\` scans common deploy-script locations (\`Procfile*\`, \`Dockerfile\`, \`bin/deploy\`, \`bin/release\`, \`bin/docker-entrypoint\`) for references to the deprecated task and surfaces a migration warning. - **Docs**: \`docs/pro/node-renderer.md\` updated to describe both modes, the non-dev env-var requirement for copy, and the deprecation story. - **CHANGELOG**: new \`Changed\` entry. ## Why this shape - The two classes shared ~90% of their logic after #3124 (both use \`RendererCacheHelpers\` for bundle validation, asset collection, and required-RSC-asset path construction). Keeping them separate forced readers to understand two entry points with parallel semantics. - Single class with \`mode:\` keeps the public surface obvious: the user picks copy vs symlink based on whether bundles need to survive Docker layer boundaries. Everything else is shared. - Soft deprecation avoids breaking users with \`Procfile\`/\`Dockerfile\`/\`bin/\` entries pinned to the old names. The doctor check gives them a clear migration nudge. ## Test plan - [x] 26 dummy specs pass for both \`pre_seed_renderer_cache_spec.rb\` and \`prepare_node_renderer_bundles_spec.rb\` (+6 new: mode validation, symlink mode via unified API, raise-in-non-dev guard, symlink mode opt-out from guard, env-var bypass for guard, deprecation warning on the shim). - [x] 2 new doctor specs for \`check_deprecated_renderer_cache_task\`: Procfile-reference warning + no-match silence. - [x] 163 total doctor specs pass (1 pre-existing unrelated failure in \`auto_fix_versions\`, not introduced by this PR). - [x] \`bundle exec rubocop\` clean on all changed files. - [ ] CI (will verify after push). ## Migration path for users - Existing \`Procfile\` / \`Dockerfile\` / \`bin/*\` entries that call \`pre_stage_bundle_for_node_renderer\` keep working unchanged — they emit a one-time deprecation warning and delegate to \`MODE=symlink\`. - Doctor surfaces affected files with migration guidance. - No changes needed for users relying on \`AssetsPrecompile.call\` auto-invocation (same behavior). - Docker builds that were using the old task name should migrate to \`MODE=copy\` (the default) for image-baked caches. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Touches Pro deployment/SSR renderer-cache staging behavior and introduces new env-var enforcement in `MODE=copy`, which could break misconfigured non-dev environments; changes are mitigated by deprecation shims and added doctor warnings/tests. > > **Overview** > **Unifies Node Renderer cache staging in Pro** by making `ReactOnRailsPro::PreSeedRendererCache.call(mode: :copy | :symlink)` the single entry point; both modes produce the same `<cache>/<bundleHash>/<bundleHash>.js` layout and the rake task `react_on_rails_pro:pre_seed_renderer_cache` now accepts `MODE=copy` (default) or `MODE=symlink`. > > **Deprecates legacy paths**: `react_on_rails_pro:pre_stage_bundle_for_node_renderer` and `ReactOnRailsPro::PrepareNodeRenderBundles` now warn and delegate to `mode: :symlink`, while `assets:precompile` auto-stages via `mode: :symlink`. > > **Adds safety/migration checks**: `MODE=copy` now raises in non-dev/test when neither `RENDERER_SERVER_BUNDLE_CACHE_PATH` nor `RENDERER_BUNDLE_PATH` is set, and `react_on_rails:doctor` scans common deploy scripts for the deprecated task and suggests the correct replacement. > > Docs/CHANGELOG are updated for the new modes and deprecations; specs are expanded for the new behavior, and the link checker ignore list adds `guavapass.com`. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit f511867. 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>
Introduces a pluggable adapter protocol so applications can eliminate
the 410→retry cold-start round-trip for previously-deployed bundle
hashes during rolling deploys — not just the current hash that
PreSeedRendererCache already handles.
Protocol (parallel to remote_bundle_cache_adapter):
module MyAdapter
def self.previous_bundle_hashes
def self.fetch(bundle_hash)
def self.upload(bundle_hash, server_bundle:, rsc_bundle: nil, assets:)
end
ReactOnRailsPro.configure do |config|
config.rolling_deploy_adapter = MyAdapter
end
Integration points:
- PreSeedRendererCache.call invokes RollingDeployCacheStager after
staging the current hash(es). Seeds each previous hash's bundle +
companion assets (loadable-stats.json, RSC manifests) into the
same <cache>/<hash>/... layout so hydration stays consistent with
the deployed asset pipeline for that hash.
- AssetsPrecompile.call auto-invokes adapter.upload(current_hash, ...)
after precompile in production-like environments.
- PREVIOUS_BUNDLE_HASHES env var (comma-separated) overrides
previous_bundle_hashes discovery for CI/testing.
- Config validation checks the adapter responds to all three required
methods at configure time.
Error handling (all degrade gracefully — runtime 410-retry remains
the fallback for any seeding failure):
- Adapter not configured: no-op.
- previous_bundle_hashes raises: warn, skip previous-hash seeding.
- fetch returns nil or raises: warn, skip that hash.
- upload raises in precompile: warn, don't fail precompile.
- Hash matching current hash: deduped.
Doctor probes:
- Verifies protocol conformance.
- Probes previous_bundle_hashes with a 3s timeout; reports latency and
hash count.
- Warns on empty list ("upload side has never run").
- Surfaces resolved renderer cache dir + hash subdirs present.
- Echoes PREVIOUS_BUNDLE_HASHES env if set without an adapter.
- Never calls fetch or upload (side effects).
Docs: new docs/pro/rolling-deploy-adapters.md with protocol spec and
three reference implementations (S3, Control Plane, Filesystem).
docs/pro/node-renderer.md points at the new page.
Tests: 7 new specs for RollingDeployCacheStager covering all
invocation paths plus 5 new doctor specs.
Refs: #3122, #3167 (PR B stacked base)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
MUST-FIX (2):
- **Nil adapter + PREVIOUS_BUNDLE_HASHES bug** (greptile P1, claude):
when the env override was set without config.rolling_deploy_adapter,
the stager proceeded past the early-return guard and crashed with
NoMethodError at adapter.fetch. Now warns and returns instead.
Doctor promotes the corresponding check from :info to :warning and
explains that both env and adapter are required.
- **RSC previous-bundle staging path** (codex P2): the renderer routes
RSC requests by rsc_bundle_hash, which differs from server_bundle_hash
and is a separate <cache>/<hash>/<hash>.js entry. The original
protocol merged both bundles under a single hash's directory using
basenames, so the RSC bundle never landed where the renderer looks.
Fix: protocol refactor — each hash is now one bundle's cache entry.
fetch(hash) → { bundle: path, assets: [paths] }
upload(hash, bundle:, assets:)
previous_bundle_hashes → flat list including server AND rsc hashes
AssetsPrecompile.publish_current_bundle_if_configured now calls
upload twice when RSC is enabled (once per hash). Stager stages each
hash at <cache>/<hash>/<hash>.js with its companion assets.
SHOULD-FIX (4):
- **Timeout on adapter.fetch** (claude): wrap with Timeout.timeout
(default 30s) so a hung external store can't block pre-seeding or
assets:precompile indefinitely.
- **Relative symlinks** (greptile P2): stage_file in :symlink mode now
uses the same Pathname#realpath + relative_path_from pattern as
PreSeedRendererCache.make_relative_symlink, matching current-hash
staging and surviving cache-dir moves.
- **Remove stray module_function** (claude): was paired with
private_class_method (an unusual combination that leaks private
instance methods to any include-r). Switched to `def self.` for all
methods, private_class_method for helpers.
- **respond_to? instead of methods.include?** (claude): more idiomatic
duck-typing in both Configuration.validate_rolling_deploy_adapter
and Doctor.report_adapter_protocol.
DOC POLISH (3):
- Control Plane reference impl: switched from backtick/system shell
interpolation to Open3.capture2e with array-form args to avoid
shell injection via env-var contents. Returns both server AND RSC
hashes via previous_bundle_hashes.
- Both S3 and Control Plane examples: lazy env-var accessor methods
instead of constants evaluated at require time, so KeyError can't
fire at class-load in environments that don't configure the adapter.
- S3 reference impl: explicit note about the read-modify-write race
in update_manifest!; documents serializing deploys or using
If-Match/ETag as strict-safety alternatives.
- Updated protocol docstrings + all reference impls (S3, Control Plane,
Filesystem) to the new fetch/upload signatures.
Tests: 13 rolling-deploy specs now (was 7) — adds nil-adapter+env
warning, fetch timeout, relative-symlink assertion. Doctor spec
updated for the warn-vs-info change. 38 Pro specs pass, 167/168
doctor specs pass (1 pre-existing unrelated failure).
Refs: #3122, #3167 (PR B), #3173 (this PR)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…3167) ## Summary Collapses `ReactOnRailsPro::PreSeedRendererCache` (copy) and `ReactOnRailsPro::PrepareNodeRenderBundles` (symlink) into a single entry point: `PreSeedRendererCache.call(mode: :copy | :symlink)`. Both modes produce the same \`<cache>/<bundleHash>/<bundleHash>.js\` layout — only the file operation differs. Stacked on #3124 (PR A). Target base is \`jg/3122-preseed-renderer-cache\`; will be rebased onto \`master\` once PR A merges. Refs #3122. ## Changes - **Unified API**: \`PreSeedRendererCache.call(mode: :copy)\` for Docker/image builds (default); \`(mode: :symlink)\` for same-filesystem workflows (local dev, CI, Heroku-style same-dyno deploys, bundle-caching restores). Unknown modes → \`ArgumentError\`. - **Non-dev env-var guard (copy mode only)**: When neither \`RENDERER_SERVER_BUNDLE_CACHE_PATH\` nor \`RENDERER_BUNDLE_PATH\` is set in a non-dev/test environment, \`mode: :copy\` raises a clear error. Motivation: the Node renderer's default cache-path resolution can differ from the Ruby side (falling back to \`/tmp\` when \`cwd\` is outside the app tree), so silent fallback is a misconfig footgun that pre-seeds bundles into a directory the renderer never reads. - **Rake task**: \`react_on_rails_pro:pre_seed_renderer_cache\` accepts \`MODE=copy\` (default) or \`MODE=symlink\`. - **Deprecations (soft)**: \`PrepareNodeRenderBundles\` and the \`pre_stage_bundle_for_node_renderer\` rake task remain as thin shims emitting once-per-process deprecation warnings; both delegate to the unified API with \`mode: :symlink\`. No behavior change for existing users. - **Auto-invocation**: \`AssetsPrecompile.call\` now calls \`PreSeedRendererCache.call(mode: :symlink)\` after precompile (previously \`PrepareNodeRenderBundles.call\` — same effect). - **Doctor check**: \`react_on_rails:doctor\` scans common deploy-script locations (\`Procfile*\`, \`Dockerfile\`, \`bin/deploy\`, \`bin/release\`, \`bin/docker-entrypoint\`) for references to the deprecated task and surfaces a migration warning. - **Docs**: \`docs/pro/node-renderer.md\` updated to describe both modes, the non-dev env-var requirement for copy, and the deprecation story. - **CHANGELOG**: new \`Changed\` entry. ## Why this shape - The two classes shared ~90% of their logic after #3124 (both use \`RendererCacheHelpers\` for bundle validation, asset collection, and required-RSC-asset path construction). Keeping them separate forced readers to understand two entry points with parallel semantics. - Single class with \`mode:\` keeps the public surface obvious: the user picks copy vs symlink based on whether bundles need to survive Docker layer boundaries. Everything else is shared. - Soft deprecation avoids breaking users with \`Procfile\`/\`Dockerfile\`/\`bin/\` entries pinned to the old names. The doctor check gives them a clear migration nudge. ## Test plan - [x] 26 dummy specs pass for both \`pre_seed_renderer_cache_spec.rb\` and \`prepare_node_renderer_bundles_spec.rb\` (+6 new: mode validation, symlink mode via unified API, raise-in-non-dev guard, symlink mode opt-out from guard, env-var bypass for guard, deprecation warning on the shim). - [x] 2 new doctor specs for \`check_deprecated_renderer_cache_task\`: Procfile-reference warning + no-match silence. - [x] 163 total doctor specs pass (1 pre-existing unrelated failure in \`auto_fix_versions\`, not introduced by this PR). - [x] \`bundle exec rubocop\` clean on all changed files. - [ ] CI (will verify after push). ## Migration path for users - Existing \`Procfile\` / \`Dockerfile\` / \`bin/*\` entries that call \`pre_stage_bundle_for_node_renderer\` keep working unchanged — they emit a one-time deprecation warning and delegate to \`MODE=symlink\`. - Doctor surfaces affected files with migration guidance. - No changes needed for users relying on \`AssetsPrecompile.call\` auto-invocation (same behavior). - Docker builds that were using the old task name should migrate to \`MODE=copy\` (the default) for image-baked caches. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Touches Pro deployment/SSR renderer-cache staging behavior and introduces new env-var enforcement in `MODE=copy`, which could break misconfigured non-dev environments; changes are mitigated by deprecation shims and added doctor warnings/tests. > > **Overview** > **Unifies Node Renderer cache staging in Pro** by making `ReactOnRailsPro::PreSeedRendererCache.call(mode: :copy | :symlink)` the single entry point; both modes produce the same `<cache>/<bundleHash>/<bundleHash>.js` layout and the rake task `react_on_rails_pro:pre_seed_renderer_cache` now accepts `MODE=copy` (default) or `MODE=symlink`. > > **Deprecates legacy paths**: `react_on_rails_pro:pre_stage_bundle_for_node_renderer` and `ReactOnRailsPro::PrepareNodeRenderBundles` now warn and delegate to `mode: :symlink`, while `assets:precompile` auto-stages via `mode: :symlink`. > > **Adds safety/migration checks**: `MODE=copy` now raises in non-dev/test when neither `RENDERER_SERVER_BUNDLE_CACHE_PATH` nor `RENDERER_BUNDLE_PATH` is set, and `react_on_rails:doctor` scans common deploy scripts for the deprecated task and suggests the correct replacement. > > Docs/CHANGELOG are updated for the new modes and deprecations; specs are expanded for the new behavior, and the link checker ignore list adds `guavapass.com`. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit f511867. 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>
Introduces a pluggable adapter protocol so applications can eliminate
the 410→retry cold-start round-trip for previously-deployed bundle
hashes during rolling deploys — not just the current hash that
PreSeedRendererCache already handles.
Protocol (parallel to remote_bundle_cache_adapter):
module MyAdapter
def self.previous_bundle_hashes
def self.fetch(bundle_hash)
def self.upload(bundle_hash, server_bundle:, rsc_bundle: nil, assets:)
end
ReactOnRailsPro.configure do |config|
config.rolling_deploy_adapter = MyAdapter
end
Integration points:
- PreSeedRendererCache.call invokes RollingDeployCacheStager after
staging the current hash(es). Seeds each previous hash's bundle +
companion assets (loadable-stats.json, RSC manifests) into the
same <cache>/<hash>/... layout so hydration stays consistent with
the deployed asset pipeline for that hash.
- AssetsPrecompile.call auto-invokes adapter.upload(current_hash, ...)
after precompile in production-like environments.
- PREVIOUS_BUNDLE_HASHES env var (comma-separated) overrides
previous_bundle_hashes discovery for CI/testing.
- Config validation checks the adapter responds to all three required
methods at configure time.
Error handling (all degrade gracefully — runtime 410-retry remains
the fallback for any seeding failure):
- Adapter not configured: no-op.
- previous_bundle_hashes raises: warn, skip previous-hash seeding.
- fetch returns nil or raises: warn, skip that hash.
- upload raises in precompile: warn, don't fail precompile.
- Hash matching current hash: deduped.
Doctor probes:
- Verifies protocol conformance.
- Probes previous_bundle_hashes with a 3s timeout; reports latency and
hash count.
- Warns on empty list ("upload side has never run").
- Surfaces resolved renderer cache dir + hash subdirs present.
- Echoes PREVIOUS_BUNDLE_HASHES env if set without an adapter.
- Never calls fetch or upload (side effects).
Docs: new docs/pro/rolling-deploy-adapters.md with protocol spec and
three reference implementations (S3, Control Plane, Filesystem).
docs/pro/node-renderer.md points at the new page.
Tests: 7 new specs for RollingDeployCacheStager covering all
invocation paths plus 5 new doctor specs.
Refs: #3122, #3167 (PR B stacked base)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
MUST-FIX (2):
- **Nil adapter + PREVIOUS_BUNDLE_HASHES bug** (greptile P1, claude):
when the env override was set without config.rolling_deploy_adapter,
the stager proceeded past the early-return guard and crashed with
NoMethodError at adapter.fetch. Now warns and returns instead.
Doctor promotes the corresponding check from :info to :warning and
explains that both env and adapter are required.
- **RSC previous-bundle staging path** (codex P2): the renderer routes
RSC requests by rsc_bundle_hash, which differs from server_bundle_hash
and is a separate <cache>/<hash>/<hash>.js entry. The original
protocol merged both bundles under a single hash's directory using
basenames, so the RSC bundle never landed where the renderer looks.
Fix: protocol refactor — each hash is now one bundle's cache entry.
fetch(hash) → { bundle: path, assets: [paths] }
upload(hash, bundle:, assets:)
previous_bundle_hashes → flat list including server AND rsc hashes
AssetsPrecompile.publish_current_bundle_if_configured now calls
upload twice when RSC is enabled (once per hash). Stager stages each
hash at <cache>/<hash>/<hash>.js with its companion assets.
SHOULD-FIX (4):
- **Timeout on adapter.fetch** (claude): wrap with Timeout.timeout
(default 30s) so a hung external store can't block pre-seeding or
assets:precompile indefinitely.
- **Relative symlinks** (greptile P2): stage_file in :symlink mode now
uses the same Pathname#realpath + relative_path_from pattern as
PreSeedRendererCache.make_relative_symlink, matching current-hash
staging and surviving cache-dir moves.
- **Remove stray module_function** (claude): was paired with
private_class_method (an unusual combination that leaks private
instance methods to any include-r). Switched to `def self.` for all
methods, private_class_method for helpers.
- **respond_to? instead of methods.include?** (claude): more idiomatic
duck-typing in both Configuration.validate_rolling_deploy_adapter
and Doctor.report_adapter_protocol.
DOC POLISH (3):
- Control Plane reference impl: switched from backtick/system shell
interpolation to Open3.capture2e with array-form args to avoid
shell injection via env-var contents. Returns both server AND RSC
hashes via previous_bundle_hashes.
- Both S3 and Control Plane examples: lazy env-var accessor methods
instead of constants evaluated at require time, so KeyError can't
fire at class-load in environments that don't configure the adapter.
- S3 reference impl: explicit note about the read-modify-write race
in update_manifest!; documents serializing deploys or using
If-Match/ETag as strict-safety alternatives.
- Updated protocol docstrings + all reference impls (S3, Control Plane,
Filesystem) to the new fetch/upload signatures.
Tests: 13 rolling-deploy specs now (was 7) — adds nil-adapter+env
warning, fetch timeout, relative-symlink assertion. Doctor spec
updated for the warn-vs-info change. 38 Pro specs pass, 167/168
doctor specs pass (1 pre-existing unrelated failure).
Refs: #3122, #3167 (PR B), #3173 (this PR)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Introduces a pluggable adapter protocol so applications can eliminate
the 410→retry cold-start round-trip for previously-deployed bundle
hashes during rolling deploys — not just the current hash that
PreSeedRendererCache already handles.
Protocol (parallel to remote_bundle_cache_adapter):
module MyAdapter
def self.previous_bundle_hashes
def self.fetch(bundle_hash)
def self.upload(bundle_hash, server_bundle:, rsc_bundle: nil, assets:)
end
ReactOnRailsPro.configure do |config|
config.rolling_deploy_adapter = MyAdapter
end
Integration points:
- PreSeedRendererCache.call invokes RollingDeployCacheStager after
staging the current hash(es). Seeds each previous hash's bundle +
companion assets (loadable-stats.json, RSC manifests) into the
same <cache>/<hash>/... layout so hydration stays consistent with
the deployed asset pipeline for that hash.
- AssetsPrecompile.call auto-invokes adapter.upload(current_hash, ...)
after precompile in production-like environments.
- PREVIOUS_BUNDLE_HASHES env var (comma-separated) overrides
previous_bundle_hashes discovery for CI/testing.
- Config validation checks the adapter responds to all three required
methods at configure time.
Error handling (all degrade gracefully — runtime 410-retry remains
the fallback for any seeding failure):
- Adapter not configured: no-op.
- previous_bundle_hashes raises: warn, skip previous-hash seeding.
- fetch returns nil or raises: warn, skip that hash.
- upload raises in precompile: warn, don't fail precompile.
- Hash matching current hash: deduped.
Doctor probes:
- Verifies protocol conformance.
- Probes previous_bundle_hashes with a 3s timeout; reports latency and
hash count.
- Warns on empty list ("upload side has never run").
- Surfaces resolved renderer cache dir + hash subdirs present.
- Echoes PREVIOUS_BUNDLE_HASHES env if set without an adapter.
- Never calls fetch or upload (side effects).
Docs: new docs/pro/rolling-deploy-adapters.md with protocol spec and
three reference implementations (S3, Control Plane, Filesystem).
docs/pro/node-renderer.md points at the new page.
Tests: 7 new specs for RollingDeployCacheStager covering all
invocation paths plus 5 new doctor specs.
Refs: #3122, #3167 (PR B stacked base)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
MUST-FIX (2):
- **Nil adapter + PREVIOUS_BUNDLE_HASHES bug** (greptile P1, claude):
when the env override was set without config.rolling_deploy_adapter,
the stager proceeded past the early-return guard and crashed with
NoMethodError at adapter.fetch. Now warns and returns instead.
Doctor promotes the corresponding check from :info to :warning and
explains that both env and adapter are required.
- **RSC previous-bundle staging path** (codex P2): the renderer routes
RSC requests by rsc_bundle_hash, which differs from server_bundle_hash
and is a separate <cache>/<hash>/<hash>.js entry. The original
protocol merged both bundles under a single hash's directory using
basenames, so the RSC bundle never landed where the renderer looks.
Fix: protocol refactor — each hash is now one bundle's cache entry.
fetch(hash) → { bundle: path, assets: [paths] }
upload(hash, bundle:, assets:)
previous_bundle_hashes → flat list including server AND rsc hashes
AssetsPrecompile.publish_current_bundle_if_configured now calls
upload twice when RSC is enabled (once per hash). Stager stages each
hash at <cache>/<hash>/<hash>.js with its companion assets.
SHOULD-FIX (4):
- **Timeout on adapter.fetch** (claude): wrap with Timeout.timeout
(default 30s) so a hung external store can't block pre-seeding or
assets:precompile indefinitely.
- **Relative symlinks** (greptile P2): stage_file in :symlink mode now
uses the same Pathname#realpath + relative_path_from pattern as
PreSeedRendererCache.make_relative_symlink, matching current-hash
staging and surviving cache-dir moves.
- **Remove stray module_function** (claude): was paired with
private_class_method (an unusual combination that leaks private
instance methods to any include-r). Switched to `def self.` for all
methods, private_class_method for helpers.
- **respond_to? instead of methods.include?** (claude): more idiomatic
duck-typing in both Configuration.validate_rolling_deploy_adapter
and Doctor.report_adapter_protocol.
DOC POLISH (3):
- Control Plane reference impl: switched from backtick/system shell
interpolation to Open3.capture2e with array-form args to avoid
shell injection via env-var contents. Returns both server AND RSC
hashes via previous_bundle_hashes.
- Both S3 and Control Plane examples: lazy env-var accessor methods
instead of constants evaluated at require time, so KeyError can't
fire at class-load in environments that don't configure the adapter.
- S3 reference impl: explicit note about the read-modify-write race
in update_manifest!; documents serializing deploys or using
If-Match/ETag as strict-safety alternatives.
- Updated protocol docstrings + all reference impls (S3, Control Plane,
Filesystem) to the new fetch/upload signatures.
Tests: 13 rolling-deploy specs now (was 7) — adds nil-adapter+env
warning, fetch timeout, relative-symlink assertion. Doctor spec
updated for the warn-vs-info change. 38 Pro specs pass, 167/168
doctor specs pass (1 pre-existing unrelated failure).
Refs: #3122, #3167 (PR B), #3173 (this PR)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…3167) ## Summary Collapses `ReactOnRailsPro::PreSeedRendererCache` (copy) and `ReactOnRailsPro::PrepareNodeRenderBundles` (symlink) into a single entry point: `PreSeedRendererCache.call(mode: :copy | :symlink)`. Both modes produce the same \`<cache>/<bundleHash>/<bundleHash>.js\` layout — only the file operation differs. Stacked on #3124 (PR A). Target base is \`jg/3122-preseed-renderer-cache\`; will be rebased onto \`master\` once PR A merges. Refs #3122. ## Changes - **Unified API**: \`PreSeedRendererCache.call(mode: :copy)\` for Docker/image builds (default); \`(mode: :symlink)\` for same-filesystem workflows (local dev, CI, Heroku-style same-dyno deploys, bundle-caching restores). Unknown modes → \`ArgumentError\`. - **Non-dev env-var guard (copy mode only)**: When neither \`RENDERER_SERVER_BUNDLE_CACHE_PATH\` nor \`RENDERER_BUNDLE_PATH\` is set in a non-dev/test environment, \`mode: :copy\` raises a clear error. Motivation: the Node renderer's default cache-path resolution can differ from the Ruby side (falling back to \`/tmp\` when \`cwd\` is outside the app tree), so silent fallback is a misconfig footgun that pre-seeds bundles into a directory the renderer never reads. - **Rake task**: \`react_on_rails_pro:pre_seed_renderer_cache\` accepts \`MODE=copy\` (default) or \`MODE=symlink\`. - **Deprecations (soft)**: \`PrepareNodeRenderBundles\` and the \`pre_stage_bundle_for_node_renderer\` rake task remain as thin shims emitting once-per-process deprecation warnings; both delegate to the unified API with \`mode: :symlink\`. No behavior change for existing users. - **Auto-invocation**: \`AssetsPrecompile.call\` now calls \`PreSeedRendererCache.call(mode: :symlink)\` after precompile (previously \`PrepareNodeRenderBundles.call\` — same effect). - **Doctor check**: \`react_on_rails:doctor\` scans common deploy-script locations (\`Procfile*\`, \`Dockerfile\`, \`bin/deploy\`, \`bin/release\`, \`bin/docker-entrypoint\`) for references to the deprecated task and surfaces a migration warning. - **Docs**: \`docs/pro/node-renderer.md\` updated to describe both modes, the non-dev env-var requirement for copy, and the deprecation story. - **CHANGELOG**: new \`Changed\` entry. ## Why this shape - The two classes shared ~90% of their logic after #3124 (both use \`RendererCacheHelpers\` for bundle validation, asset collection, and required-RSC-asset path construction). Keeping them separate forced readers to understand two entry points with parallel semantics. - Single class with \`mode:\` keeps the public surface obvious: the user picks copy vs symlink based on whether bundles need to survive Docker layer boundaries. Everything else is shared. - Soft deprecation avoids breaking users with \`Procfile\`/\`Dockerfile\`/\`bin/\` entries pinned to the old names. The doctor check gives them a clear migration nudge. ## Test plan - [x] 26 dummy specs pass for both \`pre_seed_renderer_cache_spec.rb\` and \`prepare_node_renderer_bundles_spec.rb\` (+6 new: mode validation, symlink mode via unified API, raise-in-non-dev guard, symlink mode opt-out from guard, env-var bypass for guard, deprecation warning on the shim). - [x] 2 new doctor specs for \`check_deprecated_renderer_cache_task\`: Procfile-reference warning + no-match silence. - [x] 163 total doctor specs pass (1 pre-existing unrelated failure in \`auto_fix_versions\`, not introduced by this PR). - [x] \`bundle exec rubocop\` clean on all changed files. - [ ] CI (will verify after push). ## Migration path for users - Existing \`Procfile\` / \`Dockerfile\` / \`bin/*\` entries that call \`pre_stage_bundle_for_node_renderer\` keep working unchanged — they emit a one-time deprecation warning and delegate to \`MODE=symlink\`. - Doctor surfaces affected files with migration guidance. - No changes needed for users relying on \`AssetsPrecompile.call\` auto-invocation (same behavior). - Docker builds that were using the old task name should migrate to \`MODE=copy\` (the default) for image-baked caches. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Touches Pro deployment/SSR renderer-cache staging behavior and introduces new env-var enforcement in `MODE=copy`, which could break misconfigured non-dev environments; changes are mitigated by deprecation shims and added doctor warnings/tests. > > **Overview** > **Unifies Node Renderer cache staging in Pro** by making `ReactOnRailsPro::PreSeedRendererCache.call(mode: :copy | :symlink)` the single entry point; both modes produce the same `<cache>/<bundleHash>/<bundleHash>.js` layout and the rake task `react_on_rails_pro:pre_seed_renderer_cache` now accepts `MODE=copy` (default) or `MODE=symlink`. > > **Deprecates legacy paths**: `react_on_rails_pro:pre_stage_bundle_for_node_renderer` and `ReactOnRailsPro::PrepareNodeRenderBundles` now warn and delegate to `mode: :symlink`, while `assets:precompile` auto-stages via `mode: :symlink`. > > **Adds safety/migration checks**: `MODE=copy` now raises in non-dev/test when neither `RENDERER_SERVER_BUNDLE_CACHE_PATH` nor `RENDERER_BUNDLE_PATH` is set, and `react_on_rails:doctor` scans common deploy scripts for the deprecated task and suggests the correct replacement. > > Docs/CHANGELOG are updated for the new modes and deprecations; specs are expanded for the new behavior, and the link checker ignore list adds `guavapass.com`. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit f511867. 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>
Introduces a pluggable adapter protocol so applications can eliminate
the 410→retry cold-start round-trip for previously-deployed bundle
hashes during rolling deploys — not just the current hash that
PreSeedRendererCache already handles.
Protocol (parallel to remote_bundle_cache_adapter):
module MyAdapter
def self.previous_bundle_hashes
def self.fetch(bundle_hash)
def self.upload(bundle_hash, server_bundle:, rsc_bundle: nil, assets:)
end
ReactOnRailsPro.configure do |config|
config.rolling_deploy_adapter = MyAdapter
end
Integration points:
- PreSeedRendererCache.call invokes RollingDeployCacheStager after
staging the current hash(es). Seeds each previous hash's bundle +
companion assets (loadable-stats.json, RSC manifests) into the
same <cache>/<hash>/... layout so hydration stays consistent with
the deployed asset pipeline for that hash.
- AssetsPrecompile.call auto-invokes adapter.upload(current_hash, ...)
after precompile in production-like environments.
- PREVIOUS_BUNDLE_HASHES env var (comma-separated) overrides
previous_bundle_hashes discovery for CI/testing.
- Config validation checks the adapter responds to all three required
methods at configure time.
Error handling (all degrade gracefully — runtime 410-retry remains
the fallback for any seeding failure):
- Adapter not configured: no-op.
- previous_bundle_hashes raises: warn, skip previous-hash seeding.
- fetch returns nil or raises: warn, skip that hash.
- upload raises in precompile: warn, don't fail precompile.
- Hash matching current hash: deduped.
Doctor probes:
- Verifies protocol conformance.
- Probes previous_bundle_hashes with a 3s timeout; reports latency and
hash count.
- Warns on empty list ("upload side has never run").
- Surfaces resolved renderer cache dir + hash subdirs present.
- Echoes PREVIOUS_BUNDLE_HASHES env if set without an adapter.
- Never calls fetch or upload (side effects).
Docs: new docs/pro/rolling-deploy-adapters.md with protocol spec and
three reference implementations (S3, Control Plane, Filesystem).
docs/pro/node-renderer.md points at the new page.
Tests: 7 new specs for RollingDeployCacheStager covering all
invocation paths plus 5 new doctor specs.
Refs: #3122, #3167 (PR B stacked base)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
MUST-FIX (2):
- **Nil adapter + PREVIOUS_BUNDLE_HASHES bug** (greptile P1, claude):
when the env override was set without config.rolling_deploy_adapter,
the stager proceeded past the early-return guard and crashed with
NoMethodError at adapter.fetch. Now warns and returns instead.
Doctor promotes the corresponding check from :info to :warning and
explains that both env and adapter are required.
- **RSC previous-bundle staging path** (codex P2): the renderer routes
RSC requests by rsc_bundle_hash, which differs from server_bundle_hash
and is a separate <cache>/<hash>/<hash>.js entry. The original
protocol merged both bundles under a single hash's directory using
basenames, so the RSC bundle never landed where the renderer looks.
Fix: protocol refactor — each hash is now one bundle's cache entry.
fetch(hash) → { bundle: path, assets: [paths] }
upload(hash, bundle:, assets:)
previous_bundle_hashes → flat list including server AND rsc hashes
AssetsPrecompile.publish_current_bundle_if_configured now calls
upload twice when RSC is enabled (once per hash). Stager stages each
hash at <cache>/<hash>/<hash>.js with its companion assets.
SHOULD-FIX (4):
- **Timeout on adapter.fetch** (claude): wrap with Timeout.timeout
(default 30s) so a hung external store can't block pre-seeding or
assets:precompile indefinitely.
- **Relative symlinks** (greptile P2): stage_file in :symlink mode now
uses the same Pathname#realpath + relative_path_from pattern as
PreSeedRendererCache.make_relative_symlink, matching current-hash
staging and surviving cache-dir moves.
- **Remove stray module_function** (claude): was paired with
private_class_method (an unusual combination that leaks private
instance methods to any include-r). Switched to `def self.` for all
methods, private_class_method for helpers.
- **respond_to? instead of methods.include?** (claude): more idiomatic
duck-typing in both Configuration.validate_rolling_deploy_adapter
and Doctor.report_adapter_protocol.
DOC POLISH (3):
- Control Plane reference impl: switched from backtick/system shell
interpolation to Open3.capture2e with array-form args to avoid
shell injection via env-var contents. Returns both server AND RSC
hashes via previous_bundle_hashes.
- Both S3 and Control Plane examples: lazy env-var accessor methods
instead of constants evaluated at require time, so KeyError can't
fire at class-load in environments that don't configure the adapter.
- S3 reference impl: explicit note about the read-modify-write race
in update_manifest!; documents serializing deploys or using
If-Match/ETag as strict-safety alternatives.
- Updated protocol docstrings + all reference impls (S3, Control Plane,
Filesystem) to the new fetch/upload signatures.
Tests: 13 rolling-deploy specs now (was 7) — adds nil-adapter+env
warning, fetch timeout, relative-symlink assertion. Doctor spec
updated for the warn-vs-info change. 38 Pro specs pass, 167/168
doctor specs pass (1 pre-existing unrelated failure).
Refs: #3122, #3167 (PR B), #3173 (this PR)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Introduces a pluggable adapter protocol so applications can eliminate
the 410→retry cold-start round-trip for previously-deployed bundle
hashes during rolling deploys — not just the current hash that
PreSeedRendererCache already handles.
Protocol (parallel to remote_bundle_cache_adapter):
module MyAdapter
def self.previous_bundle_hashes
def self.fetch(bundle_hash)
def self.upload(bundle_hash, server_bundle:, rsc_bundle: nil, assets:)
end
ReactOnRailsPro.configure do |config|
config.rolling_deploy_adapter = MyAdapter
end
Integration points:
- PreSeedRendererCache.call invokes RollingDeployCacheStager after
staging the current hash(es). Seeds each previous hash's bundle +
companion assets (loadable-stats.json, RSC manifests) into the
same <cache>/<hash>/... layout so hydration stays consistent with
the deployed asset pipeline for that hash.
- AssetsPrecompile.call auto-invokes adapter.upload(current_hash, ...)
after precompile in production-like environments.
- PREVIOUS_BUNDLE_HASHES env var (comma-separated) overrides
previous_bundle_hashes discovery for CI/testing.
- Config validation checks the adapter responds to all three required
methods at configure time.
Error handling (all degrade gracefully — runtime 410-retry remains
the fallback for any seeding failure):
- Adapter not configured: no-op.
- previous_bundle_hashes raises: warn, skip previous-hash seeding.
- fetch returns nil or raises: warn, skip that hash.
- upload raises in precompile: warn, don't fail precompile.
- Hash matching current hash: deduped.
Doctor probes:
- Verifies protocol conformance.
- Probes previous_bundle_hashes with a 3s timeout; reports latency and
hash count.
- Warns on empty list ("upload side has never run").
- Surfaces resolved renderer cache dir + hash subdirs present.
- Echoes PREVIOUS_BUNDLE_HASHES env if set without an adapter.
- Never calls fetch or upload (side effects).
Docs: new docs/pro/rolling-deploy-adapters.md with protocol spec and
three reference implementations (S3, Control Plane, Filesystem).
docs/pro/node-renderer.md points at the new page.
Tests: 7 new specs for RollingDeployCacheStager covering all
invocation paths plus 5 new doctor specs.
Refs: #3122, #3167 (PR B stacked base)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
MUST-FIX (2):
- **Nil adapter + PREVIOUS_BUNDLE_HASHES bug** (greptile P1, claude):
when the env override was set without config.rolling_deploy_adapter,
the stager proceeded past the early-return guard and crashed with
NoMethodError at adapter.fetch. Now warns and returns instead.
Doctor promotes the corresponding check from :info to :warning and
explains that both env and adapter are required.
- **RSC previous-bundle staging path** (codex P2): the renderer routes
RSC requests by rsc_bundle_hash, which differs from server_bundle_hash
and is a separate <cache>/<hash>/<hash>.js entry. The original
protocol merged both bundles under a single hash's directory using
basenames, so the RSC bundle never landed where the renderer looks.
Fix: protocol refactor — each hash is now one bundle's cache entry.
fetch(hash) → { bundle: path, assets: [paths] }
upload(hash, bundle:, assets:)
previous_bundle_hashes → flat list including server AND rsc hashes
AssetsPrecompile.publish_current_bundle_if_configured now calls
upload twice when RSC is enabled (once per hash). Stager stages each
hash at <cache>/<hash>/<hash>.js with its companion assets.
SHOULD-FIX (4):
- **Timeout on adapter.fetch** (claude): wrap with Timeout.timeout
(default 30s) so a hung external store can't block pre-seeding or
assets:precompile indefinitely.
- **Relative symlinks** (greptile P2): stage_file in :symlink mode now
uses the same Pathname#realpath + relative_path_from pattern as
PreSeedRendererCache.make_relative_symlink, matching current-hash
staging and surviving cache-dir moves.
- **Remove stray module_function** (claude): was paired with
private_class_method (an unusual combination that leaks private
instance methods to any include-r). Switched to `def self.` for all
methods, private_class_method for helpers.
- **respond_to? instead of methods.include?** (claude): more idiomatic
duck-typing in both Configuration.validate_rolling_deploy_adapter
and Doctor.report_adapter_protocol.
DOC POLISH (3):
- Control Plane reference impl: switched from backtick/system shell
interpolation to Open3.capture2e with array-form args to avoid
shell injection via env-var contents. Returns both server AND RSC
hashes via previous_bundle_hashes.
- Both S3 and Control Plane examples: lazy env-var accessor methods
instead of constants evaluated at require time, so KeyError can't
fire at class-load in environments that don't configure the adapter.
- S3 reference impl: explicit note about the read-modify-write race
in update_manifest!; documents serializing deploys or using
If-Match/ETag as strict-safety alternatives.
- Updated protocol docstrings + all reference impls (S3, Control Plane,
Filesystem) to the new fetch/upload signatures.
Tests: 13 rolling-deploy specs now (was 7) — adds nil-adapter+env
warning, fetch timeout, relative-symlink assertion. Doctor spec
updated for the warn-vs-info change. 38 Pro specs pass, 167/168
doctor specs pass (1 pre-existing unrelated failure).
Refs: #3122, #3167 (PR B), #3173 (this PR)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Introduces a pluggable adapter protocol so applications can eliminate
the 410→retry cold-start round-trip for previously-deployed bundle
hashes during rolling deploys — not just the current hash that
PreSeedRendererCache already handles.
Protocol (parallel to remote_bundle_cache_adapter):
module MyAdapter
def self.previous_bundle_hashes
def self.fetch(bundle_hash)
def self.upload(bundle_hash, server_bundle:, rsc_bundle: nil, assets:)
end
ReactOnRailsPro.configure do |config|
config.rolling_deploy_adapter = MyAdapter
end
Integration points:
- PreSeedRendererCache.call invokes RollingDeployCacheStager after
staging the current hash(es). Seeds each previous hash's bundle +
companion assets (loadable-stats.json, RSC manifests) into the
same <cache>/<hash>/... layout so hydration stays consistent with
the deployed asset pipeline for that hash.
- AssetsPrecompile.call auto-invokes adapter.upload(current_hash, ...)
after precompile in production-like environments.
- PREVIOUS_BUNDLE_HASHES env var (comma-separated) overrides
previous_bundle_hashes discovery for CI/testing.
- Config validation checks the adapter responds to all three required
methods at configure time.
Error handling (all degrade gracefully — runtime 410-retry remains
the fallback for any seeding failure):
- Adapter not configured: no-op.
- previous_bundle_hashes raises: warn, skip previous-hash seeding.
- fetch returns nil or raises: warn, skip that hash.
- upload raises in precompile: warn, don't fail precompile.
- Hash matching current hash: deduped.
Doctor probes:
- Verifies protocol conformance.
- Probes previous_bundle_hashes with a 3s timeout; reports latency and
hash count.
- Warns on empty list ("upload side has never run").
- Surfaces resolved renderer cache dir + hash subdirs present.
- Echoes PREVIOUS_BUNDLE_HASHES env if set without an adapter.
- Never calls fetch or upload (side effects).
Docs: new docs/pro/rolling-deploy-adapters.md with protocol spec and
three reference implementations (S3, Control Plane, Filesystem).
docs/pro/node-renderer.md points at the new page.
Tests: 7 new specs for RollingDeployCacheStager covering all
invocation paths plus 5 new doctor specs.
Refs: #3122, #3167 (PR B stacked base)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
MUST-FIX (2):
- **Nil adapter + PREVIOUS_BUNDLE_HASHES bug** (greptile P1, claude):
when the env override was set without config.rolling_deploy_adapter,
the stager proceeded past the early-return guard and crashed with
NoMethodError at adapter.fetch. Now warns and returns instead.
Doctor promotes the corresponding check from :info to :warning and
explains that both env and adapter are required.
- **RSC previous-bundle staging path** (codex P2): the renderer routes
RSC requests by rsc_bundle_hash, which differs from server_bundle_hash
and is a separate <cache>/<hash>/<hash>.js entry. The original
protocol merged both bundles under a single hash's directory using
basenames, so the RSC bundle never landed where the renderer looks.
Fix: protocol refactor — each hash is now one bundle's cache entry.
fetch(hash) → { bundle: path, assets: [paths] }
upload(hash, bundle:, assets:)
previous_bundle_hashes → flat list including server AND rsc hashes
AssetsPrecompile.publish_current_bundle_if_configured now calls
upload twice when RSC is enabled (once per hash). Stager stages each
hash at <cache>/<hash>/<hash>.js with its companion assets.
SHOULD-FIX (4):
- **Timeout on adapter.fetch** (claude): wrap with Timeout.timeout
(default 30s) so a hung external store can't block pre-seeding or
assets:precompile indefinitely.
- **Relative symlinks** (greptile P2): stage_file in :symlink mode now
uses the same Pathname#realpath + relative_path_from pattern as
PreSeedRendererCache.make_relative_symlink, matching current-hash
staging and surviving cache-dir moves.
- **Remove stray module_function** (claude): was paired with
private_class_method (an unusual combination that leaks private
instance methods to any include-r). Switched to `def self.` for all
methods, private_class_method for helpers.
- **respond_to? instead of methods.include?** (claude): more idiomatic
duck-typing in both Configuration.validate_rolling_deploy_adapter
and Doctor.report_adapter_protocol.
DOC POLISH (3):
- Control Plane reference impl: switched from backtick/system shell
interpolation to Open3.capture2e with array-form args to avoid
shell injection via env-var contents. Returns both server AND RSC
hashes via previous_bundle_hashes.
- Both S3 and Control Plane examples: lazy env-var accessor methods
instead of constants evaluated at require time, so KeyError can't
fire at class-load in environments that don't configure the adapter.
- S3 reference impl: explicit note about the read-modify-write race
in update_manifest!; documents serializing deploys or using
If-Match/ETag as strict-safety alternatives.
- Updated protocol docstrings + all reference impls (S3, Control Plane,
Filesystem) to the new fetch/upload signatures.
Tests: 13 rolling-deploy specs now (was 7) — adds nil-adapter+env
warning, fetch timeout, relative-symlink assertion. Doctor spec
updated for the warn-vs-info change. 38 Pro specs pass, 167/168
doctor specs pass (1 pre-existing unrelated failure).
Refs: #3122, #3167 (PR B), #3173 (this PR)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
## Summary Adds a pluggable **`config.rolling_deploy_adapter`** protocol that eliminates 410→retry cold-start round-trips for _previously-deployed_ bundle hashes during rolling deploys — complementing the current-hash seeding already handled by PRs #3124 + #3167. Stacked on #3167 (PR B). Target base is \`jg/3122-unify-renderer-cache-staging\`; will rebase onto \`master\` once PRs A + B merge. Refs #3122. Design doc previously shared: see earlier session context. ## Why PR A seeded the current hash. PR B unified copy/symlink staging. But during a rolling deploy: - Old Rails instances (hash \`abc\`) still drain traffic. - New Rails instances (hash \`def\`) serve new traffic. - New renderer instances receive requests for **both** hashes. Pre-seeding only the current hash leaves \`abc\` cold on new renderers → every draining-version request hits the 410 retry path. This PR closes that gap. ## Protocol ```ruby module MyRollingDeployAdapter def self.previous_bundle_hashes # Array<String> def self.fetch(bundle_hash) # Hash(:bundle, :assets) | nil def self.upload(bundle_hash, bundle:, assets:) end ReactOnRailsPro.configure do |config| config.rolling_deploy_adapter = MyRollingDeployAdapter end ``` Parallel in shape to \`remote_bundle_cache_adapter\` — same duck-typed module pattern, same validation at configure time. ## The loadable-stats wrinkle Each bundle hash must carry its **own** companion \`loadable-stats.json\` and RSC manifests (built in lockstep with that bundle). Otherwise the renderer emits HTML referencing chunk URLs for the wrong build → client-side hydration breakage. The \`fetch\` signature returns bundle + assets _together_ to force callers to think about this; see \`docs/pro/rolling-deploy-adapters.md\` for the full discussion. ## Integration points 1. **\`PreSeedRendererCache.call\`** (from PR B) now invokes \`RollingDeployCacheStager\` after staging the current hash(es). Deduplicates against current hashes. 2. **\`AssetsPrecompile.call\`** auto-invokes \`adapter.upload(current_hash, ...)\` after precompile in production-like environments, closing the publish side so the next deploy can fetch. 3. **\`PREVIOUS_BUNDLE_HASHES\`** env var (comma-separated) overrides \`previous_bundle_hashes\` discovery for CI / testing. ## Error handling — degrades gracefully Runtime 410-retry remains the fallback for any seeding failure, so rolling-deploy seeding can never cause a correctness regression: | Scenario | Behavior | | --------------------------------- | --------------------------------- | | Adapter not configured | No-op. | | \`previous_bundle_hashes\` raises | Warn, skip previous-hash seeding. | | \`fetch\` returns nil or raises | Warn, skip that hash. | | \`upload\` raises in precompile | Warn, don't fail precompile. | | Returned hash matches current | Deduped. | ## Doctor (\`react_on_rails:doctor\`) probes - ✅ Protocol conformance (all three required methods). - ✅ \`previous_bundle_hashes\` probe with 10s timeout — reports latency and count. -⚠️ Empty list (\"upload side has never run on a prior deploy\"). - ℹ️ Resolved renderer cache dir + bundle-hash subdirs present. - ℹ️ \`PREVIOUS_BUNDLE_HASHES\` env set without an adapter. Doctor never calls \`fetch\` or \`upload\` — those have side effects. ## Docs - **New**: \`docs/pro/rolling-deploy-adapters.md\` — protocol spec, three reference implementations (S3, Control Plane, Filesystem), loadable-stats wrinkle discussion, relationship to \`remote_bundle_cache_adapter\`. - \`docs/pro/node-renderer.md\` rolling-deploys section updated to point at the new page. - Sidebar entry added. ## Test plan - [x] 7 new specs for \`RollingDeployCacheStager\` covering every invocation path: adapter unset, hash list discovery, env override, copy mode, symlink mode, fetch nil, fetch raises, previous_bundle_hashes raises, missing :server_bundle in payload, dedup vs current. - [x] 5 new doctor specs for \`check_rolling_deploy_adapter\`. - [x] Existing Pro specs updated where \`instance_double(Configuration, ...)\` needed the new accessor. - [x] 36 Pro specs pass locally (27 from PR B + 9 new). - [x] 168 doctor specs pass locally (1 pre-existing unrelated failure in \`auto_fix_versions\`). - [x] \`bundle exec rubocop\` clean on all changed files. - [ ] CI (will verify after push). ## Relationship to other adapters | | \`remote_bundle_cache_adapter\` | \`rolling_deploy_adapter\` | | ------------ | --------------------------------------------- | ----------------------------------------- | | **Scope** | Webpack build outputs (pre-compile caching) | Deployed bundle hashes (rolling deploy) | | **When** | Build phase | Post-precompile + pre-seed phase | | **Avoids** | Rebuilding webpack when source hasn't changed | 410 retries for draining-version requests | | **Keyed by** | Source digest | Bundle hash | Complementary — configure both if useful. 🤖 Generated with [Claude Code](https://claude.com/claude-code) ## <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Touches Pro deploy-time asset publication and renderer-cache staging paths (`assets:precompile` and `PreSeedRendererCache`), which can impact production deploy behavior if misconfigured. Changes are largely additive and guarded with timeouts/warnings, but involve filesystem operations and external adapter calls. > > **Overview** > Introduces a new Pro `config.rolling_deploy_adapter` protocol (`previous_bundle_hashes`, `fetch`, `upload`) to **seed prior bundle hashes** into the Node Renderer cache during `PreSeedRendererCache`, with `PREVIOUS_BUNDLE_HASHES` available as a discovery override. > > Hooks `assets:precompile` to **publish the current server/RSC bundle hashes + companion assets** via the adapter (best-effort with per-hash timeouts and warnings), and extends `react_on_rails:doctor` to validate/probe the adapter (including cache-dir reporting and env-var misuse warnings). > > Unifies cache staging helpers and updates staging to **auto-include `loadable-stats.json` when present**, adds extensive specs for the stager/doctor/config validation, and documents the protocol with new `docs/pro/rolling-deploy-adapters.md` plus sidebar/node-renderer doc updates (plus a minor lychee exclude tweak). > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit ca729db. 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 * **New Features** * Rolling-deploy adapter to pre-seed renderer caches and optionally publish bundles during assets precompile. * Automatic inclusion/staging of loadable-stats.json companion assets when present. * **Configuration** * New rolling_deploy_adapter option with validation of the adapter’s required interface. * **Documentation** * Comprehensive rolling-deploy adapter docs, examples, and expanded Node Renderer rolling-deploy guidance; sidebar entry added. * **Improved Diagnostics** * Doctor now reports rolling-deploy adapter status, discovery, and warnings. * **Tests** * Extensive spec coverage for rolling-deploy, pre-seed, publishing, and configuration validation. * **Chores** * CI link-checker exclusion added for an intermittently failing GitHub URL. [](https://app.coderabbit.ai/change-stack/shakacode/react_on_rails/pull/3173) <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Collapses
ReactOnRailsPro::PreSeedRendererCache(copy) andReactOnRailsPro::PrepareNodeRenderBundles(symlink) into a single entry point:PreSeedRendererCache.call(mode: :copy | :symlink). Both modes produce the same `//.js` layout — only the file operation differs.Stacked on #3124 (PR A). Target base is `jg/3122-preseed-renderer-cache`; will be rebased onto `master` once PR A merges. Refs #3122.
Changes
Why this shape
Test plan
Migration path for users
🤖 Generated with Claude Code
Note
Medium Risk
Touches Pro deployment/SSR renderer-cache staging behavior and introduces new env-var enforcement in
MODE=copy, which could break misconfigured non-dev environments; changes are mitigated by deprecation shims and added doctor warnings/tests.Overview
Unifies Node Renderer cache staging in Pro by making
ReactOnRailsPro::PreSeedRendererCache.call(mode: :copy | :symlink)the single entry point; both modes produce the same<cache>/<bundleHash>/<bundleHash>.jslayout and the rake taskreact_on_rails_pro:pre_seed_renderer_cachenow acceptsMODE=copy(default) orMODE=symlink.Deprecates legacy paths:
react_on_rails_pro:pre_stage_bundle_for_node_rendererandReactOnRailsPro::PrepareNodeRenderBundlesnow warn and delegate tomode: :symlink, whileassets:precompileauto-stages viamode: :symlink.Adds safety/migration checks:
MODE=copynow raises in non-dev/test when neitherRENDERER_SERVER_BUNDLE_CACHE_PATHnorRENDERER_BUNDLE_PATHis set, andreact_on_rails:doctorscans common deploy scripts for the deprecated task and suggests the correct replacement.Docs/CHANGELOG are updated for the new modes and deprecations; specs are expanded for the new behavior, and the link checker ignore list adds
guavapass.com.Reviewed by Cursor Bugbot for commit f511867. Bugbot is set up for automated code reviews on this repo. Configure here.