Merge nucleic/mellow-dewy-falcon-rjhr into main

Sources/RunnerCore/PathResolution.swift

@@ -108,3 +108,80 @@ public enum PathResolution {
         #endif
     }
 }
+
+/// Cheap sanity checks on a `.ipsw` before Virtualization.framework sees it.
+///
+/// Lives beside ``PathResolution`` because it is the other half of the same
+/// job — what the CLI does with a path an operator typed — and because
+/// `RunnerCore` is the only target that unit-tests on both platforms.
+///
+/// The checks earn their place: `VZMacOSRestoreImage.image(from:)` reports no
+/// progress at all while it works, and on a partial download it can sit for a
+/// very long time rather than failing. Without these, "I pointed it at the
+/// wrong file" and "my 21 GB download stopped at 4 GB" both present to the
+/// operator as an unexplained hang.
+public enum IPSWFile {
+    /// The floor a real macOS restore image clears by an order of magnitude —
+    /// they run ~15-22 GB. Anything under this is a truncated download, a
+    /// placeholder, or the wrong file entirely.
+    public static let minimumBytes: Int64 = 1_000_000_000
+
+    /// The first two bytes of every `.ipsw`: an IPSW is a zip archive.
+    static let zipMagic = Data([0x50, 0x4B])  // "PK"
+
+    /// Fails fast if `path` cannot be a usable restore image.
+    ///
+    /// - Parameters:
+    ///   - path: An already-resolved absolute path (see ``PathResolution``).
+    ///   - label: What the file is, for error messages.
+    /// - Throws: ``CoreError/notFound(_:)`` if it is not there or unreadable,
+    ///   ``CoreError/configInvalid(_:)`` if it is there but cannot be an IPSW.
+    public static func validate(path: String, label: String = "restore image") throws {
+        var isDirectory: ObjCBool = false
+        guard FileManager.default.fileExists(atPath: path, isDirectory: &isDirectory) else {
+            throw CoreError.notFound("\(label) not found at \(path)")
+        }
+        guard !isDirectory.boolValue else {
+            throw CoreError.configInvalid(
+                "\(label) at \(path) is a directory, not an .ipsw file"
+            )
+        }
+
+        let attributes = try? FileManager.default.attributesOfItem(atPath: path)
+        let size = (attributes?[.size] as? NSNumber)?.int64Value ?? 0
+        guard size >= minimumBytes else {
+            throw CoreError.configInvalid(
+                "\(label) at \(path) is only \(describeSize(size)) — a macOS restore image is "
+                    + "~15-22 GB. The download is most likely incomplete; delete the file and "
+                    + "fetch it again."
+            )
+        }
+
+        guard let handle = FileHandle(forReadingAtPath: path) else {
+            throw CoreError.notFound("\(label) at \(path) could not be opened for reading")
+        }
+        defer { try? handle.close() }
+
+        let magic = (try? handle.read(upToCount: zipMagic.count)) ?? Data()
+        guard magic == zipMagic else {
+            let found = magic.map { String(format: "%02x", $0) }.joined()
+            throw CoreError.configInvalid(
+                "\(label) at \(path) does not look like an .ipsw: expected a zip archive "
+                    + "(magic \"PK\", 504b) but the file starts with \(found.isEmpty ? "nothing" : found). "
+                    + "Check the path, or re-download the file."
+            )
+        }
+    }
+
+    /// A byte count an operator can compare against "~15 GB" at a glance.
+    static func describeSize(_ bytes: Int64) -> String {
+        let units = ["B", "KB", "MB", "GB", "TB"]
+        var value = Double(bytes)
+        var unit = 0
+        while value >= 1024, unit < units.count - 1 {
+            value /= 1024
+            unit += 1
+        }
+        return unit == 0 ? "\(Int(value)) B" : String(format: "%.1f %@", value, units[unit])
+    }
+}

Sources/RunnerHost/IPSW.swift

