improve(ci): persist warm Vitest and Node caches#109425
Merged
Merged
Conversation
openclaw-barnacle
Bot
added
docs
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 20387d5d65
ℹ️ 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".
Comment on lines
+229
to
+230
| echo "NODE_COMPILE_CACHE=$cache_root" >> "$GITHUB_ENV" | ||
| echo "NODE_COMPILE_CACHE_PORTABLE=1" >> "$GITHUB_ENV" |
Contributor
There was a problem hiding this comment.
When node-compile-cache is enabled for the Node shard job, this exports NODE_COMPILE_CACHE into every later command, including scripts/ci-run-node-test-shard.mjs which invokes node scripts/test-projects.mjs; that path spawns Vitest from scripts/test-projects.mjs without going through the new resolveRunVitestSpawnEnv stripping in scripts/run-vitest.mjs. In CI shards that generate V8 coverage, those Vitest processes now inherit the compile cache, while the Node module docs recommend disabling it for precise V8 coverage, so coverage results can become less accurate even though the direct run-vitest wrapper is protected.
Useful? React with 👍 / 👎.
github-actions
Bot
added
the
dependencies-changed
Contributor
Dependency GuardThis PR changes dependency-related files. Maintainers should confirm these changes are intentional. Changed files:
Maintainer follow-up:
|
Contributor
Dependency graph changes notedThis PR includes dependency graph changes. The dependency guard is informational because the PR author is a repository admin or a member of
Security review is still recommended before merge when the dependency graph change is intentional. |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Contributor
Author
|
Merged via squash.
|
github-actions Bot
pushed a commit
to Desicool/openclaw
that referenced
this pull request
Jul 17, 2026
* perf(ci): harden persistent test caches * ci: document trusted cache cleanup trigger * ci: harden composite cache inputs * perf(ci): isolate Node build bytecode cache
steipete
added a commit
that referenced
this pull request
Jul 17, 2026
…109929) The installation hit Blacksmith's backing-disk cap because sticky keys embedded PR numbers and content hashes, minting a new disk per PR and per input change until every mount 429-failed fleet-wide. node-deps (#109752) and ext-boundary (#109804) were already re-keyed; this converts the last two minters and audits the third suspect. Keys changed: - Vitest fs transform cache: `vitest-fs-v2-<pr-N|protected>-...` -> the one existing `vitest-fs-v2-protected-<os>-<arch>-node-<ver>` disk. Entries are content-hash keyed (vitest sha1 over id+content+NODE_ENV+version+env config), so cross-PR sharing is safe by construction: PRs now mount the protected snapshot directly (read-only, commit false) and the separate PR-only seed mount is deleted. Writers (non-PR shard writer + scheduled warm run) keep explicit commit true. Disk count: 1 + O(open PRs) -> 1. - Gradle: `gradle-v1-<task>-<pr-N|protected>-<hashFiles(deps)>` -> `gradle-v2-<task>`. Task scope stays (light ktlint must not seed heavy build lanes); PR number and dependency hash leave the key. The dependency hash moved to an in-job fingerprint marker that makes the non-PR writer rebuild its snapshot cold when inputs change, bounding disk growth; PR mounts are read-only and safely reuse stale snapshots because Gradle caches are content-addressed. Disk count: O(tasks x PRs x dependency bumps) -> O(tasks) (7 today). - Docker builder (useblacksmith/setup-docker-builder): audited, out of scope. The action owns its key internally and always uses the repo name only (src/setup_builder.ts getStickyDisk), so it is already one disk per installation repo and cannot mint per-PR disks. PR warm-path behavior changes: - Vitest: PRs lose only PR-local warm entries for files the PR itself changed (previously persisted on the pr-N disk between pushes); changed files re-transform once per push, unchanged files still hit the protected snapshot. PRs touching transform inputs (lockfile/tsconfig/package.json) run cold per push since the generation wipe is now local to the discarded clone. - Gradle: dependency-bump PRs get warmer (stale-but-valid protected snapshot instead of a cold fresh key); other PRs are unchanged. Deleted `.github/workflows/pr-cache-cleanup.yml`: it existed for the per-PR cache layer (#109425) and only deleted GitHub actions/cache archives, which GitHub's own LRU/TTL eviction already handles for the remaining fork/Windows per-PR archive paths. Guards: pinned the O(1) vitest key + non-PR commit gate, added a Gradle per-task key/writer/fingerprint guard, added a repo-wide scan asserting no useblacksmith/stickydisk key ever contains github.event.pull_request.number or hashFiles(), and replaced the cleanup-workflow assertions with a stays-deleted check.
github-actions Bot
pushed a commit
to Desicool/openclaw
that referenced
this pull request
Jul 18, 2026
…penclaw#109929) The installation hit Blacksmith's backing-disk cap because sticky keys embedded PR numbers and content hashes, minting a new disk per PR and per input change until every mount 429-failed fleet-wide. node-deps (openclaw#109752) and ext-boundary (openclaw#109804) were already re-keyed; this converts the last two minters and audits the third suspect. Keys changed: - Vitest fs transform cache: `vitest-fs-v2-<pr-N|protected>-...` -> the one existing `vitest-fs-v2-protected-<os>-<arch>-node-<ver>` disk. Entries are content-hash keyed (vitest sha1 over id+content+NODE_ENV+version+env config), so cross-PR sharing is safe by construction: PRs now mount the protected snapshot directly (read-only, commit false) and the separate PR-only seed mount is deleted. Writers (non-PR shard writer + scheduled warm run) keep explicit commit true. Disk count: 1 + O(open PRs) -> 1. - Gradle: `gradle-v1-<task>-<pr-N|protected>-<hashFiles(deps)>` -> `gradle-v2-<task>`. Task scope stays (light ktlint must not seed heavy build lanes); PR number and dependency hash leave the key. The dependency hash moved to an in-job fingerprint marker that makes the non-PR writer rebuild its snapshot cold when inputs change, bounding disk growth; PR mounts are read-only and safely reuse stale snapshots because Gradle caches are content-addressed. Disk count: O(tasks x PRs x dependency bumps) -> O(tasks) (7 today). - Docker builder (useblacksmith/setup-docker-builder): audited, out of scope. The action owns its key internally and always uses the repo name only (src/setup_builder.ts getStickyDisk), so it is already one disk per installation repo and cannot mint per-PR disks. PR warm-path behavior changes: - Vitest: PRs lose only PR-local warm entries for files the PR itself changed (previously persisted on the pr-N disk between pushes); changed files re-transform once per push, unchanged files still hit the protected snapshot. PRs touching transform inputs (lockfile/tsconfig/package.json) run cold per push since the generation wipe is now local to the discarded clone. - Gradle: dependency-bump PRs get warmer (stale-but-valid protected snapshot instead of a cold fresh key); other PRs are unchanged. Deleted `.github/workflows/pr-cache-cleanup.yml`: it existed for the per-PR cache layer (openclaw#109425) and only deleted GitHub actions/cache archives, which GitHub's own LRU/TTL eviction already handles for the remaining fork/Windows per-PR archive paths. Guards: pinned the O(1) vitest key + non-PR commit gate, added a Gradle per-task key/writer/fingerprint guard, added a repo-wide scan asserting no useblacksmith/stickydisk key ever contains github.event.pull_request.number or hashFiles(), and replaced the cleanup-workflow assertions with a stays-deleted check.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related: #109330
What Problem This Solves
Resolves a CI performance problem where every Node test shard repeatedly pays cold Vitest transform/import costs because its filesystem module cache does not survive the job. The original persistent-cache lane also depended on a cancellable protected writer, had no bounded eviction, and could not safely reuse protected seeds after partial configuration changes.
Why This Change Was Made
Persist Vitest transform data on the same Blacksmith sticky-disk mechanism as Node dependencies, with a single semantic writer, private per-job live directories, exact transform-generation validation, PR overlays, a non-cancelling default-branch warmer, GitHub-hosted fallback archives, and bounded pruning. Also update Vitest to 4.1.10 and persist Node's portable compile cache in independent test/build namespaces; trusted main builds publish build-specific bytecode without racing the test writer. Vitest children disable inherited bytecode caching so config-driven V8 coverage remains precise.
Trust boundaries stay explicit: pull requests cannot publish protected snapshots, protected seeds merge only when their transform fingerprints match, fork fallbacks remain PR-scoped, and on-demand warming uses default-branch-only repository dispatch rather than ref-selectable workflow dispatch.
User Impact
No product behavior changes. Maintainers should see faster warm Node CI, especially on partially changed pull requests that reuse most of the shared source graph. Cache growth is bounded at 2 GiB for Vitest transforms and 1 GiB per Node bytecode namespace, pruning back to 75%; closed PR fallback archives are deleted.
Evidence
main; currentmainincludes its fix.checks-node-compact-large-1350s to 301s (-14.0%);checks-node-compact-small-1675s to 65s (-13.3%).checks-node-compact-large-7had 5s of job-level noise (91s to 96s), while its three Vitest plans still improved 66.03s to 62.79s (-4.9%), transform 123.51s to 109.54s (-11.3%), and import 140.92s to 122.63s (-13.0%).buildsnapshot; later job 87777911183 mounted and recommitted the same protected key. Both had per-SHA dist-cache misses, whileBuild distimproved from 58s to 44s (-24.1%). This is promising one-pair evidence rather than a guaranteed 24% gain because source and runner load also changed.AI-assisted: yes. I reviewed the implementation, dependency contracts, trust boundaries, and validation results.