ci: harden Rust CI/CD - #1051
Conversation
|
Warning Review limit reached
Next review available in: 26 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. 📝 WalkthroughWalkthroughThis PR splits CI linting into separate ChangesCI, toolchain, and formatting sweep
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
34-57: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueConsider making
testdepend onfmtas well.The
testjob only depends onclippy(needs: clippy), notfmt. Iffmtfails butclippyandtestpass, the CI may appear green depending on branch protection configuration. Addingfmtto theneedslist ensures formatting is enforced before tests run.♻️ Proposed fix
test: name: Test (${{ matrix.os }}) runs-on: ${{ matrix.os }} - needs: clippy + needs: [clippy, fmt]🤖 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/workflows/ci.yml around lines 34 - 57, The test workflow currently only waits on clippy, so formatting failures from fmt can be missed. Update the test job in the CI workflow to also depend on fmt by adjusting its needs list alongside clippy, so formatting is enforced before the test steps run.
🤖 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/workflows/ci.yml:
- Around line 12-21: Harden the CI workflow by adding a top-level permissions
block with contents: read so the read-only jobs do not inherit broader token
access, and update every actions/checkout@v4 step in the workflow to set
persist-credentials: false. Apply this consistently across the fmt, clippy, and
test jobs so the checkout in each job does not leave GITHUB_TOKEN configured in
git for the rest of the run.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 34-57: The test workflow currently only waits on clippy, so
formatting failures from fmt can be missed. Update the test job in the CI
workflow to also depend on fmt by adjusting its needs list alongside clippy, so
formatting is enforced before the test steps run.
🪄 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
Run ID: f4bbf49f-7d6e-4ebf-81ee-f301de6801a9
📒 Files selected for processing (2)
.github/workflows/ci.ymlrust-toolchain.toml
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
21-21: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTrack
continue-on-error: trueon fmt and clippy as a temporary measure.Both
fmtandclippyjobs havecontinue-on-error: true, which means CI will never fail on formatting or lint issues — including newly introduced ones. The PR objectives confirm this is intentional to avoid failing on existing issues, but without a follow-up tracking issue, these quality gates risk remaining permanently advisory.Consider adding a TODO or tracking issue to fix existing violations and remove
continue-on-erroronce the codebase is clean.Also applies to: 34-34
🤖 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/workflows/ci.yml at line 21, The fmt and clippy jobs are currently marked continue-on-error, so add a clear TODO or tracking reference in the CI workflow near the affected job definitions to record that this is temporary. Use the workflow job names for fmt and clippy as the anchors, and note that the flag should be removed once existing violations are resolved so new formatting or lint regressions can fail CI again.
🤖 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.
Nitpick comments:
In @.github/workflows/ci.yml:
- Line 21: The fmt and clippy jobs are currently marked continue-on-error, so
add a clear TODO or tracking reference in the CI workflow near the affected job
definitions to record that this is temporary. Use the workflow job names for fmt
and clippy as the anchors, and note that the flag should be removed once
existing violations are resolved so new formatting or lint regressions can fail
CI again.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5daaadfb-ad6f-4235-b854-f4fc340fb010
📒 Files selected for processing (2)
.github/workflows/ci.ymlrust-toolchain.toml
✅ Files skipped from review due to trivial changes (1)
- rust-toolchain.toml
- Add rust-toolchain.toml pinning to 1.91 - Standardize CI workflow (fmt + clippy -D warnings + test, Swatinem/rust-cache) - Align cargo-dist 0.31.0 config (where applicable) - Add tag-based crate publish workflow (where applicable) - Adopt dolos docker tag convention for services/backends (where applicable) - Add cliff.toml + cargo-release pre-release-hook
The rust-toolchain.toml file takes precedence over dtolnay/rust-toolchain's components parameter, so rustfmt and clippy were not being installed in CI.
- RUSTFLAGS: -D warnings was causing test failures (turns compile warnings into errors during cargo test); clippy already passes -D warnings directly - fmt and clippy are now continue-on-error: true to avoid blocking on pre-existing code quality issues; maintainers can remove this once clean
- Add permissions: contents: read (least-privilege GITHUB_TOKEN)
- Add persist-credentials: false to all checkout steps
- Add concurrency group to cancel superseded CI runs
- Fix pre-release-hook: use --tag v{{version}} instead of --latest
(--latest resolves the previous tag since the new tag doesn't exist
yet during cargo-release's pre-release-hook)
The rustfmt.toml uses wrap_comments=true (nightly-only). Use nightly rustfmt in CI and re-format all code accordingly.
98ce94a to
3d38ecb
Compare
… fmt - Remove continue-on-error from fmt and clippy so failures block merge (the whole point of -D warnings). - fmt now runs on nightly rustfmt, since rustfmt.toml's wrap_comments is an unstable option that stable silently ignores; stable would never enforce the comment wrapping this branch applied. - Run fmt/clippy/test as independent parallel jobs (drop test's needs: clippy) for faster feedback; all three gate via branch protection. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
Modernizes the Rust CI workflow and pins the toolchain, plus a one-time
cargo fmtsweep to bring the tree into compliance with the pinned formatter.Changes
.github/workflows/ci.yml— split the old monolithiccheckjob into three jobs (fmt,clippy,test) that run in parallel; migrate off the archivedactions-rs/*actions todtolnay/rust-toolchain; addpermissions: contents: read,concurrencywithcancel-in-progress,persist-credentials: false,Swatinem/rust-cache@v2, and matrixfail-fast: false.fmtruns on nightly rustfmt, becauserustfmt.tomlenableswrap_comments— an unstable option that stable rustfmt silently ignores. Running nightly makes the comment-wrapping check actually enforced. Local devs should format withcargo +nightly fmt.clippyruns--all-targets --all-features -- -D warningson the pinned toolchain.fmtandclippyare blocking (nocontinue-on-error); all three jobs should be marked required in branch protection.rust-toolchain.toml(new) — pinschannel = "1.91"withrustfmt+clippycomponents so local and CI toolchains agree.cargo fmtsweep — one-time reformat bringing the codebase into compliance with the pinned formatter (maincurrently failscargo fmt --check). Formatting only; no logic changes.Diff stat
Only
.github/workflows/ci.ymlandrust-toolchain.tomlare functional; the remaining 86 files are the formatting sweep.Not in scope
This PR does not touch cargo-dist (
dist-workspace.toml), Docker (docker.yml), crate publishing, or changelog (cliff.toml) — those remain as they are onmain.