Wave-33 C06: exact rustc toolchain pin + SelfCheck - #275
Conversation
Close L60 soft gap: floating stable -> exact channel with commit-hash identity JSON, ops doc, and hermetic CI wiring check. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
📝 WalkthroughSummaryPins the repository to Rust Must FixNone identified. Should Fix
ConsiderThe workflow updates preserve nightly overrides for fuzzing and Miri while ensuring primary and exotic-target jobs use the exact pinned toolchain. Approve / Request ChangesApprove — no blocking compilation, test, security, or architecture issues are evident from the changes. WalkthroughThe repository now pins Rust 1.96.0, documents the pin and its metadata, validates consistency with a PowerShell self-check and Rust test, and updates CI workflows to use or explicitly override the pinned toolchain. ChangesRust toolchain pin
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
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: 2
🤖 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/cross-platform-build.yml:
- Around line 67-71: Remove the explicit toolchain: 1.96.0 input from the
dtolnay/rust-toolchain action in the cross-platform build workflow, leaving the
action to read the active compiler version and targets from rust-toolchain.toml.
In `@scripts/rustc-toolchain-check.ps1`:
- Around line 53-65: Rename the Assert-Contains function to a singular-noun name
such as Assert-Substring or Assert-Content, then update every call site to use
the new name while preserving its parameters and behavior.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: db361715-e217-404f-81c5-1cddaecdd943
📒 Files selected for processing (12)
.github/workflows/ci.yml.github/workflows/cross-platform-build.yml.github/workflows/fuzz-cadence.yml.github/workflows/hermetic.yml.github/workflows/miri-smoke.ymlAGENTS.mddocs/ops/runbook.mddocs/ops/rustc-toolchain-pin.jsondocs/ops/rustc-toolchain-pin.mdrust-toolchain.tomlscripts/rustc-toolchain-check.ps1tests/rustc_toolchain.rs
📜 Review details
⏰ Context from checks skipped due to timeout. (9)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: soft loom · loom_model
- GitHub Check: clean-host smoke · Windows unsigned portable install
- GitHub Check: visual contract · WCAG AA
- GitHub Check: race smoke + channel/cancel model · windows-latest
- GitHub Check: pipeline perf regression gate
- GitHub Check: sl-daemon build · windows-latest
- GitHub Check: sl-daemon build · macos-latest
- GitHub Check: session-ledger build · windows-latest
🧰 Additional context used
📓 Path-based instructions (4)
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Perform feature work in a dedicated git worktree under.claude/worktrees/, created fromorigin/mainon a branch named<type>/<topic>, where<type>isfeat,fix,chore,ci, ordocs; never work directly onmain.
Do not directly commit tomain; do not usegit reset --hard,git stash,git clean, or--no-verify/hook bypass without operator approval; do not add AI attribution to commit or PR metadata; and do not use another actor's branch or worktree.
Files:
AGENTS.mdrust-toolchain.tomldocs/ops/runbook.mddocs/ops/rustc-toolchain-pin.jsondocs/ops/rustc-toolchain-pin.mdtests/rustc_toolchain.rsscripts/rustc-toolchain-check.ps1
**/*.{rs,toml}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,toml}: Use the pinned Rust toolchain and workspace MSRV of Rust 1.85; keepCargo.lockcommitted and current because CI uses--locked.
Before completion, use the prescribed Cargo validation commands:cargo build --all-targets --locked,cargo test --all-features --locked,cargo clippy --all-targets --all-features, andcargo fmt --all --check; use the documented targeted commands for the daemon and viewer when appropriate.
Files:
rust-toolchain.tomltests/rustc_toolchain.rs
docs/**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
Use the documented functional requirements, user journeys, operational runbook, feedback budgets, and toolchain-pin documentation as the authoritative references for corresponding behavior and development workflows.
Files:
docs/ops/runbook.mddocs/ops/rustc-toolchain-pin.md
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
Fix Clippy warnings rather than suppressing them with
#[allow]; any necessary suppression must include a tracking-issue comment.
Files:
tests/rustc_toolchain.rs
🪛 LanguageTool
docs/ops/rustc-toolchain-pin.md
[uncategorized] ~33-~33: The official name of this software platform is spelled with a capital “H”.
Context: ...
Evidence workflows (non-exhaustive): .github/workflows/ci.yml,
`.github/workflows/...
(GITHUB)
[uncategorized] ~33-~33: The official name of this software platform is spelled with a capital “H”.
Context: ...haustive): .github/workflows/ci.yml,
.github/workflows/hermetic.yml, `.github/workf...
(GITHUB)
[uncategorized] ~34-~34: The official name of this software platform is spelled with a capital “H”.
Context: ...ml, .github/workflows/hermetic.yml, .github/workflows/release.yml`.
Bump proc...
(GITHUB)
[uncategorized] ~61-~61: The official name of this software platform is spelled with a capital “H”.
Context: ...| done |
Soft CI (non-blocking): .github/workflows/hermetic.yml job
`rustc-too...
(GITHUB)
🪛 PSScriptAnalyzer (1.25.0)
scripts/rustc-toolchain-check.ps1
[warning] 53-53: The cmdlet 'Assert-Contains' uses a plural noun. A singular noun should be used instead.
Suggested fix: Singularized correction of 'Assert-Contains'
(PSUseSingularNouns)
🪛 zizmor (1.26.1)
.github/workflows/cross-platform-build.yml
[info] 69-69: action functionality is already included by the runner (superfluous-actions): use rustup and/or cargo in a script step
(superfluous-actions)
.github/workflows/hermetic.yml
[warning] 43-43: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[warning] 38-46: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
🔇 Additional comments (10)
rust-toolchain.toml (1)
2-5: LGTM!docs/ops/rustc-toolchain-pin.json (1)
1-14: LGTM!docs/ops/rustc-toolchain-pin.md (1)
1-62: LGTM!.github/workflows/hermetic.yml (1)
37-47: LGTM!.github/workflows/ci.yml (1)
184-186: LGTM!AGENTS.md (1)
53-53: LGTM!docs/ops/runbook.md (1)
7-7: LGTM!tests/rustc_toolchain.rs (1)
1-33: LGTM!.github/workflows/fuzz-cadence.yml (1)
41-41: LGTM!.github/workflows/miri-smoke.yml (1)
20-20: LGTM!
| # Must match rust-toolchain.toml channel — floating `stable` leaves the | ||
| # pin override without the exotic target std (E0463 missing `core`). | ||
| - uses: dtolnay/rust-toolchain@4be7066ada62dd38de10e7b70166bc74ed198c30 # pinned | ||
| with: | ||
| toolchain: 1.96.0 |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Remove the explicit toolchain override to align with the CI policy and prevent breakage on the next bump.
Hardcoding 1.96.0 here violates the CI policy documented in docs/ops/rustc-toolchain-pin.md, which states: "Primary GitHub Actions jobs use dtolnay/rust-toolchain without a toolchain: input so the action installs from rust-toolchain.toml."
More importantly, on the next toolchain bump (e.g., to 1.97.0), this explicit config will install exotic targets against the hardcoded 1.96.0. However, the subsequent cargo check command will automatically use the updated version from rust-toolchain.toml, resulting in the exact same E0463 missing core error you are trying to prevent.
By omitting the toolchain input entirely, the action automatically reads the channel from rust-toolchain.toml and installs the targets for the correct, active compiler version.
♻️ Proposed fix to rely on the pinned toolchain file
- # Must match rust-toolchain.toml channel — floating `stable` leaves the
- # pin override without the exotic target std (E0463 missing `core`).
- - uses: dtolnay/rust-toolchain@4be7066ada62dd38de10e7b70166bc74ed198c30 # pinned
- with:
- toolchain: 1.96.0
- targets: ${{ matrix.target }}
+ # Omit `toolchain` so the action reads from rust-toolchain.toml, ensuring
+ # the exotic target std is added to the exact pinned compiler.
+ - uses: dtolnay/rust-toolchain@4be7066ada62dd38de10e7b70166bc74ed198c30
+ with:
+ targets: ${{ matrix.target }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Must match rust-toolchain.toml channel — floating `stable` leaves the | |
| # pin override without the exotic target std (E0463 missing `core`). | |
| - uses: dtolnay/rust-toolchain@4be7066ada62dd38de10e7b70166bc74ed198c30 # pinned | |
| with: | |
| toolchain: 1.96.0 | |
| # Omit `toolchain` so the action reads from rust-toolchain.toml, ensuring | |
| # the exotic target std is added to the exact pinned compiler. | |
| - uses: dtolnay/rust-toolchain@4be7066ada62dd38de10e7b70166bc74ed198c30 | |
| with: | |
| targets: ${{ matrix.target }} |
🧰 Tools
🪛 zizmor (1.26.1)
[info] 69-69: action functionality is already included by the runner (superfluous-actions): use rustup and/or cargo in a script step
(superfluous-actions)
🤖 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/cross-platform-build.yml around lines 67 - 71, Remove the
explicit toolchain: 1.96.0 input from the dtolnay/rust-toolchain action in the
cross-platform build workflow, leaving the action to read the active compiler
version and targets from rust-toolchain.toml.
| function Assert-Contains { | ||
| param( | ||
| [Parameter(Mandatory = $true)][string]$Doc, | ||
| [Parameter(Mandatory = $true)][string]$Needle, | ||
| [Parameter(Mandatory = $true)][string]$Label, | ||
| [string]$Context = "document" | ||
| ) | ||
| $ok = $Doc.Contains($Needle) | ||
| [void](Write-Check -Label $Label -Ok $ok) | ||
| if (-not $ok) { | ||
| throw "$Context missing required anchor: '$Needle'" | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Fix PSScriptAnalyzer warning by using a singular noun.
PowerShell best practices dictate using singular nouns for cmdlet names. PSScriptAnalyzer flags Assert-Contains because "Contains" is not a singular noun. Consider renaming it to something like Assert-Substring or Assert-Content to comply with standard naming conventions.
♻️ Proposed refactor
-function Assert-Contains {
+function Assert-Substring {
param(Make sure to update the calls to this function as well:
-Assert-Contains -Doc $doc -Needle "Exact rustc toolchain pin (C06 L60)" `
+Assert-Substring -Doc $doc -Needle "Exact rustc toolchain pin (C06 L60)" `
-Label "doc heading" -Context "docs/ops/rustc-toolchain-pin.md"
-Assert-Contains -Doc $doc -Needle "rust-toolchain.toml" `
+Assert-Substring -Doc $doc -Needle "rust-toolchain.toml" `
-Label "toml reference" -Context "docs/ops/rustc-toolchain-pin.md"
-Assert-Contains -Doc $doc -Needle "rustc_commit_hash" `
+Assert-Substring -Doc $doc -Needle "rustc_commit_hash" `
-Label "commit-hash identity" -Context "docs/ops/rustc-toolchain-pin.md"
-Assert-Contains -Doc $doc -Needle "scripts/rustc-toolchain-check.ps1" `
+Assert-Substring -Doc $doc -Needle "scripts/rustc-toolchain-check.ps1" `
-Label "SelfCheck script reference" -Context "docs/ops/rustc-toolchain-pin.md"
-Assert-Contains -Doc $doc -Needle "-SelfCheck" `
+Assert-Substring -Doc $doc -Needle "-SelfCheck" `
-Label "SelfCheck invocation" -Context "docs/ops/rustc-toolchain-pin.md"
-Assert-Contains -Doc $doc -Needle "Exact rustc toolchain pin SelfCheck | **done**" `
+Assert-Substring -Doc $doc -Needle "Exact rustc toolchain pin SelfCheck | **done**" `
-Label "SelfCheck gate marked done" -Context "docs/ops/rustc-toolchain-pin.md"
-Assert-Contains -Doc $doc -Needle "dtolnay/rust-toolchain" `
+Assert-Substring -Doc $doc -Needle "dtolnay/rust-toolchain" `
-Label "CI action named" -Context "docs/ops/rustc-toolchain-pin.md"🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)
[warning] 53-53: The cmdlet 'Assert-Contains' uses a plural noun. A singular noun should be used instead.
Suggested fix: Singularized correction of 'Assert-Contains'
(PSUseSingularNouns)
🤖 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 `@scripts/rustc-toolchain-check.ps1` around lines 53 - 65, Rename the
Assert-Contains function to a singular-noun name such as Assert-Substring or
Assert-Content, then update every call site to use the new name while preserving
its parameters and behavior.
Source: Linters/SAST tools
1 participant
Summary
rust-toolchain.tomlto exact rustc 1.96.0 (not floatingstable) with matchingdocs/ops/rustc-toolchain-pin.jsoncommit-hash identity.scripts/rustc-toolchain-check.ps1 -SelfCheck(pin file exists, exact semver, primary CI usesdtolnay/rust-toolchainwithouttoolchain: stableoverride).hermetic.yml; Rust wrapper testtests/rustc_toolchain.rs.Test plan
pwsh ./scripts/rustc-toolchain-check.ps1 -SelfCheckrustc-toolchain-pincargo test --test rustc_toolchain --locked(optional local)Made with Cursor