Repository navigation
ci: let the macOS 15 and 26 pools share one Swift package cache - #13925
Conversation
test-e2e.yml, test-depot.yml and perf-activation.yml key the Swift package
cache by the runner pool string, so a warm cache on blacksmith-6vcpu-macos-15
does nothing for a job on blacksmith-6vcpu-macos-26 and vice versa. Every move
between pools pays a cold package resolve.
Nothing about .ci-source-packages is pool-specific. It holds cloned package
sources and downloaded binary artifacts, not build products, and all three
workflows already run sanitize-xcode-source-packages-cache.py, whose whole job
is dropping the SourcePackages state that stores checkout-absolute paths.
The bare key is already this repository's convention, including on the paths
that matter most: ci-macos.yml, release.yml, nightly.yml and
cli-pipe-regressions.yml all use spm-<Package.resolved hash> with a spm-
restore prefix, and cli-pipe-regressions already spans two pools that way.
These three were the outliers.
ci-macos-compat.yml keeps spm-${{ matrix.os }}- deliberately: it is an OS
compatibility matrix, so partitioning by OS is the point.
Keying by Xcode version instead would not help here. The two pools ship
different Xcodes (26.3 on macos-15, 26.5 on macos-26), so that partitions them
exactly as the pool string does.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
|
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 (3)
Included review availability: Your plan provides up to 10 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughThree GitHub Actions workflows now key Swift package caches by the ChangesSwift Package Cache
Priority: ⬇️ Low Estimated code review effort: 1 (Trivial) | ~4 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The shared Swift package caches do not expose the claimed untrusted cache-writing path. No actionable merge-blocking risk remains after normal checks. 🚥 Pre-merge checks | ✅ 25✅ Passed checks (25 passed)
✨ Finishing Touches🧪 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 |
Review — holds up, mergingThree workflow files, I checked the thing that would make this unsafe rather than taking the premise: whether Keying on Worth being precise about the win, since a shared key changes hit rate rather than content: this converts two pools that each miss on the other's warm cache into one pool that hits either. It does not make any individual resolve faster. Non-blocking, but the one thing I would keep an eye on: Enabling auto-merge; required checks remain the gate. — Zarathustra g1 🌱 |
ca867b7 ci(ios): bound the xcodebuild test invocation so a teardown wedge fails fast (manaflow-ai#13927) 9ffbb6a ci: compile the E2E test product once, in its own job (manaflow-ai#13908) 827f614 ci: let the macOS 15 and 26 pools share one Swift package cache (manaflow-ai#13925) b79a83b Price GPT-6 models in coderouter API-equivalent estimates (manaflow-ai#13892) ff20a22 Expose in-flight drag intent to custom JavaScript sidebars (manaflow-ai#13841) 3344583 Capture Cloud Desktop click destinations before queued opens (manaflow-ai#13897) ac041c1 test(ios): assert the letterbox a daemon-push shrink actually produces (manaflow-ai#13920) ce1c55c Catch guard-group drift between ci.yml and GROUPS (manaflow-ai#13924) 3466781 ci: keep leading whitespace in workload profile git output (manaflow-ai#13883) 78e0d83 Make the shortcut reference list every action the schema accepts (manaflow-ai#13911) 94fc7e8 ci: stop buying a universal Release build for CI janitors and reporters (manaflow-ai#13912) b91fff1 fix(ios): restore the package conventions lint to green on main (manaflow-ai#13904) 6defb93 ci: skip the nightly publish when no changed path reaches the app (manaflow-ai#13899) c57b001 ci: let E2E runs seed the compilation cache from any revision on main (manaflow-ai#13900) # Conflicts: # .github/workflows/nightly.yml # .github/workflows/perf-activation.yml # .github/workflows/test-depot.yml # .github/workflows/test-e2e.yml # .github/workflows/test-ios.yml
Problem
Three workflows key the Swift package cache by runner pool:
test-e2e.ymldefaults toblacksmith-6vcpu-macos-26but is routinely dispatched onto the macOS 15 pool. Because the pool string is in the key and in the restore prefix, a run on one pool can never warm a run on the other — the samePackage.resolvedresolves and re-fetches from scratch on each side. The macOS 15 pool is the contended one, so this is exactly where the cold fetch hurts most.It also silently broke a stated intent.
test-depot.yml:150-151says it exists to "consume the existing E2E dependency cache", but its fallback literal wasblacksmith-6vcpu-macos-15whiletest-e2e.yml's wasblacksmith-6vcpu-macos-26. WithMACOS_RUNNER_TESTSunset — the documented intended state — the two never shared a key at all.Fix
Drop the pool from the key and the restore prefix in the three workflows whose cached directory is pool-independent:
test-e2e.yml:396test-depot.yml:152perf-activation.yml:168all become
key: spm-${{ hashFiles('cmux.xcodeproj/.../Package.resolved') }}/restore-keys: spm-.Why this is safe
The arm64 pools have already been sharing this exact directory in production.
vars.CI_CACHE_BACKENDisr2and every wrapper call resolvesvars.CI_CACHE_BACKEND || 'r2', soci-macos.yml:455— which restores.ci-source-packagesunder a barespm-<hash>— goes to R2, andscripts/ci/r2-cache.sh:36namespaces byv1/${RUNNER_OS}-${RUNNER_ARCH}:macOS-ARM64, with no OS version in it. macOS 15 and macOS 26 have been reading and writing one shared object there all along. This PR brings the GitHub-store side in line with what the R2 side already does.Supporting points:
.ci-source-packages, passed as-clonedSourcePackagesDirPath. It holds package checkouts and binary artifacts, not build products — unlike DerivedData, whichci-macos.yml:91-93correctly keeps pool-scoped ("two pools lay the workspace out differently, and a product built under one cannot be relocated into the other").sanitize-xcode-source-packages-cache.pyremoves exactly one file, the top-levelworkspace-state.json(the only checkout-absolute-path state); what survives ischeckouts/,repositories/andartifacts/, all pinned byPackage.resolved, which is in the key. The onlybinaryTargetin the macOS graph is GhosttyKit via a localpath:(Packages/macOS/CmuxTerminalCore/Package.swift:32-35), so it never lands inartifacts/.spm-<hash>is the repo's existing convention for this key:ci-macos.yml:455(same.ci-source-packagespath),nightly.yml:700, and — on the GitHub store, not the R2 wrapper —release.yml:259.ci-macos.yml:3232,release.yml:259andcli-pipe-regressions.yml:57cache.spm-cacherather than.ci-source-packages; sincepathis part of the GitHub cache version, those are distinct entries and cannot collide.Second commit: closing the two edges this opens
Sharing one namespace makes two previously-unreachable edges reachable, so
079de32handles both.ci-macos-compat.ymlmoves to acompat-spm-prefix. It is an OS/arch compatibility matrix with an x86_64 leg (macos-15-intel) and an older-Xcodemacos-14leg, caching the same.ci-source-packagespath. Same path means same cache version, so a barerestore-keys: spm-would prefix-match an Intel-produced entry.r2-cache.sh:36namespaces this directory by$RUNNER_ARCH, so the repo already treats arch as identity-bearing. Prefixing makes the namespaces disjoint by construction instead of by luck.perf-activation.ymlgets the poisoned-cache guard. It was the one consumer of the shared namespace without one:test-e2e.yml:412-419andtest-depot.yml:168-175both verify Sparkle and Sentry actually materialized andrm -rfbetween attempts, because resolve can report success with no binary artifacts. perf-activation retried against the same possibly-poisoned directory and failed opaquely later in "Build tagged app".Verification
actionlintrc=0. Its single SC2129 finding is atperf-activation.yml:70, confirmed pre-existing onorigin/main; these edits are at:168and:180.test_ci_pull_request_caches_are_read_only.py,test_ci_manual_macos_package_cache.pyandtest_ci_sanitize_xcode_source_packages_cache.py.🤖 Generated with Claude Code