@@ -155,12 +155,10 @@ public struct IPSWProvider: Sendable {
         // not a Swift error — when handed a non-file or missing path, and an
         // ObjC exception cannot be caught here. So the existence check is not
         // politeness; it is the only thing standing between a typo and a crash.
-        var isDirectory: ObjCBool = false
-        guard FileManager.default.fileExists(atPath: url.path, isDirectory: &isDirectory),
-            !isDirectory.boolValue
-        else {
-            throw CoreError.notFound("restore image not found at \(url.path)")
-        }
+        // The size and magic-byte checks alongside it cover the other way this
+        // call goes wrong: handed a partial download it neither fails nor
+        // reports progress, it simply stops responding.
+        try IPSWFile.validate(path: url.path)
 
         do {
             return try await VZMacOSRestoreImage.image(from: url)

Sources/RunnerHost/ImageBuilder.swift

@@ -6,10 +6,16 @@ import Virtualization
 public enum ImageBuildStage: Sendable, Equatable {
     /// Downloading the IPSW.
     case downloadingIPSW(fraction: Double)
-    /// Reading the restore image and deriving a hardware configuration.
+    /// Working out which file the operator meant and whether it is usable.
     case preparing
+    /// Inside `VZMacOSRestoreImage.image(from:)`, which reports no progress of
+    /// its own and is the longest silent stretch of a local-IPSW build.
+    case loadingRestoreImage
     /// Creating the disk, NVRAM, and bundle metadata.
-    case creatingBundle
+    case creatingBundle(diskGB: Int)
+    /// An out-of-band remark about the stage in flight — printed on its own
+    /// line rather than replacing the status line.
+    case note(String)
     /// `VZMacOSInstaller` is writing macOS onto the disk.
     case installing(fraction: Double)
     /// First boot; waiting for Setup Assistant, a DHCP lease, and SSH.
@@ -89,7 +95,28 @@ public struct ImageBuilder: Sendable {
         let provider = IPSWProvider(downloadDirectory: store.ipswDir)
         let restoreImage: VZMacOSRestoreImage
         if let ipswPath {
-            progress?(.downloadingIPSW(fraction: 1.0))
+            progress?(.preparing)
+            progress?(.loadingRestoreImage)
+
+            // Reading a 21 GB archive's metadata takes a while and the
+            // framework says nothing while it does. A build that looks frozen
+            // is the single most-reported symptom of this command, so say out
+            // loud what the other likely explanation is rather than letting the
+            // operator guess.
+            let watchdog = Task {
+                try? await Task.sleep(for: .seconds(60))
+                // Cancelling is how a *fast* load ends this task, and a
+                // cancelled sleep returns rather than throwing past `try?` — so
+                // without this the note prints on every quick failure, which is
+                // precisely when it is misleading.
+                guard !Task.isCancelled else { return }
+                progress?(
+                    .note(
+                        "still loading — a truncated or partially downloaded .ipsw can block here; "
+                            + "verify the download completed"))
+            }
+            defer { watchdog.cancel() }
+
             restoreImage = try await provider.load(localPath: ipswPath)
         } else {
             progress?(.downloadingIPSW(fraction: 0))
@@ -100,8 +127,7 @@ public struct ImageBuilder: Sendable {
         }
 
         // 2/3. Hardware model and bundle.
-        progress?(.preparing)
-        progress?(.creatingBundle)
+        progress?(.creatingBundle(diskGB: config.guest.diskGB))
         let bundle = try await createBundle(name: name, restoreImage: restoreImage, config: config)
 
         // 4. Install.

Sources/gitea-macos-runner/CommandDaemon.swift

@@ -110,9 +110,10 @@ struct DaemonCommand: AsyncParsableCommand {
                     // Expected on shutdown.
                 } catch {
                     logger.critical("daemon stopped", metadata: ["error": .string("\(error)")])
-                    // Fully qualified: inside a ParsableCommand a bare `exit`
-                    // resolves to ParsableCommand.exit(withError:).
-                    await MainActor.run { Foundation.exit(1) }
+                    // Not `MainActor.run`: the main actor is parked inside
+                    // `app.run()` for the life of the process, so hopping onto
+                    // it to exit is its own deadlock. See `VZAppRuntime.run`.
+                    VZAppRuntime.flushAndExit(1)
                 }
             }
         )
@@ -123,18 +124,40 @@ struct DaemonCommand: AsyncParsableCommand {
 /// run loop it requires, while the real work runs in a `Task`.
 ///
 /// Shared by `daemon` and `vm boot`: any command that starts a VM needs this.
-@MainActor
 enum VZAppRuntime {
     /// Signal sources have to outlive the call that creates them or they are
     /// cancelled on deinit and the signals go nowhere.
-    private static var signalSources: [DispatchSourceSignal] = []
-    private static var isTerminating = false
+    private nonisolated(unsafe) static var signalSources: [DispatchSourceSignal] = []
+    private nonisolated(unsafe) static var isTerminating = false
+    private static let stateLock = NSLock()
+
+    /// Signals land here rather than on `.main`. See ``run(onSignal:body:)``.
+    private static let signalQueue = DispatchQueue(
+        label: "xyz.blakeslee.gitea-macos-runner.signals")
 
     /// Starts the run loop and runs `body` alongside it. Never returns.
     ///
+    /// ## Nothing here may touch the main queue
+    ///
+    /// This is reached from Swift's async `main`, so the frame that calls
+    /// `app.run()` is *itself* a block executing on the main dispatch queue —
+    /// and it never returns. libdispatch will not re-enter a serial queue that
+    /// already has a block in flight, so from this moment the main queue is
+    /// closed for business: a plain `Task { }` inheriting a `@MainActor`
+    /// context, a `DispatchSource` handler on `.main`, or an
+    /// `await MainActor.run { … }` all enqueue work that can never be drained.
+    ///
+    /// The symptom is exact and was reported as a hang in `image build`: a live
+    /// run loop, zero CPU, and no output past the last line printed before this
+    /// call — `body` had been enqueued behind `app.run()` and never got a first
+    /// tick. Hence `Task.detached`, a private signal queue, and ``exit(_:)``
+    /// called straight from whichever thread reaches it. A normal AppKit app
+    /// does not hit this because its `main()` is not a main-queue block.
+    ///
     /// - Parameters:
     ///   - onSignal: Cleanup to perform on `SIGINT`/`SIGTERM` before exiting.
     ///   - body: The work to run. When it returns, the process exits zero.
+    @MainActor
     static func run(
         onSignal: @escaping @Sendable () async -> Void,
         body: @escaping @Sendable () async -> Void
@@ -149,30 +172,48 @@ enum VZAppRuntime {
             // DispatchSourceSignal only observes; the default disposition still
             // kills the process unless it is ignored first.
             signal(signalNumber, SIG_IGN)
-            let source = DispatchSource.makeSignalSource(signal: signalNumber, queue: .main)
+            let source = DispatchSource.makeSignalSource(signal: signalNumber, queue: signalQueue)
             source.setEventHandler {
-                Task { @MainActor in
-                    guard !isTerminating else { return }
-                    isTerminating = true
+                guard beginTerminating() else { return }
+                Task.detached {
                     CLI.note("received signal; shutting down…")
                     await onSignal()
-                    NSApp.terminate(nil)
-                    exit(0)
+                    flushAndExit(0)
                 }
             }
             source.resume()
+            stateLock.lock()
             signalSources.append(source)
+            stateLock.unlock()
         }
 
-        Task {
+        // Detached on purpose: an inheriting `Task { }` would be queued behind
+        // the `app.run()` below and never start. See the note above.
+        Task.detached {
             await body()
-            await MainActor.run {
-                NSApp.terminate(nil)
-                exit(0)
-            }
+            flushAndExit(0)
         }
 
         app.run()
-        exit(0)
+        flushAndExit(0)
+    }
+
+    /// Wins the race to shut down, exactly once.
+    private static func beginTerminating() -> Bool {
+        stateLock.lock()
+        defer { stateLock.unlock() }
+        guard !isTerminating else { return false }
+        isTerminating = true
+        return true
+    }
+
+    /// Exits from any thread, without hopping to the unusable main actor.
+    ///
+    /// `NSApp.terminate(nil)` is deliberately not called: it requires the main
+    /// actor, which is exactly what is not available here.
+    nonisolated static func flushAndExit(_ code: Int32) -> Never {
+        fflush(stdout)
+        fflush(stderr)
+        exit(code)
     }
 }

Sources/gitea-macos-runner/CommandImage.swift

@@ -91,14 +91,15 @@ struct ImageCommand: AsyncParsableCommand {
                             name: imageName,
                             ipswPath: ipswPath,
                             config: frozenConfig,
-                            progress: { stage in printer.update(ImageCommand.describe(stage)) }
+                            progress: { stage in ImageCommand.report(stage, to: printer) }
                         )
                     } catch {
                         printer.finish()
                         CLI.error("\(error)")
-                        // Fully qualified: inside a ParsableCommand a bare `exit`
-                        // resolves to ParsableCommand.exit(withError:).
-                        await MainActor.run { Foundation.exit(1) }
+                        // Not `MainActor.run`: the main actor is parked inside
+                        // `app.run()` for the life of the process, so hopping onto
+                        // it to exit is its own deadlock. See `VZAppRuntime.run`.
+                        VZAppRuntime.flushAndExit(1)
                     }
                     printer.finish("done")
 
@@ -254,14 +255,15 @@ struct ImageCommand: AsyncParsableCommand {
                             name: imageName,
                             config: frozenConfig,
                             xcodeXIPPath: xipPath,
-                            progress: { stage in printer.update(ImageCommand.describe(stage)) }
+                            progress: { stage in ImageCommand.report(stage, to: printer) }
                         )
                     } catch {
                         printer.finish()
                         CLI.error("\(error)")
-                        // Fully qualified: inside a ParsableCommand a bare `exit`
-                        // resolves to ParsableCommand.exit(withError:).
-                        await MainActor.run { Foundation.exit(1) }
+                        // Not `MainActor.run`: the main actor is parked inside
+                        // `app.run()` for the life of the process, so hopping onto
+                        // it to exit is its own deadlock. See `VZAppRuntime.run`.
+                        VZAppRuntime.flushAndExit(1)
                     }
                     printer.finish("done")
                     print("provisioned image '\(imageName)'")
@@ -270,19 +272,36 @@ struct ImageCommand: AsyncParsableCommand {
         }
     }
 
+    /// Routes a stage to the progress printer.
+    ///
+    /// Notes get a line of their own: they are the reason the operator is still
+    /// watching, and a status line that is about to be overwritten is no place
+    /// to put "this may be a truncated download".
+    static func report(_ stage: ImageBuildStage, to printer: ProgressPrinter) {
+        if case .note(let text) = stage {
+            printer.line(text)
+        } else {
+            printer.update(describe(stage))
+        }
+    }
+
     /// Renders a build stage as one status line.
     static func describe(_ stage: ImageBuildStage) -> String {
         switch stage {
         case .downloadingIPSW(let fraction):
             return "downloading IPSW  " + CLI.progressBar(fraction)
         case .preparing:
-            return "preparing"
-        case .creatingBundle:
-            return "creating bundle"
+            return "resolving restore image…"
+        case .loadingRestoreImage:
+            return "loading restore image metadata…"
+        case .creatingBundle(let diskGB):
+            return "creating VM bundle (disk \(diskGB) GB)…"
+        case .note(let text):
+            return text
         case .installing(let fraction):
             return "installing macOS  " + CLI.progressBar(fraction)
         case .firstBoot:
-            return "first boot (Setup Assistant)"
+            return "first boot + guest provisioning…"
         case .provisioning(let step):
             return "provisioning: \(step)"
         case .finalizing:

Sources/gitea-macos-runner/CommandVM.swift

@@ -85,9 +85,10 @@ struct VMCommand: AsyncParsableCommand {
                     } catch {
                         CLI.error("\(error)")
                         await session.teardown()
-                        // Fully qualified: inside a ParsableCommand a bare `exit`
-                        // resolves to ParsableCommand.exit(withError:).
-                        await MainActor.run { Foundation.exit(1) }
+                        // Not `MainActor.run`: the main actor is parked inside
+                        // `app.run()` for the life of the process, so hopping onto
+                        // it to exit is its own deadlock. See `VZAppRuntime.run`.
+                        VZAppRuntime.flushAndExit(1)
                     }
                 }
             )

Sources/gitea-macos-runner/Main.swift

@@ -137,24 +137,50 @@ final class ProgressPrinter: @unchecked Sendable {
     private let lock = NSLock()
     private var lastLine = ""
 
+    /// Whether carriage-return rewriting means anything here.
+    ///
+    /// Piped to a file or captured by `launchd`, `\r` produces one unreadable
+    /// mega-line, so each update becomes its own line instead. `FileHandle`
+    /// writes go straight to the descriptor either way — there is no buffer to
+    /// flush, which is what makes a stall attributable to the stage last
+    /// printed rather than to output sitting unwritten.
+    private let isInteractive = isatty(fileno(stderr)) == 1
+
     /// Rewrites the current line.
     func update(_ line: String) {
         lock.lock()
         defer { lock.unlock() }
         guard line != lastLine else { return }
         lastLine = line
+        guard isInteractive else {
+            FileHandle.standardError.write(Data((line + "\n").utf8))
+            return
+        }
         let padding = String(repeating: " ", count: max(0, 78 - line.count))
         FileHandle.standardError.write(Data(("\r" + line + padding).utf8))
     }
 
+    /// Emits a standalone line without losing the status line under it.
+    func line(_ text: String) {
+        lock.lock()
+        let carried = lastLine
+        lock.unlock()
+
+        finish(text)
+        if !carried.isEmpty {
+            update(carried)
+        }
+    }
+
     /// Ends the line so subsequent output starts cleanly.
     func finish(_ line: String? = nil) {
         lock.lock()
         defer { lock.unlock() }
         if let line {
-            let padding = String(repeating: " ", count: max(0, 78 - line.count))
-            FileHandle.standardError.write(Data(("\r" + line + padding + "\n").utf8))
-        } else if !lastLine.isEmpty {
+            let padding = isInteractive ? String(repeating: " ", count: max(0, 78 - line.count)) : ""
+            let prefix = isInteractive ? "\r" : ""
+            FileHandle.standardError.write(Data((prefix + line + padding + "\n").utf8))
+        } else if !lastLine.isEmpty, isInteractive {
             FileHandle.standardError.write(Data("\n".utf8))
         }
         lastLine = ""