fix(ci): scope the Next.js build cache sticky disk per branch - #6072
Conversation
The Turbopack persistent build cache disk was keyed on github.event_name alone, so every open PR shared one mutable volume. A sticky disk mount clones the last committed snapshot and commits back last-write-wins, so each PR build restored a cache produced by a different branch. Measured on the same staging commit, two runs minutes apart: the single-writer push disk compiled in 9.3 min, the shared pull_request disk in 14.0 min. Across 22 recent runs, push builds land at 3.5-11 min and PR builds at 13.5-17.8 min. Correctness matters more than the minutes here. turbopackFileSystemCacheForBuild is beta and cross-commit restore is not a documented-supported mode - vercel/next.js#87283 reports stale HTML from a cache built at another commit, with no maintainer answer. Every other cache layer in this repo already isolates by branch: GitHub's cache cannot read sibling branches, and Blacksmith's cache product branch-scopes by default. Only this key didn't. Adds pre/post-build cache size reporting, because whether the cache was warm is otherwise invisible - the disk mounts either way and turbo buffers the build log. Adds ci-cache-cleanup.yml to delete a branch's disk when its PR closes, so per-branch disks track open PRs instead of accumulating at ~5 GB each. Also corrects the 105s-cold/22s-warm comment: those were local numbers and have never reproduced in CI.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryLow Risk Overview Adds Observability: pre- and post-build Reviewed by Cursor Bugbot for commit cb77e1e. Configure here. |
Greptile SummaryScopes the Next.js/Turbopack build-cache sticky disk per branch and adds PR-close cleanup plus cache-size reporting.
Confidence Score: 5/5This PR appears safe to merge; no blocking failures remain from prior or current review. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| .github/workflows/test-build.yml | Per-branch sticky-disk key for Next.js cache plus pre/post-build size reporting; no app runtime impact. |
| .github/workflows/ci-cache-cleanup.yml | New PR-close job deletes the pull_request-scoped sticky disk with a key meant to match the mount key. |
Reviews (2): Last reviewed commit: "refactor(ci): trim the cache-key comment..." | Re-trigger Greptile
Keeps the mechanism (one mutable volume per key, clone-on-mount, last-write-wins) and the two numbers, drops the restated reasoning. Both report steps collapse to a single du, dropping a second full walk of a ~5 GB tree. Cleanup workflow loses a concurrency group a once-per-PR delete never needed.
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit cb77e1e. Configure here.
| uses: useblacksmith/stickydisk-delete@b41313d28b8647d72114c9ba3c96bb04061562b6 # v1 | ||
| continue-on-error: true | ||
| with: | ||
| delete-key: ${{ github.repository }}-nextjs-cache-pull_request${{ github.event.pull_request.head.repo.fork && '-fork' || '' }}-${{ github.head_ref }} |
There was a problem hiding this comment.
Fork cleanup key can miss
Low Severity
The delete key relies on github.event.pull_request.head.repo.fork, but on pull_request closed that head.repo can be null when the source fork was deleted. The expression then omits the -fork suffix, so cleanup targets a different key than the mount used and the ~5 GB disk is left behind. continue-on-error hides the failed delete.
Reviewed by Cursor Bugbot for commit cb77e1e. Configure here.
| uses: useblacksmith/stickydisk-delete@b41313d28b8647d72114c9ba3c96bb04061562b6 # v1 | ||
| continue-on-error: true | ||
| with: | ||
| delete-key: ${{ github.repository }}-nextjs-cache-pull_request${{ github.event.pull_request.head.repo.fork && '-fork' || '' }}-${{ github.head_ref }} |
There was a problem hiding this comment.
Sibling PR disk wiped early
Medium Severity
Cleanup deletes the Next.js sticky disk by head_ref on every PR close, but the mount key is also only head_ref-scoped. Closing one PR from a branch that still has another open PR (for example into main and staging, or a non-CI base) removes the shared volume the surviving PR still uses. The next build goes cold, and continue-on-error hides a wrong delete.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit cb77e1e. Configure here.
…uilds) (#6080) * perf(ci): disable the Turbopack persistent build cache It is a net loss at this app's size. A controlled A/B on one branch (#6078), three runs with a byte-identical module graph so only cache state varied: cache OFF 113s compile, 2m53s job cache ON, cold 162s compile, 3m54s job cache ON, warm 360s compile, 8m18s job The cache made the same build 3.2x slower. It also grew 5.1 GB -> 12 GB across two runs of an unchanged tree, which explains the progressive degradation seen on longer-lived disks (up to 11.7 min): the more a disk is written, the more the next run must read and revalidate. Flag manipulation is visible in the logs — the cache-on runs print `✓ turbopackFileSystemCacheForBuild`, the cache-off run omits it — and every run used `turbo --force` so none is a replayed log. #5869 enabled this on locally-measured numbers (105s cold -> 22s warm) that never reproduced in CI and are inverted here. #6072 then branch-scoped the disk to stop PRs restoring each other's caches; that fixed a real problem, but with the cache off the disk is unnecessary, so the mount, the pre/post size reporting, and the env gate all go with it. Pins `turbopackFileSystemCacheForBuild: false` explicitly rather than relying on the Next default: upstream already flips that default to true in canary/preview builds (vercel/next.js#94616), so leaning on the default would let a version bump silently re-enable this. Keeps ci-cache-cleanup.yml, re-scoped to draining the 5-12 GB volumes that PRs opened while the per-branch key was live still hold — nothing else reclaims them. It is a no-op for new PRs and can be deleted once drained. Caveat: n=1 per cell. The 3.2x effect size and agreement with ~15 prior observations make it convincing, but this is three runs, not a distribution. * docs(ci): correct the cleanup key comment after the mount was removed Greptile P2: the delete step still claimed its key must stay byte-identical to the Mount Next.js build cache step in test-build.yml, but this PR removes that mount. It is now a hard-coded legacy drain key that mirrors nothing.
1 participant
Summary
The Turbopack persistent build cache sticky disk was keyed on
github.event_namealone, so every open PR shared one mutable volume. Per Blacksmith's docs a sticky disk mount clones the last committed snapshot and the job commits back last-write-wins — so each PR build restored a Turbopack cache produced by a different branch.Keys it per branch, reports whether the cache was actually warm, and deletes a branch's disk when its PR closes.
Evidence
Same staging commit, two runs minutes apart, identical runner — the only variable is which disk was mounted:
push…-nextjs-cache-pushpull_request…-nextjs-cache-pull_requestAcross 22 recent runs the split is clean: push builds 3.5–11 min, PR builds 13.5–17.8 min.
Why this is the correctness fix, not just a perf tweak
turbopackFileSystemCacheForBuildis beta, and restoring a cache built at a different commit is not a documented-supported mode. In vercel/next.js#87283 a user reports stale HTML in production from exactly that — the same commit producing different output depending on which cache was restored — and asks maintainers whether cross-build restore is supported at all. Unanswered. Others report build errors and 404s after branch switches.Sharing one mutable Turbopack build cache across ~10 concurrent PR branches with last-write-wins is precisely that configuration, which means
Build Appmay not faithfully reflect the branch under test.Per-branch keying is also the documented pattern, not a workaround. Next.js's own CI-caching guide uses
key: \${CI_COMMIT_REF_SLUG}(per-branch) in its GitLab example, and its GitHub Actions example only looks shared — GHA cache is branch-scoped by design ("cannot restore caches created for child branches or sibling branches"; PR caches are scoped to the merge ref). Blacksmith's own cache product branch-scopes by default too. This key was the only layer in the stack with no branch isolation.What this does NOT fix
This will not get
Build Appback to 3 minutes. On the single-writerpushdisk — genuinely warm — compile has itself roughly doubled today:The inflection sits between the 11:54 PT and 14:50 PT builds. That regression is real and separate; it needs a bisect and is not addressed here.
Trade-offs
restore-keysfallback, so the first build on a new branch is fully cold. Given cross-commit restore is dubious anyway, cold-first is the safe behavior — and the new reporting gives us the actual cold number instead of a guess.ci-cache-cleanup.ymlon PR close.continue-on-error: truebecause PRs that skippedBuild Appviapaths-ignorehave no disk to delete.Type of Change
Testing
actionlintclean on both workflows (only the pre-existing unknown-blacksmith-*-label false positive).stickydisk-deletepinned to the real v1 commit SHA with itsdelete-keyinput verified against the action'saction.yml.This PR's own
Build Apprun is the first check: it should reportPRE_BUILD_CACHE=cold(new branch, new key), and a second push to this branch should reportPRE_BUILD_CACHE=warmwith a faster compile. That is the direct verification of the whole premise — worth confirming before merge.