Conversation
🚀 Quick Review App CommandsWelcome! Here are the commands you can use in this PR:
|
|
No description provided. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 4 included reviews per hour; 1 remains after this review. WalkthroughThe control plane starts the Node renderer directly and increases review-app one-off job memory from 1Gi to 2Gi. CPU remains 300m, and the 15-minute timeout remains unchanged. ChangesControl plane workload updates
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to No concrete merge-blocking risk remains; the remaining deployment check is normal end-to-end validation. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
|
+review-app-deploy |
|
No description provided. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| # retain a 15-minute server-side active deadline. | ||
| runner_job_default_cpu: "300m" | ||
| runner_job_default_memory: "1Gi" | ||
| runner_job_default_memory: "2Gi" |
There was a problem hiding this comment.
Good empirically-justified fix for the one-off runner job. One thing worth double-checking: the persistent rails workload template (.controlplane/templates/rails.yml:26) still requests only 1Gi as its baseline memory for booting the same Rails image (with capacityAI: true to grow it afterward). If a 1Gi cold start was hitting a startup-probe/connection failure for the one-off runner here, the persistent workload's very first boot on a fresh review app could plausibly hit the same problem before capacityAI has a chance to scale it up — worth confirming that path is actually distinct (no startup probe on rails.yml, or otherwise already covered) rather than assuming it's fine because it hasn't failed yet.
There was a problem hiding this comment.
🤖 Codex (configured gpt-5.6-sol, medium): Confirmed in the effective head a0be59f review app: the persistent Rails workload retained 300m/1Gi, produced a replica, became ready and readyLatest, and served / and /server-components with HTTP 200. Its separate startup path worked; no Rails memory change was needed.
Review summaryThis is a minimal, well-scoped config change: bumps Code quality: Clean, scoped to the Correctness/verification: The PR description cites concrete repro data (isolated Control Plane job with the same image failed at 300m/1Gi with a startup-probe connection failure, succeeded at 300m/2Gi), which is solid evidence for the fix. Left an inline note on a related consistency question: Security: No security implications — this is a resource-request tuning change only. Performance/cost: Doubles the memory reservation for review-app one-off jobs (migrations via No blocking issues found. |
 |
❌ Review App Deployment FailedDeployment failed for PR #827, commit 93afba3 🎮 Control Plane Console |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
+review-app-delete |
|
No description provided. |
✅ Review App DeletedReview app for PR #827 is deleted |
|
+review-app-deploy |
|
No description provided. |
❌ Review App Deployment FailedDeployment failed for PR #827, commit 93afba3 🎮 Control Plane Console |
|
No description provided. |
|
+review-app-delete |
|
No description provided. |
✅ Review App DeletedReview app for PR #827 is deleted |
|
+review-app-deploy |
|
No description provided. |
justin808
changed the title
| set -e | ||
| bundle exec rake react_on_rails_pro:pre_seed_renderer_cache | ||
| exec yarn node-renderer | ||
| exec /usr/local/node/bin/node renderer/node-renderer.js |
There was a problem hiding this comment.
This works around the symptom rather than the cause. The container is launched via bash -lc (line 24), which makes bash act as a login shell and source /etc/profile. Debian's default /etc/profile unconditionally overwrites PATH (e.g. PATH="/usr/local/bin:/usr/bin:/bin:..." for non-root), which wipes out /usr/local/node/bin — the directory the Dockerfile adds via ENV PATH=/usr/local/node/bin:$PATH (.controlplane/Dockerfile:17). That's almost certainly why plain yarn node-renderer (and even bare node) couldn't be found here, while bundle still resolves fine because it lives in /usr/local/bin, which Debian's profile keeps.
Hardcoding the absolute path fixes this one exec, but:
- It's now duplicated across 4 files (this template,
readme.md,shakacode-team.md,bin/test-cpflow-github-flow) that all need to stay in sync if the Node install location in the Dockerfile ever changes. - Any future command added to this same script that relies on
/usr/local/node/bin(anotheryarn/npxcall, etc.) will silently hit "command not found" again.
A more robust fix would be to drop the -l (use bash -c here, matching the Dockerfile's own CMD ["bash", "-c", ...] at line 89, which does not use a login shell) so the image's PATH survives, letting you keep exec yarn node-renderer (or plain node ...) instead of a hardcoded path.
There was a problem hiding this comment.
🤖 Codex (configured gpt-5.6-sol, medium): Agreed. I reproduced the PATH reset with bash -lc in the same Ruby slim base image and confirmed bash -c retains /usr/local/node/bin. Head a0be59f now uses bash -c and exec node renderer/node-renderer.js; the exact-head review app reached Node Renderer readiness and passed deployed HTTP/RSC checks. The offline template contract was updated and passed.
ReviewSmall, well-scoped config PR combining two fixes for the review-app deployment pipeline. Overall looks reasonable — flagging one item and a couple of minor notes.
|
❌ Review App Deployment FailedDeployment failed for PR #827, commit 03448df 🎮 Control Plane Console |
|
+review-app-delete |
|
No description provided. |
✅ Review App DeletedReview app for PR #827 is deleted |
|
No description provided. |
|
+review-app-deploy |
|
No description provided. |
🚀 Deploying to Control Plane...
|
Review SummaryThis is a narrowly-scoped, well-tested infra/config fix. Both root causes check out against the actual image:
No security concerns — no secrets were added to the repo, consistent with the PR description. Minor, non-blocking note: doubling the review-app runner memory default is a real resource/cost increase for every review-app deploy/delete job, not just the currently-stuck one. Worth a quick sanity check that this fits within the staging org's provider quota, but not a reason to block given the reproduced failure without it. Nice, well-verified fix overall. |
justin808
left a comment
justin808
left a comment
There was a problem hiding this comment.
🤖 Codex walkthrough for head a0be59f952e41a394271dfb7b587d7ed0df59d79
The review app built the RC5 image but its 300m/1Gi release runner never exposed a replica. The QA runner default now uses 2Gi memory with the same CPU and 900-second deadline. An isolated provider job using the exact RC5 image completed at 300m/2Gi and failed at 300m/1Gi; the real #827 release execution also completed with the new memory setting.
The next workload previously exited after cache seeding with exec: yarn: not found. In the Ruby slim base image, bash -lc resets the Dockerfile's Node PATH; bash -c keeps it. The renderer template now uses the non-login shell and the same direct Node entrypoint as the Dockerfile fallback. The offline contract check pins that launch command and keeps the existing port, probes, deploy order, and resource ratio. The Control Plane docs and team note describe the corrected command.
Local validation passed: 59 Ruby files with no RuboCop offenses, the released review-app contract and renderer template check, workflow YAML/actionlint, a Ruby slim PATH reproduction, and git diff --check. The shared QA environment's missing secret reference was repaired in its provider dictionary without committing secret values. The exact-head provider deployment completed its release runner, renderer and Rails readiness, health, and URL retrieval; /, /server-components, and a nonempty RSC NDJSON payload returned HTTP 200. GitHub recorded a success deployment status but returned an HTTP 500 malformed response during the final github-script status step, leaving that workflow conclusion red; the PR description links both records and does not call the workflow green. The deployment configuration can be rolled back by reverting this PR if a default-branch rollout later regresses.
|
+review-app-delete |
|
No description provided. |
✅ Review App DeletedReview app for PR #827 is deleted |
1 participant
🤖 Codex
Commit-pinned reviewer walkthrough.
The RC5 tutorial review app in #826 built its image but stalled before a release-runner replica appeared, blocking deployed validation (#784). A provider probe with that exact image succeeded at 300m CPU and 2Gi memory, while the prior 300m/1Gi setting failed to start. This PR raises only the review-app one-off runner memory default to 2Gi; CPU and the 900-second deadline stay unchanged.
Once the release job ran, the next workload failed at
exec: yarn: not found. The Dockerfile puts Node onPATH, but the renderer workload used a loginbash -lcshell that resetPATHin the Ruby slim base image. The workload now uses non-loginbash -cand startsnode renderer/node-renderer.jsdirectly after the existing cache-seeding rake task. I reproduced the PATH difference in that base image. Its deploy order, probes, ports, and resource settings remain unchanged. The same repairs are also carried by #826, whose RC5 dependency commit is unchanged.The shared QA review environment had a missing secret reference, which was repaired in the provider dictionary without adding secret values to this repository. This was an external prerequisite for inherited-environment workloads to start.
Local verification passed:
.agents/bin/validate(59 Ruby files, no offenses),bin/conductor-exec bin/test-cpflow-github-flow --offline(released review-app contract, renderer template, workflow YAML, actionlint), a YAML check of the non-login Node launch command, andgit diff --check. The effective exact-head review-app run built and deployed imagea0be59f, completed its 300m/2Gi release runner, made the Node Renderer and Rails ready, passed workload health, and retrieved the URL. Direct checks of the deployed app returned HTTP 200 for/and/server-components;/rsc_payload/LiveActivityreturned HTTP 200,application/x-ndjson, and a nonempty three-line stream. The run concluded red only in Finalize deployment status: GitHub created asuccessdeployment status, then its API response returned HTTP 500 with malformed JSON togithub-script. The provider app and recorded GitHub deployment status were successful; the workflow conclusion is not being presented as green.Native usage
a0be59f / implementation
SHARED source interval: 2026-09-15T08:07:13.022Z through 2026-09-15T09:30:42.748Z. Snapshot through the last observed response.
Source selection: host context.
Codex source versions: 0.154.0-alpha.6.2.