perf(ci): collapse remaining per-PR sticky-disk minters to O(1) keys by steipete · Pull Request #109929 · openclaw/openclaw

steipete · 2026-07-17T10:37:15Z

What Problem This Solves

The Blacksmith installation is at its 1000-backing-disk cap and every sticky mount currently 429s fleet-wide — the reason the landed warm-path levers (#109752 node-deps, #109804 ext-boundary) cannot produce a single warm hit. The cap was reached because sticky keys embedded PR numbers and content hashes, minting a new disk per PR and per input change. Two minters remained after the earlier re-keys.

Why This Change Was Made

Vitest fs transform cache: vitest-fs-v2-<pr-N|protected>-… collapses to the one existing protected disk. Cache entries are content-hash keyed (sha1 over id+content+NODE_ENV+version+config), so cross-PR sharing is safe by construction — PRs now mount the protected snapshot read-only (commit: "false"), the separate PR-only seed mount is deleted, and writers (non-PR shard writer + scheduled warm run) keep explicit commit: true. The PR-close cleanup workflow (pr-cache-cleanup.yml) is deleted with the per-PR disks it existed to reap. Trade-off named: a PR loses warm entries only for files it changed itself, which re-transform once per push either way. Disk count: 1 + O(open PRs) → 1.
Gradle: gradle-v1-<task>-<pr-N|hashFiles> → gradle-v2-<task>. Task scope stays (a light ktlint lane must not seed the heavy build lanes); the dependency hash moves into an in-job fingerprint marker that makes the non-PR writer rebuild cold on input changes; PR mounts are read-only and safely reuse stale snapshots because Gradle caches are content-addressed. Disk count: O(tasks × PRs × dependency bumps) → O(tasks), 7 today.
Docker builder: audited out of scope — useblacksmith/setup-docker-builder keys internally by repo name only (src/setup_builder.ts getStickyDisk) and cannot mint per-PR disks.
Guards updated: no sticky key may reference github.event.pull_request.number or hashFiles(), single-writer/read-only invariants pinned, and a stays-deleted check replaces the cleanup-workflow assertions.

User Impact

None at runtime — CI-only. This stops the disk-cap regrowth so the operator purge is durable; combined with the purge it unlocks the warm paths of every sticky lever.

Evidence

node scripts/run-vitest.mjs test/scripts/ci-workflow-guards.test.ts — 80 passed, 1 skipped (rerun green after rebase onto current main).
pnpm check:test-types clean; actionlint clean; git diff --check clean.
Disk-count math per surface in the commit body; docker-builder audit cites the action source.

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.

clawsweeper · 2026-07-17T10:45:49Z

Codex review: needs real behavior proof before merge. Reviewed July 17, 2026, 6:42 AM ET / 10:42 UTC.

Summary
The PR replaces PR/hash-scoped Vitest and Gradle sticky-disk keys with fixed shared keys, makes pull-request mounts non-committing, removes the closed-PR Actions cache cleanup workflow, and expands CI guard coverage.

PR surface: Tests +60, Config +1. Total +61 across 4 files.

Reproducibility: yes. at source level: the previous sticky keys visibly include pull-request or content-hash dimensions, and the PR provides a concrete fleet-wide 429 symptom and run context. This review did not independently repeat the installation-cap failure.

Review metrics: 2 noteworthy metrics.

Sticky-key surfaces: 2 collapsed to fixed cardinality. Vitest and Gradle stop minting disks by pull-request number or dependency hash, directly addressing Blacksmith backing-disk growth.
Independent cleanup paths: 1 removed. The deleted workflow manages GitHub Actions caches, so its removal is not a necessary consequence of re-keying Blacksmith sticky disks.

Stored data model
Persistent data-model change detected: migration/backfill/repair: .github/workflows/ci.yml, persistent cache schema: .github/actions/setup-node-env/action.yml, persistent cache schema: .github/workflows/pr-cache-cleanup.yml, persistent cache schema: test/scripts/ci-workflow-guards.test.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: canonical
Canonical: #109929
Summary: This PR is the canonical remaining fix for the same Blacksmith sticky-disk cardinality problem partially addressed by two earlier merged PRs.

Members:

same_root_cause: perf(ci): serve node_modules snapshots from O(1) protected sticky disks #109752 - Merged the O(1) node-dependency sticky-disk re-key and explicitly identified the remaining Vitest and Gradle minters.
same_root_cause: perf(ci): O(1) boundary sticky key and 32 vCPU for the ext-package-boundary lane #109804 - Merged the O(1) extension-boundary sticky-disk re-key under the same installation-cap failure.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🌊 off-meta tidepool
Patch quality: 🦐 gold shrimp
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

Restore .github/workflows/pr-cache-cleanup.yml and the guard that prevents its accidental deletion while PR-scoped Actions caches remain.

Risk before merge

[P1] Removing .github/workflows/pr-cache-cleanup.yml leaves closed-PR GitHub Actions caches alive until retention or size-based eviction; because pull-request caches are merge-ref scoped and current CI still creates them, this can increase billed storage or cause cache thrashing even though Blacksmith sticky-disk cardinality improves.

Maintainer options:

Restore the independent cache cleanup (recommended)
Re-add .github/workflows/pr-cache-cleanup.yml and its guard because it cleans GitHub Actions caches rather than the Blacksmith sticky disks addressed by this PR.
Accept retention-based eviction
Merge the deletion only if maintainers explicitly intend closed-PR Actions caches to persist until GitHub retention or size eviction and accept the resulting storage and cache-thrashing exposure.

Next step before merge

[P1] The maintainer-protected PR should remain in human review until the narrow cleanup-workflow regression is removed and the final CI cache architecture is approved.

Security
Cleared: The diff keeps the existing Blacksmith action pinned, prevents pull-request jobs from committing shared sticky snapshots, and introduces no new third-party execution or expanded secret permissions.

Review findings

[P2] Restore cleanup for PR-scoped Actions caches — .github/workflows/pr-cache-cleanup.yml:1

Review details

Best possible solution:

Keep the fixed-cardinality Vitest and Gradle sticky-disk design, but restore the closed-PR GitHub Actions cache cleanup workflow and retain a guard proving it remains present while PR-scoped actions/cache consumers exist.

Do we have a high-confidence way to reproduce the issue?

Yes at source level: the previous sticky keys visibly include pull-request or content-hash dimensions, and the PR provides a concrete fleet-wide 429 symptom and run context. This review did not independently repeat the installation-cap failure.

Is this the best way to solve the issue?

No as submitted: the fixed-cardinality sticky keys are the right bounded solution, but deleting an independent GitHub Actions cache cleanup path is unnecessary and should be reverted.

Full review comments:

[P2] Restore cleanup for PR-scoped Actions caches — .github/workflows/pr-cache-cleanup.yml:1
Keep this workflow: it deletes GitHub Actions caches under the closed PR's merge ref, not Blacksmith sticky disks. Current CI still creates actions/cache entries during pull-request runs, and GitHub scopes those entries to refs/pull/.../merge; removing cleanup leaves them consuming storage until retention or size eviction and can reintroduce cache thrashing. Restore the workflow and its guard independently of the O(1) sticky-disk changes.
Confidence: 0.98

Overall correctness: patch is incorrect
Overall confidence: 0.96

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against c45f299cea4d.

Label changes

Label changes:

add P2: This is a bounded CI-capacity improvement with a concrete but limited automation regression in the proposed cleanup deletion.
add merge-risk: 🚨 automation: The PR changes shared CI cache behavior and removes cleanup for still-active PR-scoped GitHub Actions caches.
add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🌊 off-meta tidepool and patch quality is 🦐 gold shrimp.
add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: The maintainer-labeled PR is exempt from the external-contributor proof gate; its workflow guard results remain useful supplemental validation, though a post-purge warm-hit observation would best confirm the performance outcome.

Label justifications:

P2: This is a bounded CI-capacity improvement with a concrete but limited automation regression in the proposed cleanup deletion.
merge-risk: 🚨 automation: The PR changes shared CI cache behavior and removes cleanup for still-active PR-scoped GitHub Actions caches.
rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🌊 off-meta tidepool and patch quality is 🦐 gold shrimp.
status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: The maintainer-labeled PR is exempt from the external-contributor proof gate; its workflow guard results remain useful supplemental validation, though a post-purge warm-hit observation would best confirm the performance outcome.

Evidence reviewed

PR surface:

Tests +60, Config +1. Total +61 across 4 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	0	0	0	0
Tests	1	84	24	+60
Docs	0	0	0	0
Config	3	65	64	+1
Generated	0	0	0	0
Other	0	0	0	0
Total	4	149	88	+61

What I checked:

Vitest sticky-disk cardinality: The branch changes the Vitest transform cache to one protected key per runner shape and prevents pull-request jobs from committing the shared snapshot. (.github/actions/setup-node-env/action.yml:147, 5138206f8d6b)
Gradle sticky-disk cardinality: The branch changes Gradle from task-plus-PR-plus-dependency-hash keys to one stable key per task, with dependency freshness handled inside the job. (.github/workflows/ci.yml:3136, 5138206f8d6b)
Cleanup workflow targets a different cache system: The deleted workflow calls gh cache delete for refs/pull/<number>/merge; it manages GitHub Actions caches, not Blacksmith sticky disks. (.github/workflows/pr-cache-cleanup.yml:1, 5138206f8d6b)
Actions caches remain active: Current CI still uses actions/cache and setup-action caching in pull-request jobs, so closed pull requests can continue leaving merge-ref-scoped GitHub caches after this deletion. (.github/workflows/ci.yml, c45f299cea4d)
GitHub cache contract: GitHub documents that pull-request caches are scoped to refs/pull/.../merge and provides a closed-PR cleanup workflow matching the deleted file; otherwise caches remain until retention or size eviction. See https://docs.github.com/en/actions/how-tos/manage-workflow-runs/manage-caches and https://docs.github.com/en/actions/reference/workflows-and-actions/dependency-caching.
Related merged capacity work: The same contributor previously landed the O(1) node-dependency and extension-boundary cache changes in merged pull requests perf(ci): serve node_modules snapshots from O(1) protected sticky disks #109752 and perf(ci): O(1) boundary sticky key and 32 vCPU for the ext-package-boundary lane #109804. (.github/workflows/ci.yml, dfd457adc069)

Likely related people:

steipete: Introduced the two immediately preceding merged O(1) sticky-disk changes and authored the current CI cache design, making this the strongest routing signal beyond merely opening this PR. (role: feature owner and recent CI cache contributor; confidence: high; commits: f69cb2f75e49, dfd457adc069; files: .github/actions/setup-node-env/action.yml, .github/workflows/ci.yml, test/scripts/ci-workflow-guards.test.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

steipete · 2026-07-17T10:53:10Z

Merged via squash.

Prepared head SHA: 5138206f8d6bfaedec8f03d3c07c4bb27d52da5c
Landed commit: d3143ced6d85c61398d8497e87370a8e7c7b9288

…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.

openclaw-barnacle Bot added size: M maintainer Maintainer-authored PR labels Jul 17, 2026

steipete merged commit d3143ce into main Jul 17, 2026
210 of 220 checks passed

steipete deleted the perf/ci-sticky-minters-o1 branch July 17, 2026 10:52

steipete mentioned this pull request Jul 17, 2026

perf(ci): stabilize dependency snapshot refresh #109934

Merged

github-actions Bot mentioned this pull request Jul 17, 2026

📡 Upstream Digest — 2026-07-17 11:15 UTC curtismercier/openclaw-mods#1322

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

perf(ci): collapse remaining per-PR sticky-disk minters to O(1) keys#109929

perf(ci): collapse remaining per-PR sticky-disk minters to O(1) keys#109929
steipete merged 1 commit into
mainfrom
perf/ci-sticky-minters-o1

steipete commented Jul 17, 2026

Uh oh!

clawsweeper Bot commented Jul 17, 2026

Uh oh!

Uh oh!

steipete commented Jul 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

steipete commented Jul 17, 2026

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Uh oh!

clawsweeper Bot commented Jul 17, 2026

Uh oh!

Uh oh!

steipete commented Jul 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant