ci: share Rust caches by build class and save on failure#520
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe Rust composite action now supports configurable dependency-cache grouping and optional sccache. Workflows assign host or contracts cache keys, centralize Rust build settings, and configure shared disk cleanup with coverage-specific cache isolation. ChangesRust CI configuration
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/actions/rust/action.yml:
- Line 43: Update the shared-key expression in the Rust cache action so
cache-key: host is partitioned by build class, preventing clippy, binary, test,
integration-test, artifact-check, and gas-reporting jobs from sharing one
immutable archive. Include the relevant build-class input or identifier in the
key while preserving the existing runner OS and cache-key components.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0c13ee8f-2800-4217-b2e8-4962e6f0094c
📒 Files selected for processing (3)
.github/actions/rust/action.yml.github/workflows/gas-report.yml.github/workflows/test.yml
Status: caching works; NEAR now red on a pre-existing dev regression (not this change)What this PR does (working as intended)
Why NEAR is red anywayCompleting the suite for the first time unmasked 2 pre-existing failures on
Root cause: #519 (ENG-481) added an all-or-nothing migration guard in This is broken on BlockerThis PR can't go green until dev's migration tests are fixed (ENG-517 / a test update). That's a contract concern owned by ENG-481/ENG-517, not this CI change. Will rebase onto current Known follow-up (not blocking)The wasm contract prebuild is still uncached ( |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c9325a7fbf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| - name: Free disk space | ||
| uses: ./.github/actions/free-disk-space |
There was a problem hiding this comment.
This local action path now runs before actions/checkout, so ./.github/actions/free-disk-space is not present in $GITHUB_WORKSPACE when the runner tries to resolve it. The binary-build job will fail before reaching checkout, and the same ordering is repeated in fast-tests, near-integration-tests, and coverage, so the required gate can fail whenever these jobs run; move checkout before the local action or keep this cleanup inline.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/actions/rust/action.yml:
- Line 56: Update the save-if condition in the Rust action to require the
workflow ref to be the repository’s default branch for push, schedule, and
merge_group events. Preserve the existing event gating while preventing pushes
from main/dev or other non-default branches from writing to the cache.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c0078aaf-4273-49ed-a396-fc51f66454e1
📒 Files selected for processing (3)
.github/actions/free-disk-space/action.yml.github/actions/rust/action.yml.github/workflows/test.yml
| # caches; PR runs restore-only. This keeps feature branches from bloating | ||
| # the 10 GB budget or racing to write partial caches, and gives every PR a | ||
| # complete, dev-seeded cache to restore. | ||
| save-if: ${{ github.event_name == 'push' || github.event_name == 'schedule' || github.event_name == 'merge_group' }} |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
for workflow in $(fd -t f -e yml -e yaml .github/workflows); do
if rg -q 'uses:\s*\./\.github/actions/rust' "$workflow"; then
echo "===== $workflow: triggers ====="
sed -n '1,140p' "$workflow"
echo
fi
doneRepository: Templar-Protocol/contracts
Length of output: 538
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "Workflow files:"
fd -t f . .github/workflows
echo
echo "Workflows referencing ./.github/actions/rust:"
for workflow in $(fd -t f . .github/workflows -e yml -e yaml); do
if rg -q 'uses:\s*\./\.github/actions/rust' "$workflow"; then
echo "===== $workflow ====="
sed -n '1,180p' "$workflow"
echo
fi
doneRepository: Templar-Protocol/contracts
Length of output: 10650
Limit cache writes to the default branch
save-if currently allows any push, and this action is used from .github/workflows/documentation.yml on dev and .github/workflows/test.yml on main/dev, so non-default branches can still write to the cache. Gate this on the default branch if you want cache writes to stay centralized.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/actions/rust/action.yml at line 56, Update the save-if condition in
the Rust action to require the workflow ref to be the repository’s default
branch for push, schedule, and merge_group events. Preserve the existing event
gating while preventing pushes from main/dev or other non-default branches from
writing to the cache.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d34415440b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| shell: bash | ||
| run: | | ||
| { | ||
| echo "SCCACHE_GHA_ENABLED=true" |
There was a problem hiding this comment.
In pull_request runs, rust-cache is now restore-only via save-if, but this step enables sccache's GHA backend in its default read/write mode, so the same PR jobs still upload compiler outputs outside that guard. Because this action runs in the linter, build, fast-test, and NEAR jobs, feature branches can still consume the repo's GitHub cache quota and evict the default-branch host/contracts caches this change is trying to protect; set SCCACHE_GHA_RW_MODE=READ_ONLY or disable the GHA backend for non-seeding events.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 99e1af07c9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # Group by build class via `cache-key`, not `github.job`. Do not put raw | ||
| # target/component strings here: rust-cache rejects keys with commas, and | ||
| # comma-separated target lists are common in this repo. | ||
| shared-key: "rust-${{ runner.os }}-${{ inputs.cache-key || github.job }}" |
There was a problem hiding this comment.
Because save-if is true on default-branch/merge-queue runs and several jobs pass cache-key: host, this shared-key makes artifact-drift-check, binary-build, code-linter, and fast-tests all write the same exact rust-cache key. The GitHub cache docs state an existing cache cannot be changed, so if the small artifact-drift or binary job finishes first after a lockfile/key change, it seeds a partial host cache and the later test/clippy jobs restore an exact hit instead of replacing it with their fuller dependency artifacts; PRs then keep rebuilding test-only deps, undermining the timeout/cache fix. Make only the intended complete host producer save this key, or give producers distinct save keys with a shared restore prefix.
Useful? React with 👍 / 👎.
| if !reproducible { | ||
| command.arg("--no-abi"); | ||
| } |
There was a problem hiding this comment.
The cargo-near v0.19.2 README describes cargo near build as building the contract along with its ABI, and the near-sdk README calls --no-abi a way to skip schema/ABI errors. This non-reproducible branch is used by spawn_artifact_build for script/prebuild-test-contracts.sh, which feeds both just test-sandbox and script/gas-report.sh, so the required CI paths no longer exercise ABI/JsonSchema generation at all; a contract-facing type can pass tests but fail the release/reproducible build. Keep at least one required cargo-near build without this flag.
Useful? React with 👍 / 👎.
Final state — node suite <40 min, caching fixed, workflows cleaned upReworked down to a small diff (+78/−78, 4 files) after measuring several approaches and reverting the ones that didn't pay off. Root cause of the 40 min timeout
What shipped
Measured results (NEAR Integration)
Caching demonstrably works: cold run seeds all per-job caches (including the failing NEAR job), warm rerun restores them and cuts the prebuild in half. Tried and reverted (with data)
Still red (out of scope)The two |
- Replace four copy-pasted "free up disk space" blocks with one
`free-disk-space` composite action (with an `aggressive` input for coverage).
It runs after checkout, since a local action needs the repo present.
- Hoist the env vars repeated across jobs (CARGO_TERM_COLOR, CARGO_INCREMENTAL,
SQLX_OFFLINE, CARGO_PROFILE_{DEV,TEST}_DEBUG) into one workflow-level `env`
block; jobs keep only what's job-specific.
- Raise the node-integration timeout to 45 min. It's a ceiling, not billed time
(~30 min warm / ~39 min cold), so the headroom just avoids a cold-run variance
kill wasting a whole run.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
The node-integration job had `cache-on-failure: false` and timed out before rust-cache's post-step ran, so it never seeded a cache: cold build -> timeout -> still cold, permanently. Set `cache-on-failure: true` so it seeds even on a failing/timed-out run. Safe because CI runs CARGO_INCREMENTAL=0, so there is no incremental state to leave corrupt. Caching stays on per-job keys + save on same-repo PRs (rust-cache defaults), so each cache matches its job's artifact kind and a PR's later runs restore its earlier compilation. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Pass `--no-abi` to the `cargo near build` used for test artifacts. cargo-near otherwise runs a separate ABI-generation build per contract — measured at ~7.3 min (35%) of the ~20.6 min contract compile — and nothing in the suite reads the embedded ABI (tests only load and deploy the `.wasm`). This cuts the cold prebuild ~24 -> ~18 min, bringing the node suite under 40 min. Reproducible release blobs (`res/near`) keep their ABI. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
43b6cf1 to
5395027
Compare
1 participant
Problem
CI Rust compile times run ~25 min, with
near-integration-teststiming out at 40 min. The cache config in.github/actions/rust/action.ymlis the cause:shared-key: rust-${os}-${github.job}gives every job its own multi-GB Rust cache. Across all workflows that's ~11 near-identical caches — well past GitHub's 10 GB per-repo limit, which triggers LRU eviction. The least-frequently-run jobs (NEAR integration, coverage — both change-gated) are the first evicted, so they rebuild from cold nearly every run.cache-on-failure: false. A job slow enough to hit itstimeout-minutesis cancelled before rust-cache's post-step saves, so it can never seed a warm cache: cold build → timeout → still cold. Self-reinforcing.Change
cache-keybuild-class input to the rust composite action, falling back to the job id when unset.code-linter,binary-build,fast-tests,near-integration-tests,artifact-drift-check) plusgas-reportonto one sharedhostcache.coverage(instrumented),soroban/feature-matrix(wasm / no_std) keep private keys via the fallback — their artifacts don't share.cache-on-failure: trueso a timing-out job still seeds its cache. Safe: CI runsCARGO_INCREMENTAL=0, so there's no incremental state to corrupt.Net: distinct Rust caches drop ~11 → ~6, keeping the total under 10 GB so gated jobs stop getting evicted into cold rebuilds.
Validation
Reasoned from config, not from cache-hit logs — the real proof is this PR's own runs. Editing
test.ymltriggers the NEAR + soroban gates, so the suite exercises the change. Watchnear-integration-testscache restore/save and wall-clock across the first couple of runs (the very first run is expected to still be cold-ish while it re-seeds the newhostkey).🤖 Generated with Claude Code
This change is