Fix review app release runner resource defaults - #802
Conversation
🚀 Quick Review App CommandsWelcome! Here are the commands you can use in this PR:
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review. WalkthroughThe repository updates Control Plane review-app workflows to version 5.3.0, pins reusable workflows and third-party actions, restricts workflow credentials, adds composite-action input safeguards, and introduces offline release-contract validation with supporting tests and documentation. ChangesControl Plane workflows and validation
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: 🟡 Moderate · up to Review-app runners receive new resource defaults and a longer deadline, but the only deployment attempt did not expose a replica, so endpoint and behavioral validation could not complete. This leaves the release path insufficiently verified for merge. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 1 files. (2 skipped: 2 unsupported.)
✨ 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 |
ReviewOverviewSingle-file, additive config change to Code quality / correctness
Risk / potential issues
SecurityNo credentials, identities, images, permissions, or endpoints are touched — consistent with the PR's stated config-security assertions. No concerns here. Suggestions
|
Greptile SummaryUpdates review-app one-off runner defaults.
Confidence Score: 5/5The PR appears safe to merge, with no concrete blocking or non-blocking defects identified in the changed configuration. The runner overrides are confined to the prefix-matched QA configuration, align with the Rails workload resources, and do not alter production or staging settings. Important Files Changed
Reviews (1): Last reviewed commit: "Fix release runner scheduling resource r..." | Re-trigger Greptile |
Address-review summaryScan scope: full PR history after the complete current-head review wave. Mattered
Skipped
Next default scan starts after this comment. Say |
|
+review-app-deploy |
❌ Review App Deployment FailedDeployment failed for PR #802, commit ec08723 🎮 Control Plane Console |
|
+review-app-delete |
✅ Review App DeletedReview app for PR #802 is deleted |
ec08723 to
1fc3f39
Compare
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 summarySmall, well-scoped infra change (50/-11 across 4 files) that (1) gives review-app one-off jobs the same CPU/memory as the Rails workload and a 15-minute active deadline, (2) points the review-app deploy reusable workflow at an unreleased Checked and looks correct:
Two things worth a second look (left as inline comments):
No security issues found (no credentials, permissions, or identity fields touched; SHA-pinning is actually a security improvement over the previous floating |
❌ Review App Deployment FailedDeployment failed for PR #802, commit 1fc3f39 🎮 Control Plane Console |
✅ Review App DeletedReview app for PR #802 is deleted |
|
Hosted replay result for head
The conditional fresh-default verification deployment is not eligible because this repair deployment did not succeed. No protected app URL, platform identifier/value, log, token, or secret name is included here. |
Address-review summaryScan scope: since the previous summary at 2026-07-24T04:08:12Z. Status: cutoff-safe review summary. Detailed review outcomes are collapsed below. The PR itself remains not merge-ready because the sole authorized hosted deployment failed. Detailed review outcomesMattered
Skipped
Deferred-work tracking: existing issue #784. Next default scan starts after this comment. Say |
|
No description provided. |
Reviewed security repair — deployment still blockedCandidate: The maintainer-approved security expansion pins existing action sources, closes shell-expression injection paths, narrows test permissions and checkout credential persistence, and adds regression coverage. The review-app deploy/delete pair remains on released cpflow 5.3.0; older workflow callers remain on the same immutable 5.2.0 cohort. No unrelated product changes are included. Independent maker-distinct native Astra/high review found and verified a packaging regression in the proposed remote-action conversion. This candidate fixes it with matching source-checkout Git metadata scoped only to the setup step. An independent local archive/package reproduction verified the actual gem contains its library and caller template (222 packaged files); regression checks enforce the exact source identity and step-only binding. Exact-candidate local verification:
Publication is not a deployment authorization. A fresh read-only observation confirmed the exact review app absent, no accepted intent markers, and no active/pending Actions runs before publication. The reviewed synchronize path skips app creation/build/deploy when the app is absent. This is a snapshot, not an atomic lock against another actor changing state. Status: NOT COMPLETE / external-gate-failing. The last authorized repair deployment failed. No automatic retries, merge, or deployed-behavior claim are authorized by this update. Current-head hosted checks/reviews still require fresh triage. A skipped deploy workflow is not deployment evidence. Required closeout remains: successful explicitly authorized repair replay; gates-clean merge; the already-conditionally-authorized fresh-default verification PR and deployment; image/deploy/endpoint/behavior smoke and default reachability; cleanup and close the temporary PR unmerged. Retain only public-safe pass/fail evidence. Issue #784 remains the coordinator-owned tracker. Changed default-only workflows also need their appropriate post-merge exercise evidence; staging/production operations require separate authority and are not exercised here. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 235563ec8c
ℹ️ 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".
|
No description provided. |
Review updatePublished Verification and dispositionMattered
Skipped
The repair deployment and deployed behavior remain unverified. No automatic retry is authorized; the conditional fresh-default verification remains reserved until successful repair deployment and gates-clean merge. Issue #784 remains open. Non-cutoff status only. The next review pass must use |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6377676c74
ℹ️ 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".
| template_root = File.join(Gem::Specification.find_by_name("cpflow", VERSION).full_gem_path, | ||
| "lib/github_flow_templates") |
There was a problem hiding this comment.
Activate Bundler before locating cpflow
When dependencies are installed under a configured bundle path such as vendor/bundle, the documented direct command and the helper's plain ruby bin/check-cpflow-review-app-contract invocation do not activate Bundler, so Gem::Specification.find_by_name cannot see the installed cpflow gem and validation exits with Gem::LoadError. The RSpec case does not catch this because its child Ruby inherits the environment established by bundle exec rspec; activate bundler/setup in this executable or consistently invoke it via bundle exec ruby.
Useful? React with 👍 / 👎.
| Do not run blanket `cpflow update-github-actions` regeneration or | ||
| `bin/pin-cpflow-github-ref` for this split. They can overwrite the separate |
There was a problem hiding this comment.
Align the remaining split-migration instructions
Although this guide now warns against blanket regeneration, a repo-wide search still finds .github/testing-github-actions.md:27-35 directing prerelease testers to pin all generated reusable workflows to one SHA via bin/pin-cpflow-github-ref, while .controlplane/shakacode-team.md:131-145 still says every wrapper is on v5.2.0 and should be regenerated together. Following either guide overwrites the intentional v5.3.0 review-app/v5.2.0 non-review split and produces a state the new validators reject, so these remaining instructions need the same exception or a link to this section.
Useful? React with 👍 / 👎.
Why
Review-app builds could finish successfully but then stall while the release runner never exposed a replica. The previous resource-only repair did not solve that observation failure and left the workflow without a bounded, useful outcome.
What changed
How to review
.controlplane/controlplane.ymlchanges only the review-app runner defaults and deadline.bin/test-cpflow-github-flowfor the narrow canary exception and its rejection paths.Validation
git diff --check: passed.Authorized hosted replay
1fc3f390bea00655b0846d83b2831a38b3589e34.Current disposition: external-gate-failing; do not merge. No retry or waiver is inferred.
Tracking and post-merge exercise
#784 remains the required exercise tracker for this semantic workflow change. If a later authorized repair succeeds and this PR becomes mergeable, that issue owns one fresh-default verification PR, deployed behavior and endpoint evidence, cleanup, and closing the verification PR unmerged. Owner: batch coordinator.
Review decisions:
Agent details
QA Evidence
ror17-fleet-c-tutorial-checker-4, observedcodex-collaboration/gpt-5.6-sol/xhigh.1fc3f390bea00655b0846d83b2831a38b3589e34.Coordination
ror17-fleet-c-20260717.ror17-fleet-c-tutorial-repair-3, exact requested routecodex-collaboration/gpt-5.6-sol/high.ror17-fleet-c-tutorial-checker-4, exact requested routecodex-collaboration/gpt-5.6-sol/xhigh.b8676066d3344098bef8e35ee4abce1d0ca9d8a3.Summary by CodeRabbit
New Features
Improvements