From 371345d1716d611284dac890070f56c15c9628f8 Mon Sep 17 00:00:00 2001 From: mintaka Date: Fri, 11 Sep 2026 11:25:57 -0400 Subject: [PATCH] fix(ci): make the darwin DMG edge leak-proof (RIG-3335) The darwin lane intermittently failed at `hdiutil create` with "Resource busy", taking the rollup red on unrelated PRs and ejecting an approved fix from the merge queue. A stale attachment left by an earlier job on a reused runner makes the create's source busy. The mount was released on the happy path only, so an abort between attach and verify skipped the detach and leaked the device along with its diskimages-help process. Release the mount from an EXIT trap so any exit path detaches, and force-detach a prior run's Compass attachment before attaching. The bundle tool does the same before create, via a pure plist parser that selects only our own image path or volume so an unrelated volume is never touched. No retry: a retry would hide the leak, and the leak is the bug. Co-authored-by: Matt Wilkinson --- .github/workflows/ci.yml | 21 ++++++++++- tools/macos-bundle/index.test.ts | 57 +++++++++++++++++++++++++++- tools/macos-bundle/index.ts | 64 +++++++++++++++++++++++++++++++- 3 files changed, 139 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 225c0adcd..1da7d26ca 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1780,6 +1780,25 @@ jobs: exit 1 fi mnt="$(mktemp -d)" + # Release the mount on ANY step exit — errexit mid-verify or the job's + # timeout signal must not leak the attachment (a reused runner then + # hits "Resource busy" on the next hdiutil create). Idempotent and + # error-swallowing: a failing trap during exit would mask the real + # status. + trap 'hdiutil detach "$mnt" >/dev/null 2>&1 || hdiutil detach "$mnt" -force >/dev/null 2>&1 || true' EXIT + # Force-detach any Compass attachment a prior job on this reused runner + # left behind. Narrow by design: only mount points whose basename is + # exactly "Compass" or a "Compass N" duplicate, so an unrelated volume + # (e.g. /Volumes/CompassUnrelated) is never touched. The mount path is + # cut from the tab-delimited entity line rather than taken as the last + # field, so a path containing spaces survives intact. + hdiutil info 2>/dev/null \ + | sed -n 's#^/dev/[^ ]* *\(/Volumes/Compass.*\)$#\1#p' \ + | while read -r stale; do + case "$(basename "$stale")" in + Compass|Compass\ [0-9]*) hdiutil detach "$stale" -force >/dev/null 2>&1 || true ;; + esac + done hdiutil attach "$out" -mountpoint "$mnt" -nobrowse -readonly rc=0 BINARIES=( @@ -1810,7 +1829,7 @@ jobs: rc=1 fi done - hdiutil detach "$mnt" || hdiutil detach "$mnt" -force || true + # Detach runs via the EXIT trap installed above, unconditionally. if [ "$rc" != 0 ]; then exit 1 fi diff --git a/tools/macos-bundle/index.test.ts b/tools/macos-bundle/index.test.ts index eb3d966e8..fa3412f01 100644 --- a/tools/macos-bundle/index.test.ts +++ b/tools/macos-bundle/index.test.ts @@ -11,7 +11,7 @@ // import.meta.main-guarded, so importing index.ts never runs it. import { describe, expect, test } from "bun:test"; -import { parseArgs, renderInfoPlist } from "./index.ts"; +import { parseArgs, renderInfoPlist, staleMountPoints } from "./index.ts"; /** The canonical render inputs used across the plist cases. */ function plistOpts() { @@ -273,3 +273,58 @@ describe("parseArgs — fails loud on malformed input", () => { ); }); }); + +/** + * Build one `hdiutil info -plist` image block for an image at `imagePath` with + * the given mount points. The real plist is larger; these are the only keys + * staleMountPoints reads, in the order hdiutil emits them. + */ +function imageBlock(imagePath: string, mountPoints: string[]): string { + const entities = mountPoints + .map((mp) => `mount-point${mp}`) + .join(""); + return `image-path${imagePath}system-entities${entities}`; +} + +const TARGET = { + imagePath: "/tmp/compass-app-darwin-arm64.dmg", + volumeName: "Compass", +}; + +describe("staleMountPoints — selects only this build's leaked attachment", () => { + test("no attachments → empty", () => { + expect(staleMountPoints("", TARGET)).toEqual([]); + }); + + test("empty input → empty, does not throw", () => { + expect(staleMountPoints("", TARGET)).toEqual([]); + }); + + test("malformed input → empty, does not throw", () => { + expect(staleMountPoints("not a plist <<< >>>", TARGET)).toEqual([]); + }); + + test("matches by image path", () => { + const info = imageBlock(TARGET.imagePath, ["/Volumes/Compass"]); + expect(staleMountPoints(info, TARGET)).toEqual(["/Volumes/Compass"]); + }); + + test("matches by Compass volume name even when the image path differs", () => { + const info = imageBlock("/tmp/some-other-run.dmg", [ + "/private/tmp/Compass", + ]); + expect(staleMountPoints(info, TARGET)).toEqual(["/private/tmp/Compass"]); + }); + + test("does NOT select an unrelated volume", () => { + const info = imageBlock("/tmp/unrelated.dmg", ["/Volumes/SomethingElse"]); + expect(staleMountPoints(info, TARGET)).toEqual([]); + }); + + test("selects only the matching block when both are attached", () => { + const info = + imageBlock("/tmp/unrelated.dmg", ["/Volumes/SomethingElse"]) + + imageBlock(TARGET.imagePath, ["/Volumes/Compass"]); + expect(staleMountPoints(info, TARGET)).toEqual(["/Volumes/Compass"]); + }); +}); diff --git a/tools/macos-bundle/index.ts b/tools/macos-bundle/index.ts index ebb937348..9b8cf9c3a 100755 --- a/tools/macos-bundle/index.ts +++ b/tools/macos-bundle/index.ts @@ -206,6 +206,48 @@ function assertSidecarBasenamesDistinct(sidecars: string[]): void { } } +/** Extract one image block's `mount-point` values, in document order. */ +function blockMountPoints(block: string): string[] { + const mountRe = /mount-point<\/key>\s*([^<]*)<\/string>/g; + const mounts: string[] = []; + for (let m = mountRe.exec(block); m; m = mountRe.exec(block)) { + if (m[1]) mounts.push(m[1]); + } + return mounts; +} + +/** + * Parse `hdiutil info -plist` text and return the mount points that a stale + * attachment of THIS build holds — so the caller can force-detach them before + * `hdiutil create`, which fails with "Resource busy" when an earlier run on a + * reused runner leaked the attachment of our image path or volume. Pure: it + * only reads text, so the parsing (where a bug would hide) is unit-testable. + * + * Conservative by construction: a block is selected ONLY when its image-path + * equals `imagePath` or one of its mount points is named `volumeName`, so an + * unrelated volume is never returned. Unparseable/empty input yields `[]`. + */ +export function staleMountPoints( + hdiutilInfoPlist: string, + target: { imagePath: string; volumeName: string }, +): string[] { + // Each attached image is a block introduced by its image-path; + // splitting on that key isolates one image's system-entities per chunk. + const blocks = hdiutilInfoPlist.split("image-path").slice(1); + const found = new Set(); + for (const block of blocks) { + const imagePath = /^\s*([^<]*)<\/string>/.exec(block)?.[1]; + const mounts = blockMountPoints(block); + const matches = + imagePath === target.imagePath || + mounts.some((mp) => basename(mp) === target.volumeName); + if (matches) { + for (const mp of mounts) found.add(mp); + } + } + return [...found]; +} + // ── The edge (impure) ────────────────────────────────────────────────────── /** Fail loud if a required input path does not exist (build.sh sanity posture). */ @@ -218,6 +260,24 @@ async function assertExists(path: string, what: string): Promise { } } +/** + * Force-detach any stale attachment of our volume/image left by an earlier run + * on a reused runner, so `hdiutil create` does not hit "Resource busy". Never + * throws: a clean system (nothing to detach) and an unavailable/unparseable + * `hdiutil info` both leave the build untouched — a cleanup that reds a green + * system is worse than the leak. + */ +async function detachStaleAttachments(target: { + imagePath: string; + volumeName: string; +}): Promise { + const probe = await $`hdiutil info -plist`.quiet().nothrow(); + if (probe.exitCode !== 0) return; + for (const mount of staleMountPoints(probe.stdout.toString(), target)) { + await $`hdiutil detach ${mount} -force`.quiet().nothrow(); + } +} + async function main(): Promise { const args = parseArgs(Bun.argv.slice(2)); @@ -276,8 +336,10 @@ async function main(): Promise { await $`codesign --sign - --force --deep ${appDir}`; // Wrap the staging dir into a compressed (UDZO) .dmg. -ov overwrites an - // existing image so a re-run is idempotent. + // existing image so a re-run is idempotent. Detach any stale attachment of + // this volume first — on a reused runner a leaked mount makes create EBUSY. await rm(args.out, { force: true }); + await detachStaleAttachments({ imagePath: args.out, volumeName: "Compass" }); await $`hdiutil create -volname Compass -srcfolder ${stageRoot} -ov -format UDZO ${args.out}`; console.log(`macos-bundle: wrote ${args.out}`);