Repository navigation
Conversation
The tart command hook rsyncs the whole agent checkout into the guest, .git included. The agent's fetch leaves a detached `git gc --auto` running, which rewrites .git while rsync reads it. A file that vanishes mid-transfer (.git/gc.pid) fails the sync with exit 23, and the job dies before any test runs. Nothing in the guest reads the checkout's .git: the runner takes the commit and branch from the forwarded BUILDKITE_* env, and vendor tests clone their own repositories. Exclude it, anchored to the checkout root so a nested repository still copies whole. The guest also receives about 1 GB less per job.
|
Warning Review limit reached
On-demand reviews are free for the next 23 days. After that, they cost $0.25 per reviewed file. Or wait 20 minutes for your next included review. View limit detailsLimit details: You’ve used all 5 included reviews currently available. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — a single anchored rsync exclude in CI tooling, with a test that runs the real command.
What was reviewed:
- rsync semantics:
--exclude=/.gitis anchored to the transfer root, so nestedvendor/*/.gitstill copies and.gitignoreis unaffected;--deletewithout--delete-excludedwon't touch a pre-existing.giton the receiver (and the baked image's~/workis empty). - Bun shell array interpolation of
checkoutExcludesexpands as one argv entry, not a quoted string. - The test's fake
sshshim correctly consumes-i/-o/-lpairs and the host before exec'ing the rsync server side in the temp guest home;192.0.2.1is TEST-NET-1 and never reaches the network.PATHis restored infinally.
Extended reasoning...
Overview
This PR touches two files: scripts/darwin-ci/lib/guest.ts adds an exported checkoutExcludes = ["--exclude=/.git"] and interpolates it into the syncTo() rsync invocation, and test/internal/darwin-ci-guest-sync.test.ts is a new test that drives the real guest().syncTo() through a fake ssh on PATH so the rsync server side runs against a local temp directory. The change is pure CI infrastructure for the macOS Tart hosts — it does not touch any code that ships in the Bun binary.
Security risks
None. This is internal CI tooling that assembles an rsync command. No user-controlled input flows into the new exclude flag (it's a literal), and the test is hermetic: the fake ssh shim intercepts before any network is contacted, the target IP is in the RFC 5737 documentation range, and everything is confined to a tempDir. The PATH mutation is process-local to the test runner and restored in a finally.
Level of scrutiny
Low. The functional change is a one-flag addition to an rsync command line whose semantics are well-known: a leading / in an rsync filter anchors it to the transfer root, so only the checkout-root .git is skipped while nested repositories and .gitignore still transfer. The Bun $ shell expands an array argument as separate argv entries, so ${checkoutExcludes} becomes exactly --exclude=/.git. The PR description's claim that nothing in the guest reads the checkout's .git is plausible and, more importantly, the blast radius of being wrong is a red CI job on a Tart host — not user-facing behavior.
Other factors
The test follows repo conventions (tempDir from harness, skipIf gating on Windows/missing rsync, resource restoration in finally, module-scope imports) and lives alongside sibling test/internal/ infrastructure tests like runner-junit.test.ts and build-download-retry.test.ts, so the new-file placement is consistent. No CODEOWNERS entry covers scripts/darwin-ci/ or test/internal/. The PR timeline shows no outstanding reviewer objections. The bug-hunt exit reason was dry_streak with zero findings.
|
Status: ready for review. Reproduced the failure class locally with rsync 3.4.1: a copy of a tree whose Second commit (ce08238): CI on ce08238 (build 107624) and on 235da7c (build 107304): every lane is green except |
…host The agent runs the hooks from the copy installed in /usr/local/share/darwin-ci at provision time. A change under scripts/darwin-ci reaches a host only once that copy is refreshed. Expose installSelf() as `main.ts install-self` and write the steps down in the README.
|
Updated 3:35 AM PT - Aug 28th, 2026
❌ @robobun, your commit ce08238 has 1 failures in
🧪 To try this PR locally: bunx bun-pr 40693That installs a local version of the PR into your bun-40693 --bun |
There was a problem hiding this comment.
LGTM — internal CI tooling change, small and well-contained.
Checked that --exclude=/.git is anchored (nested vendor/*/.git still copies) and that Bun.$ array interpolation expands checkoutExcludes as separate argv entries in the rsync command. The new test drives the real syncTo through a fake ssh shim, uses tempDir, restores PATH in finally, and skipIfs on Windows/missing rsync. For the follow-up commit: installSelf was already imported and used by provision(), so the new install-self case just exposes it as a standalone subcommand; config.installDir renders in the usage string. README addition is ops docs only.
Extended reasoning...
Overview
This PR touches only macOS Tart CI orchestration tooling under scripts/darwin-ci/ plus a test in test/internal/. The core fix is a single rsync flag: checkoutExcludes = ["--exclude=/.git"] spliced into guest().syncTo() so the agent checkout's .git is not copied into the guest VM, avoiding rsync exit-23 when a background git gc --auto rewrites .git mid-transfer. The second commit adds an install-self subcommand to main.ts (wiring the already-imported installSelf() from lib/host.ts) and a README section documenting how script changes reach a host. A new test in test/internal/darwin-ci-guest-sync.test.ts runs the real syncTo through a fake ssh on PATH that executes the rsync server side against a local tempDir.
Security risks
None. This is host-side CI orchestration that runs on trusted BuildKite macOS hosts, not shipped runtime code. No user input parsing, no auth/crypto/permissions changes. The fake ssh shim in the test is scoped to a tempDir prepended to PATH and restored in finally. The install-self subcommand rsyncs the script directory into /usr/local/share/darwin-ci under sudo — the same call provision already makes.
Level of scrutiny
Low. The functional change is one rsync argument with a well-documented failure mode (vanished .git/gc.pid → exit 23) and a clear proof that nothing in the guest reads the checkout's .git (runner uses forwarded BUILDKITE_* env for commit metadata). The anchored /-prefixed exclude pattern is the standard rsync idiom for "root-level only", and the test explicitly asserts a nested vendor/elysia/.git still transfers. The install-self addition is a three-line dispatch to an existing helper.
Other factors
The test follows harness conventions: tempDir with using, test.skipIf(isWindows || !rsync), module-scope imports, PATH mutation guarded by try/finally, content assertions before implicit exit-code check (rsync failure would throw from Bun.$). No CODEOWNERS entries cover scripts/darwin-ci/ or test/internal/. The bug hunt exited on dry_streak with no findings and no ruled-out candidates. The PR description notes a one-line rebase interaction with #37771 — informational only, not a blocker for this review.
|
Another occurrence on 2026-09-18: build 117649, job |
Problem
step failed outside runnerbefore any test runs. The sync step printsShellError: Failed with exit code 23andrsync(45694): error: .../.git/gc.pid: open (2) ...: No such file or directory(build 107284, agentdarwin-arm64-baguette-tart-26-1)..gitincluded (scripts/darwin-ci/lib/guest.ts:53). The agent'sgit fetchleaves a detachedgit gc --autorunning. It removes.git/gc.pidand rewrites packs while rsync reads them. A vanished file is exit 23, fatal to the hook.Fix
syncTopasses--exclude=/.git. The pattern is anchored to the checkout root, so a nested repository still copies whole..git. The runner takes commit, branch, and repository from the forwardedBUILDKITE_*env (scripts/utils.mjs). Vendor tests clone their own repositories, and tests that need one create it in a temp dir. Nothing else writes to the checkout during the sync.main.ts install-selfexposes the existinginstallSelf(), and the README gets an "Updating the scripts on a host" section. See Rollout.test/internal/darwin-ci-guest-sync.test.tsruns the realsyncTothrough a fakesshthat runs the rsync server side locally. It fails on main and passes here. Alsotest/internal/runner-junit.test.ts.Rollout
/usr/local/share/darwin-ciat provision time. This PR's own darwin lanes ran that old copy, so a green tart lane here is not evidence the new sync ran. The unit test is.rsyncthe directory over, thenmain.ts install-self. No agent restart. cc @aliiBackground
test-darwinTart job boots a fresh macOS guest, rsyncs the checkout to~/work, and runsguest/job.shover ssh.git gc --autoruns after a fetch when a repository has too many packs or loose objects. It detaches by default, so it outlives the checkout step.Notes
.git.lib/agent.ts) sets nogit-clone-flagsand nogit-mirrors-path, so the checkout is a full clone that is fetched on every job and wiped nightly. Each fetch adds a pack or loose objects. Once the count crossesgc.autoPackLimit(50) orgc.auto(6700), the next fetch triggers the background repack. On a multi-GB repository that takes minutes, so the race window covers the whole sync.gc.pidis not enough. The repack also writesobjects/pack/tmp_pack_*, then renames them and deletes the old packs, so other files vanish too..gitagain. Leaving.gitout removes the only concurrent writer to the checkout.--deleteis kept. An excluded path is not deleted on the receiver without--delete-excluded, and the baked image ships an empty~/workanyway..git/gc.pidis removed mid-copy exits 24 ("some files vanished") without the exclude and 0 with it.hooks/commandrun the checkout'sscripts/darwin-ci/hooks/command.tswhen present, so a hook fix ships with its commit and a PR's own darwin lane exercises it. Give the Tart agent agit-mirrors-pathlikescripts/agent.mjsdoes.syncTocall to an argv array. Whichever lands second needs a one-line rebase.rsyncis not on PATH (Windows). The fakesshhandles the-l userform that rsync uses foruser@host.test/internal/build-download-retry.test.ts.no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/internal/darwin-ci-guest-sync.test.ts