Conversation
…hain-respecting flow Two broken action references surfaced on the first post-merge CI run of consumer rust-ci.yml calls: 1. taiki-e/install-action@a416ddee... -- non-existent SHA. Fixed: @a2352fc6... (authoritative v2 tag SHA). 2. EmbarkStudios/cargo-deny-action runs cargo-deny inside a Docker container that does NOT respect the consumer's rust-toolchain.toml override. When a caller (e.g. resq-software/crates) pins a non-default target like stable-x86_64-unknown-linux-musl, the container errors out: override toolchain 'stable-x86_64-unknown-linux-musl' is not installed Fixed: replaced with dtolnay/rust-toolchain (sets up the requested toolchain) + taiki-e/install-action (installs prebuilt cargo-deny binary) + cargo deny --all-features check (direct invocation on host) This path respects rust-toolchain.toml and is also faster (precompiled cargo-deny vs Docker image pull). No input/output schema change. Consumers (programs, crates) will need their @sha pin updated to pick up this fix.
|
Note Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported. |
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 10 minutes and 16 seconds. ⌛ 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. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughGitHub Actions workflow updates bumped the coverage tooling version and restructured the cargo-deny job to use direct command invocation instead of a dedicated action. The test job coverage setup now references a newer Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
…ws in audit Two fixes for the same underlying cause: 1. resq-software/.github doesn't emit the `required` status-check context on its own PRs. The existing required.yml is a reusable workflow (on: workflow_call) that only fires when called via uses:, never on pull_request events. Adds a standalone required-gate.yml with a pass-through `required` job, same pattern used on docs/dev/ardupilot/resq-proto. Unblocks every PR on this repo (including this one and #13). 2. audit-required-job.yml had a false-positive: it matched the `required:` job key inside the reusable aggregator and reported .github as "ok" when it wasn't. Patched the detection to first check that the workflow's `on:` block contains pull_request or push — reusable-only workflows are skipped. Correct behavior.
* feat(governance): weekly required-job audit workflow Weekly cron (Monday 08:00 UTC) + workflow_dispatch that scans every non-archived repo under resq-software/ and verifies at least one CI workflow on its default branch emits a `required` status context. If any repo is missing it (and not in the ruleset exclude list), opens or comments on a tracking issue in this repo with the repo list and a reference fix. Why: the `default-branch-baseline` ruleset (id 15191038) requires that specific context. Any repo lacking such a job has every PR blocked on 'Expected - Waiting for status to be reported'. Previously hit npm (now fixed via resq-software/npm#46). This audit catches future drift before it bites. - Uses SYNC_TOKEN (preferred, covers private repos) or GITHUB_TOKEN (public-only fallback). - Does NOT fail the run on gaps; only opens/updates an issue. - Matches either `required:` job key or `name: required` anywhere in .github/workflows/*.yml on the default branch. * fix(governance): add required-gate.yml + reject reusable-only workflows in audit Two fixes for the same underlying cause: 1. resq-software/.github doesn't emit the `required` status-check context on its own PRs. The existing required.yml is a reusable workflow (on: workflow_call) that only fires when called via uses:, never on pull_request events. Adds a standalone required-gate.yml with a pass-through `required` job, same pattern used on docs/dev/ardupilot/resq-proto. Unblocks every PR on this repo (including this one and #13). 2. audit-required-job.yml had a false-positive: it matched the `required:` job key inside the reusable aggregator and reported .github as "ok" when it wasn't. Patched the detection to first check that the workflow's `on:` block contains pull_request or push — reusable-only workflows are skipped. Correct behavior. * fix(audit): reject reusable-only workflows in required-job detection The audit was false-positive for resq-software/.github: matched the required: job key inside required.yml (on: workflow_call, reusable- only) and reported .github as having a required job on its default branch. Reusables never fire on pull_request so the ruleset saw the context as unreported, blocking every PR on .github. Fix: before matching required: or name: required, extract the workflow's on: block and skip any workflow whose triggers don't include pull_request or push.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/rust-ci.yml (1)
161-172: Deny-job restructuring looks correct; note the toolchain input contract.The new flow (dtolnay/rust-toolchain + taiki-e/install-action + direct
cargo deny --all-features check) is sound and addresses the Docker/override mismatch described in the PR.However, there's a contract coupling worth documenting:
inputs.toolchainmust match (or be a superset of) the toolchain named in the consumer'srust-toolchain.toml, otherwise rustup's cargo proxy will fail on the host with the same "override toolchain not installed" error. Current callers (resq-software/programs with stable, resq-software/crates) don't expose this yet, but it should be captured in the block comment for future maintainability.Two optional improvements:
Expand the rationale comment to explicitly call out the contract:
- Clarify that the host (not Docker) execution means
rust-toolchain.tomloverride is resolvable- Add a NOTE that
inputs.toolchainmust match the consumer's rust-toolchain.toml to avoid the same failure modeInvoking via binary path (after
taiki-e/install-actionputs it on PATH) is an alternative:cargo-deny check --all-featuresbypasses the rustup/cargo proxy entirely and sidesteps the toolchain coupling. This is optional and less idiomatic thancargo deny, but removes the strict input dependency if future callers need flexibility.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/rust-ci.yml around lines 161 - 172, Update the workflow comment near the dtolnay/rust-toolchain and taiki-e/install-action steps to document the toolchain input contract: state that because the job runs on the host and relies on rust-toolchain.toml, inputs.toolchain must match (or be a superset of) the consumer's rust-toolchain.toml to avoid "override toolchain not installed" errors from rustup/cargo; also add a NOTE describing this host-resolution behavior and, optionally, mention the alternative invocation using the installed cargo-deny binary (cargo-deny or cargo-deny check / cargo-deny check --all-features or invoking cargo-deny directly) as a way to bypass the rustup proxy if callers want to avoid the strict inputs.toolchain coupling.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/rust-ci.yml:
- Line 136: Update the pinned reference for the taiki-e/install-action usage:
either change the commit SHA in the uses entry
(taiki-e/install-action@a2352fc6ce487f030a3aa709482d57823eadfb37) to the current
v2 tag commit (58e862542551f667fa44c8a2a4a1d64ad477c96a) if you intend to follow
the floating v2 tag, or change the inline comment (“# v2”) to reflect the
specific release you pinned (e.g., “# v2.75.16”) to make the intent explicit;
apply the same change for the other occurrence of this action so both usages are
consistent.
---
Nitpick comments:
In @.github/workflows/rust-ci.yml:
- Around line 161-172: Update the workflow comment near the
dtolnay/rust-toolchain and taiki-e/install-action steps to document the
toolchain input contract: state that because the job runs on the host and relies
on rust-toolchain.toml, inputs.toolchain must match (or be a superset of) the
consumer's rust-toolchain.toml to avoid "override toolchain not installed"
errors from rustup/cargo; also add a NOTE describing this host-resolution
behavior and, optionally, mention the alternative invocation using the installed
cargo-deny binary (cargo-deny or cargo-deny check / cargo-deny check
--all-features or invoking cargo-deny directly) as a way to bypass the rustup
proxy if callers want to avoid the strict inputs.toolchain coupling.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7b119449-635a-41f9-ac29-f24ff470c831
📒 Files selected for processing (1)
.github/workflows/rust-ci.yml
CodeRabbit flagged that my `# v2` comment was misleading — the pinned SHA a2352fc6... is actually release v2.75.16 (not the current `v2` floating tag, which now resolves to 58e86254...). Keeping the SHA (pinned to a specific release for reproducibility) and correcting the comment to reflect the truth. Applied to both call sites: the cargo-llvm-cov install (line 136) and the cargo-deny install (line 169).
2 participants
Summary
Two broken references in
rust-ci.ymlsurfaced on first real post-merge CI runs:taiki-e/install-actionSHA was wrong —@a416ddee...doesn't exist on GitHub. Replaced with the authoritative v2 tag SHA@a2352fc6ce487f030a3aa709482d57823eadfb37.EmbarkStudios/cargo-deny-actionruns inside Docker and doesn't respect the consumer'srust-toolchain.toml. Whenresq-software/cratespinnedstable-x86_64-unknown-linux-musl, the Docker container errored with "override toolchain not installed". Replaced withdtolnay/rust-toolchain+taiki-e/install-action cargo-deny+ directcargo deny check— runs on the host with the right toolchain.Blast radius
Callers (
programs#20-merged,crates#71) pin@f4b51a6and are unaffected until their SHAs are re-pinned.Test plan
Summary by CodeRabbit