Repository navigation
Conversation
…per-tier lanes on main PR builds schedule one darwin aarch64 test-bun step with no release-tier agent rule, so its shards take whichever arm64 box in test-darwin frees up first. main keeps the latest and previous lanes so the canary gate still covers both macOS versions. A manual build can still pick the pinned lanes on any branch. Platform.release is now optional so the unpinned lane has a key and label without a version; output for main is unchanged.
|
Warning Review limit reached
Next review available in: 12 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
Comment |
|
Updated 6:07 PM PT - Aug 12th, 2026
❌ @robobun, your commit 7552c4c has 1 failures in
🧪 To try this PR locally: bunx bun-pr 37938That installs a local version of the PR into your bun-37938 --bun |
|
Status: ready for review; CI result on 7552c4c is in. Reproduced from the Buildkite API rather than locally: builds 92897 and 93345 (#37739) each had both The change is confined to Build 93637 (this PR): the unpinned darwin shards were generated as intended and both passed (on Related fleet-side change: #37934 (independent; its effect also depends on upgrading the four tart hosts, which run macOS 15.7.9 today). |
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. The change looks correct and is well-verified (byte-identical pipeline on main, expected diff on branches, no key collisions), but it's a CI coverage policy decision — PR builds drop guaranteed macOS 26 test coverage in exchange for throughput — so a maintainer sign-off would be worthwhile.
Checked: getPlatformKey/getPlatformLabel/getImageKey produce identical strings for every platform that still has a release; the unpinned lane's group label :darwin: aarch64 merges into the existing build group without a duplicate step key; EXPECTED_PLATFORM_RELEASE is correctly omitted for the unpinned lane (tier is undefined); traceOrderTargets still pins darwin trace to latest and only runs on main so it's unaffected by the capacity issue.
Extended reasoning...
Overview
This PR touches only .buildkite/ci.mjs, the Buildkite pipeline generator. It adds a third darwin aarch64 entry to testPlatforms with no tier or release, introduces runsByDefault() to select the two tier-pinned lanes on main and the single unpinned lane everywhere else, and makes Platform.release optional so getPlatformKey/getPlatformLabel/getImageKey can handle the release-less entry. getTestAgent() now emits the release-tier agent rule only when the lane declares a tier (previously: whenever arch === 'aarch64'), and getPipelineOptions() returns testPlatforms.filter(runsByDefault) instead of the full list for both webhook builds and manual builds that leave the platform selector blank.
Security risks
None. This is CI pipeline configuration; no runtime code, no auth, no user input handling. The change only affects which Buildkite agents pick up darwin test shards.
Level of scrutiny
Medium. The code change itself is small and mechanical, and the author verified it end-to-end (generated pipeline byte-identical on main, expected delta on a branch, no duplicate step keys, this PR's own build exercises the new lane). I traced each modified helper and confirmed output is unchanged for every existing platform, that the unpinned lane's group merges cleanly into the darwin-aarch64 build group via the label-keyed stepsByGroup merge, that EXPECTED_PLATFORM_RELEASE is not set for the unpinned lane so the platform smoke check passes on any macOS version, and that darwin is already excluded from imagePlatforms so the release-less getImageKey output is harmless there.
However, the effect is a CI coverage policy change: PR builds (and merge-queue builds, since isMainBranch() is false there) will no longer be guaranteed a run on macOS 26 — with the current 8:2 fleet split they'll land on macOS 14/15 ~80% of the time. Main keeps both pinned lanes, so canary coverage is preserved, and the alternative (145 of 220 PR builds failing on expired jobs) is clearly worse. But dropping per-PR OS-version coverage is the kind of tradeoff a maintainer should explicitly acknowledge rather than have auto-approved.
Other factors
The PR description is unusually thorough — it quantifies the capacity problem from the Buildkite API, explains why retrying can't help (priority dispatch + back-of-queue), and cross-references the independent fleet-side rebalance in #37934. There are no outstanding reviewer comments. The change is easily reversible once the latest pool has more capacity, and the code comment in testPlatforms says as much.
|
On the coverage point from the review: agreed that this is a policy trade, which is why it is assigned to @alii. One refinement pushed in 7552c4c: merge-queue builds ( |
There was a problem hiding this comment.
I reviewed this and didn't find any bugs. The code looks correct — step-key uniqueness holds (the unpinned lane's group merges into the :darwin: aarch64 build group, so no duplicate darwin-aarch64 key reaches the output), getPlatformKey/getPlatformLabel/getImageKey are unchanged for every platform that has a release, and EXPECTED_PLATFORM_RELEASE is correctly omitted for the floating lane. That said, this is a CI policy change — PR builds drop guaranteed per-macOS-version coverage in favor of throughput, with the merge queue as the backstop — so a maintainer should sign off on the tradeoff.
Extended reasoning...
Overview
Single-file change to .buildkite/ci.mjs. Adds a third darwin/aarch64 entry to testPlatforms with no tier/release, introduces runsByDefault() to select pinned lanes on main/merge-queue and the unpinned lane everywhere else, makes Platform.release optional (with getPlatformKey/getPlatformLabel/getImageKey updated to skip the segment when absent), switches getTestAgent() to emit release-tier only when the lane declares a tier, and routes getPipelineOptions() defaults through the new filter.
Security risks
None. Pipeline-generator only; no auth, secrets, or shell-string construction changed.
Level of scrutiny
Medium. Not runtime code, but it changes which test lanes gate PRs. I traced key/label generation for the new release-less entry through the group-merge in getPipeline(): on a PR the test group's label :darwin: aarch64 matches the build group's getTargetLabel, so it folds in and only one group with key darwin-aarch64 reaches the YAML (test step key darwin-aarch64-test-bun is distinct from darwin-aarch64-build-bun). On main the pinned entries keep their existing keys/labels, and runsByDefault excludes the unpinned entry there, consistent with the byte-identical claim. imagePlatforms already filters out darwin so the release-less getImageKey output isn't consumed there; traceOrderTargets still targets tier: latest but only runs on main, where those agents remain adequate.
Other factors
The mechanics look right and the PR description is thorough with local pipeline-diff verification. What warrants a human look is the coverage decision itself: PR builds will no longer be guaranteed to run on macOS 26 (they'll land on whichever of the ten arm64 boxes is free, which today is 80% previous). The merge queue still runs both pinned lanes so nothing lands untested on 26, but a maintainer should confirm that's the intended backstop and that the capacity math justifies the tradeoff versus alternatives like #37934.
|
One correction to the review's framing, so the trade is judged on the right facts: the repo does not currently use a merge queue (none of today's 979 builds was a For reference, this PR's own build (93637) generated the intended step: one |
Problem
:darwin: 26 aarch64 - test-bunshards red, in stateexpired: no agent ever picked them up. node:http: enforce headersTimeout and requestTimeout on HTTP/1 fallback connections #37739 hit this on both of its full builds today (92897, 93345); each time the shards expired, were retried, and expired again, four attempts over five hours, while main builds in the same hours (92941, 93041, 93528) got the lane in 2 to 14 minutes.release-tier=latest(getTestAgent,.buildkite/ci.mjs:401), and exactly two agents carry that tag: the bare macOS 26 boxesdarwin-arm64-breadstickanddarwin-arm64-crouton(both register asdarwin-aarch64-26.6.1-1). The eight tart agents on the four M2 Ultra hosts are allrelease-tier=previous, so thepreviouslane has four times the capacity oflatest.getPriority()gives main 2 and PRs 0, so main always goes first and PRs absorb the whole shortfall. Of the non-canceled builds created on 2026-08-12 (UTC, through about 21:40Z): 293 PR builds reached the darwin tests; thelatestlane ran on 75 of them and expired for good on 145 (79 were still queued, 170 jobs for two agents); thepreviouslane expired for good on 0. Median queue wait for PR jobs that did run: 45 min onlatest(p90 96 min), 29 min onprevious; 5 min and 1 min for main.Fix
testPlatformsgains a third darwin aarch64 entry with notier, andrunsByDefault()picks per branch: main and merge-queue builds (getPriority()2 and 1, so they are never the ones starving) keep the pinnedlatestandpreviouslanes and what lands is still checked on both macOS versions; every other build gets the one unpinned lane, whose agent rules arequeue=test-darwin, os=darwin, arch=aarch64, so its two shards take whichever of the ten arm64 boxes frees up first.getPipelineOptions()uses it for webhook builds and for manual builds that leave the platform field blank; a manual build that picks a pinned lane in the form still gets it on any branch.getTestAgent()emitsrelease-tieronly when the lane declares a tier (x64 never did; its commented-out entry drops the now-meaninglesstier).EXPECTED_PLATFORM_RELEASEwas already only set for thelatestlane, so the unpinned lane passes the platform smoke check on a 15 or a 26 box, same aspreviousdoes today.Platform.releasebecomes optional so the unpinned lane has a key and label without a version: stepdarwin-aarch64-test-bun, label:darwin: aarch64 - test-bun. Its group label matches the darwin aarch64 build group, sogetPipeline()'s group merge puts the test shards next todarwin-aarch64-build-bunin the UI.getPlatformKey/getPlatformLabel/getImageKeyproduce the same strings as before for every platform that has a release.latest) and 32 (previous); unpinned, the same PRs need about 28 jobs an hour from a pool that does about 40. Peak hours (74 and 84 builds created in the 13Z and 17Z hours) will still queue, and more boxes is the only fix for that. darwin-ci: bake a macOS 26 guest image too and run one agent per image, so tart hosts serve the latest tier #37934 rebalances the tart hosts to one guest per tier, which helps main'slatestredundancy but still leaves each pinned PR lane short on its own (5 agents, about 20 jobs an hour), and needs the four tart hosts upgraded first: they run macOS 15.7.9 today and a tart guest cannot be newer than its host. Both changes are independent; darwin-ci: bake a macOS 26 guest image too and run one agent per image, so tart hosts serve the latest tier #37934 only touches a comment ingetTestAgent()in this file.node .buildkite/ci.mjs, which writes.buildkite/ci.ymland uploads nothing outside Buildkite):main, the generated pipeline is byte for byte identical before and after this change; on a localgh-readonly-queue/...branch it haspriority: 1and both pinned lanes;darwin-aarch64-26anddarwin-aarch64-14test groups (4 jobs,release-tierrules,EXPECTED_PLATFORM_RELEASE: 26) replaced by onedarwin-aarch64-test-bunstep (2 shards, norelease-tierrule, no release assertion) inside the:darwin: aarch64group; no duplicate step keys;darwin-aarch64-26,darwin-aarch64-14anddarwin-aarch64;Background
release-tieris a tag the agents put on themselves from the macOS major version the jobs run on:latestis 26,previousis 14 and 15 (scripts/agent.mjson bare boxes,scripts/darwin-ci/lib/agent.tson tart hosts, where it is the guest image's version). A Buildkite job only goes to an agent whose tags satisfy every rule on the step; extra agent tags do not matter, which is why a step with norelease-tierrule matches both tiers.baremode inscripts/darwin-ci/README.md) and are the only macOS 26 agents.expiredis the state Buildkite gives a scheduled job that no agent accepted within the configured window (about an hour here); it fails the build like a red job. Within a priority level the darwin boxes hand out jobs in build order (today's dispatch log on the tart boxes is a strictly ascending sequence of build numbers, retried jobs included), so a retry keeps its build's place in line; a build dies on a lane when its retries run out before the lane gets down to it. Thelatestlane was running about five hours behind, and builds get about four hours of attempts: 92897's last attempt expired at 11:05Z and the lane reached the builds on either side of it at 11:03Z and 11:08Z.getPriority()returns 2 on main, 1 in the merge queue, 0 on PRs, -1 on forks; higher priority is dispatched first regardless of age, so a saturated pool serves main first no matter how long PR jobs have waited.no test proof · iteration 0 · build/CI scripts only; test-proof not applicable