Skip to content

ci: cut PR wall-clock by fixing cache thrash and redundant work - #1338

Merged
beqaabu merged 8 commits into
usc-devfrom
ci/reduce-pr-wall-clock
Sep 16, 2026
Merged

beqaabu merged 8 commits into
usc-devfrom
ci/reduce-pr-wall-clock

Conversation

@beqaabu

@beqaabu beqaabu commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Why

Measured baseline over recent PR runs (via the new .github/ci-timing.py):
median end-to-end wait 16 min, p90 91 min, max 144 min. The tail is the problem.

I expected the cause to be jobs firing when they shouldn't, so I replayed each workflow's
real path filter over the last 80 commits on usc-dev. The gating mostly already works:
try-runtime fires on 8% of commits, benchmarks 6%, chainspec 20%. The actual cost is
that the work each job does is duplicated and uncached.

workflow dur fires internal breakdown
attestor-compatibility 82m 28% build-sut 46m + provision 7m + test 28m, serial
validator-compatibility 81m 20% build-sut 47m + provision 10m + test 23m, serial
docker 64m 62% provision 9m + docker-build 52m + test 2m
rust 55m 37% test 55m, clippy 43m, check 21m, parallel

What this changes

rust.yml: the cache save was a race the useless jobs won

All six jobs passed shared-key: build-creditcoin-node, as do eight other files. Cache keys
are immutable, so cargo-fmt (2 min) and cargo-machete (0 min) saved first with an
essentially empty target/, and the 21/43/55-minute jobs had their artifacts discarded on
every run.
Those three now restore but never save (save-if: false). cargo-check,
cargo-clippy and cargo-test get their own keys, since their fingerprints
(release+default, debug+all-features, release+coverage) are mutually unusable anyway.

Also here: SKIP_WASM_BUILD=1 on cargo-clippy and cargo-test, neither of which consumed the
runtime wasm blob it was building; removal of cargo check --features=runtime-benchmarks --release, which clippy's --all-targets --all-features strictly subsumes; and job-level
timeout-minutes throughout.

Coverage moves off the PR path

cargo-test now runs cargo nextest run --release --workspace. cargo llvm-cov injects
-C instrument-coverage into RUSTFLAGS, which changes every dependency's fingerprint and
forced a fourth from-cold compile of the whole graph per PR. Nothing gated on the output:
no codecov upload, no threshold, no PR comment, only an HTML artifact someone opens by hand to
tick the PR-template checkbox. It moves to a new rust-coverage.yml (nightly on weekdays +
workflow_dispatch), together with the PROPTEST_CASES=10000 soak and its regression-file
auto-commit. PRs keep a 256-case proptest run so the invariants stay exercised.

cargo check is kept, on purpose

I checked whether clippy and the release builds already subsume it. They don't.
clippy --all-features turns runtime-benchmarks ON, so it never compiles the
#[cfg(not(feature = "runtime-benchmarks"))] blocks at runtime/src/lib.rs:1081,
node/src/client.rs:17 and node/src/command.rs:262. Nothing else type-checks the graph with
default features on a usc-dev PR either: ci.yml builds --features=fast-runtime, docker
builds --features metadata-hash, and the plain --release builds are in chainspec.yml (20%
of PRs) and runtime-upgrade.yml (restricted to branches: [main, usc-testnet]). So only the
redundant second invocation is gone; the cache fix should make what remains much cheaper.

Compat workflows: stop re-downloading immutable files

Both re-fetched every historical release zip from the API on every run. Now cached, keyed on a
new digest output from discover-versions (short sha256; the raw JSON version array is
unsafe in a cache key, it contains quotes and commas). The unzip and --version checks still
run on a cache hit, so a corrupt entry fails loudly instead of silently testing nothing. Only
the archives are cached, to limit the draw on the repo-wide cache budget the Rust caches share.

Attestor's build-sut no longer needs: discover-versions, which was gating a ~46-minute
build behind an API-paging job whose output the build never uses.

ci.yml

Dropped the no-op trigger-integration-test-cli job. Its only step was a checkout, and
because it also depended on the node build it could not report its skip decision until that
build had finished. integration-test-cli is now gated directly. Added timeout-minutes to
all 10 non-reusable jobs, which previously had none, so a hang held a runner for GitHub's
360-minute default.

The metadata hand-off stops being a patch

regenerate-metadata uploaded git diff --binary and its three consumers ran git apply on
it. A diff records the blob it was computed against, so that only works while producer and
consumers check out an identical tree. They do not:

job checkout role
regenerate-metadata merge ref producer
attestor-cli-testing merge ref consumer
integration-test-cli branch head consumer
rebuild-new-metadata (build-native.yml) branch head consumer, and commits back

So any PR whose branch was behind a base that had moved metadata.scale failed:

error: patch failed: cli/creditcoin.json:1
error: the patch applies to 'common/cc-client/artifacts/metadata.scale' (d5f373c0...),
       which does not match the current contents.

This PR hit it on its own six integration-test-cli legs, because usc-dev had moved
metadata.scale under it via #1320 and #1326 and bumped creditcoin.json to 3.137.0. A
rebase clears the symptom; it recurs on the next PR that lags a metadata-changing merge.
Flipping the producer to the branch head would only move the breakage onto the other two
consumers.

The producer now ships the regenerated files themselves, the same three paths
rebuild-new-metadata already commits back, and the consumers copy them into place:

path: |
  cli/creditcoin.json
  cli/src/lib/interfaces/
  common/cc-client/artifacts/metadata.scale

