chore(rust): pin the toolchain in-repo - #489
Conversation
The local default toolchain and CI's pin were free to drift, so a change could be clean locally and red on CI — 1.95's clippy carries lints 1.94 does not. `rust-toolchain.toml` at the repository root closes that: rustup resolves it by walking up from the working directory, so it governs both `cargo --manifest-path src-tauri/Cargo.toml ...` from the root and a bare `cargo ...` from inside `src-tauri/`. The agreement between that file and the workflows is load-bearing rather than cosmetic. Both release workflows pass `targets: aarch64-apple-darwin,x86_64-apple-darwin` to the toolchain action, which installs those targets into the toolchain *the action* selects. If the toolchain file then selected a different one, cargo would pick a toolchain with no Apple targets and the macOS build would fail — on a workflow that only runs at release time, where nobody is watching. `scripts/check-toolchain-pin.sh` enforces the agreement. It enumerates the workflow directory rather than checking the four pins that exist today, because a script that verifies what it already knows about passes happily on the day someone adds a fifth — the only day it matters. It also rejects a moving pin (`@stable`, `@master`) and a workflow that builds Rust with no pin at all. Deliberately no `targets = [...]` in the toolchain file: the release builds install their Apple targets through the action, so listing them here would make every Linux contributor download a macOS standard library. Verified the gate goes red three ways — a mismatched pin in an existing workflow, a newly added workflow pinning `@stable`, and a cargo-building workflow with no pin — and green again after each revert. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 36 minutes Limit details: You’ve used all 4 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. 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 within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (21)
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.
Pull request overview
Pins the Rust toolchain at the repository root to keep local development and CI using the same Rust/Clippy version, and adds a CI gate to prevent future drift between rust-toolchain.toml and workflow toolchain installs.
Changes:
- Add root
rust-toolchain.tomlpinning Rust 1.95.0 plusclippy/rustfmtcomponents. - Add
scripts/check-toolchain-pin.shand a fast CI job that enforces toolchain pin consistency across workflows. - Update the backend CI toolchain install to include
rustfmtand document the new pinning rule insrc-tauri/CLAUDE.md.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src-tauri/CLAUDE.md |
Documents the new in-repo Rust toolchain pin and the requirement to update workflow pins in the same commit. |
scripts/check-toolchain-pin.sh |
New enforcement script that checks rust-toolchain.toml vs workflow pins and detects Rust-building workflows without a pinned toolchain install. |
rust-toolchain.toml |
New root toolchain pin (1.95.0) with clippy and rustfmt components. |
.github/workflows/build-check.yml |
Adds a dedicated “toolchain-pin” job and updates the backend Rust install to include rustfmt. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| # Match how Rust actually gets built — a cargo subcommand, or the action that shells out to | ||
| # cargo — not the bare word. `nightly-release-manifest.yml` names "nightly-tauri-action.yml" | ||
| # in an env var and builds nothing. | ||
| grep -qE '(^|[^[:alnum:]_-])cargo[[:space:]]+[a-z]|uses:[[:space:]]*tauri-apps/tauri-action' "$workflow" || continue | ||
| grep -q "dtolnay/rust-toolchain@" "$workflow" && continue |
There was a problem hiding this comment.
Taken in 3798232, though not by anchoring to run:/uses:. That would have made things worse:
a run: | block puts the cargo line on a continuation line, so anchoring the pattern to the key
stops matching the repository's real Rust steps — including backend's own cargo test, which is
the one this check most needs to see.
Full-line comments are stripped before the detector runs instead. That covers the case you found —
build-check.yml has exactly the shape you describe, a comment mentioning cargo above a step —
without losing multi-line run: blocks. Trailing comments after code are left alone deliberately,
since stripping them would need to know about quoting.
There is a fixture for it now: scripts/tests/toolchain-pin/cargo-only-in-a-comment/ is a workflow
whose only mention of cargo is in a comment, and check-toolchain-pin.test.sh asserts it is
accepted. Against the previous version of the script that fixture exits 1, which is the false
positive you were describing.
Worth recording that the same pass found a real hole nearby, in the opposite direction: cargo +nightly test matched neither branch of the detector, because the pattern required a lowercase
letter immediately after cargo . That one is not a spurious red — cargo +<toolchain> overrides
rust-toolchain.toml, so it defeated the pin and the check guarding it at once. It is now rejected
outright rather than pin-checked.
`cargo +nightly build` beats `rust-toolchain.toml` outright — rustup honours the
`+toolchain` argument above the file — and the detector never saw it, because it
required a lowercase letter straight after `cargo ` and `+nightly` starts with a
`+`. One line in one workflow defeated the pin *and* the check protecting it,
while the check reported success. It is now banned rather than pin-checked: there
is no version it could name that would be safe, since the point of the file is to
be the single answer.
Two smaller corrections come with it. A Rust build spelled `npx tauri build` —
already how `tauri-smoke` builds in this very workflow — was not recognised as
building Rust at all, so `tauri build` now counts alongside a cargo subcommand and
the tauri-action. And full-line comments are stripped before the detector runs:
the word `cargo` in a comment used to demand a toolchain pin from a workflow that
builds nothing.
The comment on that last check was wrong and is rewritten. It claimed an unpinned
job "silently gets whatever the runner ships", which stopped being true the moment
this branch added `rust-toolchain.toml` — rustup walks up from the checkout and
installs the channel it names. What the action actually supplies is `targets`, and
the macOS release matrices cannot build `--target aarch64-apple-darwin` without it.
`scripts/tests/toolchain-pin/` holds one deliberately broken repository per rule
and `check-toolchain-pin.test.sh` asserts each is rejected, wired into CI ahead of
the real run. Without it a loosened regex still exits 0 here, because this
repository is correct. Against the previous script the fixtures read:
cargo-plus-toolchain exit 0 -> 1
unpinned-tauri-cli-build exit 0 -> 1
cargo-only-in-a-comment exit 1 -> 0
(the other four unchanged, now guarded)
The script takes optional workflow-dir and toolchain-file arguments so the
fixtures can drive it; nothing else should pass them.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
What and why
The local default toolchain and CI's pin were free to drift. CI installs Rust 1.95.0; a contributor
whose default is 1.94.0 gets a different clippy, with a different lint set — so a change can be
clean locally and red on CI for reasons that have nothing to do with the change.
rust-toolchain.tomlat the repository root fixes that. rustup resolves it by walking up fromthe working directory, so it governs both
cargo --manifest-path src-tauri/Cargo.toml …run fromthe root and a bare
cargo …run from insidesrc-tauri/. Both were verified.The agreement between that file and the workflows is load-bearing, not cosmetic. Both release
workflows pass
targets: aarch64-apple-darwin,x86_64-apple-darwinto the toolchain action, whichinstalls those targets into the toolchain the action selects. If
rust-toolchain.tomlthenselected a different toolchain, cargo would pick one with no Apple targets and the macOS build
would break — on a workflow that only runs at release time, where nobody is watching. That is why
the consistency check exists.
scripts/check-toolchain-pin.shenumerates.github/workflows/rather than checking the fourpins that exist today. A script that verifies what it already knows about passes happily on the day
someone adds a fifth, which is the only day it matters. It rejects a moving pin (
@stable,@master), a pin that disagrees with the toolchain file, a workflow that builds Rust withoutinstalling the toolchain through the action, and — see below — any
cargo +<toolchain>.Deliberately no
targets = [...]in the toolchain file: the release builds install their Appletargets through the action, so listing them here would make every Linux contributor download a
macOS standard library they will never build against.
No linked issue — this is the first of a small series of CI/infrastructure changes, not a feature.
What review changed
The first version of this check had a bypass and a false positive, and its central comment was
wrong. All three are fixed in
37982322.cargo +<toolchain>walked straight through it. The detector required a lowercase letterimmediately after
cargo, and+nightlystarts with a+, so a workflow whose only Rust step wasrun: cargo +nightly testwas skipped entirely and the script exited 0. This is the one spellingthat matters most, because
cargo +<toolchain>overridesrust-toolchain.tomlitself — rustuphonours the
+toolchainargument above the file. One line defeated the pin and the check guardingthe pin at once, and the check reported success. It is now rejected outright rather than
pin-checked: there is no version it could name that would be safe, since the point of the file is to
be the single answer.
npx tauri buildwas not recognised as building Rust — despite being howtauri-smokebuilds inthis very workflow.
tauri buildnow counts alongside a cargo subcommand andtauri-apps/tauri-action.The word
cargoin a comment counted as a Rust build, so a workflow that builds nothing could beasked for a toolchain pin. Full-line comments are stripped before the detector runs. (Anchoring the
pattern to
run:/uses:keys instead would have been worse: arun: |block puts the cargo line ona continuation line, so anchoring stops matching this repo's real Rust steps.)
The comment on that last check was simply wrong, and is rewritten. It claimed an unpinned job
"silently gets whatever the runner ships". That stopped being true the moment this branch added
rust-toolchain.toml: rustup walks up from the checkout and installs the channel the file names, onubuntu-latestas anywhere else. What the action actually supplies istargets— the macOS releasematrices cannot build
--target aarch64-apple-darwinwithout it — plus warm downloads. The rule isworth keeping; the stated reason was not the real one.
For the same reason, one thing this check deliberately does not do is treat
with: toolchain: <other>as an override. At a version tag,dtolnay/rust-toolchain'saction.ymldeclares no
toolchaininput at all and hardcodesenv: toolchain: 1.95.0; the documented inputexists only on
@master, which this script rejects as a moving pin anyway.How to verify
scripts/tests/toolchain-pin/holds one deliberately broken repository per rule, andscripts/check-toolchain-pin.test.shasserts each is rejected. It runs in CI before the realcheck, because the real repository is correct — a loosened regex would sail through the real run and
report success.
Run against the previous version of the script, the same fixtures read:
cargo-plus-toolchainunpinned-tauri-cli-buildcargo-only-in-a-commentmismatched-action-pinmoving-channelunpinned-cargo-buildagreeingOne thing this PR cannot verify from a pull request. No
pull_requestworkflow exercisestauri-action.ymlornightly-tauri-action.yml, so the toolchain file's effect on the five-platformrelease matrices is untested until a release runs. The pin and the action both name 1.95.0 so they
agree, and the script keeps them agreeing — but please trigger
nightly-tauri-action.ymlviaworkflow_dispatchafter merging and confirm all five platforms still build.Incidental findings
There are five workflow files, not four.
nightly-release-manifest.ymlwas missed by the manualsurvey that preceded this change; it builds no Rust, so it is not a gap. The enumerating script is
the reason the question got asked.
rustup auto-installs the
componentsdeclared in the toolchain file on the first cargoinvocation, so
rustfmtfor 1.95.0 now arrives without anyone runningrustup component add— whichcloses a stated precondition for the pending format sweep.
Separately, and outside this diff:
developcurrently requires zero status checks to merge(
required_status_checks.contextsis[]). The newtoolchain-pinjob will run and can go redwithout blocking anything until it is added to branch protection.
Checklist
develop, branched from an up-to-datedevelopTests
check-toolchain-pin.test.sh,with the before/after table above
npm test— not run; this PR touches no frontend codecargo test --manifest-path src-tauri/Cargo.toml --workspacepassescargo clippy --manifest-path src-tauri/Cargo.toml --workspace --all-targets -- -D warningsis clean
If this changes docs-worthy behaviour
src-tauri/CLAUDE.md§6 notes the pin, what changing it requires, and thecargo +<toolchain>banAI assistance