From 1aaa6df7599f153d3927b880be05bf9ff414c4b9 Mon Sep 17 00:00:00 2001 From: akramj13 <125495000+akramj13@users.noreply.github.com> Date: Fri, 4 Sep 2026 19:55:58 -0400 Subject: [PATCH 1/2] Fix CI on main: SwiftLint 0.65.1 line length and aria2 stderr race Two main-branch failures surfaced on every open PR once the macos-latest image moved from SwiftLint 0.65.0 to 0.65.1. SwiftLint 0.65.1 fixed `ignores_urls` so that property accesses whose member names are valid top-level domains (`.app`, `.info`) no longer make a line count as a URL. That exposed the 144-character launch log line in AppDelegate, which 0.65.0 had silently skipped. Wrap it. Aria2DownloadService read its stderr buffer inside the termination handler while the readability handler, which runs on its own queue, could still be holding the process's final write. On the slower runner the buffer was empty and the error degraded to "Process terminated with exit code 7", failing test_downloadSurfacesProcessExitAndStderr. Drain both pipes to EOF in the termination handler before building the result; that cannot block because the child's write ends closed with it. The parsing is shared between the handlers and the drain so bytes are treated the same either way. Co-Authored-By: Claude Fable 5.1 --- Cotabby/App/Core/AppDelegate.swift | 4 ++- .../Aria2DownloadService.swift | 25 +++++++++++++++---- 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/Cotabby/App/Core/AppDelegate.swift b/Cotabby/App/Core/AppDelegate.swift index 4e755b4a..8e827f00 100644 --- a/Cotabby/App/Core/AppDelegate.swift +++ b/Cotabby/App/Core/AppDelegate.swift @@ -138,7 +138,9 @@ final class AppDelegate: NSObject, NSApplicationDelegate { let version = Bundle.main.object(forInfoDictionaryKey: "CFBundleShortVersionString") as? String ?? "?" let build = Bundle.main.object(forInfoDictionaryKey: "CFBundleVersion") as? String ?? "?" - CotabbyLogger.app.info("Cotabby \(version) (build \(build)) launching on macOS \(ProcessInfo.processInfo.operatingSystemVersionString)") + CotabbyLogger.app.info( + "Cotabby \(version) (build \(build)) launching on macOS \(ProcessInfo.processInfo.operatingSystemVersionString)" + ) applyLaunchAtLoginDefaultIfNeeded() startRuntimeIfPreferredEngineRequiresIt() focusModel.start() diff --git a/Cotabby/Services/ModelManagement/Aria2DownloadService.swift b/Cotabby/Services/ModelManagement/Aria2DownloadService.swift index cd83b245..c97c8ce9 100644 --- a/Cotabby/Services/ModelManagement/Aria2DownloadService.swift +++ b/Cotabby/Services/ModelManagement/Aria2DownloadService.swift @@ -89,8 +89,9 @@ nonisolated final class Aria2DownloadService: @unchecked Sendable { process.standardOutput = outputPipe process.standardError = errorPipe - outputPipe.fileHandleForReading.readabilityHandler = { [progressHandler] handle in - let data = handle.availableData + // Shared by the readability handlers and the termination drain below, so bytes that arrive + // either way are parsed identically. + let consumeOutput: @Sendable (Data) -> Void = { [progressHandler] data in guard !data.isEmpty, let text = String(data: data, encoding: .utf8) else { return } @@ -101,20 +102,34 @@ nonisolated final class Aria2DownloadService: @unchecked Sendable { } } } - - errorPipe.fileHandleForReading.readabilityHandler = { handle in - let data = handle.availableData + let consumeError: @Sendable (Data) -> Void = { data in guard !data.isEmpty, let text = String(data: data, encoding: .utf8) else { return } errorBuffer.append(text) } + outputPipe.fileHandleForReading.readabilityHandler = { handle in + consumeOutput(handle.availableData) + } + + errorPipe.fileHandleForReading.readabilityHandler = { handle in + consumeError(handle.availableData) + } + return try await withCheckedThrowingContinuation { continuation in process.terminationHandler = { [processState] terminatedProcess in outputPipe.fileHandleForReading.readabilityHandler = nil errorPipe.fileHandleForReading.readabilityHandler = nil + // The readability handlers run on their own queue, so a process that exits right + // after its last write can terminate before those bytes were consumed; on a slow + // machine that turned "simulated aria failure" into a bare exit code. Draining to + // EOF here cannot block: the child's write ends closed when it exited, and the + // parent's copies were closed at launch. + consumeOutput(outputPipe.fileHandleForReading.readDataToEndOfFile()) + consumeError(errorPipe.fileHandleForReading.readDataToEndOfFile()) + continuation.resume( with: Self.completionResult( status: terminatedProcess.terminationStatus, From c6a7d26ff16248edfc7b180355cdee98c0de6ab1 Mon Sep 17 00:00:00 2001 From: akramj13 <125495000+akramj13@users.noreply.github.com> Date: Fri, 4 Sep 2026 20:09:06 -0400 Subject: [PATCH 2/2] Serialize aria2 pipe reads with the termination drain Detaching a readability handler does not wait for a callback that is already running, so one could have pulled the final stderr bytes with availableData and not yet appended them when the termination handler drained the pipe and read the buffer. Run every pipe read, callbacks and drain alike, on one serial queue and snapshot the message on that same queue: an in-flight callback finishes appending before the drain starts, and a late callback finds the pipe at EOF. Co-Authored-By: Claude Fable 5.1 --- .../Aria2DownloadService.swift | 32 +++++++++++++------ 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/Cotabby/Services/ModelManagement/Aria2DownloadService.swift b/Cotabby/Services/ModelManagement/Aria2DownloadService.swift index c97c8ce9..2b3db582 100644 --- a/Cotabby/Services/ModelManagement/Aria2DownloadService.swift +++ b/Cotabby/Services/ModelManagement/Aria2DownloadService.swift @@ -109,12 +109,20 @@ nonisolated final class Aria2DownloadService: @unchecked Sendable { errorBuffer.append(text) } + // Every read from either pipe, whether a readability callback or the termination drain + // below, runs on this one serial queue. That is what makes the drain complete: a callback + // that already pulled bytes with `availableData` finishes appending them before the drain + // starts, and a callback that lands after the drain finds the pipe at EOF. Detaching a + // handler alone does not wait for an in-flight callback, so without the queue the final + // stderr write could still be lost on a slow machine. + let pipeReadQueue = DispatchQueue(label: "com.cotabby.aria2.pipe-read") + outputPipe.fileHandleForReading.readabilityHandler = { handle in - consumeOutput(handle.availableData) + pipeReadQueue.sync { consumeOutput(handle.availableData) } } errorPipe.fileHandleForReading.readabilityHandler = { handle in - consumeError(handle.availableData) + pipeReadQueue.sync { consumeError(handle.availableData) } } return try await withCheckedThrowingContinuation { continuation in @@ -122,19 +130,23 @@ nonisolated final class Aria2DownloadService: @unchecked Sendable { outputPipe.fileHandleForReading.readabilityHandler = nil errorPipe.fileHandleForReading.readabilityHandler = nil - // The readability handlers run on their own queue, so a process that exits right - // after its last write can terminate before those bytes were consumed; on a slow - // machine that turned "simulated aria failure" into a bare exit code. Draining to - // EOF here cannot block: the child's write ends closed when it exited, and the - // parent's copies were closed at launch. - consumeOutput(outputPipe.fileHandleForReading.readDataToEndOfFile()) - consumeError(errorPipe.fileHandleForReading.readDataToEndOfFile()) + // A process that exits right after its last write can terminate before the + // readability callbacks consumed those bytes; on a slow machine that turned + // "simulated aria failure" into a bare exit code. Drain both pipes to EOF on the + // read queue, then snapshot the message on the same queue so nothing appends after + // the snapshot. The drain cannot block: the child's write ends closed when it + // exited, and the parent's copies were closed at launch. + let errorMessage = pipeReadQueue.sync { + consumeOutput(outputPipe.fileHandleForReading.readDataToEndOfFile()) + consumeError(errorPipe.fileHandleForReading.readDataToEndOfFile()) + return errorBuffer.value + } continuation.resume( with: Self.completionResult( status: terminatedProcess.terminationStatus, requestedOutcome: processState.finish(), - errorMessage: errorBuffer.value, + errorMessage: errorMessage, targetURL: targetURL ) )