[codex] Update cpflow review app guidance#764
Conversation
🚀 Quick Review App CommandsWelcome! Here are the commands you can use in this PR:
|
|
Important Review skippedDraft detected. 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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
+review-app-deploy |
❌ Review App Deployment FailedDeployment failed for PR #764, commit 8b4be0c 🎮 Control Plane Console |
github-actions
Bot
had a problem deploying
to
review/qa-react-webpack-rails-tutorial-764
Review app ready |
github-actions
Bot
temporarily deployed
to
review/qa-react-webpack-rails-tutorial-764
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
🎉 Deploy Complete!Open Review AppDeployment successful for PR #764, commit 7a11703 🎮 Control Plane Console |
github-actions
Bot
temporarily deployed
to
review/qa-react-webpack-rails-tutorial-764
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Bugbot Autofix is ON. A cloud agent has been kicked off to fix the reported issues.
Reviewed by Cursor Bugbot for commit 7a11703. Configure here.
| "yarn res:build && RAILS_ENV=test bin/rails react_on_rails:locale && RAILS_ENV=test bin/shakapacker" | ||
| config.build_production_command = | ||
| "yarn res:build && RAILS_ENV=production NODE_ENV=production bin/rails react_on_rails:locale && " \ | ||
| "RAILS_ENV=production NODE_ENV=production bin/shakapacker" |
There was a problem hiding this comment.
Production build omits packs
Medium Severity
build_production_command still runs ReScript, locale, and Shakapacker but never react_on_rails:generate_packs, even though the same upgrade adds that task to the Docker image build and test asset scripts. Any production precompile that relies only on this configured command can miss generated pack entrypoints.
Reviewed by Cursor Bugbot for commit 7a11703. Configure here.
| fi | ||
|
|
||
| echo "⚠️ Application does not exist: $APP_NAME" | ||
| exit 0 |
There was a problem hiding this comment.
Exists errors skip deletion
Medium Severity
When cpflow exists exits non-zero, the script treats any output that does not match a short list of substrings as “application does not exist” and exits successfully without calling delete. Transient API, auth, or network failures can therefore be misread as a missing app and leave review apps undeleted.
Reviewed by Cursor Bugbot for commit 7a11703. Configure here.
| "build:test": "rm -rf public/packs-test && RAILS_ENV=test NODE_ENV=test bin/shakapacker", | ||
| "build:dev": "rm -rf public/packs && RAILS_ENV=development NODE_ENV=development bin/shakapacker", | ||
| "build:test": "rm -rf public/packs-test && yarn res:build && RAILS_ENV=test NODE_ENV=test bundle exec rails react_on_rails:locale && RAILS_ENV=test NODE_ENV=test bundle exec rake react_on_rails:generate_packs && RAILS_ENV=test NODE_ENV=test bin/shakapacker", | ||
| "build:dev": "rm -rf public/packs && yarn res:build && RAILS_ENV=development NODE_ENV=development bundle exec rails react_on_rails:locale && RAILS_ENV=development NODE_ENV=development bin/shakapacker", |
There was a problem hiding this comment.
Dev build skips pack generation
Low Severity
build:test runs react_on_rails:generate_packs before Shakapacker, but build:dev in the same edit does not. Local development builds that use only yarn build:dev can diverge from CI and fail or serve stale packs after v17 pack generation changes.
Reviewed by Cursor Bugbot for commit 7a11703. Configure here.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| "build:test": "rm -rf public/packs-test && RAILS_ENV=test NODE_ENV=test bin/shakapacker", | ||
| "build:dev": "rm -rf public/packs && RAILS_ENV=development NODE_ENV=development bin/shakapacker", | ||
| "build:test": "rm -rf public/packs-test && yarn res:build && RAILS_ENV=test NODE_ENV=test bundle exec rails react_on_rails:locale && RAILS_ENV=test NODE_ENV=test bundle exec rake react_on_rails:generate_packs && RAILS_ENV=test NODE_ENV=test bin/shakapacker", | ||
| "build:dev": "rm -rf public/packs && yarn res:build && RAILS_ENV=development NODE_ENV=development bundle exec rails react_on_rails:locale && RAILS_ENV=development NODE_ENV=development bin/shakapacker", |
There was a problem hiding this comment.
build:dev is missing react_on_rails:generate_packs
build:test (line above) now runs react_on_rails:generate_packs before shakapacker, and the Dockerfile was updated the same way. But build:dev only adds react_on_rails:locale. If v17 requires generate_packs before webpack (which the other changes imply), direct yarn build:dev calls will produce a broken dev bundle.
| "build:dev": "rm -rf public/packs && yarn res:build && RAILS_ENV=development NODE_ENV=development bundle exec rails react_on_rails:locale && RAILS_ENV=development NODE_ENV=development bin/shakapacker", | |
| "build:dev": "rm -rf public/packs && yarn res:build && RAILS_ENV=development NODE_ENV=development bundle exec rails react_on_rails:locale && RAILS_ENV=development NODE_ENV=development bundle exec rake react_on_rails:generate_packs && RAILS_ENV=development NODE_ENV=development bin/shakapacker", |
| config.build_production_command = | ||
| "yarn res:build && RAILS_ENV=production NODE_ENV=production bin/rails react_on_rails:locale && " \ | ||
| "RAILS_ENV=production NODE_ENV=production bin/shakapacker" |
There was a problem hiding this comment.
build_production_command is missing react_on_rails:generate_packs
The Dockerfile was updated to run bundle exec rake react_on_rails:generate_packs before assets:precompile, and build:test in package.json also runs it. But this production command only adds react_on_rails:locale — generate_packs is absent. All three build paths should be consistent.
Also note that build_test_command (line 11–12) has the same omission compared to the build:test yarn script, which does include generate_packs. Consider whether the initializer's test command should also include it.
| config.build_production_command = | |
| "yarn res:build && RAILS_ENV=production NODE_ENV=production bin/rails react_on_rails:locale && " \ | |
| "RAILS_ENV=production NODE_ENV=production bin/shakapacker" | |
| config.build_production_command = | |
| "yarn res:build && RAILS_ENV=production NODE_ENV=production bin/rails react_on_rails:locale && " \ | |
| "RAILS_ENV=production NODE_ENV=production bin/rails react_on_rails:generate_packs && " \ | |
| "RAILS_ENV=production NODE_ENV=production bin/shakapacker" |
| if [[ -n "${{ inputs.docker_build_extra_args }}" ]]; then | ||
| while IFS= read -r arg; do | ||
| arg="${arg%$'\r'}" | ||
| [[ -n "${arg}" ]] || continue | ||
|
|
||
| if [[ "${arg}" =~ [[:space:]] ]]; then | ||
| echo "docker_build_extra_args entries must be single docker-build tokens. " \ | ||
| "Use key=value forms like --build-arg=FOO=bar." >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| docker_build_args+=("${arg}") | ||
| done <<< "${{ inputs.docker_build_extra_args }}" |
There was a problem hiding this comment.
Shell injection risk: template expressions in inline script
${{ inputs.docker_build_extra_args }} is rendered into the script before the shell runs it. GitHub Actions does not shell-escape template values, so a crafted input like "; malicious_command; echo " could escape the here-string context. The same issue applies to ${{ inputs.docker_build_ssh_key }} on line 83.
The GitHub hardening guide recommends routing such values through env: variables instead:
env:
DOCKER_BUILD_EXTRA_ARGS: ${{ inputs.docker_build_extra_args }}
run: |
...
if [[ -n "$DOCKER_BUILD_EXTRA_ARGS" ]]; then
while IFS= read -r arg; do
...
done <<< "$DOCKER_BUILD_EXTRA_ARGS"
fiSince this action is only called from controlled workflows today the practical risk is low, but this pattern prevents future misuse if the action is ever invoked with PR-derived inputs.
| wait_for_service $(echo $REDIS_URL | sed -e 's|redis://||' -e 's|/.*$||') | ||
|
|
||
| echo " -- Finishing entrypoint.sh, executing '$@'" | ||
| echo " -- Finishing entrypoint.sh, executing '$*'" |
There was a problem hiding this comment.
Minor: $* vs $@ in the display message
The actual exec "$@" below is correct. But in a double-quoted string, $* joins all positional parameters with the first character of IFS (usually a space), while $@ expands them as separate words. For a display message they behave identically in normal cases, but $@ is the more precise choice and consistent with the exec below.
| echo " -- Finishing entrypoint.sh, executing '$*'" | |
| echo " -- Finishing entrypoint.sh, executing '$@'" |
| if [[ -z "$TOKEN" ]]; then | ||
| echo "Error: Control Plane token not provided" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| if [[ -z "$ORG" ]]; then | ||
| echo "Error: Control Plane organization not provided" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
There was a problem hiding this comment.
Dead code: both token and org inputs are declared required: true at lines 4 and 8, so GitHub Actions will fail the job before the shell script ever runs if they are absent. These runtime checks add noise without providing any additional safety. They can be removed.
🎉 Deploy Complete!Open Review AppDeployment successful for PR #764, commit 7a11703 🎮 Control Plane Console |
1 participant
Summary
5.1.117.0.0-rc.1Validation
ruby -S cpflow --version.github/workflows/cpflow-*.ymland.controlplane/controlplane.ymlgit diff --checkactionlint .github/workflows/cpflow-*.ymlwas run and only reported existing shellcheck style/info findingsNotes
Note
Medium Risk
Touches production Docker precompile, CI deploy/delete paths, and a major React on Rails RC upgrade; misordered or failed pack generation would break deploys and SSR/RSC.
Overview
Aligns the tutorial with cpflow 5.1.1 and React on Rails 17.0.0.rc.1 by adding reusable GitHub composite actions (setup, Docker build with optional SSH, guarded review-app delete) and refreshing Control Plane secret-template comments.
Build and deploy: The production Dockerfile now runs
react_on_rails:generate_packsbefore ReScript andassets:precompile. Rails initializer and yarnbuild:test/build:devscripts run locale generation (and testgenerate_packs) ahead of Shakapacker so direct bundler invocations match v17 expectations. Webpack resolvesclient/appand aliasesreact-on-railsto Pro to avoid mixing core and Pro at runtime.App surface: Gems and npm lock to RoR 17 / react-on-rails-rsc
19.0.5-rc.5; README documents the new Control Plane Actions flow and versions. Small client updates add'use client'where needed and formatting-only RSC/demo tweaks. entrypoint logging uses$*instead of$@.Reviewed by Cursor Bugbot for commit 7a11703. Bugbot is set up for automated code reviews on this repo. Configure here.