ci: add GitHub Actions workflow for Rust and Flutter checks#173
ci: add GitHub Actions workflow for Rust and Flutter checks#173AndreaDiazCorreia wants to merge 4 commits into
Conversation
- Add a CI workflow that runs on pull requests and pushes to main, with concurrency control to cancel superseded runs. - Rust job: build, test, clippy (deny warnings), and check wasm32 target. - Flutter job: install Rust toolchain (required for flutter_rust_bridge_codegen), generate Dart bindings, analyze, and test. - Cache cargo registry/target, Flutter SDK, and codegen tools (flutter_rust_bridge_codegen + cargo-expand) to speed up runs. - Pin Flutter 3.38.2 and flutter_rust_bridge_codegen 2.11.1.
|
Warning Review limit reached
Next review available in: 18 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. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughAdds a GitHub Actions CI workflow triggered by pull requests and pushes to ChangesCI validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Poem
🚥 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 |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/ci.yml (1)
1-101: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winAdd a top-level
permissionsblock with least-privilege.The workflow has no
permissionsblock, so theGITHUB_TOKENgets the repo's default permissions — often overly broad (e.g.,contents: write). This job only needs to read source and report status checks, socontents: readis sufficient. This is flagged by zizmor'sexcessive-permissionscheck.🔒️ Suggested change
on: pull_request: push: branches: [main] +permissions: + contents: read + # Cancel superseded runs on the same ref to save CI minutes. concurrency:🤖 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 1 - 101, Add a top-level permissions block near the workflow metadata, setting contents to read so all jobs use least-privilege GITHUB_TOKEN access. Do not grant write permissions or add unnecessary scopes.Source: Linters/SAST tools
🧹 Nitpick comments (2)
.github/workflows/ci.yml (2)
25-25: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winSet
persist-credentials: falseon bothactions/checkoutsteps.By default,
actions/checkout@v4persists the GitHub token in the local git config. Neither job pushes commits, so there's no need to keep credentials. Disabling this reduces the risk of token leakage in artifacts or logs (zizmorartipacked).🔒️ Suggested change (apply to both checkout steps)
- - uses: actions/checkout@v4 + - uses: actions/checkout@v4 + with: + persist-credentials: falseAlso applies to: 54-54
🤖 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 25, Both actions/checkout@v4 steps currently persist GitHub credentials unnecessarily. Add persist-credentials: false to each checkout action configuration, including the steps in both jobs.Source: Linters/SAST tools
82-88: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPin
cargo-expandand include it in the cache key.
flutter_rust_bridge_codegenis pinned with--versionand--locked, butcargo-expandis installed unpinned. On a cache miss, a differentcargo-expandversion may be installed than on a previous run, making codegen non-reproducible. Additionally, the cache key only includesFRB_VERSION, so a newcargo-expandrelease won't invalidate the cache — the stale binary is kept (acceptable), but the inconsistency between pinned and unpinned tools is a reproducibility gap.♻️ Suggested changes
env: FLUTTER_VERSION: "3.38.2" FRB_VERSION: "2.11.1" + CARGO_EXPAND_VERSION: "1.0.0" # pin to a specific versionkey: frb-tools-${{ runner.os }}-frb${{ env.FRB_VERSION }}-expand${{ env.CARGO_EXPAND_VERSION }}- name: Install codegen tools if: steps.tools-cache.outputs.cache-hit != 'true' run: | cargo install flutter_rust_bridge_codegen --version ${{ env.FRB_VERSION }} --locked - cargo install cargo-expand + cargo install cargo-expand --version ${{ env.CARGO_EXPAND_VERSION }} --locked🤖 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 82 - 88, Pin the cargo-expand installation to an explicit version using --version and --locked, and define that version alongside FRB_VERSION in the workflow environment. Update the tools cache key to include the cargo-expand version so cache entries invalidate when either codegen tool changes; adjust the Install codegen tools step accordingly.
🤖 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.
Outside diff comments:
In @.github/workflows/ci.yml:
- Around line 1-101: Add a top-level permissions block near the workflow
metadata, setting contents to read so all jobs use least-privilege GITHUB_TOKEN
access. Do not grant write permissions or add unnecessary scopes.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Line 25: Both actions/checkout@v4 steps currently persist GitHub credentials
unnecessarily. Add persist-credentials: false to each checkout action
configuration, including the steps in both jobs.
- Around line 82-88: Pin the cargo-expand installation to an explicit version
using --version and --locked, and define that version alongside FRB_VERSION in
the workflow environment. Update the tools cache key to include the cargo-expand
version so cache entries invalidate when either codegen tool changes; adjust the
Install codegen tools step accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c5ea95f7-9691-4ace-a670-2df052898540
📒 Files selected for processing (1)
.github/workflows/ci.yml
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bf9a2efa63
ℹ️ 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".
- Pin dtolnay/rust-toolchain to 1.97.0 in both rust and flutter jobs so a newer stable clippy can't introduce lints that break `-D warnings` on unrelated PRs. - Replace `sort_by` with `sort_by_key` in list_trades for cleaner code.
- Drop wasm32-unknown-unknown target installation and check step: the project doesn't currently build for wasm32, so the check adds no value and only slows down CI.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 781513fa10
ℹ️ 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".
| - name: Clippy (deny warnings) | ||
| run: cargo clippy --locked -- -D warnings |
There was a problem hiding this comment.
Add the wasm check to the Rust CI job
The Rust job stops after native build/test/clippy, but this repo has a supported web/WASM target and the existing pre-commit hook still runs cargo check --target wasm32-unknown-unknown. With this workflow, a PR can break the cfg(target_arch = "wasm32") code or WASM-only dependencies while all Rust CI still passes; add the wasm target install/check after clippy so CI covers the same web build path.
Useful? React with 👍 / 👎.
- Set workflow permissions to contents:read (least-privilege: jobs only read source and report status). - Add persist-credentials:false to checkout steps to prevent credential leakage in subsequent steps. - Pin cargo-expand to 1.0.123 and include version in cache key so updates don't silently break the build.
Adds
.github/workflows/ci.yml, running on every PR and on push tomain.cargo build,cargo test,cargo clippy -- -D warnings,cargo check --target wasm32-unknown-unknownflutter_rust_bridge_codegen generate),flutter analyze,flutter testCaches cargo, the Flutter SDK, and the codegen tools (incl. cargo-expand, required by codegen).
Closes #151.
Note: the
flutter analyzestep stays red until the pending Dart lint cleanup lands onmain(3 pre-existing infos); the Rust job is already green after the clippy cleanup.Summary by CodeRabbit