Skip to content

darwin-ci: bake a macOS 26 guest image too and run one agent per image, so tart hosts serve the latest tier - #37934

Open
robobun wants to merge 4 commits into
mainfrom
farm/615d438a/darwin-ci-latest-tier-guests
Open

robobun wants to merge 4 commits into
mainfrom
farm/615d438a/darwin-ci-latest-tier-guests

Conversation

@robobun

@robobun robobun commented Aug 12, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

  • The two :darwin: 26 aarch64 - test-bun shards (release-tier=latest) almost never run on PR builds: they queue, expire after about an hour, and the build ends failed with everything else green. Of the 600 builds created on 2026-08-12 between 07:04Z and 20:20Z, the darwin 26 shards passed on 55 PR jobs and expired on 170 (200 more still queued); the darwin 14 shards (release-tier=previous) passed 367 times in the same window.
  • Every latest job that did run ran on the agent name darwin-aarch64-26.6.1-1 (bare boxes, scripts/agent.mjs naming) with exactly two ever in flight: the latest tier is two agents for the whole PR stream, about four builds an hour. main builds are priority 2 and PR builds priority 0, so PRs are what starves.
  • Cause: scripts/darwin-ci (the tart fleet provisioning ci: run darwin tests on the test-darwin fleet again, for every PR #37633 brought the fleet back with) bakes every host one image from macos-sequoia-xcode and tags all of its agents release=15,release-tier=previous (lib/config.ts baseRemote/guestRelease, lib/agent.ts tags line). All eight tart agents (darwin-arm64-{ciabatta,challah,focaccia,sourdough}-tart-15-{1,2}) serve the darwin 14 lane and nothing provisions agents for the darwin 26 lane, although .buildkite/ci.mjs schedules the same test step once per tier on every build (testPlatforms, plus traceOrderTargets on latest). Before the reimage these hosts ran macOS 26 guests (darwin-*-arm64-tart-26).
  • The split is the in-repo defect, but the fleet is also short overall: the same window had 293 non-canceled builds in 13.25 h (about 22/h), each wanting two 14-minute shards per tier, i.e. roughly 10 busy agents per tier against 8 previous + 2 latest today. By the evening previous shards were expiring too (this PR's build 93698: all other jobs passed, both darwin 14 shards expired once and passed on retry, both darwin 26 shards never ran).
  • The waiting_failed states from build ~93364 onward were a separate problem (build-bun lanes failing on github.com 503s for dep tarballs), tracked separately.

Fix

  • config.tart.guests lists one guest per tier ci.mjs targets, { 26, macos-tahoe-xcode } and { 15, macos-sequoia-xcode }; images are named bun-ci-<release>. spawn is now per image and defaults to 1, and images x spawn is checked against maxGuests = 2 (the Virtualization.framework limit the old spawn: 2 filled exactly), so a host still runs two guests, one per lane.
  • bake is per release: it refuses a release newer than the host's own macOS before pulling anything, and refuses a base image whose sw_vers major is not the release its agents will advertise (the cirruslabs tags are Xcode versions, and the latest lane asserts EXPECTED_PLATFORM_RELEASE=26 inside the guest).
  • install-agent retires whatever agent jobs the host has (per-image or the single unsuffixed tart job, plus a bare scripts/agent.mjs daemon and the nightly reboot daemon), rsyncs these scripts into installDir, then installs one config + launchd job per image (com.buildkite.buildkite-agent.<release>, agent name %hostname-tart-<release>-%spawn, tags release=<N>,release-tier=<tier>) and re-arms the nightly reboot for all of them.
  • The command hook boots bun-ci-<release> for the agent's own release tag, which buildkite-agent exposes to hooks as BUILDKITE_AGENT_META_DATA_RELEASE (ci.mjs already reads ..._QUEUE this way); the previous lane carries no release in the job, so the agent's tag is the only thing that can select the image.
  • provision on a host that is already serving bakes from a staged copy (installDir.next) while the existing agents keep using the installed hooks, and runs install-agent only after both bakes verified, so a failed or interrupted re-provision leaves the host serving as before. Re-running provision is therefore the upgrade path for the four current hosts (README: needs a macOS 26 host or --release 15 --spawn 2, disk for old and new images at once, a free guest slot for the bake, then tart delete bun-ci-base).
  • Why this is the right fix: ci.mjs deliberately tier-splits arm64 and pins latest to 26 (comments at testPlatforms / getTestAgent), a Buildkite tag has one value per agent so serving two tiers needs two agent processes, and keying image and tag to the same release number is what makes a host's tags truthful by construction; a host provisioned with the defaults can no longer land entirely in one tier, which is how this happened. What it does not do: add capacity. With the four hosts it turns 2 latest / 8 previous agents into 6 / 4; at today's volume both tiers stay oversubscribed until the fleet is about 10 tart hosts or ci.mjs schedules darwin on fewer builds, and --release N --spawn 2 per host is the knob for rebalancing. Those are fleet/policy calls left out of this PR; the numbers above are for whoever makes them.
  • Verified with test/internal/source-lints/darwin-ci-release-tiers.test.ts (runs in the GitHub "Source lints" job, which now also triggers on scripts/darwin-ci/** and .buildkite/ci.mjs): it reads the darwin aarch64 lanes out of ci.mjs and checks the configured guests serve exactly those tiers, that the latest guest is the release ci.mjs pins on the latest lane, that each guest has its own base and image, that the defaults fit maxGuests, that each image's agent config carries its release/tier tags and the hook resolves the tag back to that release, and that the hook rejects a missing or non-numeric tag. With main's guest list the first two tests fail (latest unserved, [26] expected, [] configured); bumping only ci.mjs to a hypothetical 27 fails the second; on this branch all six pass under bun bd test, as does the whole test/internal/source-lints/ directory.
  • Also exercised here: every main.ts argument path fails before touching anything (usage, --base without --release, --release 0, --spawn two, --release 14 with no configured base, 2 images x --spawn 2 and 1 x 3 against the cap, tart mode on a non-arm64 host); tsc --strict --noUnusedLocals over scripts/darwin-ci reports only the two errors already present on main; prettier clean; both ghcr.io/cirruslabs/macos-{tahoe,sequoia}-xcode:latest exist. provision/bake/install-agent have not been run against a real host, so the operator doing the rollout should watch the first host; after it, the main-only darwin-aarch64-trace-order step will start landing on tart hosts (it runs on the host, not in a guest, and is soft-fail).
  • darwin-ci: address review of the tart host scripts #37771 rewrites these scripts' shell plumbing and keeps one image per host; the overlap (installTartAgent, main.ts option parsing, config.ts helpers, imports in bake.ts/command.ts) is mechanical to rebase in either order and I will do it for whichever lands second.

Background

  • release-tier: an agent tag derived from the macOS major (latest = 26, previous = 14/15, oldest below). getTestAgent() in .buildkite/ci.mjs targets the darwin aarch64 test step by it, so every build has one step (two shards) per tier; Buildkite only gives a job to an agent whose tags match all of the step's agent query rules.
  • tart host: an Apple Silicon box on queue=test-darwin running only buildkite-agent and Tart; for each test job the command hook (scripts/darwin-ci/hooks/command.ts) clones a baked image into a fresh guest VM, runs the job there over ssh and deletes it, so the release an agent advertises is the guest image's macOS, not the host's. Virtualization.framework allows two macOS guests per host.
  • bare host: a box that runs tests on the host itself via scripts/agent.mjs, tagged from its own macOS version; the only latest agents today are bare.
  • BUILDKITE_AGENT_META_DATA_<TAG>: buildkite-agent exports each of the agent's own tags into the job environment under this name, so a hook can tell which agent config it is running under.
  • job expiry: a scheduled job no agent picks up within the configured window ends expired, which fails the build.
Buildkite evidence (2026-08-12)

Non-retried darwin 26 test-bun job states across builds 92951 to 93549 (600 builds), by branch type:

main  passed          4   (all on darwin-aarch64-26.6.1-1)
pr    passed         55   (all on darwin-aarch64-26.6.1-1)
pr    expired       170
pr    scheduled     200   (still waiting at 20:40Z)
pr    waiting_failed 64   (build-bun failed: github.com 503s, separate issue)
pr    canceled      566

Darwin 14 test-bun in the same builds: 367 passed, 137 scheduled, 15 failed, 64 waiting_failed, 466 canceled; agents darwin-arm64-{ciabatta,challah,focaccia,sourdough}-tart-15-{1,2}.

Every latest job that started between 09:28Z and 20:16Z ran on darwin-aarch64-26.6.1-1, each starting as a previous one finished with exactly two in flight, working through PR builds in order (93002, 93004, ..., 93311 at 20:09Z, builds created five hours earlier). Build 93310's darwin 26 shards: created 15:05Z, expired 16:05Z, retried and expired again at 18:05Z, 19:05Z and 20:05Z; its darwin 14 shards passed at 18:11Z and 18:19Z. Passed darwin shards in the window: 429, median 14 min, p90 15 min; 293 of the 600 builds were not canceled.

Step agent query rules (unchanged by this PR): os=darwin,arch=aarch64,queue=test-darwin,release-tier=latest.

On 2026-08-10 (builds ~91506 to 91542, before the reimage) the same lane ran on darwin-{baguette,challah,ciabatta,focaccia,sourdough}-arm64-tart-26 plus darwin-{breadstick,crouton}-arm64-1.

How the branch evolved during self-review
  1. 8365505986: two guests, per-release bake/agents/hook, the lint test.
  2. bb6e264931: retire a host's agents before replacing what they use; install-agent re-installs the scripts it points the agents at.
  3. edda299189: replaced the early retirement (which left a host idle if a re-provision stopped early) with baking from a staged copy and swapping at the end; host macOS check in bake; images x spawn <= maxGuests.
  4. abd93545c4: lint also checks the latest guest against the release ci.mjs pins and checks the tier sets both ways; a converted bare host's daemon plist is removed, not just unloaded; docs corrected to match.

[stamp-90s] gate passed · iteration 0 · 11 files touched

passes on PR (with fix)
Test-only change.

Debug/ASAN (expected pass):
$ bun bd test 'test/internal/source-lints/darwin-ci-release-tiers.test.ts'
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test test/internal/source-lints/darwin-ci-release-tiers.test.ts
bun test v1.4.0 (abd93545c)

test/internal/source-lints/darwin-ci-release-tiers.test.ts:
(pass) scripts/darwin-ci serves every darwin release-tier that .buildkite/ci.mjs schedules > the configured guest images serve exactly the tiers ci.mjs targets [299.44ms]
(pass) scripts/darwin-ci serves every darwin release-tier that .buildkite/ci.mjs schedules > the latest guest is the macOS release ci.mjs pins on the latest lane [454.03ms]
(pass) scripts/darwin-ci serves every darwin release-tier that .buildkite/ci.mjs schedules > each configured guest bakes from its own base to its own image [27.55ms]
(pass) scripts/darwin-ci serves every darwin release-tier that .buildkite/ci.mjs schedules > the default agents per image fit the guests a host can run at once [8.09ms]
(pass) scripts/darwin-ci serves every darwin release-tier that .buildkite/ci.mjs schedules > each image's agents are tagged with its release and tier, and the hook boots that image [44.12ms]
(pass) scripts/darwin-ci serves every darwin release-tier that .buildkite/ci.mjs schedules > the hook refuses an agent whose release tag is missing or not numeric [5.54ms]

 6 pass
 0 fail
 19 expect() calls
Ran 6 tests across 1 file. [6.28s]
Exit: 0
diff hotspot
.buildkite/ci.mjs                                  |   9 +-
 .github/workflows/source-lints.yml                 |   4 +
 scripts/darwin-ci/README.md                        |  51 ++++++-
 scripts/darwin-ci/guest/bake.sh                    |   2 +-
 scripts/darwin-ci/hooks/command.ts                 |  10 +-
 scripts/darwin-ci/lib/agent.ts                     | 169 ++++++++++++++-------
 scripts/darwin-ci/lib/bake.ts                      |  24 ++-
 scripts/darwin-ci/lib/config.ts                    |  36 ++++-
 scripts/darwin-ci/lib/host.ts                      |   7 +-
 scripts/darwin-ci/main.ts                          |  89 ++++++++---
 .../source-lints/darwin-ci-release-tiers.test.ts   | 128 ++++++++++++++++
 11 files changed, 430 insertions(+), 99 deletions(-)

gate history · 1 passed · 0 rejected · iteration 0

evidence per changed file
file                                                      reads  edits  tests
.buildkite/ci.mjs                                             4      3      0
.github/workflows/source-lints.yml                            1      2      0
scripts/darwin-ci/README.md                                   4     10      0
scripts/darwin-ci/guest/bake.sh                               1      1      0
scripts/darwin-ci/hooks/command.ts                            2      4      0
scripts/darwin-ci/lib/agent.ts                                9     20      0
scripts/darwin-ci/lib/bake.ts                                 2      3      0
scripts/darwin-ci/lib/config.ts                               3      6      0
scripts/darwin-ci/lib/host.ts                                 2      1      0
scripts/darwin-ci/main.ts                                     9     19      0
…t/internal/source-lints/darwin-ci-release-tiers.test.ts      3     12      0

root cause · written by the author bot

The darwin 26 test shards never ran because the tart host provisioning in scripts/darwin-ci/lib/config.ts baked a single macOS 15 guest image and tagged every agent release-tier=previous, leaving the release-tier=latest lane that ci.mjs schedules on every build with only two bare-box agents fleet-wide, so priority-0 PR jobs expired behind main's. The fix bakes one guest image per tier (26 and 15), runs one buildkite-agent launchd job per image tagged with its own release and tier, and has the command hook boot the image matching the agent's release tag, with sw_vers checks in bake, a guest-…

…e, so tart hosts serve the latest tier

scripts/darwin-ci baked every tart host a single macos-sequoia guest and
tagged all of its agents release-tier=previous, so the whole tart fleet
served the darwin 14 lane and the darwin 26 lane (release-tier=latest)
was left to two bare boxes. Every build schedules one job of each tier,
so latest backed up and PR builds' darwin 26 shards expired unrun.

- config.tart lists one guest per tier ci.mjs targets (26 tahoe, 15
  sequoia); images are named bun-ci-<release>
- bake takes a release, and refuses a base image whose sw_vers major
  does not match the release the agents will advertise
- install-agent installs one launchd job + config per image, tagged
  release=<N>,release-tier=<tier>, after retiring whatever agent jobs
  the host had; the nightly cleanup boots out all of them
- the command hook boots the image for the agent's release tag
  (BUILDKITE_AGENT_META_DATA_RELEASE)
- spawn defaults to 1 per image, so a host still runs two guests
- main.ts: --release N pins a host to one image, --base overrides that
  image's base; bases are resolved before the host is touched
- test/internal/source-lints/darwin-ci-release-tiers.test.ts checks
  the configured guests cover every tier ci.mjs schedules darwin
  aarch64 jobs on, and that agent tags, image names and the hook's
  lookup agree; source-lints.yml runs on darwin-ci and ci.mjs changes
@coderabbitai

coderabbitai Bot commented Aug 12, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 6 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 9542efcc-3c27-4bbb-8167-7aad8c581187

📥 Commits

Reviewing files that changed from the base of the PR and between 165dc9f and abd9354.

📒 Files selected for processing (11)
  • .buildkite/ci.mjs
  • .github/workflows/source-lints.yml
  • scripts/darwin-ci/README.md
  • scripts/darwin-ci/guest/bake.sh
  • scripts/darwin-ci/hooks/command.ts
  • scripts/darwin-ci/lib/agent.ts
  • scripts/darwin-ci/lib/bake.ts
  • scripts/darwin-ci/lib/config.ts
  • scripts/darwin-ci/lib/host.ts
  • scripts/darwin-ci/main.ts
  • test/internal/source-lints/darwin-ci-release-tiers.test.ts

Comment @coderabbitai help to get the list of available commands.

@robobun

robobun commented Aug 12, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status: ready for a maintainer with fleet access (head abd9354). Self-review done; four bot review passes (one per push) found no issues; the GitHub "Source lints" job that runs the new test is green on each push. The Buildkite status will stay red on this PR regardless: the darwin shards it needs are the ones the fleet cannot currently run (build 93698: every other job passed, the darwin 26 shards never got an agent), and the other reds so far were a github.com download outage and a main-side worker_threads failure, both reported separately.

Reproduced from the Buildkite API rather than locally: across the 600 builds created on 2026-08-12 (92951 to 93549) every release-tier=latest darwin job that ran landed on the two-slot agent name darwin-aarch64-26.6.1-1, while all eight tart agents were *-tart-15-* (release-tier=previous); 170 PR darwin 26 shards expired in that window, 55 passed. scripts/darwin-ci/lib/config.ts on main bakes only a sequoia (15) guest, so that is the fleet shape its provisioning produces. The same data says the fleet is about half the size the current build volume needs even once split evenly (numbers in the PR body); that part is a fleet decision outside this diff.

test/internal/source-lints/darwin-ci-release-tiers.test.ts fails on main's guest list and passes on this branch. Taking effect needs provision re-run on the four tart hosts (README in the diff has the steps and caveats); nothing changes on the hosts when this merges.

@robobun

robobun commented Aug 12, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 2:05 AM PT - Aug 13th, 2026

❌ @robobun, your commit abd9354 has 2 failures in Build #94076 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 37934

That installs a local version of the PR into your bun-37934 executable, so you can run:

bun-37934 --bun

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed this PR and didn't find any bugs. Because it restructures the darwin CI fleet's provisioning (per-release launchd jobs, per-release images, hook image selection), hasn't been run against a real tart host per the description, and needs coordinating with #37771, a human look before merge would be worthwhile.

What was reviewed:

  • config.ts guests → guestImage/guestBase/agentGuestRelease and their use in bake.ts, agent.ts, hooks/command.ts, main.ts — the release tag round-trips (agent config → BUILDKITE_AGENT_META_DATA_RELEASE → guestImage).
  • installTartAgent's retire-then-install ordering: old plists (including the unsuffixed com.buildkite.buildkite-agent) are booted out and removed before the new per-release ones are written; the nightly cleanup glob covers both shapes.
  • main.ts arg validation: --base without --release and non-positive-integer --release/--spawn fail before any side effect; fail() is process.exit(1) so the ?? fail(...) narrowing holds.
  • The new source-lint test's ci.mjs scan matches the actual testPlatforms/traceOrderTargets line shape and the tartAgentConfig tag format.
Extended reasoning...

Overview

This PR changes scripts/darwin-ci/ from baking one macOS guest image per host to baking one image per configured release (26 and 15) and running one buildkite-agent launchd job per image, so every tart host serves both the latest and previous darwin aarch64 test lanes. The only runtime-repo changes outside scripts/darwin-ci/ are a comment in .buildkite/ci.mjs, two path triggers in .github/workflows/source-lints.yml, and a new source-lint test. The bug hunting system found no issues.

Security risks

None material. This is internal CI-host provisioning run by an operator over ssh with passwordless sudo; no new external inputs are parsed, no secrets are added or moved, and the agent token handling is unchanged. The command hook's new input is BUILDKITE_AGENT_META_DATA_RELEASE, which comes from the agent's own config file and is regex-gated to ^\d+$.

Level of scrutiny

Medium-high for an infra change: it doesn't touch shipped code, but it rewrites how the macOS CI fleet is configured (launchd job naming, config file naming, image naming, nightly cleanup glob), the author states bake/install-agent were not run on a real host, and rollout is a manual operation on four production hosts. A mistake here strands or duplicates darwin agents rather than breaking Bun itself, but it directly affects every PR's CI.

Other factors

  • The PR description is unusually thorough, the source-lint test proves the config-level invariant (tiers scheduled by ci.mjs are covered by configured guests, and the tag → image round-trip holds), and the CLI arg-validation paths were exercised.
  • Deferring rather than approving because: (a) the provisioning path is untested on a real host by the author's own note, (b) #37771 rewrites the same scripts and whichever lands second needs a rebase — a maintainer should decide sequencing, and (c) this is fleet-provisioning code where a human who runs these hosts should sign off on the launchd/image naming migration before it's rolled out.

@robobun

robobun commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator Author

Both caveats in the review are the ones in the PR description: bake/install-agent have only been exercised up to their argument handling here (no tart host in this environment; the image/tag/hook agreement is what the source-lint test pins), and the rollout is an operator step on the four tart hosts. On sequencing with #37771: this change is confined to the image list, the per-release agent install, and the hook's image lookup, so it rebases onto #37771's argv-array rewrite (or the reverse) without changing either's intent; happy to do that rebase whichever lands first.

CI state for the record: the GitHub "Source lints" job (where the new test runs) is green; the Buildkite build's red lanes are build-bun steps failing to download dep tarballs from github.com (503s), which is hitting every build today and is being tracked separately.

…install-agent refreshes the hooks

On an already provisioned host, provision replaced the hooks (and now the
image names they resolve) while the old agents were still taking jobs,
and a standalone install-agent left the previously installed hooks in
place next to agent configs written by a newer layout. Both paths now
retire the host's agent jobs first, and install-agent rsyncs hooks/ and
lib/ into installDir before writing the configs that point at them.

Also covers guestBase() in the source-lint test and documents the
upgrade path for hosts that still have the single bun-ci-base image.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed this PR and didn't find any bugs. Since it reworks the darwin CI host provisioning (per-release launchd jobs, agent retirement, hook image selection) and by the author's note hasn't been exercised on a real tart host yet, a human look from someone with fleet access would still be worthwhile before rollout.

What was reviewed:

  • Config → agent tags → hook image resolution round-trips for both releases; agentGuestRelease rejects missing/non-numeric tags.
  • retireTartAgents prefix match covers the legacy unsuffixed com.buildkite.buildkite-agent job; /Library/LaunchAgents always exists on macOS so readdirSync is safe on a fresh host.
  • installSelf() now runs inside install-agent too — redundant when called via provision but harmless, and matches the README's new contract.
  • .buildkite/ci.mjs change is comment-only; source-lints workflow paths and the new lint test look correct.
Extended reasoning...

Overview

This PR reshapes scripts/darwin-ci/ so each tart host bakes one guest image per macOS release tier (bun-ci-26 for latest, bun-ci-15 for previous) and runs one buildkite-agent launchd job per image, instead of a single bun-ci-base image tagged previous. The command hook picks the image from the agent's release tag via BUILDKITE_AGENT_META_DATA_RELEASE. It also adds retireTartAgents() so re-provisioning a live host stops its existing agents before replacing hooks/images, adds a sw_vers guard in bake so a base image can't be baked under the wrong release, updates the README, adds a source-lint test that keeps the guest list in step with the tiers ci.mjs schedules, and wires scripts/darwin-ci/** + .buildkite/ci.mjs into the source-lints workflow triggers. The .buildkite/ci.mjs change itself is comment-only.

Security risks

None material. This is operator-run provisioning tooling that already assumes passwordless sudo on a CI host; no new trust boundaries are crossed. The new readdirSync(/Library/LaunchAgents) and sudo find ... -delete operate on fixed system paths under existing sudo privileges. No runtime (src/) code is touched.

Level of scrutiny

Medium-high, but for operational rather than code-correctness reasons. The code itself is a clean refactor and the source-lint test pins the config↔tag↔hook agreement. However: (a) the author explicitly notes bake/install-agent have not been run against a real tart host — only argument validation was exercised; (b) rollout requires an operator to run provision on four production CI hosts, and a mistake in the launchd/retire logic would take a host off the queue mid-bake; (c) PR #37771 rewrites overlapping shell plumbing and needs sequencing. These are decisions someone with fleet access should sign off on.

Other factors

The new source-lint test is well-constructed (fails on main's config, passes here) and the GitHub source-lints job is green. The Buildkite red is unrelated (github.com 503s on build-bun). No prior human reviews on the thread. Given the untested-on-host caveat and the operational rollout step, deferring rather than approving.

…n at the two guests macOS allows

provision retired a host's agents before it had checked anything, so a
re-provision that stopped early (console user not yet ci, host too old
for the tahoe guest, bake failure, or the nightly reboot firing during
the hour-long bake) left the host serving nothing. It now bakes from a
staged copy of these scripts while the existing agents keep using the
installed hooks and images, and install-agent swaps everything over at
the end; retiring also stops the nightly reboot daemon until the agents
are back. bake refuses a release newer than the host's macOS before
pulling anything.

Virtualization.framework runs at most two macOS guests per host, which
the old spawn=2 default filled exactly; with spawn now counted per image,
images x spawn is checked against that limit up front, and the lint test
checks the defaults fit it.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed this PR and didn't find any bugs. Because it restructures the tart-host provisioning (per-release launchd agents, staged re-provision, hook image selection) and the bake/install-agent paths haven't been exercised on a real host, someone with fleet access should look it over before merge — they'll be the ones running provision on the four hosts.

What was reviewed:

  • config → agent tags → BUILDKITE_AGENT_META_DATA_RELEASE → guestImage() round-trip; the source-lint test pins it and rejects missing/non-numeric tags.
  • retireTartAgents() prefix match covers the legacy unsuffixed com.buildkite.buildkite-agent job; nightly cleanup's glob does too.
  • checkTartAgentOptions caps images × spawn at maxGuests; default (2×1) fits.
  • installSelf() from within installTartAgent() — checked that main.ts no longer runs bake from installDir in the tart path (it uses ${installDir}.next), so the rsync-over-self hazard doesn't apply.
Extended reasoning...

Overview

This PR reworks scripts/darwin-ci so each tart host bakes one guest image per macOS release that .buildkite/ci.mjs schedules a darwin-aarch64 lane on (26 = latest, 15 = previous) and runs one buildkite-agent launchd job per image, instead of the current single bun-ci-base image with all agents tagged release-tier=previous. It touches the config (guests[] list, guestImage/guestBase/agentGuestRelease helpers), the bake step (per-release, with host-vs-guest and base-vs-release macOS-major checks), install-agent (retire whatever agent jobs exist by prefix, re-install hooks/lib, write one config+plist per release), the command hook (boot bun-ci-<release> from the agent's release tag), main.ts (multi-release CLI, staged re-provision), README/comments, a new source-lint test, and workflow path filters. .buildkite/ci.mjs itself only gains a comment.

Security risks

None material. The scripts already run with passwordless sudo on CI hosts by design; nothing here widens that surface. New shell interpolations (launchAgents, agentLabelPrefix, staging, configDir) are all constants derived from config, not user input. agentGuestRelease gates on /^\d+$/ before Number(), so the hook can't be steered to an arbitrary image name via a malformed tag.

Level of scrutiny

This is CI-fleet infrastructure, not runtime code — a defect here fails or misroutes CI jobs rather than shipping in the binary. That lowers the blast radius, but the change is not mechanical: it's a ~300-line restructuring of provisioning logic (launchd job lifecycle, staged rsync-then-swap, per-release configs) that the author explicitly says has only been argument-path tested and not run on a real tart host. The rollout requires an operator with fleet access to re-run provision on four production hosts, so the person doing that should be the one to sign off on the retirement/swap ordering and the migration note (delete bun-ci-base after).

Other factors

  • The new source-lint test covers the config↔tag↔hook agreement and is green; it does not (and cannot) cover the launchd/tart side effects.
  • There is a stated conflict with open PR #37771 that rewrites the same scripts' shell plumbing; whichever lands second needs a rebase, which is a sequencing call for a maintainer.
  • No prior human review comments to address; no CODEOWNERS entry for these paths.
  • I checked the one refactor hazard that stood out: installTartAgent() now calls installSelf() (rsync-over-installDir). In the provision tart path main.ts no longer executes from installDir — it runs from the operator's checkout and stages to ${installDir}.next, then installTartAgent() (still running from the checkout) rsyncs the checkout into installDir. A standalone main.ts install-agent run from installDir would rsync installDir onto itself, which is a no-op, so that path is also safe.

Given the untested-on-hardware provisioning paths, the required operator rollout, and the pending #37771 conflict, this warrants a human look rather than an auto-approval.

…ire a bare daemon's plist too; doc fixes

The source-lint test now reads each darwin aarch64 lane's release as well
as its tier, so bumping the latest lane to the next macOS in ci.mjs fails
until config.tart.guests follows, and it checks the served tiers equal
the scheduled ones in both directions (an image for an unscheduled tier
would take one of the host's two guest slots).

install-agent removes the scripts/agent.mjs LaunchDaemon plist it was
already booting out, so a converted bare host stays converted across the
nightly reboot. README, usage and comments now say what the code does:
images are swapped in per bake and only hooks and agents flip at the
end, a live bake needs a free guest slot, the bun-ci-base migration
needs a macOS 26 host (or --release 15 --spawn 2) and disk for the old
and new images at once, and --release/--spawn is the per-host knob for
rebalancing the tiers.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed this PR and the automated bug hunt found no issues. Because it restructures the tart-host provisioning flow (per-release launchd jobs, agent retirement, staged re-provision) and has not been exercised on a real host, a maintainer with fleet access should still review before landing.

What was reviewed:

  • agentGuestRelease reads BUILDKITE_AGENT_META_DATA_RELEASE, which matches how buildkite-agent exposes agent tags to hooks; the hook fails closed on a missing/non-numeric tag.
  • retireAgents()'s prefix match on com.buildkite.buildkite-agent covers both the old unsuffixed plist and the new .<release> plists; the bare-daemon plist is now removed, not just booted out.
  • installSelf() inside installTartAgent copies from import.meta.dir/.. (the tree the operator scp'd), so hooks and agent configs are installed from the same source atomically after bakes verify.
  • The source-lint test's ci.mjs text scan matches the current one-object-per-line shape of testPlatforms/traceOrderTargets and asserts latest is present as a guard against the scan going stale.
Extended reasoning...

Overview

This PR reworks scripts/darwin-ci so each tart host bakes and serves one macOS guest image per release-tier that .buildkite/ci.mjs schedules (26 for latest, 15 for previous), instead of a single macOS 15 image that left the latest lane with only two bare-box agents fleet-wide. It touches the config (guests[] list, maxGuests cap), per-release bake with host/guest sw_vers checks, per-release launchd jobs and agent configs, an agent-retirement path that handles all prior layouts, a staged-copy re-provision so a failed bake leaves live agents untouched, the command hook's image selection via BUILDKITE_AGENT_META_DATA_RELEASE, a new source-lint test pinning the config against ci.mjs, workflow path triggers, and docs. The .buildkite/ci.mjs change is comment-only.

Security risks

None to the runtime. The scripts run under passwordless sudo on CI hosts as before; new sudo rm -f/rm -rf calls target fixed paths (/Library/LaunchAgents/com.buildkite.buildkite-agent*.plist, /Library/LaunchDaemons/buildkite-agent.plist, ${installDir}.next). No new secrets handling — the agent token path is unchanged.

Level of scrutiny

Medium-high, and specifically needs a human with fleet access. This is not user-facing runtime code, but it is the provisioning path for the entire darwin test fleet: a bug here could take hosts offline or mislabel agents. The author explicitly notes provision/bake/install-agent were only exercised up to argument validation (no tart host available), and the PR body itself says "ready for a maintainer with fleet access" and "the operator doing the rollout should watch the first host". The source-lint test pins the config↔ci.mjs↔hook agreement but cannot exercise launchd, tart, or the sw_vers checks.

Other factors

  • ~430 lines across 11 files, including a substantial rewrite of installTartAgent and new retireAgents logic that runs launchctl bootout + rm over discovered plists.
  • Overlaps with open PR #37771 (rewrites the same files' shell plumbing); sequencing needs a maintainer call.
  • Merging alone changes nothing on the hosts — an operator must re-run provision on each, which is another reason this needs a human in the loop rather than an automated approval.
  • No prior human or claude[bot] review on the thread; coderabbit was rate-limited.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants