Repository navigation
ci: key reload-build caches on the commit and fall back across branches - #14099
Conversation
Run 35938367902 dispatched reload-build with a short SHA and rebuilt cold in 753 seconds; a follow-up dispatch with the full SHA on the same branch also restored nothing. The DerivedData key embeds the raw `ref` input, and the only cross-branch fallback is a `main-` prefix that no dispatch saves. This guard runs the workflow's real cache-metadata script and asserts that every spelling of one commit keys identically, and that a later commit under any ref text restores an earlier entry. It fails on main. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
reload-build derived its DerivedData key from the raw `ref` input, so a short SHA, a full SHA, and a branch name for one commit each got their own key, and reload-cloud's per-call ephemeral branches never shared an entry. The only cross-branch fallback was `<prefix>main-`, which a run saves only when someone dispatches `ref=main`, so it was effectively never present. Run 35938367902 restored nothing and built cold in 753 seconds. The DerivedData key is now `<prefix>commit-<resolved sha>`, and the last restore-key is the Xcode/runner/workspace prefix alone: the newest entry from any commit or branch with the same toolchain and checkout path. That is safe to adopt because tracked-source mtimes are already derived from blob ids, so every file that differs from the cached build recompiles, and a failed cached build already retries cold. The SPM cache gains the same prefix fallback, so a Package.resolved bump no longer discards every other package checkout. actions/cache scopes entries to the dispatch ref. A run dispatched on an ephemeral branch still restores default-branch entries but saves where no later run can read; the workflow now says so with a notice, and timings.json records `cache_scope_ref` in place of the retired `cache_branch_key`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
All contributors have signed the CLA ✍️ ✅ |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: manaflow-ai/cmux/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 10 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe reload-build workflow now builds macOS cache keys from the resolved commit and restores compatible caches across commits and branches. A new regression test checks the cache metadata and restore behavior, and CI runs that test. ChangesReload-build cache behavior
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The cache-key test covers the documented dispatch path, and no issue identified here prevents merging after normal checks. 🚥 Pre-merge checks | ✅ 24 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (24 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 7.69% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 1 files. (3 skipped: 3 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The spelling guard injected SOURCE_REF, which the fixed step no longer reads, so it could not catch ref-text keying reintroduced under another name. It now evaluates the step's own env block with each spelling substituted for every ref-bearing expression, fails on an expression it does not model, and checks the notice for dispatches off the default branch. It still fails against the workflow on main. Also rewraps the workflow header. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Pushed
The review also raised two points I have not changed:
A live A/B check is in progress. Run A, 35944989183, is a cold baseline at — Rivetmoss g1 🦉 |
5b646b7 ci: apply the queue janitor threshold per runner pool (manaflow-ai#14131) d49a1b1 ci: reuse the headless cmux-tui build in SDK conformance (manaflow-ai#14108) ba85a1b ci: key reload-build caches on the commit and fall back across branches (manaflow-ai#14099) 27fb3bf ci: hand focused test-macos-suite dispatches to run-e2e.sh (manaflow-ai#14075) d18c1b9 ci: let a failed compile admission mark a run doomed for the queue janitor (manaflow-ai#14129) dfdce2c ci: bind pull request product reuse to the merge it compiled (manaflow-ai#14080) afacff3 ci: sparse-checkout the Claude wrapper regression job (manaflow-ai#14088) 35a6bb1 ci: stop pinning remote-daemon macOS tests to the macOS 26 pool (manaflow-ai#14128) 1ba6d77 ci: run macOS jobs on GitHub-hosted runners alongside Blacksmith (manaflow-ai#14097) 587de87 Import CmuxWorkspaces where CodexTurnRestoreIntentPolicy names its liveness type (manaflow-ai#14123) # Conflicts: # .github/workflows/ci-guards.yml # .github/workflows/ci-macos.yml # .github/workflows/ci-queue-janitor.yml # .github/workflows/ci.yml # .github/workflows/cmux-tui-sdks.yml # .github/workflows/reload-build.yml # .github/workflows/remote-daemon.yml # .github/workflows/test-macos-suite.yml
reload-build.ymlrebuilt cold on dispatches that should have been warm. Run 35938367902 (ref=9a0702e0f5) restored nothing and spent 753 s in the build step (derived_data_cache_status: miss, emptymatched_key). A second dispatch for the same branch with a full SHA, 35941854226, missed the same way.Cause. The DerivedData key embedded a slug of the raw
refinput. A short SHA, a full SHA and a branch name for one commit therefore produced three different keys, and each of reload-cloud's per-call branches got a key of its own. The only cross-branch restore-key was<prefix>main-, which a run writes only when dispatched withref=main. Nothing does that, so the fallback never matched.Now.
<prefix>commit-<resolved sha>-run-…. Every spelling of a commit keys identically.<spm prefix>with noPackage.resolvedhash. A package bump no longer throws away every other checkout; fallback hits already go through the sanitizer.tmp/ios-buildfix×24 andreload-blacksmith/*). The workflow now emits a notice when that happens, and the header says to dispatch onmainwith-f ref=<branch>. The recent reload-cloud dispatches already do this.timings.json:cache_branch_keyis replaced bycache_scope_ref, the ref whose cache scope the run saves into.Validation.
tests/test_ci_reload_build_cache_keys.pyis new and registered aslinux-guard. It runs in theapp-host-cachegroup inci-guards.yml. It runs the workflow's real metadata script against a scratch repository and resolves the restore-keys the workflow passes to actions/cache. It asserts that the empty ref, short SHA, full SHA, branch andrefs/heads/<branch>all give one key. It also asserts that a later commit whosePackage.resolvedchanged restores an earlier entry for both caches under any ref text.actionlintis clean onreload-build.ymlandci-guards.yml.tests/test_*.pyandtests/test_*.shon this branch and on anorigin/mainworktree. The results are identical except for the new test.test_ci_app_host_xcodebuild_retry.shfails in both trees when rerun alone: it relies on a 0.1 s idle timeout and is load-sensitive.Same footgun elsewhere: none with ref-text keys. Every other cache key uses a resolved SHA or a content hash. The related gaps are listed below, with none changed here:
ci-macos-compat.yml:132-135: the DerivedData key has norestore-keys, and saves land in the dispatch branch's scope. A restore-key alone would not help, because this workflow does not normalize mtimes, so a restored DerivedData recompiles everything anyway.ci-macos.yml:454-457(also:2464,:3271): theghosttykit-sentry-off-v1-<sha>restore has no main-scoped writer. Only dispatch-onlyci-macos-compat.ymlsaves that key, so it misses and falls through todownload-prebuilt-ghosttykit.sh. It costs time, not correctness.test-ios.yml:443-457: the only writer is gated oninputs.seed_cacheon main, and no caller passes it. iOS DerivedData is never cached.reload-build.ymlwithplatform=ios: every cache step is macOS-only, so iOS reloads are always cold.perf-activation.yml:210andtmux-corpus.yml:56-76: dispatch-only, with no DerivedData cache and caches saved only into the dispatch scope.test-e2e.yml:46: the concurrency group uses rawinputs.ref, so a short SHA and a full SHA of one commit do not cancel each other. This is not a cache issue.Related: #14081 seeds per-main-push DerivedData to R2 for PR admission. It is a different build (admission products, a different path contract), so reload-build cannot adopt it as is. Once it lands, pointing this workflow's fallback at an R2 seed with the same contract would remove the dependence on a recent main-scoped dispatch.
— Rivetmoss g1 🦉
🤖 Generated with Claude Code
Summary by cubic
Fixes reload-build cache misses by keying DerivedData and SPM caches on the resolved commit SHA instead of the raw
reftext, so short SHAs, full SHAs, branch names, andrefs/heads/<branch>all hit the same entry. Adds a generic Xcode/runner/workspace prefix fallback so any commit or branch built with the same toolchain and checkout path restores the newest cache.Package.resolvedbump no longer discards other package checkouts; SPM fallback hits go through the sanitizer.timings.jsonrecordscache_scope_refin place of the retiredcache_branch_key.Written for commit 5ff0959. Summary will update on new commits.
Summary by CodeRabbit