Repository navigation
fix(ci): unblock Rust CI on newer clippy/rustdoc lints and pin the toolchain - #122
Merged
Merged
Conversation
…olchain Newer stable rustc/clippy started flagging async_trait's generated #[must_use] as redundant (clippy::double_must_use) and one doc link as having a redundant explicit target, both newly-stricter lints that broke an unrelated PR. Also pins rustc/cargo via rust-toolchain.toml (mirrored automatically by rustup locally and by setup-rust-toolchain in CI/release), adds a weekly workflow that opens a PR bumping that pin to the latest stable, and skips the Rust CI job on PRs/pushes that don't touch Rust code (via a job-level `if`, not a trigger-level path filter, so the required "Rust" status check still reports instead of hanging pending). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Generated bump PRs will not trigger CI, and change-detection failures can bypass the required Rust check.
Review effort: Balanced
Findings: 1
Open (3)
What changed in this PR
Pins Rust CI and resolves lint failures introduced by newer toolchains.
Changes:
- Pins Rust 1.99.0 and adds weekly update PRs.
- Adds path-based Rust CI execution.
- Scopes Clippy allowances and fixes a rustdoc link.
| File | Description |
|---|---|
rust-toolchain.toml |
Pins Rust and required components. |
cli-engine/src/transport/injector.rs |
Allows generated async-trait lint. |
cli-engine/src/middleware/mod.rs |
Allows generated async-trait lints. |
cli-engine/src/config.rs |
Simplifies a rustdoc link. |
cli-engine/src/auth/storage.rs |
Allows generated async-trait lint. |
cli-engine/src/auth/mod.rs |
Allows generated async-trait lint. |
.github/workflows/rust-toolchain-bump.yml |
Automates weekly toolchain updates. |
.github/workflows/ci.yml |
Detects Rust-relevant changes before running CI. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Copilot review on #122 caught two real issues: the Rust job's `if` skipped (not failed) when the `changes` job itself fails, which would've let a transient paths-filter error bypass the only required Rust check; and the bump workflow's default GITHUB_TOKEN means its generated PR won't trigger ci.yml at all (GitHub's anti-recursion protection), defeating the point of opening it as a normal PR. The first is fixed outright. The second needs a PAT/GitHub App secret only a repo admin can provision, so the token input now prefers one if present (`RUST_TOOLCHAIN_BUMP_TOKEN`) and falls back to today's behavior otherwise. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
qcai-godaddy
approved these changes
Oct 2, 2026
Merged
jpage-godaddy
pushed a commit
that referenced
this pull request
Oct 5, 2026
🤖 I have created a release *beep* *boop* --- <details><summary>cli-engine: 0.10.0</summary> ## [0.10.0](cli-engine-v0.9.5...cli-engine-v0.10.0) (2026-10-05) ### ⚠ BREAKING CHANGES * **output:** `render_human_with_view` and `render_human_with_registry_selected` are now crate-internal (no longer exported from the crate root or `output`); `render_human_with_registry` and `render_human_with_registry_for_schema` are removed entirely. Any consumer calling these directly (expected mainly in command-module tests) should switch to the new `preview_human_view(data, columns)`. ### Features * **output:** mandatory/essential columns and explicit --fields opt-out ([#120](#120)) ([4ff2418](4ff2418)) ### Bug Fixes * **ci:** unblock Rust CI on newer clippy/rustdoc lints and pin the toolchain ([#122](#122)) ([6fe18d7](6fe18d7)) ### Documentation * propose cursor-first pagination (--limit/--continue) ([#114](#114)) ([d67f9d0](d67f9d0)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
async_trait's generated#[must_use]as redundant (clippy::double_must_use) on 6 trait definitions, and flags one doc link inconfig.rsas having a redundant explicit target (rustdoc::redundant_explicit_links). Both are newly-stricter lints, not real regressions — fixed with scoped#[allow]s (plus a comment explaining why) and a doc-link simplification respectively.rust-toolchain.toml(the Rust equivalent of.nvmrc—rustupandsetup-rust-toolchainboth pick it up automatically, locally and inci.yml/release.yml) so CI stops silently floating onto whateverstablehappens to be that day..github/workflows/rust-toolchain-bump.yml: a weekly (+ manually dispatchable) job that checks the real latest stable, and opens a PR bumping the pin when it's behind. That PR runs through the normalci.ymlchecks, so if the new toolchain breaks something, a maintainer pushes fixes to that same branch before merging.GITHUB_TOKEN, so GitHub's anti-recursion protection means it currently will not auto-triggerci.yml(caught by Copilot review). The workflow'screate-pull-requeststep now prefers aRUST_TOOLCHAIN_BUMP_TOKENsecret (PAT or GitHub App token) if one exists, falling back toGITHUB_TOKENotherwise. Someone with repo-admin access needs to decide on and provision that secret for bump PRs to get real CI coverage; until then they'll need a manual push/re-run to trigger checks.RustCI job on PRs/pushes that don't touch Rust-relevant paths, via achangesjob + job-levelif:(not a trigger-levelpaths:filter) —Rustis a required status check onmain, and a trigger-level path filter would leave that check stuck "Pending" forever on non-Rust PRs instead of satisfying it. A skipped job reports "Success" and doesn't block merge.if:fails safe: if thechangesjob itself fails (e.g. apaths-filtererror) rather than cleanly resolving tofalse, theRustjob still runs instead of being skipped — otherwise a transient detection failure could let real Rust changes merge without ever running Rust CI (also caught by Copilot review).Test plan
cargo fmt --all --checkcargo clippy --all-targets -- -D warnings&&--features pkce-authcargo test --all-targets&&--features pkce-auth(all passing)cargo test --doc&&--features pkce-authRUSTDOCFLAGS='-D warnings' cargo doc --no-deps&&--features pkce-authcargo rustdoc --lib -- -W missing-docs(0 missing)./cli-engine/scripts/check-module-size.shactionlinton both workflow filesrustupauto-switches to the pinned1.99.0toolchain on entering the repoif, token-triggers-CI gap, a typo) addressed in a follow-up commit; re-review came back clean (0 findings)🤖 Generated with Claude Code