CI reliability hardening: pin tooling, improve clang-tidy cache keys, and preserve failure diagnostics
- Dominant language
- C++
- Stars
- 15.9k
- Forks
- 998
- Avg merge
- 4d 7h
- Merged PRs (30d)
- 34
Description
## Summary
While reviewing `.github/workflows/01-ci-pipeline.yml` and its reusable workflows, I found several CI reliability risks that can cause flaky or hard-to-diagnose failures even when the source change is valid.
This is a proposed hardening pass rather than a claim that every item is currently failing.
## Findings
### 1. Unpinned Python build/test tools
`03-macos-linux-build.yml` pins some tools but leaves these floating:
```text
pytest
pytest-xdist
scikit-build-core
setuptools_scm
```
A new upstream release can change behavior without a repository change.
**Suggested fix:** pin tested ranges or install from a lock/constraints file shared by all runners.
### 2. Brittle exact Ubuntu package version
`clang_tidy.yml` installs:
```text
clang-tidy=1:18.0-59~exp2
```
This exact archive version can disappear or differ across runner-image refreshes.
**Suggested fix:** install `clang-tidy-18`, verify `clang-tidy --version`, or use an LLVM setup action with an explicitly supported major version.
### 3. Generated-header cache key is missing toolchain/config inputs
The cache key includes third-party CMake/proto hashes and submodule commits, but not all inputs that can affect generated artifacts, such as:
- compiler identity/version;
- CMake version;
- relevant root CMake configuration;
- workflow/schema version.
A cache hit can therefore restore incompatible generated state.
**Suggested fix:** add compiler/CMake fingerprints and a manual cache-schema version to the key, or cache only immutable downloaded dependencies rather than generated build output.
### 4. Changed-file handling is unsafe for unusual filenames
The clang-tidy workflow converts changed files through whitespace-separated shell strings and `xargs`. Paths containing spaces, quotes, or newlines can be split incorrectly.
**Suggested fix:** use NUL-delimited paths throughout (`-z`, `xargs -0`) or pass a JSON array from the changed-files action into Python and launch subprocesses directly.
### 5. Failure diagnostics are lost after the job ends
clang-tidy logs are printed in groups, but build/test workflows do not upload structured logs, `compile_commands.json`, CMake logs, or test reports on failure.
**Suggested fix:** add `if: failure()` artifact uploads for:
- `build/compile_commands.json`;
- CMake configure logs;
- clang-tidy logs;
- pytest JUnit XML;
- CTest/JUnit output;
- runner/toolchain version summary.
### 6. No explicit timeouts
Long CMake builds, package installs, tests, or hung subprocesses can consume the full default job limit.
**Suggested fix:** add job- or step-level `timeout-minutes` and terminate child processes cleanly.
### 7. Repeated `apt-get update`
Linux build jobs run `apt-get update` separately for Clang and AIO installation.
**Suggested fix:** consolidate Linux package installation into one step to reduce network flakiness and runtime.
### 8. Main-branch coverage is intentionally narrow but should be explicit
On pushes to `main`, most platform jobs are skipped and only Linux x64 runs. If this is a cost-control decision, it would help to pair it with a scheduled full matrix and document that policy in the workflow.
## Proposed validation
After the hardening patch:
1. run a PR that changes C++, Python, CMake, and docs separately;
2. test a cache hit and cache miss;
3. intentionally trigger clang-tidy, compile, and pytest failures;
4. verify diagnostic artifacts are available;
5. run the full matrix on `workflow_dispatch` and schedule;
6. confirm main pushes still follow the intended cost policy.
I can help translate this into a patch if maintainers confirm the preferred pinning and full-matrix policy.
Contributor guide
Assessment
This issue has not been assessed yet.