Conversation
Reviewer's GuideThis PR formally pins the Minimum Supported Rust Version (MSRV) to 1.89.0 by extending CI and test workflows with a dual-toolchain matrix, adding MSRV verification in the release pipeline, and bumping crate metadata and toolchain configs to 1.89.0. Class diagram for updated crate metadata with MSRVclassDiagram
class Netsuke {
+name: "netsuke"
+version: "0.1.0"
+edition: "2024"
+rust-version: "1.89.0"
}
class NinjaEnv {
+name: "ninja_env"
+version: "0.1.0"
+edition: "2024"
+rust-version: "1.89.0"
}
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Rate limit exceeded@leynos has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 2 minutes and 32 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
Summary by CodeRabbit
WalkthroughSummarise: Enforce MSRV 1.89.0 across CI and manifests; add CI matrix (stable, 1.89.0), gate coverage to stable, add MSRV verification steps in release workflow, pin many actions to commits, and update rust-toolchain and Cargo.toml rust-version fields to 1.89.0. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Push/PR
participant CI as CI Workflows
participant Matrix as Build/Test Matrix
participant Release as Release Workflow
participant CodeScene as CodeScene
Dev->>CI: Trigger workflows
CI->>Matrix: Start jobs (matrix: stable, 1.89.0)
Note over Matrix: Setup Rust with matrix.rust\nInstall components: rustfmt, clippy
Matrix->>Matrix: Build & lint (both toolchains)
alt matrix.rust == stable
Matrix->>Matrix: Run tests & measure coverage
Matrix->>CodeScene: Upload coverage (if secret present)
else matrix.rust == 1.89.0
Matrix-->>CodeScene: Skip coverage/upload
end
Dev->>Release: Tag/PR triggers release
Release->>Release: Setup stable and install 1.89.0
Release->>Release: Add targets (+stable and +1.89.0)
Release->>Release: cross build (stable) and cross +1.89.0 build (MSRV verify)
Release->>Release: Package & publish artifacts
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Poem
✨ Finishing Touches🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
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)
42-49: Fix CodeScene upload step to use secrets in the if conditionEnforce the use of the secrets context for the guard and pass the token input directly from
secrets.
- File:
.github/workflows/ci.yml- Step: Upload coverage data to CodeScene
- Remove the step-level
envblock forCS_ACCESS_TOKEN(it isn’t available whenifis evaluated).- Change the
ifto checksecrets.CS_ACCESS_TOKEN != ''.- Pass the token via
secrets.CS_ACCESS_TOKENin the action input.- name: Upload coverage data to CodeScene - env: - CS_ACCESS_TOKEN: ${{ secrets.CS_ACCESS_TOKEN }} - if: matrix.rust == 'stable' && env.CS_ACCESS_TOKEN + name: Upload coverage data to CodeScene + if: matrix.rust == 'stable' && secrets.CS_ACCESS_TOKEN != '' uses: leynos/shared-actions/.github/actions/upload-codescene-coverage@c6559452842af6a83b83429129dccaf910e34562 with: format: lcov - access-token: ${{ env.CS_ACCESS_TOKEN }} + access-token: ${{ secrets.CS_ACCESS_TOKEN }} installer-checksum: ${{ vars.CODESCENE_CLI_SHA256 }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
.github/workflows/ci.yml(2 hunks).github/workflows/netsukefile-test.yml(1 hunks).github/workflows/release.yml(2 hunks)Cargo.toml(1 hunks)ninja_env/Cargo.toml(1 hunks)rust-toolchain.toml(1 hunks)test_support/Cargo.toml(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
Cargo.toml
📄 CodeRabbit inference engine (AGENTS.md)
Cargo.toml: Use explicit version ranges and keep dependencies up to date
Mandate SemVer caret requirements for all dependencies (e.g., "1.2.3")
Prohibit wildcard (*) and open-ended (>=) version specifiers; use ~ only with a specific, documented reason
Files:
Cargo.toml
🔍 Remote MCP
Additional context for reviewing the MSRV bump to Rust 1.89.0:
-
The
rust-versionfield in eachCargo.tomlspecifies the minimum supported Rust toolchain and is enforced by Cargo (since v1.56) — Cargo will refuse to build with a compiler older than the declared version. (doc.rust-lang.org) -
Rust 1.89.0 became the stable release on August 7, 2025, ensuring that the MSRV pinned in CI and in
rust-toolchain.tomlcorresponds to an officially published toolchain. (blog.rust-lang.org)
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-test (stable)
🔇 Additional comments (5)
rust-toolchain.toml (1)
2-2: Pin the toolchain to 1.89.0 — LGTM.Keep the components list aligned with CI to avoid drift.
Run the verification script to ensure MSRV is consistently declared and referenced across the repo.
#!/bin/bash set -euo pipefail echo "1) Verify rust-toolchain.toml channel:" rg -n "^channel\s*=\s*\"1\.89\.0\"" rust-toolchain.toml || { echo "Mismatch in rust-toolchain.toml"; exit 1; } echo "2) Verify all Cargo.toml files declare rust-version = \"1.89.0\":" rg -n --glob 'Cargo.toml' '^\s*rust-version\s*=\s*"([^"]+)"' -H | sed 's/^/FOUND: /' if rg -n --glob 'Cargo.toml' -P '^\s*rust-version\s*=\s*"(?!1\.89\.0)"' ; then echo "ERROR: One or more crates do not declare rust-version = \"1.89.0\"" exit 1 fi echo "3) Verify no workflows still reference nightly or an older MSRV:" rg -n ".github/workflows" -e "nightly" -e "1\.85\.0" -e "1\.86\.0" -e "1\.87\.0" -e "1\.88\.0" || true echo "4) Verify CI and other workflows reference 1.89.0:" rg -n ".github/workflows" -e "1\.89\.0"test_support/Cargo.toml (1)
5-5: Align crate MSRV to 1.89.0 — LGTM.Match this with CI to ensure tests run on the declared MSRV.
Cargo.toml (1)
5-5: Declare rust-version = "1.89.0" in the root crate — LGTM.Pin the workspace MSRV via manifests as done here and in member crates to let Cargo fail-fast on older compilers.
ninja_env/Cargo.toml (1)
5-5: Update crate MSRV to 1.89.0 — LGTM.This keeps the workspace consistent with CI and rust-toolchain.toml.
.github/workflows/ci.yml (1)
36-36: Gate coverage generation to stable — LGTM.Keep coverage noise down while still validating MSRV builds in the matrix.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/ci.yml (1)
21-35: Speed up builds by caching the Cargo registry and git index per toolchain.Add a cache step keyed by matrix.rust. Omit target to avoid stale artefacts across rustc revisions.
steps: - uses: actions/checkout@v4 + - name: Cache Cargo (registry + git) + uses: actions/cache@5a3ec84eff668545956fd18022155c47e93e2684 # v4 + with: + path: | + ~/.cargo/registry + ~/.cargo/git + key: ${{ runner.os }}-cargo-${{ matrix.rust }}-${{ hashFiles('**/Cargo.lock') }} + restore-keys: | + ${{ runner.os }}-cargo-${{ matrix.rust }}- + ${{ runner.os }}-cargo- - name: Setup Rust uses: leynos/shared-actions/.github/actions/setup-rust@c6559452842af6a83b83429129dccaf910e34562.github/workflows/release.yml (1)
109-117: Avoid caching target across different toolchains; scope the cache or drop target.The cache key ignores the compiler, and target contains artefacts compiled with +1.89.0 before the stable build. Prevent stale or incompatible incremental states.
Apply one of these diffs:
Option A — only cache registry and git:
with: path: | ~/.cargo/registry ~/.cargo/git - target key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }} restore-keys: | ${{ runner.os }}-cargo-Option B — keep target but scope key by toolchain channel:
- key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }} + key: ${{ runner.os }}-cargo-stable-${{ hashFiles('**/Cargo.lock') }} restore-keys: | - ${{ runner.os }}-cargo- + ${{ runner.os }}-cargo-stable-Then add a separate cache step earlier for the MSRV build keyed with cargo-1.89.0-… if you truly need to cache its target.
♻️ Duplicate comments (9)
.github/workflows/netsukefile-test.yml (3)
12-15: Keep matrix with fail-fast: false (LGTM).Run both toolchains to completion to surface independent failures.
27-31: Emit toolchain versions for auditability (LGTM).Log rustup, rustc, and cargo versions to disambiguate matrix runs.
39-42: Scope cache keys by toolchain (LGTM).Isolate caches between stable and MSRV to prevent artefact bleed.
.github/workflows/ci.yml (2)
12-15: Keep fail-fast: false (LGTM).Run the full matrix to surface both stable and MSRV failures.
24-27: Install rustfmt and clippy for each toolchain (LGTM).Guarantee fmt/lint availability on stable and 1.89.0.
.github/workflows/release.yml (4)
88-97: Install MSRV, add target, and emit versions (LGTM).Set up the 1.89.0 toolchain and target triple and log versions to make the MSRV verification reproducible.
106-107: Verify MSRV build in the release pipeline (LGTM).Fail fast if 1.89.0 cannot build the release target.
86-97: Reorder to fail earlier if MSRV is broken (optional).Install and add target for 1.89.0, then run MSRV verify before installing cross to save time on broken tags. You already follow this ordering; keep it as policy for future edits.
1-10: Validate repo-wide MSRV alignment to 1.89.0.Run the checks below and patch any stragglers (rust-toolchain.toml and all Cargo.toml rust-version fields).
#!/bin/bash set -euo pipefail echo "== rust-toolchain.toml channel ==" rg -n '^channel\s*=\s*"1\.89\.0"$' rust-toolchain.toml || true echo echo "== Manifests with rust-version 1.89.0 ==" rg -nP '^\s*rust-version\s*=\s*"1\.89\.0"\s*$' -g '!**/target/**' Cargo.toml || true echo echo "== Workflows using MSRV ==" rg -n "rust: \\[stable, '1.89.0'\\]" .github/workflows || true rg -n "toolchain: \\$\\{\\{ matrix.rust \\}\\}" .github/workflows || true rg -n "cross \\+1\\.89\\.0 build" .github/workflows || true echo echo "Review any missing hits and update to 1.89.0."
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
.github/workflows/ci.yml(3 hunks).github/workflows/netsukefile-test.yml(1 hunks).github/workflows/release.yml(5 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/release.yml
[warning] 58-58: too few spaces before comment: expected 2
(comments)
[warning] 61-61: too few spaces before comment: expected 2
(comments)
🔍 Remote MCP Ref
Relevant facts for reviewing MSRV changes
- The Cargo manifest supports a rust-version field in [package]; this declares the minimum supported Rust compiler and is placed in Cargo.toml’s [package] section.
- Cargo will refuse to build with a compiler older than the declared rust-version (i.e., the field is enforced by Cargo).
- The rust-version entry affects workspace/crate compatibility expectations and should be kept aligned with CI/toolchain pins (rust-toolchain.toml and CI matrices) to avoid drift.
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-test (stable)
🔇 Additional comments (3)
.github/workflows/netsukefile-test.yml (1)
18-19: Export RUSTUP_TOOLCHAIN per job (LGTM).Align Makefile invocations with the selected toolchain via env propagation.
.github/workflows/ci.yml (1)
37-37: Gate coverage to stable (LGTM).Avoid divergent coverage deltas from MSRV builds.
.github/workflows/release.yml (1)
130-144: Pin upload/download actions (LGTM).Use commit SHAs for upload-artifact and download-artifact to harden supply chain.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/release.yml (1)
111-119: Scope Cargo cache key by target triple to prevent cross‑target cache thrashKey the cache by matrix.target so x86_64/aarch64 artefacts don’t overwrite each other.
Apply this diff:
- name: Cache cargo registry uses: actions/cache@0400d5f644dc74513175e3cd8d07132dd4860809 # v4 with: path: | ~/.cargo/registry ~/.cargo/git target - key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }} + key: ${{ runner.os }}-cargo-${{ matrix.target }}-${{ hashFiles('**/Cargo.lock') }} restore-keys: | - ${{ runner.os }}-cargo- + ${{ runner.os }}-cargo-${{ matrix.target }}- + ${{ runner.os }}-cargo-
♻️ Duplicate comments (9)
.github/workflows/netsukefile-test.yml (3)
12-15: Disable fail-fast and add matrix — LGTMRun the full matrix and surface all failures; the configuration is correct.
27-31: Echo active Rust toolchain — LGTMPrint rustup, rustc, and cargo versions for traceability across matrix axes.
33-41: Scope Cargo cache by toolchain; stop caching target — LGTMPrevent cross‑toolchain artefact bleed and avoid defeating the cache when cleaning.
.github/workflows/ci.yml (2)
12-15: Disable fail-fast and add Rust matrix — LGTMExecute both stable and MSRV jobs to completion.
25-32: Install rustfmt/clippy and echo toolchain — LGTMGuarantee formatter/linter availability on each toolchain and log versions.
.github/workflows/release.yml (4)
13-14: Declare least‑privilege permissions for build job — LGTMKeep contents: read for the build job.
90-109: Install MSRV, add target, log versions, and verify MSRV build — LGTMValidate MSRV early and log both MSRV and stable toolchain versions.
139-140: Declare least‑privilege permissions for release job — LGTMGrant contents: write to publish artefacts and notes.
60-60: Pin marketplace actions to SHAs — LGTMHarden supply‑chain by pinning checkout, setup‑python, cache, upload‑artifact, gh‑release, and download‑artifact to commits.
Also applies to: 63-63, 100-100, 132-132, 143-147
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
.github/workflows/ci.yml(2 hunks).github/workflows/netsukefile-test.yml(1 hunks).github/workflows/release.yml(5 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/ci.yml
48-48: context "secrets" is not allowed here. available contexts are "env", "github", "inputs", "job", "matrix", "needs", "runner", "steps", "strategy", "vars". see https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability for more details
(expression)
🔍 Remote MCP Ref
Relevant facts to help review this PR
-
Cargo's rust-version in Cargo.toml is enforced by Cargo as the minimum compiler; builds will fail on older compilers — ensure CI actually runs 1.89.0 to match the new rust-version.
-
GitHub Actions docs show how to specify a Rust toolchain in workflows and recommend verifying toolchain versions in CI (e.g., rustup show; rustc --version; cargo --version) — this supports adding the "Show rustc version" steps.
-
Pinning third‑party Actions to commit SHAs is recommended for security/reproducibility; docs describe using commit SHAs instead of tags/major versions. Review pinned SHAs for desired tradeoffs (stability vs. automatic updates).
-
actions/cache guidance: choose cache keys and be cautious about caching build outputs (target) when workflows run clean or different toolchains; the PR's change to not cache target and to include matrix.rust in cache keys aligns with best practices.
-
Clippy and some tooling document considerations for minimum-supported-Rust-version; if linters or formatters are run under multiple toolchains, confirm they behave identically across stable and 1.89.0.
-
When verifying MSRV for cross-compilation, using cross with an explicit toolchain (e.g., cross +1.89.0 build) is a valid approach and matches the PR's MSRV verification in release.yml.
Sources used: github Actions docs (specifying Rust version, pinning SHAs); rust-bitcoin README MSRV examples; clippy MSRV docs; actions/cache docs; cross usage example
🔇 Additional comments (3)
.github/workflows/netsukefile-test.yml (1)
18-19: Keep RUSTUP_TOOLCHAIN in env — aligns with setup inputExport the toolchain and also pass it to the setup action. With the added “Show rustc version” step this is auditable.
.github/workflows/ci.yml (2)
42-42: Gate coverage generation to stable — LGTMAvoid toolchain drift in coverage metrics.
48-53: Confirm step-level secret context fixAll instances of
secrets.CS_ACCESS_TOKENin step-levelifconditions have been replaced withenv.CS_ACCESS_TOKENand the correspondingenvblock added. No remaining step-levelifusessecrets.*. Workflow now passes actionlint.
1 participant
Summary
closes #119
Testing
RUSTUP_TOOLCHAIN=1.89.0 make fmtRUSTUP_TOOLCHAIN=1.89.0 make lintRUSTUP_TOOLCHAIN=1.89.0 make testhttps://chatgpt.com/codex/tasks/task_e_68a977ffdd4c832291b4e2a63e2e8b57
Summary by Sourcery
Set MSRV to Rust 1.89 and enforce it across CI, release, and project manifests
Enhancements:
CI: