fix(cmux NEXT agent pane): a failed screencapture is an error, not an Esc (carries part of #20827)

Packages/macOS/CmuxNext/Sources/CmuxNextAgentPane/AgentPaneCapture.swift

@@ -1,17 +1,21 @@
+import CmuxNextCompat
 import CmuxNextWakeups
 import CoreGraphics
 import Foundation
 import ImageIO
+import os
 import UniformTypeIdentifiers
 
 /// The composer's + > Attach Screenshot… and Attach Window… (cx-ncc.71, the Codex app's Attach):
 /// `composer.capture {mode: "screen" | "window"}`. After a click in the pane, macOS `screencapture`
 /// runs its own region pick (`-i`) or window pick (`-iW`), so the selection UI and the Screen
 /// Recording permission are the system's. It answers `{name, mimeType, data}` (base64) for the
 /// page to attach as an image, `{tooLarge: true, name}` for one that does not fit the page's cap,
-/// or `{cancelled: true}` when the person presses Esc.
+/// `{cancelled: true}` when the person presses Esc, or the `capture.failed` error when
+/// `screencapture` could not start or ended with an error.
 enum AgentPaneCapture {
     nonisolated static let tool = "/usr/sbin/screencapture"
+    private nonisolated static let logger = Logger(subsystem: "com.cmuxterm.app.next", category: "agent-pane.capture")
     /// The page's cap on an attached image (attachments.ts `MAX_IMAGE_BYTES`).
     nonisolated static let maxBytes = 5 * 1024 * 1024
     /// A Retina capture over the cap is scaled to this longest side first, then sent as JPEG.
@@ -22,11 +26,14 @@ enum AgentPaneCapture {
         // The screen goes to the agent: only a click in the pane starts a capture.
         guard model.transport.gestures.consume() else { return AgentPaneModel.transportFailure(.gestureRequired) }
         let file = FileManager.default.temporaryDirectory.appendingPathComponent("cmux-capture-\(UUID().uuidString).png")
-        guard await capture(window: mode == "window", to: file) else { return AgentPaneReply.success(["cancelled": true]) }
+        if case .failed(let reason) = await capture(window: mode == "window", to: file) {
+            logger.error("composer.capture: screencapture failed: \(reason, privacy: .public)")
+            return AgentPaneReply.failure(code: "capture.failed", message: AgentPaneModel.captureFailedMessage)
+        }
         let taken = Date()
         // A person waits for the attachment: the read and the scale run at userBlocking's priority.
         let value = await Task.detached(priority: WorkPriority.userBlocking.taskPriority) { () -> [String: any Sendable] in
-            defer { try? FileManager.default.removeItem(at: file) }
+            defer { try? FileManager.checked.removeItem(at: file) }
             return Self.answer((try? Data(contentsOf: file)) ?? Data(), taken: taken)
         }.value
         return AgentPaneReply.success(value)
@@ -46,12 +53,38 @@ enum AgentPaneCapture {
         return ["name": name, "mimeType": image.mimeType, "data": image.data.base64EncodedString()]
     }
 
-    /// Runs the pick; false when `screencapture` could not start. Its caller is the main actor: the spawn
-    /// runs on AsyncProcess's own queue, and the wait holds no thread.
-    nonisolated static func capture(window: Bool, to file: URL) async -> Bool {
+    /// What one pick produced.
+    nonisolated enum CaptureResult: Equatable, Sendable {
+        /// `screencapture` ran; its file holds the image, or nothing (Esc).
+        case taken
+        /// It could not start or ended with an error, in the system's words.
+        case failed(String)
+    }
+
+    /// Runs the pick with `executable` (``tool``; tests pass a stub); `.failed` when it could not start.
+    /// Its caller is the main actor, so it runs off it (`@concurrent`): the spawn runs on AsyncProcess's own
+    /// queue, the wait holds no thread, and the file check and cleanup never touch the disk on the main thread.
+    @concurrent nonisolated static func capture(window: Bool, to file: URL, executable: String = tool) async -> CaptureResult {
         // -x: no shutter sound; -o: no window shadow.
         let arguments = (window ? ["-i", "-W", "-o"] : ["-i"]) + ["-x", "-t", "png", file.path]
-        return (try? await AsyncProcess().run(ProcessRequest(URL(fileURLWithPath: tool), arguments))) != nil
+        let result: CaptureResult
+        do {
+            let outcome = try await AsyncProcess().run(ProcessRequest(URL(fileURLWithPath: executable), arguments,
+                                                                      errorOutput: .separate))
+            let said = String(decoding: outcome.errorOutput, as: UTF8.self).trimmingCharacters(in: .whitespacesAndNewlines)
+            // With a file, the status decides (0: the image). With none, an Esc says nothing, whatever
+            // status the macOS version gives it, and an end that says why is an error (no Screen
+            // Recording grant). A signal is always an error (a crash).
+            let wrote = FileManager.checked.fileExists(atPath: file.path)
+            let ok = outcome.exited && (wrote ? outcome.status == 0 : said.isEmpty)
+            result = ok ? .taken
+                : .failed("\(said.isEmpty ? "no message" : said) (\(outcome.exited ? "exit" : "signal") \(outcome.status))")
+        } catch {
+            result = .failed(String(describing: error))
+        }
+        // A failed pick's partial file is the person's screen: never left in the temp folder.
+        if result != .taken { try? FileManager.checked.removeItem(at: file) }
+        return result
     }
 
     /// The capture as an image under ``maxBytes``: the PNG as written, else scaled to ``maxSide`` as

Packages/macOS/CmuxNext/Sources/CmuxNextAgentPane/AgentPaneStrings.swift

@@ -21,6 +21,11 @@ extension AgentPaneModel {
         String(localized: "agentPane.error.openPreview", defaultValue: "The page could not be opened.", bundle: .module)
     }
 
+    /// The system screen capture could not start, or ended with an error and wrote no image.
+    static var captureFailedMessage: String {
+        String(localized: "agentPane.error.capture", defaultValue: "The screen capture failed.", bundle: .module)
+    }
+
     /// The host's acpmux socket refused a page frame or could not connect (AgentPaneTransport).
     static var transportFailedMessage: String {
         String(localized: "agentPane.error.transport", defaultValue: "The app refused the agent connection.", bundle: .module)

Packages/macOS/CmuxNext/Sources/CmuxNextAgentPane/Resources/Localizable.xcstrings

@@ -9709,6 +9709,137 @@
         }
       }
     },
+    "agentPane.error.capture": {
+      "extractionState": "manual",
+      "localizations": {
+        "ar": {
+          "stringUnit": {
+            "state": "translated",
+            "value": "فشل التقاط الشاشة."
+          }
+        },
+        "bs": {
+          "stringUnit": {
+            "state": "translated",
+            "value": "Snimanje ekrana nije uspjelo."
+          }
+        },
+        "da": {
+          "stringUnit": {
+            "state": "translated",
+            "value": "Skærmoptagelsen mislykkedes."
+          }
+        },
+        "de": {
+          "stringUnit": {
+            "state": "translated",
+            "value": "Die Bildschirmaufnahme ist fehlgeschlagen."
+          }
+        },
+        "en": {
+          "stringUnit": {
+            "state": "translated",
+            "value": "The screen capture failed."
+          }
+        },
+        "es": {
+          "stringUnit": {
+            "state": "translated",
+            "value": "La captura de pantalla falló."
+          }
+        },
+        "fr": {
+          "stringUnit": {
+            "state": "translated",
+            "value": "La capture d’écran a échoué."
+          }
+        },
+        "it": {
+          "stringUnit": {
+            "state": "translated",
+            "value": "L’acquisizione dello schermo non è riuscita."
+          }
+        },
+        "ja": {
+          "stringUnit": {
+            "state": "translated",
+            "value": "スクリーンキャプチャに失敗しました。"
+          }
+        },
+        "km": {
+          "stringUnit": {
+            "state": "translated",
+            "value": "ការថតអេក្រង់បានបរាជ័យ។"
+          }
+        },
+        "ko": {
+          "stringUnit": {
+            "state": "translated",
+            "value": "화면 캡처에 실패했습니다."
+          }
+        },
+        "nb": {
+          "stringUnit": {
+            "state": "translated",
+            "value": "Skjermbildet kunne ikke tas."
+          }
+        },
+        "pl": {
+          "stringUnit": {
+            "state": "translated",
+            "value": "Nie udało się przechwycić ekranu."
+          }
+        },
+        "pt-BR": {
+          "stringUnit": {
+            "state": "translated",
+            "value": "A captura de tela falhou."
+          }
+        },
+        "ru": {
+          "stringUnit": {
+            "state": "translated",
+            "value": "Не удалось сделать снимок экрана."
+          }
+        },
+        "th": {
+          "stringUnit": {
+            "state": "translated",
+            "value": "การจับภาพหน้าจอล้มเหลว"
+          }
+        },
+        "tr": {
+          "stringUnit": {
+            "state": "translated",
+            "value": "Ekran görüntüsü alınamadı."
+          }
+        },
+        "uk": {
+          "stringUnit": {
+            "state": "translated",
+            "value": "Не вдалося зробити знімок екрана."
+          }
+        },
+        "vi": {
+          "stringUnit": {
+            "state": "translated",
+            "value": "Không thể chụp màn hình."
+          }
+        },
+        "zh-Hans": {
+          "stringUnit": {
+            "state": "translated",
+            "value": "屏幕捕获失败。"
+          }
+        },
+        "zh-Hant": {
+          "stringUnit": {
+            "state": "translated",
+            "value": "螢幕擷取失敗。"
+          }
+        }
+      }
+    },
     "agentPane.error.openPreview": {
       "extractionState": "manual",
       "localizations": {

Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/AgentPaneCaptureTests.swift

@@ -23,6 +23,62 @@ struct AgentPaneCaptureTests {
         #expect(answer["data"] == nil)
     }
 
+    /// A stub `screencapture` in a fresh folder, the file it would write, and a cleanup.
+    private static func stub(_ script: String) throws -> (tool: String, file: URL, root: URL) {
+        let root = FileManager.default.temporaryDirectory.appendingPathComponent("cmux-capture-test-\(UUID().uuidString)")
+        try FileManager.default.createDirectory(at: root, withIntermediateDirectories: true)
+        let tool = root.appendingPathComponent("screencapture-stub.sh")
+        try "#!/bin/sh\n\(script)\n".write(to: tool, atomically: true, encoding: .utf8)
+        try FileManager.default.setAttributes([.posixPermissions: 0o755], ofItemAtPath: tool.path)
+        return (tool.path, root.appendingPathComponent("capture.png"), root)
+    }
+
+    /// #20827: `screencapture` that fails (no Screen Recording grant, a crash) is not an Esc.
+    @Test(.timeLimit(.minutes(1))) func aFailedScreencaptureIsAFailureNotACancel() async throws {
+        let stub = try Self.stub(#"echo "could not create image from display" >&2; exit 1"#)
+        defer { try? FileManager.default.removeItem(at: stub.root) }
+        let result = await AgentPaneCapture.capture(window: false, to: stub.file, executable: stub.tool)
+        guard case .failed = result else {
+            Issue.record("a screencapture that exited 1 read as \(result)")
+            return
+        }
+    }
+
+    /// An error that `screencapture` reports with status 0 and no image is not an Esc either.
+    @Test(.timeLimit(.minutes(1))) func anErrorSaidWithStatusZeroAndNoImageIsAFailure() async throws {
+        let stub = try Self.stub(#"echo "could not create image from display" >&2; exit 0"#)
+        defer { try? FileManager.default.removeItem(at: stub.root) }
+        let result = await AgentPaneCapture.capture(window: false, to: stub.file, executable: stub.tool)
+        guard case .failed = result else {
+            Issue.record("an error said with status 0 read as \(result)")
+            return
+        }
+    }
+
+    /// An Esc in the pick exits 0 and writes nothing: still a cancel, not an error.
+    @Test(.timeLimit(.minutes(1))) func anEscapedPickIsTakenWithNoImage() async throws {
+        let stub = try Self.stub("exit 0")
+        defer { try? FileManager.default.removeItem(at: stub.root) }
+        #expect(await AgentPaneCapture.capture(window: false, to: stub.file, executable: stub.tool) == .taken)
+        #expect(!FileManager.default.fileExists(atPath: stub.file.path))
+    }
+
+    /// An Esc that a macOS version reports with a nonzero status, writing and saying nothing, is
+    /// still a cancel.
+    @Test(.timeLimit(.minutes(1))) func aQuietNonzeroExitWithNoImageIsACancel() async throws {
+        let stub = try Self.stub("exit 1")
+        defer { try? FileManager.default.removeItem(at: stub.root) }
+        #expect(await AgentPaneCapture.capture(window: false, to: stub.file, executable: stub.tool) == .taken)
+    }
+
+    /// A failed pick that left a partial file removes it (its bytes are the person's screen).
+    @Test(.timeLimit(.minutes(1))) func aFailedPickLeavesNoFile() async throws {
+        let stub = try Self.stub(#"printf partial > "$5"; exit 1"#)
+        defer { try? FileManager.default.removeItem(at: stub.root) }
+        _ = await AgentPaneCapture.capture(window: false, to: stub.file, executable: stub.tool)
+        #expect(!FileManager.default.fileExists(atPath: stub.file.path), "the failed capture's file was left behind")
+    }
+
     @Test func aCaptureTooLargeToAttachIsNamedForTheNote() {
         // Bytes over the cap that are no image: nothing can scale them under it.
         let answer = AgentPaneCapture.answer(Data(count: AgentPaneCapture.maxBytes + 1), taken: Self.taken)