mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-06 10:48:12 +02:00
perf(ci): restore master's Rust dependency cache on the PR runner lane (#13457)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Every change to Paperclip goes through the pull request CI workflow before it merges > - One lane of that workflow, `Verify Paperclip Runner`, builds and tests the native Rust runner > - That lane compiles all 313 third-party crates from scratch on every pull request, in two profiles > - Master already builds and stores exactly those compiled crates, but the pull request lane never reads them > - This pull request restores that existing cache on the pull request lane, read-only > - The benefit is about 3.9 minutes less compute per run, or about 4.7 compute-hours each day, with no change to what CI checks ## Linked Issues or Issue Description No public GitHub issue exists for this. The problem follows the enhancement issue template below. Related merged pull requests, found by searching this repository: - Refs https://github.com/paperclipai/paperclip/pull/13194 — created the `release-runner-v1` cache that this pull request reads - Refs https://github.com/paperclipai/paperclip/pull/13300 — established the restore-only dependency cache pattern that this pull request follows - Refs https://github.com/paperclipai/paperclip/pull/13326 — split the post-merge runner job into the `protocol` and `rust` lanes used as the warm-cache baseline below - Refs https://github.com/paperclipai/paperclip/pull/13259 — applied the same caching idea to the post-merge typecheck job I found no open pull request that duplicates this work. **What existing behavior does this improve?** The `Verify Paperclip Runner` job in `.github/workflows/pr-trusted.yml` recompiles the full Rust dependency tree on every pull request. **Subsystem affected** Cross-cutting (multiple of the above). The change touches CI workflow configuration only. It does not change product code. **Current behavior** The job has no Rust cache. Its only cache step restores the pnpm store. Each run therefore downloads about 280 crates and compiles all 313 third-party crates twice, once in the `dev` profile and once in the `release` profile. Measured over 12 successful runs on 2026-09-15, the job takes 15.0 minutes on average. The range is 12.1 to 17.4 minutes. | Phase | Mean | Share | |---|---|---| | `check:eval-kernel` | 0.0m | — | | `typecheck:typescript` and protocol manifest | 0.1m | 1% | | `build:rust` — cargo `dev` profile | 1.8m | 12% | | TypeScript tests (`node --test` and vitest, 143 files) | 5.8m | 39% | | `check:replay-goldens` | 0.1m | 1% | | `typecheck:rust` — `cargo fmt` and `cargo check` | 0.9m | 6% | | `test:rust` compile — cargo `release` profile | 4.9m | 33% | | `test:rust` run | 0.8m | 5% | | Parity checks | 0.0m | — | | `check:api-authority` | 0.5m | 3% | Cargo reports the two compiles directly. The `dev` profile takes 1m24s to 1m53s. The `release` profile takes 3m54s to 5m36s. **Proposed behavior** The job restores master's existing Rust dependency cache before it runs the checks. Master already writes this cache. The post-merge `rust` lane in `.github/workflows/release-verify.yml` writes `release-runner-v1`. It also warms both profiles into that entry. The entry is 677MB and lives on `refs/heads/master`. Pull request branches are allowed to read caches from the default branch. The pull request lane restores that entry read-only. Master stays the only writer. A pull request never saves a branch-scoped copy. A pull request never evicts the shared entry. This matches the rule the pnpm store in the same file already follows. **Reason and benefit** Master runs these same checks with a warm cache. That gives a direct measurement of the saving. | Check set | Cold (pull request today) | Warm (master) | Change | |---|---|---|---| | `check:runner` and `check:api-authority` | 7.1m | 3.4m | −3.7m | | `check:eval-kernel` and `check:protocol` | 7.8m | 7.3m | −0.5m | | Cache restore step | — | 20–21s | +0.35m | The net saving is about 3.9 minutes per run, or about 26%. About 73 runs execute this job each day. The daily saving is therefore about 4.7 compute-hours. The protocol side changes very little. Vitest dominates that side, not compilation. The cache should almost always hit. `packages/paperclip-runner/runner/Cargo.lock` changed in 7 of the last 1697 commits on master. A miss costs nothing more than today's behavior. **Breaking changes** None. The change adds two steps to one CI job. It does not change any check, any test, or any product code. **Additional context** No documentation covers pull request CI caching, so this pull request updates no documents. ## What Changed - Added a `Select the pinned Runner Rust toolchain` step to the `verify_paperclip_runner` job in `.github/workflows/pr-trusted.yml`. The step exports `RUSTUP_TOOLCHAIN`. `rust-cache` hashes `rustc -vV` into the cache key, so the key needs the pinned compiler. This lane had no rustup step before, so `rustc` resolved to each runner image's default instead of the pinned 1.97.1. - Made that step tolerate a missing `rustup`. `release-verify.yml` runs on one post-merge fleet image. The gate in this workflow routes to either `ubuntu-latest` or the public pull request fleet. A missing `rustup` now costs the cache. It does not fail the pull request. - Added a `Restore Runner Rust dependencies (read only)` step that uses `Swatinem/rust-cache` with `save-if: false`. - Mirrored every cache key input from the master writer in `release-verify.yml`: the same action SHA, `workspaces`, `shared-key`, `cache-workspace-crates`, and `cache-bin`. Neither side sets `prefix-key`. Any drift causes a silent miss and a full recompile. - Added `.github/scripts/tests/pr-runner-rust-cache.test.mjs`. It asserts key-input parity across the two workflow files, the step order, and the restore-only contract. ## Verification Run the workflow shape tests: ```bash node --test '.github/scripts/tests/*.test.mjs' ``` Result: 413 pass, 0 fail. I also mutation-tested the new assertions. I applied each mutation to the workflow, ran the new test file, then restored the file. All 7 mutations fail the suite: | Mutation | Result | |---|---| | `shared-key` changed to `pr-runner-v1` | Caught | | `save-if` changed to `true` | Caught | | `cache-bin` changed to `true` | Caught | | `rustup show active-toolchain` line deleted | Caught | | `RUSTUP_TOOLCHAIN` export line deleted | Caught | | `working-directory` line deleted | Caught | | `command -v rustup` guard deleted | Caught | To confirm the cache key inputs match the master writer, parse both workflows and compare: ```bash ruby -ryaml -e 'pr=YAML.safe_load(File.read(".github/workflows/pr-trusted.yml"), aliases: true); rv=YAML.safe_load(File.read(".github/workflows/release-verify.yml"), aliases: true); a=pr["jobs"]["verify_paperclip_runner"]["steps"].find{|s| s["uses"].to_s.include?("rust-cache")}; b=rv["jobs"]["verify_paperclip_runner"]["steps"].find{|s| s["uses"].to_s.include?("rust-cache")}; puts a["uses"]==b["uses"]; %w[workspaces shared-key cache-workspace-crates cache-bin].each{|k| puts "#{k}: #{a["with"][k]==b["with"][k]}"}' ``` Every line prints `true`. After the pin advances (see Risks), confirm the cache works in CI. The restore step log must show `Cache restored from key: v0-rust-release-runner-v1-Linux-x64-...`. Cargo must stop printing `Compiling` lines for third-party crates. The job should drop from about 15.0 minutes to about 11.0 minutes. ## Risks Low risk overall. A cache miss produces exactly today's behavior, so the worst case is no improvement. - **This change does nothing until a second pull request lands.** `.github/workflows/pr.yml` pins this reusable workflow by SHA. The file header describes this two-step rollout. A follow-up pull request must bump that pin. That follow-up validates itself, because GitHub uses the pull request's own `pr.yml` for `pull_request` events. - **A key mismatch would silently remove the benefit.** The runner images here may ship a different default `rustc` than the post-merge fleet. The toolchain step pins the compiler to prevent this. The new test guards the remaining key inputs. If the first runs still miss, compare `rustup show` output against a master run. - **A missing `rustup` degrades quietly.** The step prints a GitHub notice and continues. CI stays green and the run is simply uncached. - **No cache poisoning path.** Pull requests only read. `save-if: false` stops any write. GitHub also isolates pull request cache writes from the default branch. - **The cache entry can expire.** GitHub evicts unused entries after 7 days and enforces a repository size limit. Master pushes are frequent, so the entry should stay warm. Eviction only causes a miss. ## 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 to read 12 job logs and step timings from recent CI runs, the GitHub Actions cache API to read cache entry keys and sizes, `git log` to measure `Cargo.lock` churn, and local `node --test` runs to verify and mutation-test the change. 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) - [ ] 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 - [ ] All Paperclip CI gates are green - [ ] 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 branch name `claude/paperclip-runner-conditional-81a300` carries a tool-generated suffix, so it does not meet the branch naming rule. I can rename it if you want. The CI and Greptile boxes stay unchecked until those checks finish on this pull request. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
This commit is contained in:
1 parent
4cc387f907
commit
f97a3f886e
2 files changed
+91
No files matched your search
@@ -0,0 +1,55 @@
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { readFileSync } from "node:fs";
|
||||
|
||||
const read = (name) => readFileSync(new URL(`../../workflows/${name}`, import.meta.url), "utf8");
|
||||
const prWorkflow = read("pr-trusted.yml");
|
||||
const releaseWorkflow = read("release-verify.yml");
|
||||
const pr = prWorkflow.split(" verify_paperclip_runner:")[1].split(" build:")[0];
|
||||
const release = releaseWorkflow.split(" verify_paperclip_runner:")[1].split(" build:")[0];
|
||||
|
||||
// The key is computed from these inputs. A pull request that disagrees with
|
||||
// the master writer on any of them misses every time and silently recompiles
|
||||
// all 313 third-party crates in both profiles, which is exactly the cost this
|
||||
// restore exists to remove.
|
||||
const keyInputs = [
|
||||
/uses: Swatinem\/rust-cache@([0-9a-f]{40}) # v[0-9.]+/,
|
||||
/workspaces: (packages\/paperclip-runner\/runner -> target)/,
|
||||
/shared-key: (release-runner-v1)/,
|
||||
/cache-workspace-crates: (false)/,
|
||||
/cache-bin: (false)/,
|
||||
];
|
||||
|
||||
test("the PR lane restores the Rust cache under the same key the master push writes", () => {
|
||||
for (const pattern of keyInputs) {
|
||||
const mine = pr.match(pattern);
|
||||
const theirs = release.match(pattern);
|
||||
assert.ok(mine, `PR lane is missing ${pattern}`);
|
||||
assert.ok(theirs, `master writer is missing ${pattern}`);
|
||||
assert.equal(mine[1], theirs[1], `key input drifted from the master writer: ${pattern}`);
|
||||
}
|
||||
assert.doesNotMatch(pr, /prefix-key:|cache-on-failure: true|cache-all-crates: true/);
|
||||
});
|
||||
|
||||
test("the PR lane pins the compiler before the key is computed", () => {
|
||||
const select = pr.indexOf(" - name: Select the pinned Runner Rust toolchain");
|
||||
const cache = pr.indexOf(" - name: Restore Runner Rust dependencies (read only)");
|
||||
const verify = pr.indexOf(" - name: Verify Paperclip Runner\n");
|
||||
assert.ok(select >= 0 && cache > select && verify > cache);
|
||||
const setup = pr.slice(select, cache);
|
||||
assert.match(setup, /working-directory: packages\/paperclip-runner/);
|
||||
assert.match(setup, /rustup show active-toolchain/);
|
||||
assert.match(setup, /echo "RUSTUP_TOOLCHAIN=\$toolchain" >> "\$GITHUB_ENV"/);
|
||||
// The gate routes to either ubuntu-latest or the public PR fleet, so a
|
||||
// missing rustup must cost the cache, never the pull request.
|
||||
assert.match(setup, /command -v rustup/);
|
||||
assert.doesNotMatch(setup, /set -euo pipefail/);
|
||||
});
|
||||
|
||||
test("a pull request never writes to or evicts the master cache entry", () => {
|
||||
const step = pr.split(" - name: Restore Runner Rust dependencies (read only)")[1]
|
||||
.split(" - name: Verify Paperclip Runner\n")[0];
|
||||
assert.equal(step.match(/^\s*save-if: (.+)$/m)?.[1], "false");
|
||||
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)/);
|
||||
});
|
||||
@@ -658,6 +658,42 @@ jobs:
|
||||
- name: Install dependencies
|
||||
run: pnpm install --frozen-lockfile
|
||||
|
||||
# Same restore-only contract as the pnpm store above, for the Rust
|
||||
# dependency tree: master's post-merge verification is the sole writer
|
||||
# of release-runner-v1, and PR merge refs must not save branch-scoped
|
||||
# copies of a ~680MB target directory. Every key input below has to
|
||||
# match that writer in release-verify.yml exactly or each PR misses and
|
||||
# recompiles all 313 third-party crates in both profiles. A miss is a
|
||||
# slow run, never a wrong one.
|
||||
- name: Select the pinned Runner Rust toolchain
|
||||
working-directory: packages/paperclip-runner
|
||||
run: |
|
||||
set -uo pipefail
|
||||
|
||||
# release-verify.yml runs on a single post-merge fleet image; the
|
||||
# gate here can route to ubuntu-latest or the public PR fleet, so
|
||||
# this tolerates an image without rustup instead of failing every
|
||||
# pull request. Without the pin the cache key simply will not match.
|
||||
if ! command -v rustup >/dev/null 2>&1; then
|
||||
echo '::notice title=Runner Rust cache::rustup is unavailable; building with the image default toolchain'
|
||||
exit 0
|
||||
fi
|
||||
|
||||
rustup show
|
||||
toolchain="$(rustup show active-toolchain | awk '{print $1}')"
|
||||
echo "RUSTUP_TOOLCHAIN=$toolchain" >> "$GITHUB_ENV"
|
||||
|
||||
- name: Restore Runner Rust dependencies (read only)
|
||||
uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2
|
||||
with:
|
||||
workspaces: packages/paperclip-runner/runner -> target
|
||||
shared-key: release-runner-v1
|
||||
# Mirror the master writer: these also feed the cache key.
|
||||
cache-workspace-crates: false
|
||||
cache-bin: false
|
||||
# Restore only. Never let a pull request evict master's entry.
|
||||
save-if: false
|
||||
|
||||
- name: Verify Paperclip Runner
|
||||
run: pnpm --filter @paperclipai/paperclip-runner check:all
|
||||
|
||||
|
||||
Reference in new issue
Block a user