ci(rust): parallelize recorder contract and protect main cache production - #569
Conversation
|
Warning Review limit reached
Next review available in: 39 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. 📝 WalkthroughWalkthroughThe workflow preserves ChangesRust CI topology
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant CIWorkflow
participant RustJob
participant MarketRecorderContract
participant CIGate
CIWorkflow->>RustJob: Run Rust checks without market recorder release
CIWorkflow->>MarketRecorderContract: Run market recorder release contract
RustJob-->>CIGate: Report Rust job result
MarketRecorderContract-->>CIGate: Report contract job result
CIGate-->>CIWorkflow: Evaluate required job results
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7d06157281
ℹ️ 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".
7d06157 to
4691f9b
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/scripts/test-select-rust-ci-scope.sh (1)
220-225: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAssert the control gate in the scope contract.
The current checks verify the job name, release script, and Rust-job exclusion. They pass if
market_recorder_contractruns on every scope. Add assertions forneeds: scopeandif: needs.scope.outputs.control == 'true'to enforce the control-lane requirement in.github/workflows/ci.ymlLine 444.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/scripts/test-select-rust-ci-scope.sh around lines 220 - 225, Extend the market_recorder_contract assertions around recorder_block to require the job definition’s needs: scope dependency and the conditional gate if: needs.scope.outputs.control == 'true'. Keep the existing job-name, release-script, and Rust-job exclusion checks unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/scripts/test-select-rust-ci-scope.sh:
- Line 178: Update the dependency assertion in test-select-rust-ci-scope.sh to
run after job_block() extracts the ci-gate block, and grep that extracted block
rather than the full workflow. Preserve the existing market_recorder_contract
dependency check while ensuring it specifically validates the ci-gate job.
In @.github/workflows/ci.yml:
- Around line 448-449: Update the Checkout step in the release/Docker build
workflow to set persist-credentials to false, preventing the GitHub token from
remaining in .git/config before privileged Docker builds; preserve fetch-depth:
0 only in checkout steps that require full history.
---
Nitpick comments:
In @.github/scripts/test-select-rust-ci-scope.sh:
- Around line 220-225: Extend the market_recorder_contract assertions around
recorder_block to require the job definition’s needs: scope dependency and the
conditional gate if: needs.scope.outputs.control == 'true'. Keep the existing
job-name, release-script, and Rust-job exclusion checks unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 02a26e16-c15d-4c45-b770-714f616898ad
📒 Files selected for processing (2)
.github/scripts/test-select-rust-ci-scope.sh.github/workflows/ci.yml
…ng sccache binary - market_recorder_contract checkout now uses persist-credentials: false so the GITHUB_TOKEN cannot leak into the docker build context (zizmor artipacked finding on #569). - The gate-membership pin is scoped to the ci-gate needs block. - Record cache dimensions tolerates a missing sccache binary (continue-on-error fallback path). Refs #568
…tion - The market-recorder release contract (a ~5.5min docker build that cannot use host sccache) moves out of the serial Rust Workspace job into its own parallel job, keeping ci-gate membership (#568). - concurrency.cancel-in-progress is now false on main: PR runs still supersede, but main runs finish so rust-cache/sccache keep producing for all branches. Refs #568
…ng sccache binary - market_recorder_contract checkout now uses persist-credentials: false so the GITHUB_TOKEN cannot leak into the docker build context (zizmor artipacked finding on #569). - The gate-membership pin is scoped to the ci-gate needs block. - Record cache dimensions tolerates a missing sccache binary (continue-on-error fallback path). Refs #568
The default checkout died on the repo's gitlink-without-.gitmodules ccpm entry during submodule cleanup; every other job in this workflow uses fetch-depth: 0 and passes. Refs #568
857f825 to
a8a3c8c
Compare
Second gitlink without .gitmodules registration, surfaced after the ccpm removal unblocked the first. Also unreferenced. Refs #568
2 participants
Change contract
Two finishing items for the CI layering (per #558's final review):
market_recorder_contract, gated oncontrol == 'true'and a member of ci-gate. Rust Workspace critical path drops by ~5.5min on control-lane runs.concurrency.cancel-in-progressis nowfalseon main (stilltrueon PR branches) so main runs always complete and keep feeding rust-cache/sccache to everyone.Out of scope
Dependency / merge order
None; stacks on #566 (merged). Scope-contract pins updated and PASS.
Focused validation
.github/scripts/test-select-rust-ci-scope.shPASS with new pins (recorder job block, gate membership, absence from rust/fast-gates)Rollout / rollback impact
CI-only. Rollback = revert.
Issue relationship
Closes #568
Summary by CodeRabbit
Bug Fixes
Chores