Skip to content

feat(ci): add CI Doctor canary + clippy auto-fix workflows#3596

Open
cijothomas wants to merge 1 commit into
open-telemetry:mainfrom
cijothomas:cijothomas/ci-doctor
Open

feat(ci): add CI Doctor canary + clippy auto-fix workflows#3596
cijothomas wants to merge 1 commit into
open-telemetry:mainfrom
cijothomas:cijothomas/ci-doctor

Conversation

@cijothomas

Copy link
Copy Markdown
Member

Adds two scheduled workflows plus one helper script to catch and repair the CI failures that today only surface when a contributor opens an unrelated PR — new stable clippy releases, new RUSTSEC advisories, and MSRV drift under a fresh dependency resolve. Motivated by the clippy 1.97 fallout seen in #3593 / #3594.

Tier 1 — .github/workflows/ci-doctor.yml (canary)
Runs daily on main (and on workflow_dispatch). Three jobs, all on stable, all with a fresh cargo update first:

  • clippycargo clippy --workspace --all-targets --all-features -- -Dwarnings
  • cargo-denycargo deny fetch + cargo deny check advisories
  • msrvscripts/msrv.sh

On failure the reporter opens (or leaves alone) a labeled tracking issue with a stable signature so repeat runs do not spam. On the next passing run the issue is auto-closed. This alone converts "surprise red PR" into "known open tracking issue".

Tier 2 — .github/workflows/ci-doctor-autofix.yml
Runs 30 minutes after the canary. Attempts cargo clippy --fix on stable, runs cargo fmt --all, and only opens a PR if the fixed tree passes both cargo clippy … -Dwarnings and cargo test --workspace --all-features --lib. Partial fixes are refused — the Tier 1 tracking issue is enough for those. The PR uses branch ci-doctor/clippy-fix (force-pushed on each run) and is titled chore(ci-hygiene): apply clippy suggestions from Rust <version>.

Reporter — .github/scripts/ci-doctor-report.sh
Idempotent bash script used by Tier 1. Dedupes by hidden <!-- ci-doctor:signature=<key> --> marker in the issue body. Auto-creates the ci-hygiene label if missing.

Guardrails

  • Both workflows are no-ops on forks (github.repository == guard).
  • Kill switch: repo variable CI_DOCTOR_ENABLED=false.
  • Optional secret CI_DOCTOR_TOKEN (a bot PAT or GitHub App installation token) — when set, autofix opens the PR under that identity so its own CI runs; without it, the default GITHUB_TOKEN opens the PR but CI must be manually kicked (close-and-reopen or empty commit).
  • No workflow calls gh pr merge or gh pr review --approve. Bot PRs need normal review + CLA.

Local checks done

  • actionlint clean on both workflow YAMLs.
  • shellcheck clean on the reporter.
  • Reporter branching logic exercised with a stubbed gh — all four cases (fail/pass × issue exists / does not exist) call the right sub-commands and produce a well-formed issue body.

Not yet exercised end-to-end — these will only be validated on the first real run:

  • GitHub search-based dedupe of the tracking issue.
  • cargo clippy --fix behavior on a GH runner.
  • The autofix push + gh pr create flow.
  • Whether the autofix PR triggers downstream CI (see the CI_DOCTOR_TOKEN note above).

Suggested shakedown after merge: manually workflow_dispatch both workflows once and observe, then iterate with follow-up PRs if anything misbehaves.

@cijothomas
cijothomas requested a review from a team as a code owner July 15, 2026 18:33
@codecov

codecov Bot commented Jul 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.2%. Comparing base (eac3145) to head (f3c9b2f).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##            main   #3596     +/-   ##
=======================================
- Coverage   83.2%   83.2%   -0.1%     
=======================================
  Files        130     130             
  Lines      28232   28231      -1     
=======================================
- Hits       23491   23490      -1     
  Misses      4741    4741             

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

jensholdgaard added a commit to jensholdgaard/ourios that referenced this pull request Jul 25, 2026
* ci: nightly hygiene canary for clippy + advisory drift

Two things rot here without anyone touching the code. `rust-toolchain.toml`
is `channel = "stable"` (unpinned) while `clippy::pedantic` warnings are
errors, so every ~6-week Rust release can turn CI red; and a newly published
RustSec advisory fails `cargo deny` the same way. Both ran on
`pull_request`/`push` only, so either one surfaced as a surprise red check on
whatever unrelated PR came next.

Adapted from open-telemetry/opentelemetry-rust#3596 (cijothomas), but as a
`schedule:` trigger on this workflow rather than the new `ci-doctor.yml` it
proposes. That matters: the canary now runs the *literal same* clippy and
cargo-deny job definitions PR CI runs, so it cannot drift from what is
actually enforced -- a copied `cargo clippy` invocation in a second file
inevitably would.

Only the two hermetic jobs participate; the other ten carry
`if: github.event_name != 'schedule'`, and `on:` documents that new jobs must
opt out unless they are flake-free. A canary that files noise gets ignored,
so testcontainers/localstack jobs stay out -- we have already seen a registry
pull time out, and there is a known rust-lld flake.

`ci-success` is excluded too: with ten of twelve `needs` skipped it would
report "all required checks passed" off two jobs.

Dropped from the upstream design:

- The MSRV tier. It runs a `scripts/msrv.sh` we do not have, and it exists to
  catch drift under a fresh resolve -- but deps are exact-pinned densely and
  Renovate already turns bumps into reviewable PRs.
- Tier 2 auto-fix. `clippy --fix` cannot make the judgement calls pedantic
  lints require: `too_many_lines` wants a helper extracted at a sensible seam
  and `doc_markdown` wants deciding which words are code (CLAUDE.md §6.4). It
  is also the part upstream flags as not yet exercised end to end.

Changed from the upstream design: dedupe scans the labelled issues directly
instead of using GitHub's search index, which lags issue creation by seconds
to minutes -- exactly the window two consecutive canary runs land in.

Verification: actionlint clean, shellcheck clean, and the reporter's four
branches (fail/pass x issue exists/absent) exercised against a stubbed `gh`,
plus a fifth case proving the clippy and deny signals never share an issue.

Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>

* ci: restrict the hygiene reporter to main and serialize it

Two review findings on #631.

CodeRabbit: two reporter runs could race the dedupe. The specific case given
is already serialized -- the workflow-level concurrency group keys on
`github.ref` and does not cancel in progress on main, so same-ref runs queue.
But it surfaced a worse reachable path: `workflow_dispatch` can target any
branch, and the reporter's guard only checked the *event*. A dispatch on a
feature branch would therefore file an issue whose body claims `main` is
broken, on the strength of that branch's clippy result. The tracking issue
describes main, so now only main may file it -- which also removes the
cross-ref race. Added the suggested job-level concurrency group as
belt-and-braces, so a future change to the workflow-level key cannot silently
reopen it.

Copilot: the dedupe scan was capped at 100 issues. The label should never hold
more than two at once, but a truncated scan fails in the one way this function
exists to prevent -- silently missing the marker and filing a duplicate -- so
the ceiling is now far above any real value.

actionlint + shellcheck clean; the reporter's four branches re-exercised
against the stubbed `gh`.

Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>

* docs(ci): correct what a manual dispatch actually runs

The `on:` comment claimed a `workflow_dispatch` "runs every job". It does
not: `coverage` additionally requires a `push` to main, and after the last
commit `hygiene-report" requires `main` whatever the event. A future
maintainer debugging a missing coverage run would have been misled by the
comment that was supposed to help them.

Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>

---------

Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants