From bd51f157e9aade37f2f4602eda238fe51405a0c2 Mon Sep 17 00:00:00 2001 From: Nicky Leach Date: Tue, 15 Sep 2026 17:24:22 -0700 Subject: [PATCH] fix(ci): make the Runner Rust cache key independent of the image toolchain (#13500) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Every change goes through the pull request CI workflow, and its `Verify Paperclip Runner` lane builds and tests the native Rust runner > - https://github.com/paperclipai/paperclip/pull/13457 made that lane restore master's prebuilt Rust dependency cache instead of recompiling 313 crates > - Measured over 26 runs since it went live, the cache hits on the RunsOn fleet and misses on every GitHub-hosted runner > - The cause is the cache key: `rust-cache` hashes every installed toolchain, and each runner image ships a different stable Rust next to the pinned one > - This pull request removes the extra toolchains before the key is computed, in the reader and the writer > - The benefit is that the saving the cache already delivers, 4.8 minutes per run, reaches the 73% of runs that currently miss it ## Linked Issues or Issue Description No public GitHub issue exists for this. The problem follows the enhancement issue template below. - Refs https://github.com/paperclipai/paperclip/pull/13457 — added the cache this pull request repairs - Refs https://github.com/paperclipai/paperclip/pull/13459 — activated it - Refs https://github.com/paperclipai/paperclip/pull/13194 — created the `release-runner-v1` entry on master I searched this repository for other pull requests touching this cache and found no duplicate and nothing in flight. **What existing behavior does this improve?** The Rust dependency cache added in #13457 misses on GitHub-hosted runners, so most pull requests still recompile the whole dependency tree. **Subsystem affected** Cross-cutting (multiple of the above). The change touches CI workflow configuration only. It does not change product code. **Current behavior** The cache works, but only on one runner class. Across 26 successful `Verify Paperclip Runner` jobs since #13459 merged: | Runner | Runs | Before | After | Change | Cache | |---|---|---|---|---|---| | RunsOn fleet | 7 | 16.4m | 11.6m | −4.8m | 4 of 4 full hit | | ubuntu-latest | 19 | 14.9m | 14.7m | −0.2m | 0 of 6 hit | | All | 26 | 15.0m | 13.9m | −1.1m | | Only 27% of runs reach the fleet, so the fleet-wide saving is 1.1 minutes rather than the 4.8 minutes the cache delivers where it lands. The two runners compute different keys: ``` fleet: v0-rust-release-runner-v1-Linux-x64-4bb3b8ea-a95b0328 ubuntu-latest: v0-rust-release-runner-v1-Linux-x64-9fdc73e3-a95b0328 ``` The lockfile half agrees. The environment half does not. `rust-cache` logs why, under `Environment considered`: | Runner | Toolchains it found | |---|---| | fleet | 1.97.1 and **1.98.0** | | ubuntu-latest | 1.97.1 and **1.98.1** | `rust-cache` hashes every installed toolchain, not only the active one. Both images carry the pinned 1.97.1. Each also ships its own stable Rust, and those differ by a patch version. The post-merge writer runs on a RunsOn image, so the fleet agrees with it and GitHub-hosted runners cannot. Pinning `RUSTUP_TOOLCHAIN` in #13457 was necessary but not sufficient. It fixes which toolchain builds the code. It does not change which toolchains exist on the image. **Proposed behavior** Remove every toolchain except the pin, before the cache step, in both the reader and the `release-runner-v1` writer. The key then depends on the pinned compiler and the lockfile alone, not on what the image happens to carry. **Reason and benefit** The cache already proves its value where it lands: release compile drops from 5m07s to 1m30s, debug from 1m48s to 13s, and the job from 16.4m to 11.6m. This change extends that to the other 73% of runs. Expected fleet-wide mean: about 11.5m, against 13.9m today and 15.0m before #13457. It also removes a standing fragility. The fleet hits today only because two RunsOn images happen to agree. If either image updates its stable Rust on its own, the hit rate drops to zero with no code change. **Breaking changes** None. The change only affects cache key computation. A miss reproduces the current behavior. **Additional context** The typecheck writer in `release-verify.yml` keeps its current step on purpose. It restores and saves on the same post-merge image, so its key never disagrees with itself. ## What Changed - Added a toolchain normalization block to `Select the pinned Runner Rust toolchain` in `.github/workflows/pr-trusted.yml`, before the cache restore. It keeps the pinned toolchain and uninstalls the rest. - Added the identical block to the same step in the `verify_paperclip_runner` job of `.github/workflows/release-verify.yml`, which writes `release-runner-v1`. The reader and the writer must agree, or the key matches nothing. - Made the block tolerant. If a toolchain cannot be removed it prints a notice and continues, so a pull request loses the cache rather than the run. - Extended `.github/scripts/tests/pr-runner-rust-cache.test.mjs` with two tests: the block is byte-identical in both workflows, and it runs before the cache step in each. ## Verification Run the workflow shape tests: ```bash node --test '.github/scripts/tests/*.test.mjs' ./scripts/__tests__/e2e-shard.test.mjs ./scripts/__tests__/release-verify-workflow.test.mjs ./scripts/__tests__/run-vitest-stable-shard.test.mjs ./scripts/cloud-source-verification.test.mjs ``` Result: 471 pass, 0 fail. I ran the step body against a stub `rustup` to confirm the logic, rather than only checking syntax. Three paths, all exit 0: | Case | Result | |---|---| | Pin plus an extra stable toolchain | Uninstalls only the extra, exports `RUSTUP_TOOLCHAIN=1.97.1-x86_64-unknown-linux-gnu` | | Pin only, already normalized | No uninstall calls, no error | | No `rustup` on `PATH` | Prints the notice and continues | I also mutation-tested the new parity assertion. Each mutation edits only the writer, then the reader and writer disagree: | Mutation to `release-verify.yml` | Result | |---|---| | One word changed in the shared comment | Caught | | `uninstall` changed to `remove` | Caught | | Trailing `rustup toolchain list` deleted | Caught | After this merges, confirm the repair in CI. Take a `Verify Paperclip Runner` job that ran on `ubuntu-latest` and check the restore step for `full match: true`. Under `Environment considered`, `Rust Versions` must list only 1.97.1. The job should finish near 11.5m rather than 14.7m. ## Risks - **Master must republish the cache once.** This changes the key, so the existing `release-runner-v1` entry no longer matches. `cloud-readiness.yml` runs `release-verify.yml` on every push to master, so the first push after this lands writes the new entry. Pull requests merged before that miss the cache, which is exactly what most of them do today. No run breaks. - **The reader and the writer must stay in step.** If they diverge, no run hits the cache. The new parity test fails on any difference in the block, including a comment. - **Uninstalling the image toolchain is deliberate.** Everything in this job builds through the pinned 1.97.1, resolved from `packages/paperclip-runner/rust-toolchain.toml` and `RUSTUP_TOOLCHAIN`. Nothing in the job uses the image default. - **Low blast radius.** The change only affects cache key inputs. A miss compiles from scratch, as today. - **Image drift is now handled.** The key no longer depends on the image's own Rust version, so a future image update cannot silently disable the cache. ## Model Used Claude Opus 5, provider Anthropic, exact model ID `claude-opus-5`, 1M context window. Adaptive thinking was on. I used tool use throughout: the `gh` CLI and the GitHub API to pull 26 post-merge job records and 10 full job logs, log parsing in Python to isolate the per-phase timings and the two cache keys, a stub `rustup` on `PATH` to exercise the new step, and local `node --test` runs to verify and mutation-test the guards. Run through Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Note on the unchecked boxes. The CI and Greptile boxes stay unchecked until those checks finish. On documentation: no document describes the Runner cache keys, so there is nothing to update. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 --- .../tests/pr-runner-rust-cache.test.mjs | 34 +++++++++++++++++++ .github/workflows/pr-trusted.yml | 17 ++++++++++ .github/workflows/release-verify.yml | 17 ++++++++++ 3 files changed, 68 insertions(+) diff --git a/.github/scripts/tests/pr-runner-rust-cache.test.mjs b/.github/scripts/tests/pr-runner-rust-cache.test.mjs index 59f3e2698d..1987072442 100644 --- a/.github/scripts/tests/pr-runner-rust-cache.test.mjs +++ b/.github/scripts/tests/pr-runner-rust-cache.test.mjs @@ -53,3 +53,37 @@ test("a pull request never writes to or evicts the master cache entry", () => { assert.doesNotMatch(step, /^\s*if:/m, "the restore must not be conditional; a miss is already free"); assert.doesNotMatch(prWorkflow, /uses: Swatinem\/rust-cache@[0-9a-f]{40}[\s\S]*?save-if: (?!false)/); }); + +// The cache key mixes in every toolchain rust-cache can find, so the runner +// image's own stable Rust lands in it too. The fleets carried 1.98.0 while +// ubuntu-latest carried 1.98.1, which is why GitHub-hosted pull requests +// missed a cache the fleet hit. Both workflows now strip everything but the +// pin. They have to do it the same way: if the reader and the writer disagree, +// the key matches nothing and every run recompiles. +const NORMALIZE = /# rust-cache hashes every installed toolchain[\s\S]*?rustup toolchain list\n/; + +test("reader and writer strip extra toolchains identically before the key is computed", () => { + const mine = pr.match(NORMALIZE); + const theirs = release.match(NORMALIZE); + assert.ok(mine, "pr-trusted.yml must normalize the installed toolchains"); + assert.ok(theirs, "release-verify.yml must normalize the installed toolchains"); + assert.equal(mine[0], theirs[0], "the normalization must be identical in both workflows"); + + for (const [name, body] of [["reader", mine[0]], ["writer", theirs[0]]]) { + // Keep the pin, drop the rest, and never fail the job over it. + assert.match(body, /grep -vx "\$toolchain"/, name); + assert.match(body, /xargs -n1 rustup toolchain uninstall/, name); + assert.match(body, /\|\| true/, name); + } +}); + +test("the toolchain is stripped before the cache step, not after", () => { + for (const [name, body, cacheStep] of [ + ["reader", pr, " - name: Restore Runner Rust dependencies (read only)"], + ["writer", release, " - name: Cache Runner Rust dependencies"], + ]) { + const normalize = body.search(NORMALIZE); + const cache = body.indexOf(cacheStep); + assert.ok(normalize >= 0 && cache > normalize, `${name}: normalization must precede the cache step`); + } +}); diff --git a/.github/workflows/pr-trusted.yml b/.github/workflows/pr-trusted.yml index fcb8a41f39..81c5d597f6 100644 --- a/.github/workflows/pr-trusted.yml +++ b/.github/workflows/pr-trusted.yml @@ -683,6 +683,23 @@ jobs: toolchain="$(rustup show active-toolchain | awk '{print $1}')" echo "RUSTUP_TOOLCHAIN=$toolchain" >> "$GITHUB_ENV" + # rust-cache hashes every installed toolchain into the cache key, not + # only the active one. Each runner image also ships its own stable + # Rust, and those disagree across images: 1.98.0 on the RunsOn fleets + # against 1.98.1 on ubuntu-latest. Two runners that agreed on the pin + # therefore still computed different keys, and every GitHub-hosted + # pull request missed this cache while the fleet hit it. Remove every + # toolchain except the pin, so the key depends on the pinned compiler + # and the lockfile alone rather than on what the image happens to + # carry. Keep this block identical in both workflows: the reader and + # the writer must agree or the key matches nothing. + extra_toolchains="$(rustup toolchain list | awk '{print $1}' | grep -vx "$toolchain" || true)" + if [ -n "$extra_toolchains" ]; then + echo "$extra_toolchains" | xargs -n1 rustup toolchain uninstall \ + || echo '::notice title=Runner Rust cache::could not remove an extra toolchain; the cache key may not match' + fi + rustup toolchain list + - name: Restore Runner Rust dependencies (read only) uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2 with: diff --git a/.github/workflows/release-verify.yml b/.github/workflows/release-verify.yml index 505624a6b0..5144e6b52a 100644 --- a/.github/workflows/release-verify.yml +++ b/.github/workflows/release-verify.yml @@ -302,6 +302,23 @@ jobs: toolchain="$(rustup show active-toolchain | awk '{print $1}')" echo "RUSTUP_TOOLCHAIN=$toolchain" >> "$GITHUB_ENV" + # rust-cache hashes every installed toolchain into the cache key, not + # only the active one. Each runner image also ships its own stable + # Rust, and those disagree across images: 1.98.0 on the RunsOn fleets + # against 1.98.1 on ubuntu-latest. Two runners that agreed on the pin + # therefore still computed different keys, and every GitHub-hosted + # pull request missed this cache while the fleet hit it. Remove every + # toolchain except the pin, so the key depends on the pinned compiler + # and the lockfile alone rather than on what the image happens to + # carry. Keep this block identical in both workflows: the reader and + # the writer must agree or the key matches nothing. + extra_toolchains="$(rustup toolchain list | awk '{print $1}' | grep -vx "$toolchain" || true)" + if [ -n "$extra_toolchains" ]; then + echo "$extra_toolchains" | xargs -n1 rustup toolchain uninstall \ + || echo '::notice title=Runner Rust cache::could not remove an extra toolchain; the cache key may not match' + fi + rustup toolchain list + - name: Cache Runner Rust dependencies # Restore and save only within trusted master-push verification. GitHub # isolates branch/PR caches from master; other callers compile afresh.