mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-05 12:29:37 +01:00
b4c69ded9f8a79ac3569bfc2b8ae58ee7d67ff5e
4
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
a194166017 |
perf(ci): repair dependency caches and parallelize transport fixtures (#18883)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Required CI checks protect changes to its native Runner and control plane. > - Recent Runner jobs restored a Rust cache but still rebuilt dependencies for 74–92 seconds. > - New workspace install hooks disabled the materialized JavaScript dependency cache. > - A single transport file also kept 326 test cases on one worker. > - This pull request corrects the cache inputs, replays known install hooks, and separates independent transport test groups. > - The same checks can finish sooner without removing test cases or widening runner access. ## Linked Issues or Issue Description Refs #18794 and #18544. Related dependency-action version update: #17698; this change fixes the cache input contract used by the currently pinned action. **What happened?** Recent PR jobs such as #18783 and #18825 restored an exact Rust dependency cache from October 16. The virtual workspace's development profile changed on October 30. Rust Cache 2.9.2 hashes member manifests but omits that virtual manifest. Rebuilding cannot replace an immutable exact cache entry. The JavaScript cache also rejected the new server and Runner Copilot postinstall hooks. **Expected behavior** Changing a Cargo profile selects a new dependency cache. An exact JavaScript dependency restore still performs required workspace install hooks. Independent transport fixtures use the existing file workers while retaining all assertions and real deadlines. **Steps to reproduce** Inspect the Rust dependency cache hit and following `build:rust` in the recent Runner package jobs. Run `node scripts/ci-dependency-cache.mjs` on the previous revision to see `cacheable: false`. Run the original transport file to measure the serial test work. ## What Changed - Add the virtual workspace manifest and ancestor Cargo configuration to all shared Rust dependency keys. Keep reader/writer identities aligned and preserve master-only cache writes and CODEOWNERS-only AWS routing. - Recognize only the exact reviewed root, server, and Runner postinstall commands. Include the server/Runner installer scripts in the dependency key. Replay workspace postinstall hooks after a materialized restore. Unknown lifecycle hooks still disable reuse. - Move attachment recovery, warm attachment, live adoption, and suspension fixtures into separate files. Keep the large event-backlog cases in the original file to avoid starting another expensive fixture group at the same time. Share five stateless test helpers outside production source. - Extend the opt-in `profile-ci` benchmark with a PR-scoped Rust cache producer and a fresh-host consumer. Require exact dependency/workspace hits and reject any Cargo unit that is rebuilt. Retain the JSON build evidence. Existing setup qualification checks real JavaScript cache restores. - Schedule the ten measured files over 50 seconds first, in approximate duration order. Package CI uses up to four workers, bounded by available CPUs; local runs retain Vitest defaults. The AWS runner has four CPUs and 16 GiB RAM; the three-worker run averaged 44% CPU and peaked at 15% memory. Preserve Vitest selection and shard membership and default ordering for other files. Job count and AWS authorization are unchanged. - Scope the optional paired server benchmark to changes in its measured inputs. Cache-only changes qualify exact restores and run the shared package tests using restored dependencies. - Update the cache and test documentation and the existing native cache contract guard. ## Verification - All 586 `.github/scripts/tests/*.test.mjs` policy tests passed, including cache trust and runner routing guards. Actionlint passed for all three changed workflows. - All 169 original top-level transport test definitions remain byte-identical. The original file passed all 326 cases in 317.99 seconds on this macOS host. A clean preliminary split passed the exact same named cases in 183.01 seconds with the same four-worker cap. - Preserve the first failed experiment: its parallel run overlapped the policy suite and timed out in the 1,024-event settled backlog case at 60 seconds; 325 cases passed. A clean repeat passed, but that does not establish the earlier failure's sole cause. The final layout keeps the backlog cases in the original file. The final layout passed the same 326 cases in 186.35 seconds (41.4% faster than the 317.99-second baseline). The original test bodies and all timeouts remain unchanged. - Fresh-host Rust qualification passed: all 308 compilation units were fresh, and Cargo completed in 1.59 seconds on a new runner ([final-head run](https://github.com/paperclipai/paperclip/actions/runs/41745082837/job/126296908816)). JavaScript setup dropped from 46 seconds cold to 23 seconds with exact dependency/build restores; all 1,052 shared-package cases passed against the restored tree ([final-head job](https://github.com/paperclipai/paperclip/actions/runs/41745082837/job/126295937797)). The previous qualification also passed (43 to 19 seconds) and confirmed all three postinstall hooks replayed. These are qualification results, not an end-to-end PR timing claim. - Initial full AWS package run passed 4,907 cases with 10 existing skips. The original-file remainder fell to 118.7 seconds, but the 100-second deadline file started last, leaving the full package at 417.43 seconds. Starting only three long files first passed 4,911 cases (including four new sequencer guards) in 422.54 seconds, so reordering alone did not improve full-package time. Summed file execution was 1,202 seconds across three busy workers. The final four-worker configuration passed **4,912 cases, with the same 10 existing skips**, in **324.99 seconds**, a **23.1% reduction** from 422.54 seconds. The five added cases guard ordering, selection, sharding, and path handling. Peak memory was 17.95% of the 16 GiB runner; average CPU was 49.14% ([final-head package job](https://github.com/paperclipai/paperclip/actions/runs/41745082974/job/126296339480)). - Local workspace typecheck and build passed. The complete local `pnpm test:run` passed all six chat partitions but reported failures in one existing OAuth redirect case and one gateway authority case in the general-server group. Both focused repetitions passed (three OAuth cases and one gateway case), and a complete repeat of both files passed all 593 cases in 42.89 seconds; that does not explain the original failures. The broad local run was stopped after 61 minutes once full final-head CI had passed; its remaining general/workspace/serialized work was not completed locally, and it is not reported as a pass. Original failure logs were retained. Full final-head CI provides the complete suite validation. Sequencer selection/shard guards and the 11-case transport suspension smoke passed. The corrected release workflow guard passed all 71 tests. - The first CI run failed only the policy guard that still expected the old native cache key; downstream aggregate checks also failed. That guard now checks the new profile/config prefix and existing host/target/protocol suffix. A second policy run found that the new sequencer test imported Vitest in the dependency-free policy lane; it now runs in the Runner package lane, where all four cases pass. Greptile reviewed final head `8011f84189` at 5/5 with no actionable findings and confirmed this issue fixed. Final-head policy, typecheck, build, and review passed. At 23:52:46 UTC on November 3, several AWS jobs were terminated together: the Runner package process received SIGTERM and browser logs recorded a runner shutdown signal. The failed/cancelled jobs passed on the same-head retry, including required `ci / verify`; no assertion or timeout was changed in response. The final concurrency adjustment at `fe5a69bb0b` also received Greptile 5/5 with no actionable findings. Final-head required CI, including `ci / verify`, passed ([aggregate](https://github.com/paperclipai/paperclip/actions/runs/41745082974/job/126300554711)). Local timings exclude install, build, cache transfer, and CI queue time. ## Risks A new Rust key needs one successful canonical master build before ordinary PRs can hit it. The fresh-host qualification uses separate PR-scoped entries. Parallel test files can increase I/O contention; case execution inside each file, timeouts, and assertions are unchanged. Two extracted suites exceed 1,000 lines because they retain existing multi-stage recovery fixtures unchanged; one parameterized fixture alone is 1,059 lines. No production execution logic, lockfile, credentials, runner trust gates, or required check is removed. ## Model Used OpenAI GPT-6 through Codex, with reasoning, repository editing, command execution, and GitHub inspection. The host does not expose a more precise model snapshot or context-window size. ## 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 — targeted checks passed; the broad-run failures and repetitions are documented above - [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 --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
|
1d23cb6962 |
ci: pin a checkout-independent Rust cache path so PR lanes hit master's cache (#14394)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Paperclip ships a native Runner binary, written in Rust, and seven CI lanes build it on every pull request > - `Canary Dry Run` is the slowest check on every green PR run, and most of its time is `cargo build --release` on third-party crates > - Master saves a Rust dependency cache for these lanes, but every PR lane logs `No cache found` and compiles every crate from zero > - The cache key matches, but GitHub also compares a hash of the absolute cache paths, and the master writer (RunsOn fleet, `/home/runner/_work/...`) and the PR readers (GitHub-hosted, `/home/runner/work/...`) hash different paths > - This pull request gives both sides a checkout-independent workspace path, so the hashes match and the PR lanes restore master's cache > - The benefit is about 2.5 minutes less wall clock per PR run and about 18 fewer runner-minutes per run ## Linked Issues or Issue Description No public issue exists for this problem. The description below follows the enhancement template. Related prior PRs on the same cache: Refs #13194, Refs #13259, Refs #13457, Refs #13459, Refs #13500, Refs #13586. None of them pins the workspace path, so none of them fixes this miss. **What existing behavior does this improve?** The `Swatinem/rust-cache` restore step in the PR workflow lanes that build the Runner: `Canary Dry Run`, `Build`, `Typecheck + Release Registry`, and the four `Verify Paperclip Runner` lanes. **Subsystem affected** CI workflows under `.github/workflows/`, their guard tests under `.github/scripts/tests/`, and `doc/RELEASE-AUTOMATION-SETUP.md`. **Current behavior** Every PR lane logs `No cache found` although master holds an entry with the exact key. Run 36424309181 computed `v0-rust-release-runner-v1-Linux-x64-c3a3ca66-a95b0328`, and master holds a 678 MB entry with that key. GitHub matches a cache entry on the key and on a version hash of the absolute paths in the cache. The master writer runs on the RunsOn fleet, where the checkout is `/home/runner/_work/paperclip/paperclip`. The PR readers run on GitHub-hosted `ubuntu-latest`, where the checkout is `/home/runner/work/paperclip/paperclip`. The stored version `5c40870d…` is the sha256 of the `_work` paths plus `zstd-without-long|1.0`. The `work` paths hash to `1656e9ee…`. The key can never match, so each lane compiles every third-party crate again. **Proposed behavior** The writer and the readers pass the same checkout-independent path to `rust-cache`. Both runner layouts then produce the same version hash, and the PR lanes restore master's cache. **Reason and benefit** `Canary Dry Run` takes 533s on a green run. 251s of that is dependency compilation that a warm cache removes. Seven lanes pay this cost in every PR run. **Breaking changes** None. This change affects CI only. ## What Changed - Add a `Pin the Runner Rust workspace path` step before `rust-cache` in the master writer (`release-verify.yml`, typecheck and runner lanes) and in all four PR readers (`pr-trusted.yml`). The step creates the symlink `$HOME/paperclip-runner-rust` → `$GITHUB_WORKSPACE/packages/paperclip-runner/runner` and passes that path to `rust-cache` as `workspaces: <path> -> target`. `rust-cache` resolves the input with `path.resolve`, which does not follow symlinks, so both runner layouts now produce the same cache paths and the same version hash. `$HOME` is `/home/runner` on both images, which is why the `~/.cargo` paths already agreed. - Bump the shared keys `release-runner-v1` → `release-runner-v2` and `release-typecheck-v1` → `release-typecheck-v2`. The old, unreachable entries are then visibly orphaned instead of sharing a key with the new ones. - Extend the guard tests `pr-runner-rust-cache`, `release-runner-cache`, and `typecheck-rust-cache`. They now require the pin step in both workflows with identical text, placed before the cache step, and they reject a `workspaces:` value that resolves under the checkout. The `pr-runner-rust-cache` test checks all four PR reader jobs and fails if a `rust-cache` step appears in a PR job that is not in its reader list. - Update `doc/RELEASE-AUTOMATION-SETUP.md` to name the `release-runner-v2` key and to explain the pinned workspace path. ### Expected savings once merged Measured from run 36424309181. "Removed" is the dependency-compile time that a warm restore removes, minus about 18s to restore the 680 MB entry. The fleet writer's own restore shows this cost. | Lane | Today | Removed | Expected | |---|---|---|---| | Canary Dry Run | 533s | ~150s | ~380s | | Typecheck + Release Registry | 462s | ~155s | ~305s | | Verify Paperclip Runner (vitest 2/2) | 453s | ~245s | ~210s | | Verify Paperclip Runner (rust) | 400s | ~175s | ~225s | | Build | 348s | ~130s | ~220s | | Verify Paperclip Runner (static checks) | 321s | ~170s | ~150s | | Verify Paperclip Runner (vitest 1/2) | 346s | ~70s | ~275s | - Wall clock per PR run: about 533s → about 385s. That is about 2.5 minutes faster to a green check set. `Canary Dry Run` stays the longest check. The rest is the non-cargo work in `release.sh` (standalone package builds ~30s, publish-payload preview ~73s). - Runner time: about 18 runner-minutes saved per PR run across the seven lanes. - The first master push after merge compiles from zero once in the fleet writer (about 4 extra minutes on that one run) and saves the v2 entry. Later PRs hit it. When a PR changes `Cargo.lock`, the prefix restore key still gives a partial hit, as before. ## Verification - Run the guard tests for the three cache lanes: `node --test .github/scripts/tests/pr-runner-rust-cache.test.mjs .github/scripts/tests/release-runner-cache.test.mjs .github/scripts/tests/typecheck-rust-cache.test.mjs` Result: 21 pass, 0 fail. - Run the full guard suite: `node --test '.github/scripts/tests/*.test.mjs'`. Result: 376 pass, 3 fail. The 3 failures are in `docker-canary-promotion.test.mjs`. They hit a sandbox temp-file ENOENT and fail the same way on the unmodified branch. - Run `node --test scripts/__tests__/release-verify-workflow.test.mjs`. Result: 14 pass. - Local archive test: create a tar from the `_work` layout through the symlink (relative `../../../paperclip-runner-rust/target` entries, `tar -P -C $GITHUB_WORKSPACE`, the same way `@actions/cache` does). Extract it on the `work` layout. The files land in the real target directory and the symlink stays intact. - After merge, open any GitHub-hosted PR run and confirm that the seven Rust lanes log `Restored from cache key ...release-runner-v2...` in place of `No cache found`. ## Risks - Low risk. The change touches CI workflows, their tests, and one doc page. No product code changes. - If the pin step fails, `rust-cache` reports a miss and the lane compiles from zero, as it does today. The build does not break. - Both runner layouts sit four levels under `/home/runner`, so the relative `../../../` archive entries line up. The existing `~/.cargo/registry` and `~/.cargo/git` cache paths already rely on this property. A future runner image with a different `$HOME` depth would miss the cache but would not fail the job. - `rm -rf "$pinned"` acts on the symlink itself (no trailing slash), never on the checkout behind it. It only matters on a reused runner. - Squash-merge note: the branch carries commits by `Bender (Fable)`. Add `Co-Authored-By: Bender (Fable) <bender-fable@paperclip.local>` to the squash body to keep that authorship. ## Model Used - Anthropic Claude Fable 5.1 (`claude-fable-5-1`), run through Claude Code inside a Paperclip agent heartbeat. Extended thinking was on. Tool use: shell, GitHub CLI, and the GitHub REST API for workflow logs, cache listings, and PR operations. ## 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 and contains no internal ticket id. The agent execution workspace fixed this branch name, so I cannot rename it. Squash-merge drops the branch name. - [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 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: devinfoley <139239+devinfoley@users.noreply.github.com> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> Co-authored-by: Bender (Fable) <bender-fable@paperclip.local> |
||
|
|
bd51f157e9 |
fix(ci): make the Runner Rust cache key independent of the image toolchain (#13500)
## 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 <noreply@anthropic.com> |
||
|
|
f97a3f886e |
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) |