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}`);