There is no pre-image to match, so the step cannot fail this way regardless of which ref each
job sits on. if-no-files-found goes warn to error, since a missing artifact now means
consumers silently test stale metadata rather than fail loudly; it can only fire in an
already-failing run, as every consumer needs: this job. Incidental cli/yarn.lock churn is
no longer carried over, because every consumer runs its own yarn install afterwards.

integration-test-cli also drops its branch-head checkout pin. Tracing it back, the pin comes
from 542894144, the commit that introduced the typedef auto-update, where it is required
because that job pushes a commit. Only rebuild-new-metadata does that. Here it meant the job
ran CLI sources from the un-merged branch against a node binary that
build-creditcoin-node-for-testing-ci had compiled from the merge ref.

Measurement

.github/ci-timing.py with snapshot / report / compare, so the next round of this work
can be judged instead of asserted. Queue time is included, because waiting for a runner is
waiting.

.github/ci-timing.py snapshot --out before.json     # then land changes, let PRs run
.github/ci-timing.py snapshot --out after.json
.github/ci-timing.py compare before.json after.json

docker.yml: skip the node image build when nothing can affect it

The node image's Rust compile is 27.2 of the docker-build job's ~29 minutes (BuildKit
stage #21 [rust-builder 4/4]); every other stage together is ~90 seconds. Over the last 160
commits, 20 of 72 Docker runs rebuilt it for changes that cannot affect it.

Two cases, neither of which loses coverage the old CI had:

cli/src/** is dropped from the triggers. A CLI-only change cannot touch the Rust
binaries, and the one thing this workflow uniquely proved about the CLI is already asserted on
every PR by sanity.yml's npm-packages job:

yarn build; npm install -g .
which creditcoin; creditcoin help
creditcoin wizard --help; creditcoin attestor --help

That is a strict superset of docker-test's three docker exec CLI checks, and costs about a
minute. The rest of docker-test (chainspecs, other binaries, compose) exercises an image that a
cli/src change leaves byte-identical.

cc3-indexer/** still triggers the workflow but no longer builds the node image. The root
Dockerfile has no reference to cc3-indexer, and the indexer and stress-test images come from
their own external bases (subql-node-substrate, deno) rather than deriving from the node
image, so both are still built and still CVE-scanned. docker-test skips along with the node
image, because every assertion in it runs against that image.

Mechanically, a detect-changes job computes whether the node image is needed and passes it to
build-docker.yml's new build-node-image input, which gates the node build, its Scout compare,
and the export/upload of the image artifact. The input defaults to true, so release.yml, which
omits it, is unaffected. A manual workflow_dispatch run has no base ref to diff against, so it
forces the value to true rather than silently skipping.

The job is called detect-changes rather than should-run on purpose: should-run is a
required status check produced by ci.yml, and a second job of that name would emit a colliding
context into branch protection.

Replayed against the last 160 commits:

before after
workflow runs 72 61
node image builds 72 52

20 builds of ~27 min avoided, 27% of Docker runs.

One honest caveat: on a cli/src-only PR the Docker Scout comparison no longer runs, so a CVE
introduced by a new CLI npm dependency would surface in the weekly Scout report and Dependabot
rather than on the PR. That step is already continue-on-error: true with exit-code: false,
so it never gated anything.

Not done, and worth a follow-up: when the node image is skipped, the job still provisions a
Linode VM (6-17 min) to run ~2 minutes of auxiliary image builds. Making the runner conditional
would reclaim that, but if deploy-runner were skipped while docker-build still targeted the
run-id-scoped self-hosted label, the job would queue until timeout. That failure mode is worth
handling deliberately rather than in passing.

Deliberately not in this PR

Doctests still don't run. nextest doesn't run them, but neither did cargo llvm-cov
without --doctests (nightly-only; the toolchain pins stable 1.88.0). So the ~50 doctest
blocks in common/utils, common/continuity and proof-gen-api-server have never executed
in CI. I preserved that rather than silently changing it: adding cargo test --doc may
surface pre-existing failures and deserves its own PR.

Docker's remaining 27-minute compile, on the runs where it genuinely is needed (52 of 72).
The gating above removes it where it is not. Making the build itself cheaper is still open, and
adding
cache-to type=gha does not fix it, for two independent reasons:

  1. COPY . /creditcoin-node/ at Dockerfile:39 (and again at :57) sits before
    RUN cargo build, so any file change invalidates the layer the Rust build rests on.
  2. cache-to type=gha needs the docker-container driver, but the job then depends on the
    images being in the local daemon (docker image save, Scout's local:// scans, docker push), and loading back into the image store is incompatible with the
    --sbom=true --provenance=mode=max attestations these builds already produce.

The real options are (a) a dependency-layer split (cargo-chef style) or (b) assembling the PR
image from binaries build-native already produced, keeping the from-source Dockerfile for
releases. Both are bigger than a flag and want their own PR.

Sharing one build across the four identical --release --features=fast-runtime builds
(ci, attestor-compat, validator-compat, proof-gen). Blocker to resolve first:
git-lfs-checkout is inconsistent (ci and proof-gen pass false, both compat workflows pass
true) and chainspecs/mainnetSpecRaw.json is LFS-tracked and include_bytes!-compiled at
node/src/chain_spec.rs:137, so the false builds embed a pointer stub and the artifacts are
not interchangeable today.

Verification

  • All 41 workflows parse as YAML.
  • actionlint clean on every changed file. The only findings left repo-wide are pre-existing
    SC2129s in check-snapshot.yml and runtime-upgrade.yml, both untouched. One SC2129 my own
    change introduced was fixed by grouping the $GITHUB_OUTPUT redirects.
  • yamllint: zero non-line-length findings on the changed files (line-length is not an
    enforced gate here; untouched docker.yml violates it in eight places today).
  • The metadata change is verified by CI itself: the six integration-test-cli legs that were
    red are the exact consumers of the artifact. Its one local blind spot is that
    upload-artifact roots a multi-path upload at the least common ancestor of its paths, which
    for cli/... plus common/... is the workspace, so path: . on download restores them in
    place. That cannot be exercised off a runner; build-native.yml's existing
    binary-for-testing-ci upload relies on the same rule. The consumers' git diff debug step
    shows an empty diff if it were ever wrong.
  • SKIP_WASM_BUILD=1 cargo check --release --workspace passes locally.
  • Only MegaLinter and should-run are required status checks, so nothing here can block a
    merge on a skipped job.

Worth watching after merge: splitting one Rust cache key into three raises total cache demand,
and the repo shares a 10 GB budget. If entries start evicting each other, the fix is to trim
what Swatinem/rust-cache saves rather than to re-merge the keys.

🤖 Generated with Claude Code

https://claude.ai/code/session_01EQ1x9L5zqW75yehx535TPi

@cursor

cursor Bot commented Sep 10, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Changes are limited to GitHub Actions and a local timing script; no runtime or auth code is touched, though PR gates shift (coverage/soak off the default path) and cache/docker skipping could hide regressions if filters are wrong.

Overview
This PR reduces pull-request CI time by fixing Rust cache thrashing, dropping duplicate native builds, and skipping Docker/node work when path filters say it cannot matter. It also adds .github/ci-timing.py so before/after snapshots can be compared with snapshot / compare.

Rust (rust.yml) stops lightweight jobs from saving an empty build-creditcoin-node cache over the real compile artifacts (save-if: false on fmt/machete/audit), gives check/clippy/test separate Swatinem keys, runs tests with cargo nextest and SKIP_WASM_BUILD, and warms caches on usc-dev pushes when manifests change. Coverage (cargo llvm-cov) and the 10k-case proptest soak move to new rust-coverage.yml (schedule + workflow_dispatch); PRs keep a 256-case simulation run.

Native builds: cache-warm.yml warms the Linode build-native cache without uploading artifacts; build-native.yml adds optional upload-artifacts, runner-family cache namespaces, and metadata via copied files instead of git apply on a diff. ci.yml invokes attestor/validator compatibility as workflow_call jobs that reuse binary-for-testing-ci, moves compat path filters into should-run, removes the no-op trigger-integration-test-cli, and fixes CLI integration to use the merge ref like other jobs.

Compat workflows cache immutable historical release zips keyed on a version-list digest; Docker adds detect-changes and build-node-image to skip the ~27m node image when only indexer/CLI paths changed; sanity.yml installs the packed CLI tarball in a clean container to replace docker’s CLI coverage.

Reviewed by Cursor Bugbot for commit 9147c2d. Bugbot is set up for automated code reviews on this repo. Configure here.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 7a15e33. Configure here.

Comment thread .github/workflows/attestor-compatibility.yml Outdated
@gluwa-bot

gluwa-bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Overview

Image reference gluwa/creditcoin3:latest gluwa/creditcoin3:latest
- digest dfb918c3c546 9f8e7fa65a7c
- tag latest latest
- provenance 7e92a94 9e265dd
- vulnerabilities critical: 3 high: 16 medium: 21 low: 4 unspecified: 13 critical: 2 high: 13 medium: 11 low: 3 unspecified: 11
- platform linux/amd64 linux/amd64
- size 430 MB 431 MB (+550 kB)
- packages 405 404 (-1)
Base Image ubuntu:26.04
also known as:
latest
resolute
ubuntu:26.04
also known as:
latest
resolute
rolling
- vulnerabilities critical: 1 high: 7 medium: 12 low: 1 unspecified: 2 critical: 1 high: 5 medium: 2 low: 0
Labels (1 changes)
  • ± 1 changed
  • 3 unchanged
-org.opencontainers.image.created=2026-06-27T04:19:04.617438+00:00
+org.opencontainers.image.created=2026-08-17T09:02:45.677319+00:00
 org.opencontainers.image.description=The Ubuntu container image maintained by Canonical

Ubuntu is a Debian-based Linux operating system that runs from the desktop to the cloud, to all your internet connected things.
It is the world's most popular operating system across public clouds and OpenStack clouds.
It is the number one platform for containers; from Docker to Kubernetes to LXD, Ubuntu can run your containers at scale.
Fast, secure and simple, Ubuntu powers millions of PCs worldwide.

 org.opencontainers.image.title=ubuntu
 org.opencontainers.image.version=26.04
Policies (2 improved, 0 worsened)
Policy Name gluwa/creditcoin3:latest gluwa/creditcoin3:latest Change Standing
Default non-root user No Change
No copyleft licenses ⚠️ 373 ⚠️ 361 -12 Improved
No fixable critical or high vulnerabilities ⚠️ 19 ⚠️ 15 -4 Improved
No high-profile vulnerabilities No Change
No outdated base images ⚠️ ⚠️ No Change
No unapproved base images No Change
Supply chain attestations No Change
Packages and Vulnerabilities (47 package changes and 15 vulnerability changes)
  • ➖ 1 packages removed
  • ♾️ 46 packages changed
  • 337 packages unchanged
  • ✔️ 15 vulnerabilities removed
Changes for packages of type deb (35 changes)
Package Version
gluwa/creditcoin3:latest
Version
gluwa/creditcoin3:latest
♾️ base-files 14ubuntu6.1 14ubuntu6.2
♾️ bsdutils 1:2.41.3-3ubuntu2 1:2.41.3-3ubuntu2.2
♾️ curl 8.18.0-1ubuntu2.3 8.18.0-1ubuntu2.5
♾️ diffutils 1:3.12-1 1:3.12-1ubuntu0.1
♾️ gnu-coreutils 9.7-3ubuntu2 9.7-3ubuntu2.1
♾️ gpgv 2.4.8-4ubuntu3 2.4.8-4ubuntu3.1
♾️ libattr1 1:2.5.2-4 1:2.5.2-4ubuntu0.1
♾️ libaudit-common 1:4.1.2-1build1 1:4.1.2-1ubuntu0.1
♾️ libaudit1 1:4.1.2-1build1 1:4.1.2-1ubuntu0.1
♾️ libblkid1 2.41.3-3ubuntu2 2.41.3-3ubuntu2.2
♾️ libbz2-1.0 1.0.8-6build2 1.0.8-6ubuntu0.1
♾️ libc-bin 2.43-2ubuntu2.3 2.43-2ubuntu2.4
♾️ libc-gconv-modules-extra 2.43-2ubuntu2.3 2.43-2ubuntu2.4
♾️ libc6 2.43-2ubuntu2.3 2.43-2ubuntu2.4
♾️ libcurl4t64 8.18.0-1ubuntu2.3 8.18.0-1ubuntu2.5
♾️ libgcrypt20 1.12.0-2ubuntu1 1.12.0-2ubuntu1.1
♾️ libmount1 2.41.3-3ubuntu2 2.41.3-3ubuntu2.2
♾️ libpam-modules 1.7.0-5ubuntu3.1 1.7.0-5ubuntu3.2
♾️ libpam-modules-bin 1.7.0-5ubuntu3.1 1.7.0-5ubuntu3.2
♾️ libpam-runtime 1.7.0-5ubuntu3.1 1.7.0-5ubuntu3.2
♾️ libpam0g 1.7.0-5ubuntu3.1 1.7.0-5ubuntu3.2
♾️ libpq5 18.4-0ubuntu0.26.04.1 18.6-0ubuntu0.26.04.1
♾️ libsmartcols1 2.41.3-3ubuntu2 2.41.3-3ubuntu2.2
♾️ libssh2-1t64 1.11.1-1ubuntu0.26.04.3 1.11.1-1ubuntu0.26.04.4
♾️ libssl3t64 3.5.5-1ubuntu3.3 3.5.5-1ubuntu3.5
♾️ libsystemd0 259.5-0ubuntu3.3 259.5-0ubuntu3.4
♾️ libudev1 259.5-0ubuntu3.3 259.5-0ubuntu3.4
♾️ libuuid1 2.41.3-3ubuntu2 2.41.3-3ubuntu2.2
♾️ login 1:4.16.0-2+really2.41.3-3ubuntu2 1:4.16.0-2+really2.41.3-3ubuntu2.2
♾️ mount 2.41.3-3ubuntu2 2.41.3-3ubuntu2.2
♾️ openssl 3.5.5-1ubuntu3.3 3.5.5-1ubuntu3.5
♾️ openssl-provider-legacy 3.5.5-1ubuntu3.3 3.5.5-1ubuntu3.5
♾️ perl-base 5.40.1-7ubuntu0.1 5.40.1-7ubuntu0.3
♾️ util-linux 2.41.3-3ubuntu2 2.41.3-3ubuntu2.2
♾️ zlib1g 1:1.3.dfsg+really1.3.1-1ubuntu3 1:1.3.dfsg+really1.3.1-1ubuntu3.1
Changes for packages of type golang (6 changes)
Package Version
gluwa/creditcoin3:latest
Version
gluwa/creditcoin3:latest
♾️ github.com/canonical/pebble 1.31.1-0.20260528050051-33f10658d3fd 1.32.2-0.20260721212935-faa1696b477d
♾️ github.com/gorilla/websocket 1.5.1 1.5.3
critical: 0 high: 0 medium: 1 low: 0
Removed vulnerabilities (1):
  • medium : GHSA--w67g--5rqw--f597
golang.org/x/net 0.40.0
critical: 1 high: 2 medium: 7 low: 0
Removed vulnerabilities (10):
  • critical : CVE--2026--39821
  • high : CVE--2026--46600
  • high : CVE--2026--33814
  • medium : CVE--2026--25680
  • medium : CVE--2026--42506
  • medium : CVE--2026--42502
  • medium : CVE--2026--27136
  • medium : CVE--2026--25681
  • medium : CVE--2025--58190
  • medium : CVE--2025--47911
♾️ golang.org/x/sys 0.33.0 0.46.0
critical: 0 high: 0 medium: 0 low: 1
Removed vulnerabilities (1):
  • low : CVE--2026--39824
♾️ golang.org/x/term 0.32.0 0.44.0
♾️ stdlib 1.26.3 1.26.5
critical: 1 high: 6 medium: 4 low: 0 unspecified: 2 critical: 1 high: 5 medium: 2 low: 0
Removed vulnerabilities (5):
  • high : CVE--2026--42504
  • medium : CVE--2026--27145
  • medium : CVE--2026--42507
  • unspecified : CVE--2026--42505
  • unspecified : CVE--2026--39822
Changes for packages of type npm (6 changes)
Package Version
gluwa/creditcoin3:latest
Version
gluwa/creditcoin3:latest
♾️ @types/node 26.1.2 22.7.5
♾️ node-gyp 13.0.1 13.0.2
♾️ picomatch 4.0.5 4.0.7
♾️ undici 8.10.0 8.10.2
♾️ undici-types 8.3.0 6.21.0
♾️ ws 8.21.2 8.21.3

@beqaabu
beqaabu requested review from a team, BradleyOlson64, DylanVerstraete, creditcoinprotoclaw, didac-gluwa, jakerumbles and mdbig1 and removed request for a team September 14, 2026 10:24
@DylanVerstraete

Copy link
Copy Markdown
Contributor

Reviewed commit 9b217090. I recommend addressing these three points before merging:

  1. [P1] The new 45-minute CLI timeout cuts off healthy test execution. .github/workflows/ci.yml:827 sets the limit below the suite's observed duration. Four matrix jobs hit it on this PR, with passing tests in the logs until cancellation. The same variants previously passed in 61–65 minutes. Please raise the limit with headroom, or shorten the suite before lowering it. Current timeout, successful baseline.

  2. [P2] The new caches need a default-branch producer to benefit fresh PRs. The rust-check, rust-clippy, rust-test, and historical-binary caches are populated by PR runs, but caches saved under refs/pull/.../merge cannot be restored by other PRs. There is no automatic default-branch producer for those keys; the new nightly simulation also sets save-if: false. Repeated runs of this PR can therefore look much faster while fresh PRs still start cold. Please add a trusted default-branch cache producer using matching keys and configurations. GitHub cache-scope documentation.

  3. [P2] Removing cli/src/** from Docker checks loses package-install coverage. The existing sanity job installs from the working directory, where development dependencies are available. The Dockerfile instead packs the CLI and installs its tarball in a separate runtime stage. A source change importing a development-only dependency can pass sanity but break the packaged CLI, so the existing sanity check is not a strict superset. Before removing this trigger, add a cheap smoke test that installs the packed tarball into a clean environment and runs the help commands. That would preserve this coverage without rebuilding Rust. npm installation semantics.

The Rust result is encouraging: 835 tests passed, and cargo-test finished in 6m27s with a cache hit. The remaining long wait in this run is attestor compatibility: approximately 85 minutes overall, including 47 minutes waiting for five checkpoints. A useful next investigation is accelerating the simulated chain while preserving the five-checkpoint assertion. Rust run, compatibility run.

Reviewed all eight changed files and live CI logs; no local full-build rerun.

@DylanVerstraete

DylanVerstraete commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Beqa asked me to fold the useful half of #1359 into this branch and close that one, so I pushed 58410b3 here. Two changes, both in service of the "known gap" your cache-warm.yml comment already names.

A self-hosted warmer. The hosted job covers the two compatibility builds. ci.yml's build-creditcoin-node-for-testing-ci and proof-gen's build run the same --release --features=fast-runtime configuration on a Linode VM with no default-branch producer, so they stay cold on every fresh PR. The new warm-build-creditcoin-node-linode job plus its deploy/remove pair fills that namespace. Cost is one VM per manifest change on usc-dev, not per PR, and deploy-runner.yml is added to the push paths.

Key separation, which the above needs to work at all. Hosted and self-hosted runners compute the same Swatinem key today: same arch, same rustc, same manifests, and build-native.yml sets no CXXFLAGS on either side, so the env hash matches too. I confirmed that from live cache entries, which show ...-6cf672bd-... for every build-native.yml caller regardless of runner, against ...-ade765bf-... for rust.yml, exactly the CXXFLAGS split your comment describes. Their Cargo home and checkout root differ, and target/ fingerprints and .d files record absolute paths, so a cross-family restore fails every fingerprint check and rebuilds after paying for the download. Without a split the hosted warmer just wins the immutable entry and the Linode one can never save. build-native.yml now folds the runner family and $HOME into the shared key, and defaults cache-key-name so the one caller that omits it, proof-gen, stays in the namespace it should be sharing.

Two things I noticed while reading, both for you rather than changes I made:

  • The warm-build-creditcoin-node comment says it mirrors the inputs both compatibility workflows pass, but it passes git-lfs-checkout: false where they pass true. That does not affect the cache key, since LFS only changes which files are checked out, so the warming still works. The comment is just inaccurate.
  • ci: build the SUT once and share it with both compatibility suites #1360 removes both compatibility builds in favour of a shared artifact. When it lands, the hosted warmer here has no consumers left and the Linode one becomes the only one that matters. Worth deleting the hosted job in that PR rather than leaving it running weekly for nobody.

Independently of this branch, I verified the diagnosis holds: 70 of 71 cache entries are on refs/pull/*/merge with the sole branch-scoped entry being a yarn cache, and a 33-second machete job and a 41-minute clippy job on #1344 both logged a restore of the identical key with full match: true, 822 MB of registry with essentially no compiled output.

BradleyOlson64
BradleyOlson64 previously approved these changes Sep 14, 2026

@BradleyOlson64 BradleyOlson64 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty excited for this one to land 😆

mdbig1
mdbig1 previously approved these changes Sep 15, 2026
@beqaabu
beqaabu dismissed stale reviews from mdbig1 and BradleyOlson64 via 193b6bd September 16, 2026 15:02
beqaabu and others added 7 commits September 16, 2026 20:10
Measured baseline over recent PR runs: median end-to-end wait 16 min, p90 91 min,
max 144 min. The tail is what hurts, and it is not caused by jobs firing too
eagerly. Replaying each workflow's real path filter over the last 80 commits shows
the gating mostly works already (try-runtime 8%, benchmarks 6%, chainspec 20%).
The cost is that the work each job does is duplicated and uncached.

rust.yml

All six jobs passed `shared-key: build-creditcoin-node`, and so do eight other
files. Cache keys are immutable, so cargo-fmt (2 min) and cargo-machete (0 min)
saved first with an essentially empty target/ and the 21/43/55-minute jobs had
their artifacts discarded every run. Those three now restore but never save
(`save-if: false`); cargo-check, cargo-clippy and cargo-test get their own keys,
since their fingerprints (release+default, debug+all-features, release+coverage)
are mutually unusable anyway.

Also: SKIP_WASM_BUILD=1 on cargo-clippy and cargo-test, neither of which consumed
the runtime wasm blob it was building; removed `cargo check
--features=runtime-benchmarks --release`, which clippy's --all-targets
--all-features strictly subsumes; and job-level timeout-minutes throughout.

cargo-test now runs `cargo nextest run --release --workspace` instead of
`cargo llvm-cov`. Coverage instrumentation injects -C instrument-coverage into
RUSTFLAGS, changing every dependency's fingerprint and forcing a fourth from-cold
compile of the whole graph per PR, and nothing gated on the result: no codecov
upload, no threshold, no PR comment, only an HTML artifact opened by hand. It
moves to the new rust-coverage.yml (nightly + workflow_dispatch) along with the
PROPTEST_CASES=10000 soak and its regression-file auto-commit.

The remaining `cargo check --release` is kept deliberately. clippy --all-features
turns runtime-benchmarks ON, so it never compiles the
#[cfg(not(feature = "runtime-benchmarks"))] blocks in runtime/src/lib.rs,
node/src/client.rs and node/src/command.rs, and nothing else in CI type-checks the
graph with default features on a usc-dev PR.

attestor-compatibility.yml / validator-compatibility.yml

Both re-downloaded every historical release zip from the API on every run, though
published assets are immutable. Now cached, keyed on a new `digest` output from
discover-versions (a short sha256; the raw JSON version array is unsafe in a cache
key). The unzip and --version checks still run on a hit, so a corrupt entry fails
loudly. Only the archives are cached, to limit the draw on the repo-wide cache
budget the Rust caches share.

Attestor's build-sut no longer needs discover-versions, which was gating a
~46-minute build behind an API-paging job whose output it does not use.

ci.yml

Dropped the no-op trigger-integration-test-cli job, whose only step was a checkout
and which, because it also waited on the node build, could not report its skip
decision until that build had finished; integration-test-cli is now gated directly.
Added timeout-minutes to all 10 non-reusable jobs, which previously had none, so a
hang held a runner for GitHub's 360-minute default.

.github/ci-timing.py

Snapshot/report/compare over the Actions API, so this work can be measured rather
than asserted. Queue time counts, because waiting for a runner is waiting.

Doctests are deliberately still not run: nextest does not run them, but neither did
cargo llvm-cov without --doctests (nightly-only, and the toolchain pins stable
1.88.0), so the ~50 doctest blocks in common/utils, common/continuity and
proof-gen-api-server have never executed in CI. Closing that gap may surface
pre-existing failures and belongs in its own change.

Verified: all 41 workflows parse; actionlint clean on every changed file (the only
findings left are pre-existing SC2129s in two untouched workflows); yamllint reports
no non-line-length issues; `SKIP_WASM_BUILD=1 cargo check --release --workspace`
passes locally.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EQ1x9L5zqW75yehx535TPi
The node image's Rust compile is 27.2 of the docker-build job's ~29 minutes
(BuildKit stage `#21 [rust-builder 4/4]`); every other stage together is ~90s.
Measured over 160 commits, 20 of 72 Docker runs rebuilt it for changes that
cannot affect it.

Two separate cases, and neither loses coverage the old CI had:

cli/src/** is dropped from the triggers. A CLI-only change cannot touch the Rust
binaries, and the one thing this workflow uniquely proved about the CLI, that it
packages and installs and then answers --help, is already asserted on EVERY PR by
sanity.yml's npm-packages job: `yarn build; npm install -g .; which creditcoin;
creditcoin help / wizard --help / attestor --help`. That is a superset of
docker-test's three `docker exec` CLI checks and costs about a minute. The rest of
docker-test (chainspecs, other binaries, compose) exercises an image that a
cli/src change leaves byte-identical.

cc3-indexer/** still triggers the workflow but no longer builds the node image.
The root Dockerfile has no reference to cc3-indexer, and the indexer and
stress-test images come from their own external bases (subql-node-substrate,
deno) rather than deriving from the node image, so both are still built and still
CVE-scanned. docker-test skips with the node image because every assertion in it
runs against that image.

Mechanically: a `detect-changes` job computes whether the node image is needed and
passes it to build-docker.yml's new `build-node-image` input, which gates the node
build, its Scout compare, and the export/upload of the image artifact. The input
defaults to true, so release.yml, which omits it, is unaffected. A manual
workflow_dispatch run has no base ref to diff, so it forces the value to true
rather than silently skipping.

The job is named detect-changes, not should-run, because `should-run` is a required
status check produced by ci.yml and a second job of that name would emit a
colliding context into branch protection.

One honest caveat: on a cli/src-only PR the Docker Scout comparison no longer runs,
so a CVE introduced by a new CLI npm dependency would surface in the weekly Scout
report and Dependabot rather than on the PR. That step is already
`continue-on-error: true` with `exit-code: false`, so it never gated anything.

Replayed against the last 160 commits: workflow runs drop 72 -> 61, node image
builds drop 72 -> 52, avoiding 20 builds of ~27 min (27% of Docker runs).

Verified: all 42 workflows parse; actionlint clean; yamllint reports no
non-line-length issues on either changed file.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EQ1x9L5zqW75yehx535TPi
MegaLinter's checkov step failed on the new workflow with CKV_GHA_7 ("the build
output cannot be affected by user parameters other than the build entry point and
the top-level source location"), pointing at rust-coverage.yml's
workflow_dispatch.inputs.ref.

The input was redundant anyway: workflow_dispatch already presents a ref selector,
and actions/checkout defaults to the dispatched ref, so dispatching the workflow
against a chosen branch works exactly as before without it.

Verified with checkov 3.3.2 (the version MegaLinter pins) against the repo's own
.checkov.yml: 2435 passed, 0 failed, where the previous commit gave 2434 passed and
1 failed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EQ1x9L5zqW75yehx535TPi
integration-test-cli was cancelled at 45 minutes on this PR. That was my own
timeout, set from too small a sample. Measured across 45 recent CI runs
(successful and failed): median 62 min, p90 98, max 99. Raised to 150.

Timeouts here are a safety net against a hung job holding a runner for GitHub's
360-minute default, not a performance budget, so every value is now roughly 2-3x
the observed p90 rather than a tight fit:

  integration-test-cli                 45 -> 150   (p90 98)
  integration-test-attestator-network  90 -> 120   (max 42)
  integration-test-blockchain          60 ->  90   (max 25)
  attestor-cli-testing                 60 ->  90   (max 35)
  cc3-indexer-testing                  45 ->  90   (max 28)
  docs-smart-contract-...-hardhat      45 ->  90   (no sample)
  attestor/validator-compatibility     90 -> 120   (55 observed here; ~50 is the
                                                    documented soak budget)

Unchanged where the margin was already large: archiver-testing 45 (max 4),
regenerate-metadata 30 (max 3), integration-test-check-extrinsics 30 (max 2),
should-run 10, and the rust.yml budgets (cold: clippy 20, check 18, test 34).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EQ1x9L5zqW75yehx535TPi
Addresses two of the three review points; the first (the 45-minute CLI timeout) was
already fixed in the commit after the one reviewed, raised to 150 against a measured
p90 of 98.

Default-branch cache producer
-----------------------------
Confirmed the gap: every Rust-building workflow in this repo (rust, ci, docker, wasm,
chainspec, both compat, proof-gen, benchmarks, try-runtime) was triggered by
pull_request alone. A cache saved during a PR lives under refs/pull/<n>/merge and no
other PR can restore it, so nothing ever wrote an entry a fresh PR could use. That is
why re-running this PR looked fast while a new PR would still start cold.

rust.yml now also runs on push to usc-dev, populating rust-check / rust-clippy /
rust-test. It deliberately re-runs the same jobs rather than reimplementing them
elsewhere, because identical steps, env and shared-key are what make the entry
restorable.

cache-warm.yml covers the separate key that build-native.yml callers use, which is the
one both compatibility builds restore. It calls build-native.yml rather than running
cargo directly, for a concrete reason: Swatinem/rust-cache hashes CXX-prefixed env vars
into the key, and rust.yml sets CXXFLAGS while build-native.yml does not, so the two
compute different keys despite passing the same shared-key. Warming has to reproduce
the consumer's environment exactly. This also explains the reviewer's observation that
builds sharing a shared-key restored different keys.

Both triggers list only Cargo manifests and the toolchain file, not **.rs. The key is
derived from the manifests, and the cache holds dependencies rather than workspace
crates, so an ordinary source change leaves the existing key valid and the warmed entry
still applies. Warming on every push would re-pay a full build to refresh nothing.

build-native.yml gains an `upload-artifacts` input (default true, so existing callers
are unchanged) so the warming run does not leave ~200 MB of binaries nothing downloads.

Packaged-CLI coverage
---------------------
The review is right that sanity.yml's npm-packages job is not a strict superset of what
docker-test checked, and my earlier claim overstated it. `npm install -g .` runs on a
fully provisioned runner; the Dockerfile instead runs `yarn pack` and installs the
tarball into a clean stage built FROM runtime-base, which has node, yarn and node-gyp
but no compiler toolchain and none of the repo's node_modules. A CLI change adding a
dependency that builds from source would pass the former and break the latter.
cli/package.json already depends on bls-signatures-bindings, so this is not theoretical.

sanity.yml now also packs the tarball and installs it in a clean node:24-slim container
(node-gyp installed first, so the environment mirrors runtime-base rather than being
stricter), then runs the same help commands. Verified locally end to end: the tarball
installs in 26s and all three commands succeed.

Verified: all 43 workflows parse; actionlint clean on every changed file; checkov 2446
passed, 0 failed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EQ1x9L5zqW75yehx535TPi
…hosted one

The hosted warmer covers the two compatibility builds. ci.yml's
build-creditcoin-node-for-testing-ci and proof-gen-api-server build the same
`--release --features=fast-runtime` configuration on a Linode VM and had no
default-branch producer, which is the largest block of PR wall-clock left once
the hosted warmer is in place. This adds that producer: one VM per manifest
change on usc-dev, not per PR.

It needs key separation to work. Hosted and self-hosted runners compute the
same Swatinem key today — same arch, same rustc, same manifests, and
build-native.yml sets no CXXFLAGS on either side, so the env hash matches as
well (`...-6cf672bd-...` on both). Their Cargo home and checkout root differ,
and target/ fingerprints and .d files record absolute paths, so an archive
restored across families fails every fingerprint check and rebuilds anyway
after paying for the download. Without the split the hosted warmer simply wins
the immutable entry and the Linode warmer can never save.

build-native.yml therefore folds the runner family and $HOME into the shared
key, and defaults `cache-key-name` so the caller that omits it
(proof-gen-api-server) stays in the same namespace as the warmer it should be
sharing with.

Moved here from #1359, which is closed in favour of this branch.
…1360)

* ci: build the SUT once and share it with both compatibility suites

Both compatibility workflows built their own system-under-test with
`--release --features=fast-runtime`, the same configuration ci.yml already builds on
every PR. In the run I measured, both restored an identical rust-cache key
(v0-rust-build-creditcoin-node-Linux-x64-ade765bf-1dabb174) with "full match: true"
and still spent 21m42s and 20m58s compiling, because Swatinem/rust-cache stores
dependencies and deliberately excludes workspace crates. Sharing a cache was never
going to fix this; sharing the binary is.

Both workflows become reusable (workflow_call) and take a `sut-artifact` input. ci.yml
calls them after build-creditcoin-node-for-testing-ci and passes binary-for-testing-ci,
which build-native.yml already populates with all seven binaries, including the
attestor and attestor_zombienet that the attestor suite needs.

Their pull_request path filters move verbatim into ci.yml's should-run as the
attestor_compatibility and validator_compatibility filters; a parity check confirms the
lists are byte-identical, so exactly the same commits trigger exactly the same suites.
Since a matching filter makes should-run's `changes` output non-empty, the shared build
is always present when either suite runs.

git-lfs-checkout is dropped rather than standardised upward. The only LFS-tracked file
compiled into the node is chainspecs/mainnetSpecRaw.json, reached solely through
mainnet_config() at node/src/chain_spec.rs:137, and both suites run --chain dev only
(attestor starts the node with --chain dev; validator derives its shared genesis with
build-spec --chain dev --raw). validator's chainspecs/** entry is a trigger, not a
runtime read. Dropping LFS also removes ~193 MB of embedded spec from the binary, which
speeds the artifact upload and both downloads.

Details worth knowing:

  * The called workflows declare no `concurrency`. It is inherited from the calling
    run, and a group keyed on github.workflow would have collided with ci.yml's.
  * deploy-runner no longer lists build-sut in `needs`. The intent behind that edge
    (never pay for Linode time on a failed build) is preserved by the caller: ci.yml
    only invokes these workflows after the build succeeds.
  * Three Linode VMs now exist under one github.run_id. They cannot collide:
    deploy-runner.yml derives the VM name from a hash of "proxy / secret / type", and
    all three differ (proxy-build-creditcoin-node-for-testing-ci vs
    type-attestor-compatibility vs type-validator-compatibility). The runs-on label
    sets differ for the same reason.
  * Nesting is ci.yml -> compat -> deploy-runner, three levels, within GitHub's limit
    of four.
  * Incidentally fixes a latent artifact-name clash: both suites uploaded
    binary-for-PR, the same name runtime-upgrade.yml uses for a different build
    (--release, no fast-runtime).

Over the last 160 commits the attestor suite fires 33 times and the validator suite 26,
so this removes 59 duplicate builds, roughly 1240 runner-minutes at warm-cache speed and
far more cold. Wall clock per PR is unchanged: the critical path was already
build -> provision -> test, and it still is, just with one build instead of three.

Verified: all 43 workflows parse; actionlint clean; checkov 2438 passed, 0 failed; and a
call-graph check confirms every `uses:` target exists and declares workflow_call, every
required input is supplied with no unknown ones, every `needs:` and every
should-run output reference resolves, and nesting stays within the limit.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EQ1x9L5zqW75yehx535TPi

* ci(cache-warm): drop the hosted warmer, which this PR leaves without consumers

The hosted warmer existed to feed the two compatibility builds. This PR replaces those
builds with a download of ci.yml's artifact, so nothing hosted calls build-native.yml any
more. Enumerating every caller confirms it: ci.yml (x2), proof-gen-api-server, benchmarks,
chainspec (x2), runtime-upgrade and release all run on self-hosted Linode VMs, and after
this change the hosted warmer was the only hosted caller left. Since 58410b3 keys the
two runner families into separate namespaces, it was warming an entry no job can read.

The self-hosted warmer stays and is now the only one, which is the right shape: it feeds
the namespace every remaining caller actually restores.

Also corrects two comments that 58410b3's split left pointing at the wrong thing: the
file header described the compatibility builds as the consumers of this key, and the
Linode job described itself as the second half of a hosted/self-hosted pair.

This is the deletion flagged when the Linode warmer landed, rather than leaving a weekly
build running for nobody.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EQ1x9L5zqW75yehx535TPi

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@beqaabu
beqaabu force-pushed the ci/reduce-pr-wall-clock branch from 193b6bd to 7d70a99 Compare September 16, 2026 16:10
regenerate-metadata uploaded `git diff --binary` and its three consumers ran
`git apply` on it. A diff records the blob it was computed against, so this only
works while the producer and every consumer check out an identical tree. They do
not: regenerate-metadata and attestor-cli-testing use a plain checkout, i.e. the
PR merge ref, while integration-test-cli and rebuild-new-metadata pinned the
branch head. Any PR whose branch was behind a base that had moved
`metadata.scale` therefore failed with

    error: the patch applies to 'common/cc-client/artifacts/metadata.scale'
           (d5f373c...), which does not match the current contents.

Upload the three regenerated paths themselves instead, the same set
rebuild-new-metadata commits back, and let the consumers copy them into place.
There is no pre-image to match, so the step cannot fail this way regardless of
which ref each job sits on. `if-no-files-found` goes from warn to error, since a
missing artifact now means consumers silently test stale metadata. Incidental
`cli/yarn.lock` churn is no longer carried over; every consumer runs its own
`yarn install`.

integration-test-cli also drops its branch-head checkout pin. That pin was
inherited from the commit that introduced the typedef auto-update, where it is
required because the job commits back; only rebuild-new-metadata does that. Here
it meant the job ran CLI sources from the un-merged branch against a node binary
that build-creditcoin-node-for-testing-ci had compiled from the merge ref.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@beqaabu
beqaabu merged commit d1ff8d1 into usc-dev Sep 16, 2026
56 of 58 checks passed
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.

5 participants