From 296ca24eb4842b7ee903a8816b24edb46e98edf0 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Mon, 28 Sep 2026 02:12:55 -0700 Subject: [PATCH 01/15] Add the window recording request and frame model to CmuxFoundation A clip needs its parameters settled before any capture starts: the format and its default frame rate, the scale and width caps, the crop rectangle in window points, the file name, and how the frame size follows a window that is resized mid-clip. None of that needs a window or a screen, so it lives in CmuxFoundation with tests instead of inside the capture session. WindowRecordingFrameGeometry aspect-fits each captured image into the frame size chosen at the start, so a resize letterboxes rather than stretches, and a crop is adopted once and then kept for the rest of the clip. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../WindowRecordingCaptionTrack.swift | 78 +++++ .../WindowRecordingFrameGeometry.swift | 120 +++++++ .../WindowRecordingLabel.swift | 43 +++ .../WindowRecordingOutputNaming.swift | 28 ++ .../WindowRecordingRegion.swift | 40 +++ .../WindowRecordingRequest.swift | 301 ++++++++++++++++++ .../WindowRecordingCaptionTrackTests.swift | 71 +++++ .../WindowRecordingFrameGeometryTests.swift | 139 ++++++++ .../WindowRecordingOutputNamingTests.swift | 48 +++ .../WindowRecordingRequestTests.swift | 163 ++++++++++ 10 files changed, 1031 insertions(+) create mode 100644 Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingCaptionTrack.swift create mode 100644 Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingFrameGeometry.swift create mode 100644 Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingLabel.swift create mode 100644 Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingOutputNaming.swift create mode 100644 Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingRegion.swift create mode 100644 Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingRequest.swift create mode 100644 Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/WindowRecordingCaptionTrackTests.swift create mode 100644 Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/WindowRecordingFrameGeometryTests.swift create mode 100644 Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/WindowRecordingOutputNamingTests.swift create mode 100644 Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/WindowRecordingRequestTests.swift diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingCaptionTrack.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingCaptionTrack.swift new file mode 100644 index 000000000000..cd56eabaa9a4 --- /dev/null +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingCaptionTrack.swift @@ -0,0 +1,78 @@ +internal import Foundation + +/// The captions burned into a recording's frames. +/// +/// An agent that records a tour wants the clip to say what it did, not just +/// show a cursor moving: `cmux record note "open Settings"` appends one here, +/// and the recorder draws whichever note is current when it writes a frame. +/// Automation commands push their own notes through the same sink, which is +/// what `--overlay-actions` turns on. +public struct WindowRecordingCaptionTrack: Sendable, Equatable { + public struct Note: Sendable, Equatable { + public let offsetSeconds: Double + public let text: String + + public init(offsetSeconds: Double, text: String) { + self.offsetSeconds = offsetSeconds + self.text = text + } + } + + /// How long one note stays on screen after the moment it was pushed. + public static let defaultVisibleSeconds: Double = 2.5 + /// Longest caption drawn; a whole prompt would cover the window. + public static let maximumCharacters = 120 + + public let visibleSeconds: Double + private var notes: [Note] = [] + + public init(visibleSeconds: Double = WindowRecordingCaptionTrack.defaultVisibleSeconds) { + self.visibleSeconds = max(0.1, visibleSeconds) + } + + public var isEmpty: Bool { notes.isEmpty } + public var notesInOrder: [Note] { notes } + public var count: Int { notes.count } + + /// Records one caption. Returns false when the text carries nothing to draw. + /// + /// Notes arrive from the recorder's own clock, so they normally append; an + /// out-of-order offset is still inserted in order rather than dropped, + /// which keeps `caption(atOffsetSeconds:)` a plain search. + @discardableResult + public mutating func append(text: String, atOffsetSeconds offset: Double) -> Bool { + let collapsed = text + .components(separatedBy: .whitespacesAndNewlines) + .filter { !$0.isEmpty } + .joined(separator: " ") + guard !collapsed.isEmpty else { return false } + let clipped = collapsed.count > Self.maximumCharacters + ? String(collapsed.prefix(Self.maximumCharacters - 1)) + "\u{2026}" + : collapsed + let note = Note( + offsetSeconds: offset.isFinite ? max(0, offset) : 0, + text: clipped + ) + if let last = notes.last, last.offsetSeconds <= note.offsetSeconds { + notes.append(note) + } else { + let index = notes.firstIndex { $0.offsetSeconds > note.offsetSeconds } ?? notes.endIndex + notes.insert(note, at: index) + } + return true + } + + /// The caption to draw on the frame captured at `offset`, if any. + public func caption(atOffsetSeconds offset: Double) -> String? { + guard offset.isFinite else { return nil } + var current: Note? + for note in notes { + if note.offsetSeconds > offset { break } + current = note + } + guard let current, offset - current.offsetSeconds < visibleSeconds else { + return nil + } + return current.text + } +} diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingFrameGeometry.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingFrameGeometry.swift new file mode 100644 index 000000000000..098febd78199 --- /dev/null +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingFrameGeometry.swift @@ -0,0 +1,120 @@ +internal import Foundation + +/// The crop and output size every frame of one recording uses. +/// +/// A video has one frame size for its whole length, so the geometry is decided +/// once from the first captured frame and reused: a window resized mid-clip is +/// letterboxed into the size the clip started with rather than ending it. +public struct WindowRecordingFrameGeometry: Equatable, Sendable { + /// Crop rectangle in captured pixels, origin at the image's top-left. + public let cropX: Int + public let cropY: Int + public let cropWidth: Int + public let cropHeight: Int + /// Encoded frame size in pixels. + public let outputWidth: Int + public let outputHeight: Int + + public enum Failure: Error, Equatable, Sendable { + case emptyWindow + case regionOutsideWindow + + public var message: String { + switch self { + case .emptyWindow: + "the window has no capturable content" + case .regionOutsideWindow: + "region lies outside the window" + } + } + } + + public var cropsNothing: Bool { + cropX == 0 && cropY == 0 + } + + public var scalesNothing: Bool { + outputWidth == cropWidth && outputHeight == cropHeight + } + + /// Plans the geometry from the first captured frame. + /// + /// - Parameters: + /// - windowPixelWidth: width of the captured window image, in pixels. + /// - windowPixelHeight: height of the captured window image, in pixels. + /// - pointPixelScale: pixels per point of the captured image, so a region + /// given in window points lands on the right pixels on a Retina display. + /// - request: the validated recording request. + public static func plan( + windowPixelWidth: Int, + windowPixelHeight: Int, + pointPixelScale: Double, + request: WindowRecordingRequest + ) throws -> WindowRecordingFrameGeometry { + guard windowPixelWidth > 0, windowPixelHeight > 0 else { + throw Failure.emptyWindow + } + let pixelScale = pointPixelScale.isFinite && pointPixelScale > 0 ? pointPixelScale : 1 + + var cropX = 0 + var cropY = 0 + var cropWidth = windowPixelWidth + var cropHeight = windowPixelHeight + + if case let .region(region) = request.target { + let left = Int((region.x * pixelScale).rounded(.down)) + let top = Int((region.y * pixelScale).rounded(.down)) + let right = Int(((region.x + region.width) * pixelScale).rounded(.up)) + let bottom = Int(((region.y + region.height) * pixelScale).rounded(.up)) + cropX = max(0, min(left, windowPixelWidth)) + cropY = max(0, min(top, windowPixelHeight)) + cropWidth = min(right, windowPixelWidth) - cropX + cropHeight = min(bottom, windowPixelHeight) - cropY + guard cropWidth > 0, cropHeight > 0 else { + throw Failure.regionOutsideWindow + } + } + + var outputWidth = Double(cropWidth) * request.scale + var outputHeight = Double(cropHeight) * request.scale + if let maximumWidth = request.maximumWidth, outputWidth > Double(maximumWidth) { + let shrink = Double(maximumWidth) / outputWidth + outputWidth *= shrink + outputHeight *= shrink + } + + let quantum = request.format == .mp4 ? 2 : 1 + return WindowRecordingFrameGeometry( + cropX: cropX, + cropY: cropY, + cropWidth: cropWidth, + cropHeight: cropHeight, + outputWidth: quantize(outputWidth, to: quantum), + outputHeight: quantize(outputHeight, to: quantum) + ) + } + + /// Keeps this geometry's encoded frame size while taking another plan's crop. + /// + /// A window resized mid-recording has to be cropped against the image that + /// was captured, but the encoded frame size may not change, so the new crop + /// is drawn into the size the clip started with. + public func adoptingCrop( + of other: WindowRecordingFrameGeometry + ) -> WindowRecordingFrameGeometry { + WindowRecordingFrameGeometry( + cropX: other.cropX, + cropY: other.cropY, + cropWidth: other.cropWidth, + cropHeight: other.cropHeight, + outputWidth: outputWidth, + outputHeight: outputHeight + ) + } + + /// H.264 wants even dimensions; a gif only wants a positive one. + private static func quantize(_ value: Double, to quantum: Int) -> Int { + let rounded = max(quantum, Int(value.rounded())) + return rounded - (rounded % quantum) + } +} diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingLabel.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingLabel.swift new file mode 100644 index 000000000000..fd32d1adb6b7 --- /dev/null +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingLabel.swift @@ -0,0 +1,43 @@ +internal import Foundation + +/// Converts an optional recording label into one safe filename component. +/// +/// Same rules as the screenshot label: the label reaches the filesystem, and a +/// caller that names a recording after a branch or a test can put anything in +/// it, including a path separator. +public struct WindowRecordingLabel: Sendable, Equatable { + private static let maximumUTF8ByteCount = 80 + + public let value: String + + public init(_ input: String) { + let trimmed = input.trimmingCharacters(in: .whitespacesAndNewlines) + guard !trimmed.isEmpty else { + value = "" + return + } + + let allowed = CharacterSet.alphanumerics.union( + CharacterSet(charactersIn: "-_.") + ) + let scalars = trimmed.unicodeScalars.map { + allowed.contains($0) ? Character($0) : "-" + } + let cleaned = String(scalars) + .trimmingCharacters(in: CharacterSet(charactersIn: "-_.")) + guard !cleaned.isEmpty else { + value = "recording" + return + } + + var byteCount = 0 + value = String(cleaned.prefix { character in + let characterByteCount = String(character).utf8.count + guard byteCount + characterByteCount <= Self.maximumUTF8ByteCount else { + return false + } + byteCount += characterByteCount + return true + }) + } +} diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingOutputNaming.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingOutputNaming.swift new file mode 100644 index 000000000000..6588a5e7f196 --- /dev/null +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingOutputNaming.swift @@ -0,0 +1,28 @@ +public import Foundation + +/// Where a recording lands and what it is called. +/// +/// Recordings follow the screenshot convention (a sortable timestamp plus a +/// short unique suffix, under one directory in the temporary directory) so the +/// two kinds of capture can be listed together and cleaned up together. +public struct WindowRecordingOutputNaming: Sendable { + public static let directoryName = "cmux-recordings" + + /// `2026-09-28T07-14-03Z_1a2b3c4d`: sorts by time, unique per recording. + public static func identifier(date: Date, uuid: UUID = UUID()) -> String { + let timestamp = ISO8601DateFormatter().string(from: date) + .replacingOccurrences(of: ":", with: "-") + .replacingOccurrences(of: "+", with: "_") + return "\(timestamp)_\(uuid.uuidString.prefix(8).lowercased())" + } + + public static func filename( + label: String, + identifier: String, + format: WindowRecordingRequest.Format + ) -> String { + let sanitized = WindowRecordingLabel(label).value + let stem = sanitized.isEmpty ? identifier : "\(sanitized)_\(identifier)" + return "\(stem).\(format.rawValue)" + } +} diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingRegion.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingRegion.swift new file mode 100644 index 000000000000..503ebc861195 --- /dev/null +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingRegion.swift @@ -0,0 +1,40 @@ +internal import Foundation + +/// A rectangle inside a cmux window, in window points, with its origin at the +/// window's top-left corner. +/// +/// Top-left origin matches the captured image and the coordinates +/// `debug.window.screenshot` frames are read in, so an agent that found a +/// feature in one frame can hand the same numbers back to the recorder. +public struct WindowRecordingRegion: Equatable, Sendable { + public let x: Double + public let y: Double + public let width: Double + public let height: Double + + public init(x: Double, y: Double, width: Double, height: Double) { + self.x = x + self.y = y + self.width = width + self.height = height + } + + /// Parses `x,y,width,height`, the form `cmux record --region` accepts. + public init?(commaSeparated text: String) { + let parts = text.split(separator: ",", omittingEmptySubsequences: false) + guard parts.count == 4 else { return nil } + var values: [Double] = [] + values.reserveCapacity(4) + for part in parts { + guard let value = Double(part.trimmingCharacters(in: .whitespaces)) else { + return nil + } + values.append(value) + } + self.init(x: values[0], y: values[1], width: values[2], height: values[3]) + } + + public var isFinite: Bool { + x.isFinite && y.isFinite && width.isFinite && height.isFinite + } +} diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingRequest.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingRequest.swift new file mode 100644 index 000000000000..4d8480393bb7 --- /dev/null +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingRequest.swift @@ -0,0 +1,301 @@ +internal import Foundation + +/// The bounds every `window.record.start` request is held to. +/// +/// One recording writes frames for as long as it runs, so the caller cannot be +/// trusted with an unbounded frame rate, duration, or pixel count: a stuck +/// agent would fill the disk. These are the limits the socket enforces; the +/// recorder stops itself at `maximumSeconds` even when nobody calls stop. +public struct WindowRecordingLimits: Sendable { + public static let framesPerSecond = 1...30 + public static let seconds = 0.5...120.0 + public static let scale = 0.1...1.0 + public static let maximumWidth = 64...4096 + public static let minimumRegionExtent: Double = 8 +} + +/// One validated `window.record.start` request. +public struct WindowRecordingRequest: Equatable, Sendable { + public enum Format: String, Sendable, CaseIterable { + case mp4 + case gif + + /// Frame rate used when the caller does not ask for one. + /// + /// A gif carries one palette and one frame per delay slot, so it stays + /// slower and smaller than the mp4 default. + public var defaultFramesPerSecond: Int { + switch self { + case .mp4: 12 + case .gif: 8 + } + } + + /// Scale used when the caller does not ask for one. + public var defaultScale: Double { + switch self { + case .mp4: 1 + case .gif: 0.5 + } + } + + /// Width cap used when the caller does not ask for one. + /// + /// A full-resolution gif of a cmux window runs to tens of megabytes, + /// which no pull request body wants, so gifs get a default cap and mp4 + /// keeps the window's own size. + public var defaultMaximumWidth: Int? { + switch self { + case .mp4: nil + case .gif: 960 + } + } + } + + /// What the recorder frames. + public enum Target: Equatable, Sendable { + /// A whole cmux window: the one named by `window`, else the same window + /// `debug.window.screenshot` picks. + case window + /// A rectangle inside that window. + case region(WindowRecordingRegion) + } + + public enum Failure: Error, Equatable, Sendable { + case unknownFormat(String) + case notANumber(field: String) + case outOfRange(field: String, message: String) + case malformedRegion(String) + case regionTooSmall + case outputPathNotAbsolute(String) + case outputExtensionMismatch(path: String, format: Format) + + /// The message the socket returns with `invalid_params`. + public var message: String { + switch self { + case let .unknownFormat(value): + let known = Format.allCases.map(\.rawValue).joined(separator: ", ") + return "unknown format '\(value)'; expected one of \(known)" + case let .notANumber(field): + return "\(field) must be a number" + case let .outOfRange(field, message): + return "\(field) \(message)" + case let .malformedRegion(value): + return "region '\(value)' must be x,y,width,height in window points" + case .regionTooSmall: + let minimum = Int(WindowRecordingLimits.minimumRegionExtent) + return "region width and height must each be at least \(minimum) points" + case let .outputPathNotAbsolute(path): + return "out '\(path)' must be an absolute path" + case let .outputExtensionMismatch(path, format): + return "out '\(path)' must end in .\(format.rawValue) for format \(format.rawValue)" + } + } + } + + public let target: Target + public let windowHandle: String? + public let format: Format + public let framesPerSecond: Int + public let maximumSeconds: Double + public let scale: Double + public let maximumWidth: Int? + public let label: String + public let outputPath: String? + public let drawsCaptions: Bool + + public init( + target: Target = .window, + windowHandle: String? = nil, + format: Format = .mp4, + framesPerSecond: Int? = nil, + maximumSeconds: Double = 15, + scale: Double? = nil, + maximumWidth: Int?? = nil, + label: String = "", + outputPath: String? = nil, + drawsCaptions: Bool = true + ) { + self.target = target + self.windowHandle = windowHandle + self.format = format + self.framesPerSecond = framesPerSecond ?? format.defaultFramesPerSecond + self.maximumSeconds = maximumSeconds + self.scale = scale ?? format.defaultScale + self.maximumWidth = maximumWidth ?? format.defaultMaximumWidth + self.label = label + self.outputPath = outputPath + self.drawsCaptions = drawsCaptions + } + + /// The number of frames this request may write before it stops itself. + public var frameBudget: Int { + max(1, Int((maximumSeconds * Double(framesPerSecond)).rounded(.up))) + } + + /// Seconds between two frames. + public var frameInterval: Double { + 1 / Double(framesPerSecond) + } + + /// Validates one decoded `window.record.start` parameter dictionary. + /// + /// Absent keys take the format's defaults, so `{}` is a complete request: + /// a 15 second mp4 of the selected window. + public static func make(params: [String: Any]) throws -> WindowRecordingRequest { + let format = try decodeFormat(params["format"]) + let framesPerSecond = try decodeInt( + params["fps"], + field: "fps", + range: WindowRecordingLimits.framesPerSecond + ) + let maximumSeconds = try decodeDouble( + params["max_seconds"], + field: "max_seconds", + range: WindowRecordingLimits.seconds + ) + let scale = try decodeDouble( + params["scale"], + field: "scale", + range: WindowRecordingLimits.scale + ) + let maximumWidth = try decodeInt( + params["max_width"], + field: "max_width", + range: WindowRecordingLimits.maximumWidth + ) + let target = try decodeTarget(params["region"]) + let outputPath = try decodeOutputPath(params["out"], format: format) + + return WindowRecordingRequest( + target: target, + windowHandle: (params["window"] as? String)?.trimmedNonEmpty, + format: format, + framesPerSecond: framesPerSecond, + maximumSeconds: maximumSeconds ?? 15, + scale: scale, + maximumWidth: maximumWidth.map { Optional($0) }, + label: WindowRecordingLabel(params["label"] as? String ?? "").value, + outputPath: outputPath, + drawsCaptions: decodeBool(params["captions"]) ?? true + ) + } + + private static func decodeFormat(_ value: Any?) throws -> Format { + guard let raw = (value as? String)?.trimmedNonEmpty else { return .mp4 } + guard let format = Format(rawValue: raw.lowercased()) else { + throw Failure.unknownFormat(raw) + } + return format + } + + private static func decodeTarget(_ value: Any?) throws -> Target { + guard let value else { return .window } + let region: WindowRecordingRegion + if let text = value as? String { + guard let parsed = WindowRecordingRegion(commaSeparated: text) else { + throw Failure.malformedRegion(text) + } + region = parsed + } else if let numbers = value as? [Any] { + let doubles = numbers.compactMap { numericValue($0) } + guard doubles.count == 4 else { + throw Failure.malformedRegion(String(describing: value)) + } + region = WindowRecordingRegion( + x: doubles[0], + y: doubles[1], + width: doubles[2], + height: doubles[3] + ) + } else { + throw Failure.malformedRegion(String(describing: value)) + } + guard region.isFinite else { + throw Failure.malformedRegion(String(describing: value)) + } + guard region.width >= WindowRecordingLimits.minimumRegionExtent, + region.height >= WindowRecordingLimits.minimumRegionExtent else { + throw Failure.regionTooSmall + } + return .region(region) + } + + private static func decodeOutputPath(_ value: Any?, format: Format) throws -> String? { + guard let path = (value as? String)?.trimmedNonEmpty else { return nil } + guard path.hasPrefix("/") else { + throw Failure.outputPathNotAbsolute(path) + } + guard path.lowercased().hasSuffix(".\(format.rawValue)") else { + throw Failure.outputExtensionMismatch(path: path, format: format) + } + return path + } + + private static func decodeInt( + _ value: Any?, + field: String, + range: ClosedRange + ) throws -> Int? { + guard let value else { return nil } + guard let number = numericValue(value) else { + throw Failure.notANumber(field: field) + } + let rounded = Int(number.rounded()) + guard range.contains(rounded) else { + throw Failure.outOfRange( + field: field, + message: "must be between \(range.lowerBound) and \(range.upperBound)" + ) + } + return rounded + } + + private static func decodeDouble( + _ value: Any?, + field: String, + range: ClosedRange + ) throws -> Double? { + guard let value else { return nil } + guard let number = numericValue(value), number.isFinite else { + throw Failure.notANumber(field: field) + } + guard range.contains(number) else { + throw Failure.outOfRange( + field: field, + message: "must be between \(Self.trim(range.lowerBound)) and \(Self.trim(range.upperBound))" + ) + } + return number + } + + private static func decodeBool(_ value: Any?) -> Bool? { + if let flag = value as? Bool { return flag } + guard let text = (value as? String)?.trimmedNonEmpty?.lowercased() else { return nil } + switch text { + case "true", "yes", "1": return true + case "false", "no", "0": return false + default: return nil + } + } + + private static func numericValue(_ value: Any) -> Double? { + if let number = value as? Double { return number } + if let number = value as? Int { return Double(number) } + if let number = value as? NSNumber { return number.doubleValue } + if let text = value as? String { return Double(text.trimmingCharacters(in: .whitespaces)) } + return nil + } + + private static func trim(_ value: Double) -> String { + value == value.rounded() ? String(Int(value)) : String(value) + } +} + +private extension String { + /// Nil for a blank value, so an empty socket string means "not supplied". + var trimmedNonEmpty: String? { + let trimmed = trimmingCharacters(in: .whitespacesAndNewlines) + return trimmed.isEmpty ? nil : trimmed + } +} diff --git a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/WindowRecordingCaptionTrackTests.swift b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/WindowRecordingCaptionTrackTests.swift new file mode 100644 index 000000000000..9809408bddcc --- /dev/null +++ b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/WindowRecordingCaptionTrackTests.swift @@ -0,0 +1,71 @@ +import Foundation +import Testing +@testable import CmuxFoundation + +@Suite struct WindowRecordingCaptionTrackTests { + @Test func noNotesDrawNoCaption() { + let track = WindowRecordingCaptionTrack() + + #expect(track.isEmpty) + #expect(track.caption(atOffsetSeconds: 0) == nil) + } + + @Test func aNoteIsDrawnFromItsOffsetUntilItExpires() { + var track = WindowRecordingCaptionTrack(visibleSeconds: 2) + track.append(text: "open Settings", atOffsetSeconds: 1) + + #expect(track.caption(atOffsetSeconds: 0.9) == nil) + #expect(track.caption(atOffsetSeconds: 1) == "open Settings") + #expect(track.caption(atOffsetSeconds: 2.9) == "open Settings") + #expect(track.caption(atOffsetSeconds: 3) == nil) + } + + @Test func aLaterNoteReplacesAnEarlierOneImmediately() { + var track = WindowRecordingCaptionTrack(visibleSeconds: 10) + track.append(text: "first", atOffsetSeconds: 0) + track.append(text: "second", atOffsetSeconds: 1) + + #expect(track.caption(atOffsetSeconds: 0.5) == "first") + #expect(track.caption(atOffsetSeconds: 1) == "second") + #expect(track.count == 2) + } + + @Test func outOfOrderNotesAreStillOrdered() { + var track = WindowRecordingCaptionTrack(visibleSeconds: 10) + track.append(text: "late", atOffsetSeconds: 5) + track.append(text: "early", atOffsetSeconds: 1) + + #expect(track.notesInOrder.map(\.text) == ["early", "late"]) + #expect(track.caption(atOffsetSeconds: 2) == "early") + } + + @Test func whitespaceIsCollapsedAndBlankNotesAreDropped() { + var track = WindowRecordingCaptionTrack() + + let drewCaption = track.append(text: " click\n Settings ", atOffsetSeconds: 0) + let drewBlank = track.append(text: " \n ", atOffsetSeconds: 1) + + #expect(drewCaption) + #expect(!drewBlank) + #expect(track.caption(atOffsetSeconds: 0) == "click Settings") + #expect(track.count == 1) + } + + @Test func longCaptionsAreClippedRatherThanCoveringTheWindow() { + var track = WindowRecordingCaptionTrack() + track.append(text: String(repeating: "a", count: 400), atOffsetSeconds: 0) + let caption = track.caption(atOffsetSeconds: 0) + + #expect(caption?.count == WindowRecordingCaptionTrack.maximumCharacters) + #expect(caption?.hasSuffix("\u{2026}") == true) + } + + @Test func negativeAndBrokenOffsetsLandAtTheStart() { + var track = WindowRecordingCaptionTrack(visibleSeconds: 5) + track.append(text: "start", atOffsetSeconds: -3) + track.append(text: "nan", atOffsetSeconds: .nan) + + #expect(track.notesInOrder.allSatisfy { $0.offsetSeconds == 0 }) + #expect(track.caption(atOffsetSeconds: .nan) == nil) + } +} diff --git a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/WindowRecordingFrameGeometryTests.swift b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/WindowRecordingFrameGeometryTests.swift new file mode 100644 index 000000000000..cebe7f13167b --- /dev/null +++ b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/WindowRecordingFrameGeometryTests.swift @@ -0,0 +1,139 @@ +import Foundation +import Testing +@testable import CmuxFoundation + +@Suite struct WindowRecordingFrameGeometryTests { + private func plan( + windowPixelWidth: Int = 1600, + windowPixelHeight: Int = 1000, + pointPixelScale: Double = 2, + params: [String: Any] = [:] + ) throws -> WindowRecordingFrameGeometry { + try WindowRecordingFrameGeometry.plan( + windowPixelWidth: windowPixelWidth, + windowPixelHeight: windowPixelHeight, + pointPixelScale: pointPixelScale, + request: try WindowRecordingRequest.make(params: params) + ) + } + + @Test func mp4OfAWholeWindowKeepsItsPixels() throws { + let geometry = try plan() + + #expect(geometry.cropX == 0) + #expect(geometry.cropY == 0) + #expect(geometry.cropWidth == 1600) + #expect(geometry.cropHeight == 1000) + #expect(geometry.outputWidth == 1600) + #expect(geometry.outputHeight == 1000) + #expect(geometry.scalesNothing) + } + + @Test func mp4DimensionsStayEvenForH264() throws { + let geometry = try plan( + windowPixelWidth: 1001, + windowPixelHeight: 667, + params: ["scale": 0.5] + ) + + #expect(geometry.outputWidth % 2 == 0) + #expect(geometry.outputHeight % 2 == 0) + #expect(geometry.outputWidth == 500) + #expect(geometry.outputHeight == 334) + } + + @Test func gifIsHalvedAndWidthCappedByDefault() throws { + let geometry = try plan(params: ["format": "gif"]) + + #expect(geometry.outputWidth == 800) + #expect(geometry.outputHeight == 500) + } + + @Test func gifWidthCapKeepsTheAspectRatio() throws { + let geometry = try plan( + windowPixelWidth: 3200, + windowPixelHeight: 2000, + params: ["format": "gif"] + ) + + #expect(geometry.outputWidth == 960) + #expect(geometry.outputHeight == 600) + } + + @Test func regionPointsBecomePixelsOnARetinaWindow() throws { + let geometry = try plan(params: ["region": "100,50,400,300"]) + + #expect(geometry.cropX == 200) + #expect(geometry.cropY == 100) + #expect(geometry.cropWidth == 800) + #expect(geometry.cropHeight == 600) + #expect(geometry.outputWidth == 800) + #expect(geometry.outputHeight == 600) + #expect(!geometry.cropsNothing) + } + + @Test func regionOnAOnePointPerPixelWindowIsNotScaled() throws { + let geometry = try plan(pointPixelScale: 1, params: ["region": "10,10,100,100"]) + + #expect(geometry.cropX == 10) + #expect(geometry.cropWidth == 100) + } + + @Test func aRegionHangingOffTheEdgeIsClampedToTheWindow() throws { + let geometry = try plan(params: ["region": "700,400,400,400"]) + + #expect(geometry.cropX == 1400) + #expect(geometry.cropY == 800) + #expect(geometry.cropWidth == 200) + #expect(geometry.cropHeight == 200) + } + + @Test func aRegionFullyOutsideTheWindowFails() throws { + let request = try WindowRecordingRequest.make(params: ["region": "2000,0,100,100"]) + + #expect(throws: WindowRecordingFrameGeometry.Failure.regionOutsideWindow) { + try WindowRecordingFrameGeometry.plan( + windowPixelWidth: 1600, + windowPixelHeight: 1000, + pointPixelScale: 1, + request: request + ) + } + } + + @Test func anEmptyWindowFails() throws { + #expect(throws: WindowRecordingFrameGeometry.Failure.emptyWindow) { + try plan(windowPixelWidth: 0) + } + } + + @Test func aBrokenPixelScaleFallsBackToOnePixelPerPoint() throws { + let geometry = try plan(pointPixelScale: 0, params: ["region": "10,10,100,100"]) + + #expect(geometry.cropX == 10) + #expect(geometry.cropWidth == 100) + } + + @Test func outputNeverCollapsesToZero() throws { + let geometry = try plan( + windowPixelWidth: 10, + windowPixelHeight: 10, + params: ["scale": 0.1] + ) + + #expect(geometry.outputWidth == 2) + #expect(geometry.outputHeight == 2) + } + + @Test func aResizedWindowKeepsTheEncodedFrameSize() throws { + let opening = try plan(windowPixelWidth: 1000, windowPixelHeight: 800) + let resized = try plan(windowPixelWidth: 1200, windowPixelHeight: 700) + + let continued = opening.adoptingCrop(of: resized) + + #expect(continued.cropWidth == 1200) + #expect(continued.cropHeight == 700) + #expect(continued.outputWidth == opening.outputWidth) + #expect(continued.outputHeight == opening.outputHeight) + } +} diff --git a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/WindowRecordingOutputNamingTests.swift b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/WindowRecordingOutputNamingTests.swift new file mode 100644 index 000000000000..956189b62c50 --- /dev/null +++ b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/WindowRecordingOutputNamingTests.swift @@ -0,0 +1,48 @@ +import Foundation +import Testing +@testable import CmuxFoundation + +@Suite struct WindowRecordingOutputNamingTests { + private let uuid = UUID(uuidString: "1A2B3C4D-0000-0000-0000-000000000000")! + + @Test func identifiersSortByTimeAndCarryAUniqueSuffix() { + let identifier = WindowRecordingOutputNaming.identifier( + date: Date(timeIntervalSince1970: 1_790_000_000), + uuid: uuid + ) + + #expect(identifier.hasSuffix("_1a2b3c4d")) + #expect(!identifier.contains(":")) + } + + @Test func aLabelBecomesTheFilenamePrefix() { + let filename = WindowRecordingOutputNaming.filename( + label: "sidebar tour", + identifier: "2026-09-28T07-14-03Z_1a2b3c4d", + format: .gif + ) + + #expect(filename == "sidebar-tour_2026-09-28T07-14-03Z_1a2b3c4d.gif") + } + + @Test func noLabelLeavesJustTheIdentifier() { + let filename = WindowRecordingOutputNaming.filename( + label: " ", + identifier: "id", + format: .mp4 + ) + + #expect(filename == "id.mp4") + } + + @Test func aPathSeparatorInALabelCannotEscapeTheDirectory() { + let filename = WindowRecordingOutputNaming.filename( + label: "../../etc/passwd", + identifier: "id", + format: .mp4 + ) + + #expect(!filename.contains("/")) + #expect(filename == "etc-passwd_id.mp4") + } +} diff --git a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/WindowRecordingRequestTests.swift b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/WindowRecordingRequestTests.swift new file mode 100644 index 000000000000..b42a920953e9 --- /dev/null +++ b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/WindowRecordingRequestTests.swift @@ -0,0 +1,163 @@ +import Foundation +import Testing +@testable import CmuxFoundation + +@Suite struct WindowRecordingRequestTests { + @Test func emptyParametersAreACompleteMP4Request() throws { + let request = try WindowRecordingRequest.make(params: [:]) + + #expect(request.format == .mp4) + #expect(request.target == .window) + #expect(request.windowHandle == nil) + #expect(request.framesPerSecond == 12) + #expect(request.maximumSeconds == 15) + #expect(request.scale == 1) + #expect(request.maximumWidth == nil) + #expect(request.drawsCaptions) + #expect(request.frameBudget == 180) + } + + @Test func gifDefaultsStaySmallerThanMP4Defaults() throws { + let request = try WindowRecordingRequest.make(params: ["format": "GIF"]) + + #expect(request.format == .gif) + #expect(request.framesPerSecond == 8) + #expect(request.scale == 0.5) + #expect(request.maximumWidth == 960) + } + + @Test func explicitValuesOverrideFormatDefaults() throws { + let request = try WindowRecordingRequest.make(params: [ + "format": "gif", + "fps": 15, + "scale": 0.75, + "max_width": 640, + "max_seconds": 4.5, + "captions": false, + "window": " window:2 ", + "label": "settings tour!", + ]) + + #expect(request.framesPerSecond == 15) + #expect(request.scale == 0.75) + #expect(request.maximumWidth == 640) + #expect(request.maximumSeconds == 4.5) + #expect(!request.drawsCaptions) + #expect(request.windowHandle == "window:2") + #expect(request.label == "settings-tour") + #expect(request.frameBudget == 68) + } + + @Test func numbersArriveAsStringsFromV1Callers() throws { + let request = try WindowRecordingRequest.make(params: [ + "fps": "6", + "max_seconds": "2", + "captions": "no", + ]) + + #expect(request.framesPerSecond == 6) + #expect(request.maximumSeconds == 2) + #expect(!request.drawsCaptions) + #expect(request.frameBudget == 12) + } + + @Test func regionAcceptsTextAndArrayForms() throws { + let fromText = try WindowRecordingRequest.make(params: ["region": "10, 20,300,200"]) + let fromArray = try WindowRecordingRequest.make(params: ["region": [10, 20, 300, 200]]) + let expected = WindowRecordingRegion(x: 10, y: 20, width: 300, height: 200) + + #expect(fromText.target == .region(expected)) + #expect(fromArray.target == .region(expected)) + } + + @Test(arguments: [ + "10,20,300", + "10,20,300,200,5", + "", + "a,b,c,d", + ]) + func malformedRegionsAreRejected(region: String) { + #expect(throws: WindowRecordingRequest.Failure.malformedRegion(region)) { + try WindowRecordingRequest.make(params: ["region": region]) + } + } + + @Test func tinyRegionsAreRejected() { + #expect(throws: WindowRecordingRequest.Failure.regionTooSmall) { + try WindowRecordingRequest.make(params: ["region": "0,0,4,200"]) + } + } + + @Test func nonFiniteRegionsAreRejected() { + #expect(throws: (any Error).self) { + try WindowRecordingRequest.make(params: ["region": [0, 0, Double.infinity, 200]]) + } + } + + @Test func unknownFormatsNameTheKnownOnes() throws { + let failure = WindowRecordingRequest.Failure.unknownFormat("webm") + + #expect(throws: failure) { + try WindowRecordingRequest.make(params: ["format": "webm"]) + } + #expect(failure.message == "unknown format 'webm'; expected one of mp4, gif") + } + + @Test(arguments: [ + ("fps", 0), + ("fps", 61), + ("max_width", 8), + ]) + func outOfRangeIntegersAreRejected(field: String, value: Int) { + #expect(throws: (any Error).self) { + try WindowRecordingRequest.make(params: [field: value]) + } + } + + @Test func durationIsCappedSoAStuckAgentCannotFillTheDisk() { + #expect(throws: (any Error).self) { + try WindowRecordingRequest.make(params: ["max_seconds": 3600]) + } + #expect(WindowRecordingLimits.seconds.upperBound == 120) + } + + @Test func scaleNeverUpscales() { + #expect(WindowRecordingLimits.scale.upperBound == 1) + #expect(throws: (any Error).self) { + try WindowRecordingRequest.make(params: ["scale": 2]) + } + } + + @Test func nonNumericValuesSayWhichField() { + #expect(throws: WindowRecordingRequest.Failure.notANumber(field: "fps")) { + try WindowRecordingRequest.make(params: ["fps": "soon"]) + } + } + + @Test func outputPathMustBeAbsoluteAndMatchTheFormat() { + #expect(throws: WindowRecordingRequest.Failure.outputPathNotAbsolute("clip.mp4")) { + try WindowRecordingRequest.make(params: ["out": "clip.mp4"]) + } + #expect(throws: WindowRecordingRequest.Failure.outputExtensionMismatch( + path: "/tmp/clip.mov", + format: .mp4 + )) { + try WindowRecordingRequest.make(params: ["out": "/tmp/clip.mov"]) + } + } + + @Test func outputPathIsKeptWhenItMatches() throws { + let request = try WindowRecordingRequest.make(params: [ + "format": "gif", + "out": "/tmp/tour/clip.GIF", + ]) + + #expect(request.outputPath == "/tmp/tour/clip.GIF") + } + + @Test func frameIntervalFollowsTheFrameRate() throws { + let request = try WindowRecordingRequest.make(params: ["fps": 10]) + + #expect(request.frameInterval == 0.1) + } +} From 63f25e6774ad30951d06abdba65943ba67b38dbb Mon Sep 17 00:00:00 2001 From: Leo Li Date: Mon, 28 Sep 2026 02:14:30 -0700 Subject: [PATCH 02/15] Add cmux record for capturing a cmux window to mp4 or gif An agent working in its own cmux can describe what it changed, but it cannot show it. `cmux record start` films a cmux window, or a region of one, and writes an mp4 or a gif that can go straight into a pull request. `cmux record note` drops a caption into the clip as it runs, so a reader can follow what was being done. Capture uses ScreenCaptureKit's own-process content, so only cmux's own windows are reachable and no Screen Recording permission is asked for. Encoding is AVAssetWriter for mp4 and ImageIO for gif; frame times come from when each frame was sampled, so playback matches what happened. A recording stops itself at --max-seconds, and one runs at a time, so an agent that goes away cannot leave a capture running. The window.record.* socket methods run on the socket worker and are not main-thread callable: the window being filmed has to keep drawing while the sampler runs. They are deliberately absent from the remote relay allowlist, which stays default-deny, because a clip is local screen content rather than an object of the peer's workspace. Co-Authored-By: Claude Opus 5.5 (1M context) --- CLI/CMUXCLI+CommandSuggestions.swift | 1 + CLI/CMUXCLI+Record.swift | 219 ++++++++++++ CLI/CMUXCLI+TaskHelp.swift | 1 + CLI/CMUXCLI+WindowDispatch.swift | 5 + CLI/cmux.swift | 10 + .../Wire/ControlCommandExecutionPolicy.swift | 8 + .../ControlCommandExecutionPolicyTests.swift | 14 + .../RemoteRelayCoreRPCPolicyTests.swift | 14 + Resources/Localizable.xcstrings | 312 +++++++++++++++++ .../TerminalController+WindowRecording.swift | 189 ++++++++++ Sources/TerminalController.swift | 9 + Sources/WindowRecordingFrameComposer.swift | 137 ++++++++ Sources/WindowRecordingFrameWriter.swift | 285 +++++++++++++++ Sources/WindowRecordingRegistry.swift | 148 ++++++++ Sources/WindowRecordingSession.swift | 330 ++++++++++++++++++ Sources/WindowRecordingWindowSelection.swift | 45 +++ cmux.xcodeproj/project.pbxproj | 32 ++ cmuxTests/WindowRecordingPipelineTests.swift | 292 ++++++++++++++++ docs/cli-contract.md | 2 + scripts/localization-allowed-omissions.json | 4 + 20 files changed, 2057 insertions(+) create mode 100644 CLI/CMUXCLI+Record.swift create mode 100644 Sources/TerminalController+WindowRecording.swift create mode 100644 Sources/WindowRecordingFrameComposer.swift create mode 100644 Sources/WindowRecordingFrameWriter.swift create mode 100644 Sources/WindowRecordingRegistry.swift create mode 100644 Sources/WindowRecordingSession.swift create mode 100644 Sources/WindowRecordingWindowSelection.swift create mode 100644 cmuxTests/WindowRecordingPipelineTests.swift diff --git a/CLI/CMUXCLI+CommandSuggestions.swift b/CLI/CMUXCLI+CommandSuggestions.swift index 3624928c9027..ad3752c89aca 100644 --- a/CLI/CMUXCLI+CommandSuggestions.swift +++ b/CLI/CMUXCLI+CommandSuggestions.swift @@ -166,6 +166,7 @@ extension CMUXCLI { "previous-window", "read-screen", "read-selection", + "record", "refresh-surfaces", "reload-config", "remote-daemon-status", diff --git a/CLI/CMUXCLI+Record.swift b/CLI/CMUXCLI+Record.swift new file mode 100644 index 000000000000..e2c5c0ddb357 --- /dev/null +++ b/CLI/CMUXCLI+Record.swift @@ -0,0 +1,219 @@ +import Foundation + +extension CMUXCLI { + static var recordHelp: String { + String(localized: "cli.help.record", defaultValue: """ + Usage: cmux record [start] [flags] + cmux record stop|status [--id ] + cmux record note [--id ] + cmux record list + + Record a cmux window to an mp4 or gif, for a pull request or a bug + report. Only cmux's own windows can be recorded, and no Screen + Recording permission is involved. + + Flags for start: + --format mp4|gif Output format (default: mp4) + --gif Shorthand for --format gif + --fps <1-30> Frames per second (default: 12 for mp4, 8 for gif) + --max-seconds Stop by itself after n seconds, 0.5-120 (default: 15) + --scale <0.1-1> Scale the output (default: 1 for mp4, 0.5 for gif) + --max-width <64-4096> Cap the output width, keeping the aspect ratio + --region Record part of the window, in window points + --out Write here instead of a temporary directory + --label Name the file + --no-captions Do not draw `record note` captions into the clip + --window Window to record (default: the frontmost one) + + Output: ` `, or the full response with --json. + + A recording stops itself at --max-seconds, so a clip is never left + running by an agent that goes away. One recording at a time. + + Example: + cmux record start --gif --max-seconds 8 --label sidebar-drag + cmux record note "dragging the workspace" + cmux record stop + """) + } + + static var recordUsageLine: String { + String( + localized: "cli.usage.record", + defaultValue: "record [start] [--format mp4|gif] [--gif] [--fps ] [--max-seconds ] [--scale ] [--max-width ] [--region ] [--out ] [--label ] [--no-captions] [--window ] | record stop|status [--id ] | record note [--id ] | record list" + ) + } + + func runRecordCommand( + commandArgs: [String], + client: SocketClient, + jsonOutput: Bool, + windowOverride: String? + ) throws { + // `cmux record --gif` is `cmux record start --gif`: the subcommand is + // only a subcommand when it is not a flag. + let hasSubcommand = !(commandArgs.first?.hasPrefix("-") ?? true) + let subcommand = hasSubcommand ? commandArgs[0].lowercased() : "start" + let arguments = hasSubcommand ? Array(commandArgs.dropFirst()) : commandArgs + + switch subcommand { + case "start": + try runRecordStart( + arguments: arguments, + client: client, + jsonOutput: jsonOutput, + windowOverride: windowOverride + ) + case "stop", "status": + var (recordingID, trailing) = parseOption(arguments, name: "--id") + if recordingID == nil, let positional = trailing.first { + recordingID = positional + trailing = Array(trailing.dropFirst()) + } + try requireNoRecordArguments(trailing, subcommand: subcommand) + var params: [String: Any] = [:] + if let recordingID { params["id"] = recordingID } + printRecordStatus( + try client.sendV2( + method: subcommand == "stop" ? "window.record.stop" : "window.record.status", + params: params, + responseTimeout: 60 + ), + jsonOutput: jsonOutput + ) + case "note": + let (id, trailing) = parseOption(arguments, name: "--id") + let text = trailing.filter { $0 != "--" }.joined(separator: " ") + .trimmingCharacters(in: .whitespacesAndNewlines) + guard !text.isEmpty else { + throw CLIError(message: String( + localized: "cli.record.error.noteText", + defaultValue: "record note: text is required" + )) + } + var params: [String: Any] = ["text": text] + if let id { params["id"] = id } + printRecordStatus( + try client.sendV2(method: "window.record.note", params: params), + jsonOutput: jsonOutput + ) + case "list": + try requireNoRecordArguments(arguments, subcommand: subcommand) + let response = try client.sendV2(method: "window.record.list") + if jsonOutput { + print(jsonString(response)) + return + } + let recordings = (response["recordings"] as? [[String: Any]]) ?? [] + for recording in recordings { + print(Self.recordStatusLine(recording)) + } + default: + throw CLIError(message: String( + format: String( + localized: "cli.record.error.unknownSubcommand", + defaultValue: "record: unknown subcommand '%@' (start, stop, status, note, list)" + ), + subcommand + )) + } + } + + private func runRecordStart( + arguments: [String], + client: SocketClient, + jsonOutput: Bool, + windowOverride: String? + ) throws { + let (format, afterFormat) = parseOption(arguments, name: "--format") + let (fps, afterFPS) = parseOption(afterFormat, name: "--fps") + let (maxSeconds, afterSeconds) = parseOption(afterFPS, name: "--max-seconds") + let (scale, afterScale) = parseOption(afterSeconds, name: "--scale") + let (maxWidth, afterWidth) = parseOption(afterScale, name: "--max-width") + let (region, afterRegion) = parseOption(afterWidth, name: "--region") + let (out, afterOut) = parseOption(afterRegion, name: "--out") + let (label, afterLabel) = parseOption(afterOut, name: "--label") + let (window, afterWindow) = parseOption(afterLabel, name: "--window") + + var trailing = afterWindow + let wantsGIF = trailing.contains("--gif") + let drawsCaptions = !trailing.contains("--no-captions") + trailing.removeAll { $0 == "--gif" || $0 == "--no-captions" } + try requireNoRecordArguments(trailing, subcommand: "start") + + if wantsGIF, let format, format.lowercased() != "gif" { + throw CLIError(message: String( + localized: "cli.record.error.formatConflict", + defaultValue: "record start: --gif conflicts with --format" + )) + } + + // Values stay as the caller typed them: the app owns the limits, so the + // CLI cannot disagree with it about what a valid frame rate is. + var params: [String: Any] = [:] + if let format { params["format"] = format } else if wantsGIF { params["format"] = "gif" } + if let fps { params["fps"] = fps } + if let maxSeconds { params["max_seconds"] = maxSeconds } + if let scale { params["scale"] = scale } + if let maxWidth { params["max_width"] = maxWidth } + if let region { params["region"] = region } + if let label { params["label"] = label } + if !drawsCaptions { params["captions"] = false } + // The app has no idea what directory the CLI was run from. + if let out { params["out"] = Self.absoluteRecordingPath(out) } + if let windowID = try normalizeWindowHandle(window ?? windowOverride, client: client) { + params["window"] = windowID + } + + printRecordStatus( + try client.sendV2( + method: "window.record.start", + params: params, + responseTimeout: 45 + ), + jsonOutput: jsonOutput + ) + } + + private func requireNoRecordArguments(_ trailing: [String], subcommand: String) throws { + guard trailing.isEmpty else { + throw CLIError(message: String( + format: String( + localized: "cli.record.error.unexpectedArguments", + defaultValue: "record %@: unexpected arguments: %@" + ), + subcommand, + trailing.joined(separator: " ") + )) + } + } + + private func printRecordStatus(_ response: [String: Any], jsonOutput: Bool) { + if jsonOutput { + print(jsonString(response)) + return + } + print(Self.recordStatusLine(response)) + } + + static func recordStatusLine(_ response: [String: Any]) -> String { + let id = (response["id"] as? String) ?? "-" + let state = (response["state"] as? String) ?? "-" + let frames = (response["frames"] as? Int).map(String.init) ?? "-" + let path = (response["path"] as? String) ?? "-" + var line = "\(id) \(state) \(frames) \(path)" + if let error = response["error"] as? String, !error.isEmpty { + line += " (\(error))" + } + return line + } + + static func absoluteRecordingPath(_ path: String) -> String { + let expanded = NSString(string: path).expandingTildeInPath + guard !expanded.hasPrefix("/") else { return expanded } + return URL(fileURLWithPath: FileManager.default.currentDirectoryPath) + .appendingPathComponent(expanded) + .standardizedFileURL + .path + } +} diff --git a/CLI/CMUXCLI+TaskHelp.swift b/CLI/CMUXCLI+TaskHelp.swift index 66e4cce231b4..7b0ef0c784be 100644 --- a/CLI/CMUXCLI+TaskHelp.swift +++ b/CLI/CMUXCLI+TaskHelp.swift @@ -274,6 +274,7 @@ extension CMUXCLI { current-workspace [--window ] \(Self.readSelectionUsageLine) \(Self.readScreenUsageLine) + \(Self.recordUsageLine) sidebar-state [--workspace ] [--window ] markdown [open] [--focus ] (open markdown file in formatted viewer panel with live reload) diff [patch-file|-] [--source ] [--cwd ] [--base ] [--focus ] [--no-focus] [--title ] [--layout ] [--font-size ] (open patch input or git source in a browser split) diff --git a/CLI/CMUXCLI+WindowDispatch.swift b/CLI/CMUXCLI+WindowDispatch.swift index d4640b9322a6..d993c7cf6961 100644 --- a/CLI/CMUXCLI+WindowDispatch.swift +++ b/CLI/CMUXCLI+WindowDispatch.swift @@ -24,6 +24,11 @@ extension CMUXCLI { if normalizedCommand == "read-screen" || normalizedCommand == "read-selection" || normalizedCommand == "current" { return false } + // Recording films whatever is on screen; activating a window first + // would put the recording's own side effect in the clip. + if normalizedCommand == "record" { + return false + } if normalizedCommand == "rpc", commandArgs.first?.trimmingCharacters(in: .whitespacesAndNewlines).lowercased() == "surface.read_selection" { diff --git a/CLI/cmux.swift b/CLI/cmux.swift index 25f4fb68c584..94b2d9510c90 100644 --- a/CLI/cmux.swift +++ b/CLI/cmux.swift @@ -7462,6 +7462,14 @@ struct CMUXCLI { includeContextInPlainOutput: true ) + case "record": + try runRecordCommand( + commandArgs: commandArgs, + client: client, + jsonOutput: jsonOutput, + windowOverride: windowId + ) + case "read-screen": let selectionOnly = commandArgs.contains("--selection") if selectionOnly { @@ -20059,6 +20067,8 @@ struct CMUXCLI { return Self.readSelectionHelp case "read-screen": return Self.readScreenHelp + case "record": + return Self.recordHelp case "paste": return Self.pasteHelp case "send": diff --git a/Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Wire/ControlCommandExecutionPolicy.swift b/Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Wire/ControlCommandExecutionPolicy.swift index 4e73bdd3fefe..76720085af9f 100644 --- a/Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Wire/ControlCommandExecutionPolicy.swift +++ b/Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Wire/ControlCommandExecutionPolicy.swift @@ -201,6 +201,14 @@ public enum ControlCommandExecutionPolicy: Sendable, Equatable { // compositor before falling back to AppKit. Keep that wait on the // socket worker so WebKit-backed panels can render on the main actor. "debug.window.screenshot", + // Window recording samples ScreenCaptureKit on a schedule for as long + // as the clip lasts. The sampling loop must never own the main actor: + // the window it is filming has to keep drawing. + "window.record.start", + "window.record.stop", + "window.record.status", + "window.record.note", + "window.record.list", // debug.sidebar.simulate_drag intentionally runs on the socket worker // so its Thread.sleep between drag-state ticks doesn't block the main // actor (which still owns the SidebarDragState mutations via diff --git a/Packages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/ControlCommandExecutionPolicyTests.swift b/Packages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/ControlCommandExecutionPolicyTests.swift index d126aadaffe5..d7766b3630b5 100644 --- a/Packages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/ControlCommandExecutionPolicyTests.swift +++ b/Packages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/ControlCommandExecutionPolicyTests.swift @@ -124,6 +124,20 @@ struct ControlCommandExecutionPolicyTests { ) } + @Test func windowRecordingRunsOnTheWorkerAndIsNotMainThreadCallable() { + // A recording samples the window for as long as the clip lasts, so its + // verbs must never be callable inline on the main thread: the window + // being filmed has to keep drawing while the sampler runs. + for method in [ + "window.record.start", "window.record.stop", "window.record.status", + "window.record.note", "window.record.list", + ] { + let policy = ControlCommandExecutionPolicy(forMethod: method) + #expect(policy == .socketWorker(mainThreadCallable: false), "\(method)") + #expect(policy.runsOnSocketWorker, "\(method)") + } + } + @Test func v2ResolutionReadsRunOnTheWorkerAndAreMainThreadCallable() { // Tranche D (issue #5757): the implicit handle-normalization reads. // One controlResolveOnMain hop (refresh + witness + ref minting), diff --git a/Packages/macOS/CmuxRemoteWorkspace/Tests/CmuxRemoteWorkspaceTests/RemoteRelayCoreRPCPolicyTests.swift b/Packages/macOS/CmuxRemoteWorkspace/Tests/CmuxRemoteWorkspaceTests/RemoteRelayCoreRPCPolicyTests.swift index 707b63f4c86c..403c7b112302 100644 --- a/Packages/macOS/CmuxRemoteWorkspace/Tests/CmuxRemoteWorkspaceTests/RemoteRelayCoreRPCPolicyTests.swift +++ b/Packages/macOS/CmuxRemoteWorkspace/Tests/CmuxRemoteWorkspaceTests/RemoteRelayCoreRPCPolicyTests.swift @@ -86,6 +86,20 @@ struct RemoteRelayCoreRPCPolicyTests { #expect(decision(method, [:]) != .allowed) } + @Test("window recording is never reachable from a relay", arguments: [ + "window.record.start", "window.record.stop", "window.record.status", + "window.record.note", "window.record.list" + ]) + func windowRecordingIsDenied(method: String) { + // Recording films the local screen. A relay peer is authorized for the + // objects of one workspace, not for whatever the local user has on + // display, so these verbs stay off the allowlist entirely. + #expect(decision(method, [:]) == .denied(code: "remote_relay_method_denied", + message: "Relay method is not permitted")) + #expect(decision(method, ["workspace_id": owner.uuidString]) != .allowed) + #expect(RemoteRelayCommandPolicy().permittedMethods(from: [method]).isEmpty) + } + @Test("remote reconnect remains withheld until its surface execution is scoped") func reconnectIsDenied() { #expect(decision("workspace.remote.reconnect", [ diff --git a/Resources/Localizable.xcstrings b/Resources/Localizable.xcstrings index 79f82380c4bc..0db518cbb891 100644 --- a/Resources/Localizable.xcstrings +++ b/Resources/Localizable.xcstrings @@ -566153,6 +566153,318 @@ } } } + }, + "cli.help.record": { + "extractionState": "manual", + "localizations": { + "en": { + "stringUnit": { + "state": "translated", + "value": "Usage: cmux record [start] [flags]\n cmux record stop|status [--id ]\n cmux record note [--id ] \n cmux record list\n\nRecord a cmux window to an mp4 or gif, for a pull request or a bug\nreport. Only cmux's own windows can be recorded, and no Screen\nRecording permission is involved.\n\nFlags for start:\n --format mp4|gif Output format (default: mp4)\n --gif Shorthand for --format gif\n --fps <1-30> Frames per second (default: 12 for mp4, 8 for gif)\n --max-seconds Stop by itself after n seconds, 0.5-120 (default: 15)\n --scale <0.1-1> Scale the output (default: 1 for mp4, 0.5 for gif)\n --max-width <64-4096> Cap the output width, keeping the aspect ratio\n --region Record part of the window, in window points\n --out Write here instead of a temporary directory\n --label Name the file\n --no-captions Do not draw `record note` captions into the clip\n --window Window to record (default: the frontmost one)\n\nOutput: ` `, or the full response with --json.\n\nA recording stops itself at --max-seconds, so a clip is never left\nrunning by an agent that goes away. One recording at a time.\n\nExample:\n cmux record start --gif --max-seconds 8 --label sidebar-drag\n cmux record note \"dragging the workspace\"\n cmux record stop" + } + }, + "ar": { + "stringUnit": { + "state": "translated", + "value": "الاستخدام: cmux record [start] [flags]\n cmux record stop|status [--id ]\n cmux record note [--id ] \n cmux record list\n\nتسجيل نافذة cmux بصيغة mp4 أو gif، من أجل طلب سحب أو تقرير خطأ.\nلا يمكن تسجيل سوى نوافذ cmux نفسها،\nولا يتطلب ذلك إذن تسجيل الشاشة.\n\nخيارات start:\n --format mp4|gif صيغة الإخراج (الافتراضي: mp4)\n --gif اختصار لـ --format gif\n --fps <1-30> الإطارات في الثانية (الافتراضي: 12 لـ mp4، 8 لـ gif)\n --max-seconds يتوقف من تلقاء نفسه بعد n ثانية، 0.5-120 (الافتراضي: 15)\n --scale <0.1-1> تحجيم الإخراج (الافتراضي: 1 لـ mp4، 0.5 لـ gif)\n --max-width <64-4096> تحديد أقصى عرض للإخراج مع الحفاظ على نسبة الأبعاد\n --region تسجيل جزء من النافذة، بوحدات نقاط النافذة\n --out الكتابة هنا بدلاً من دليل مؤقت\n --label تسمية الملف\n --no-captions عدم رسم تعليقات `record note` داخل المقطع\n --window النافذة المراد تسجيلها (الافتراضي: النافذة الأمامية)\n\nالإخراج: ` `، أو الاستجابة الكاملة مع --json.\n\nيتوقف التسجيل من تلقاء نفسه عند --max-seconds، حتى لا يترك وكيل غادر\nمقطعًا قيد التسجيل. تسجيل واحد في كل مرة.\n\nمثال:\n cmux record start --gif --max-seconds 8 --label sidebar-drag\n cmux record note \"dragging the workspace\"\n cmux record stop" + } + }, + "de": { + "stringUnit": { + "state": "translated", + "value": "Verwendung: cmux record [start] [Flags]\n cmux record stop|status [--id ]\n cmux record note [--id ] \n cmux record list\n\nEin cmux-Fenster als mp4 oder gif aufzeichnen, für einen Pull Request\noder einen Fehlerbericht. Nur cmux-eigene Fenster können aufgezeichnet\nwerden, und es ist keine Bildschirmaufnahme-Berechtigung beteiligt.\n\nFlags für start:\n --format mp4|gif Ausgabeformat (Standard: mp4)\n --gif Kurzform für --format gif\n --fps <1-30> Bilder pro Sekunde (Standard: 12 für mp4, 8 für gif)\n --max-seconds Nach n Sekunden selbst beenden, 0.5-120 (Standard: 15)\n --scale <0.1-1> Ausgabe skalieren (Standard: 1 für mp4, 0.5 für gif)\n --max-width <64-4096> Ausgabebreite begrenzen, Seitenverhältnis bleibt\n --region Teil des Fensters aufzeichnen, in Fensterpunkten\n --out Hierhin schreiben statt in ein temporäres Verzeichnis\n --label Datei benennen\n --no-captions Keine `record note`-Untertitel in den Clip zeichnen\n --window Fenster zum Aufzeichnen (Standard: das vorderste)\n\nAusgabe: ` `, oder mit --json die vollständige Antwort.\n\nEine Aufzeichnung beendet sich bei --max-seconds selbst, damit ein Agent, der\nverschwindet, keinen laufenden Clip hinterlässt. Eine Aufzeichnung zur Zeit.\n\nBeispiel:\n cmux record start --gif --max-seconds 8 --label sidebar-drag\n cmux record note \"dragging the workspace\"\n cmux record stop" + } + }, + "es": { + "stringUnit": { + "state": "translated", + "value": "Uso: cmux record [start] [flags]\n cmux record stop|status [--id ]\n cmux record note [--id ] \n cmux record list\n\nGraba una ventana de cmux en mp4 o gif, para un pull request o un informe\nde error. Solo se pueden grabar las ventanas propias de cmux, y no\ninterviene ningún permiso de grabación de pantalla.\n\nFlags para start:\n --format mp4|gif Formato de salida (predeterminado: mp4)\n --gif Atajo para --format gif\n --fps <1-30> Fotogramas por segundo (predeterminado: 12 para mp4, 8 para gif)\n --max-seconds Se detiene solo tras n segundos, 0.5-120 (predeterminado: 15)\n --scale <0.1-1> Escala la salida (predeterminado: 1 para mp4, 0.5 para gif)\n --max-width <64-4096> Limita el ancho de salida manteniendo la proporción\n --region Graba parte de la ventana, en puntos de ventana\n --out Escribe aquí en lugar de en un directorio temporal\n --label Da nombre al archivo\n --no-captions No dibujar los subtítulos de `record note` en el clip\n --window Ventana que se graba (predeterminado: la del frente)\n\nSalida: ` `, o la respuesta completa con --json.\n\nUna grabación se detiene sola en --max-seconds, para que un agente que\ndesaparece nunca deje un clip en marcha. Una grabación a la vez.\n\nEjemplo:\n cmux record start --gif --max-seconds 8 --label sidebar-drag\n cmux record note \"dragging the workspace\"\n cmux record stop" + } + }, + "fr": { + "stringUnit": { + "state": "translated", + "value": "Utilisation : cmux record [start] [flags]\n cmux record stop|status [--id ]\n cmux record note [--id ] \n cmux record list\n\nEnregistrer une fenêtre cmux en mp4 ou gif, pour une pull request ou un\nrapport de bug. Seules les fenêtres de cmux peuvent être enregistrées, et\naucune autorisation d'enregistrement de l'écran n'intervient.\n\nFlags pour start :\n --format mp4|gif Format de sortie (par défaut : mp4)\n --gif Raccourci pour --format gif\n --fps <1-30> Images par seconde (par défaut : 12 pour mp4, 8 pour gif)\n --max-seconds S'arrête seul après n secondes, 0.5-120 (par défaut : 15)\n --scale <0.1-1> Mettre la sortie à l'échelle (par défaut : 1 pour mp4, 0.5 pour gif)\n --max-width <64-4096> Limiter la largeur de sortie en gardant les proportions\n --region N'enregistrer qu'une partie de la fenêtre, en points fenêtre\n --out Écrire ici au lieu d'un répertoire temporaire\n --label Nommer le fichier\n --no-captions Ne pas dessiner les légendes `record note` dans le clip\n --window Fenêtre à enregistrer (par défaut : celle au premier plan)\n\nSortie : ` `, ou la réponse complète avec --json.\n\nUn enregistrement s'arrête de lui-même à --max-seconds, afin qu'un agent qui\ndisparaît ne laisse jamais un clip en cours. Un seul enregistrement à la fois.\n\nExemple :\n cmux record start --gif --max-seconds 8 --label sidebar-drag\n cmux record note \"dragging the workspace\"\n cmux record stop" + } + }, + "ja": { + "stringUnit": { + "state": "translated", + "value": "使用方法: cmux record [start] [flags]\n cmux record stop|status [--id ]\n cmux record note [--id ] \n cmux record list\n\nプルリクエストやバグ報告のために、cmux のウィンドウを mp4 または gif\nで録画します。録画できるのは cmux 自身のウィンドウだけで、画面収録の\n権限は必要ありません。\n\nstart のフラグ:\n --format mp4|gif 出力形式（既定: mp4）\n --gif --format gif の短縮形\n --fps <1-30> 1 秒あたりのフレーム数（既定: mp4 は 12、gif は 8）\n --max-seconds n 秒後に自動停止、0.5-120（既定: 15）\n --scale <0.1-1> 出力を拡大縮小（既定: mp4 は 1、gif は 0.5）\n --max-width <64-4096> 縦横比を保ったまま出力の幅を制限\n --region ウィンドウの一部だけを録画（ウィンドウポイント単位）\n --out 一時ディレクトリではなくこのパスに書き出す\n --label ファイルに名前を付ける\n --no-captions `record note` のキャプションをクリップに描画しない\n --window 録画するウィンドウ（既定: 最前面のウィンドウ）\n\n出力: ` `、または --json で完全なレスポンス。\n\n録画は --max-seconds で自動停止するので、いなくなったエージェントが\n録画を実行したままにすることはありません。録画は同時に 1 つだけです。\n\n例:\n cmux record start --gif --max-seconds 8 --label sidebar-drag\n cmux record note \"dragging the workspace\"\n cmux record stop" + } + }, + "ko": { + "stringUnit": { + "state": "translated", + "value": "사용법: cmux record [start] [flags]\n cmux record stop|status [--id ]\n cmux record note [--id ] \n cmux record list\n\n풀 리퀘스트나 버그 리포트에 쓰려고 cmux 창을 mp4 또는 gif로\n녹화합니다. cmux 자신의 창만 녹화할 수 있고, 화면 기록 권한은\n필요하지 않습니다.\n\nstart 플래그:\n --format mp4|gif 출력 형식 (기본값: mp4)\n --gif --format gif의 단축형\n --fps <1-30> 초당 프레임 수 (기본값: mp4는 12, gif는 8)\n --max-seconds n초 후 스스로 정지, 0.5-120 (기본값: 15)\n --scale <0.1-1> 출력 크기 조정 (기본값: mp4는 1, gif는 0.5)\n --max-width <64-4096> 가로 세로 비율을 유지하며 출력 너비 제한\n --region 창의 일부만 녹화, 창 포인트 단위\n --out 임시 디렉터리 대신 이 경로에 기록\n --label 파일 이름 지정\n --no-captions `record note` 캡션을 클립에 그리지 않음\n --window 녹화할 창 (기본값: 맨 앞 창)\n\n출력: ` `, 또는 --json으로 전체 응답.\n\n녹화는 --max-seconds에서 스스로 멈추므로 사라진 에이전트가 녹화를 계속\n남겨 두는 일은 없습니다. 한 번에 하나의 녹화만 가능합니다.\n\n예:\n cmux record start --gif --max-seconds 8 --label sidebar-drag\n cmux record note \"dragging the workspace\"\n cmux record stop" + } + }, + "zh-Hans": { + "stringUnit": { + "state": "translated", + "value": "使用方法: cmux record [start] [flags]\n cmux record stop|status [--id ]\n cmux record note [--id ] \n cmux record list\n\n将 cmux 窗口录制为 mp4 或 gif，用于 pull request 或错误报告。\n只能录制 cmux 自己的窗口，\n而且不需要任何屏幕录制权限。\n\nstart 的标志:\n --format mp4|gif 输出格式（默认: mp4）\n --gif --format gif 的简写\n --fps <1-30> 每秒帧数（默认: mp4 为 12，gif 为 8）\n --max-seconds n 秒后自行停止，0.5-120（默认: 15）\n --scale <0.1-1> 缩放输出（默认: mp4 为 1，gif 为 0.5）\n --max-width <64-4096> 限制输出宽度并保持宽高比\n --region 只录制窗口的一部分，以窗口点为单位\n --out 写入此处而非临时目录\n --label 为文件命名\n --no-captions 不要把 `record note` 字幕画进视频\n --window 要录制的窗口（默认: 最前面的窗口）\n\n输出: ` `，或用 --json 获取完整响应。\n\n录制会在 --max-seconds 时自行停止，因此离开的代理不会留下仍在录制的\n视频。同一时间只能有一份录制。\n\n示例:\n cmux record start --gif --max-seconds 8 --label sidebar-drag\n cmux record note \"dragging the workspace\"\n cmux record stop" + } + }, + "zh-Hant": { + "stringUnit": { + "state": "translated", + "value": "使用方法: cmux record [start] [flags]\n cmux record stop|status [--id ]\n cmux record note [--id ] \n cmux record list\n\n將 cmux 視窗錄製成 mp4 或 gif，用於 pull request 或錯誤報告。\n只能錄製 cmux 自己的視窗，\n而且不需要任何螢幕錄製權限。\n\nstart 的旗標:\n --format mp4|gif 輸出格式（預設: mp4）\n --gif --format gif 的簡寫\n --fps <1-30> 每秒影格數（預設: mp4 為 12，gif 為 8）\n --max-seconds n 秒後自行停止，0.5-120（預設: 15）\n --scale <0.1-1> 縮放輸出（預設: mp4 為 1，gif 為 0.5）\n --max-width <64-4096> 限制輸出寬度並維持長寬比\n --region 只錄製視窗的一部分，以視窗點為單位\n --out 寫入此處而非暫存目錄\n --label 為檔案命名\n --no-captions 不要將 `record note` 字幕畫進影片\n --window 要錄製的視窗（預設: 最前面的視窗）\n\n輸出: ` `，或以 --json 取得完整回應。\n\n錄製會在 --max-seconds 時自行停止，因此離開的代理程式不會留下仍在\n錄製的影片。同一時間只能有一份錄製。\n\n範例:\n cmux record start --gif --max-seconds 8 --label sidebar-drag\n cmux record note \"dragging the workspace\"\n cmux record stop" + } + } + } + }, + "cli.record.error.formatConflict": { + "extractionState": "manual", + "localizations": { + "en": { + "stringUnit": { + "state": "translated", + "value": "record start: --gif conflicts with --format" + } + }, + "ar": { + "stringUnit": { + "state": "translated", + "value": "record start: لا يمكن دمج --gif مع --format" + } + }, + "de": { + "stringUnit": { + "state": "translated", + "value": "record start: --gif kann nicht mit --format kombiniert werden" + } + }, + "es": { + "stringUnit": { + "state": "translated", + "value": "record start: --gif no se puede combinar con --format" + } + }, + "fr": { + "stringUnit": { + "state": "translated", + "value": "record start: --gif ne peut pas être combiné avec --format" + } + }, + "ja": { + "stringUnit": { + "state": "translated", + "value": "record start: --gif は --format と併用できません" + } + }, + "ko": { + "stringUnit": { + "state": "translated", + "value": "record start: --gif은 --format과 함께 사용할 수 없습니다" + } + }, + "zh-Hans": { + "stringUnit": { + "state": "translated", + "value": "record start: --gif 不能与 --format 同用" + } + }, + "zh-Hant": { + "stringUnit": { + "state": "translated", + "value": "record start: --gif 無法與 --format 同時使用" + } + } + } + }, + "cli.record.error.noteText": { + "extractionState": "manual", + "localizations": { + "en": { + "stringUnit": { + "state": "translated", + "value": "record note: text is required" + } + }, + "ar": { + "stringUnit": { + "state": "translated", + "value": "record note: النص مطلوب" + } + }, + "de": { + "stringUnit": { + "state": "translated", + "value": "record note: Text ist erforderlich" + } + }, + "es": { + "stringUnit": { + "state": "translated", + "value": "record note: se requiere el texto" + } + }, + "fr": { + "stringUnit": { + "state": "translated", + "value": "record note : le texte est requis" + } + }, + "ja": { + "stringUnit": { + "state": "translated", + "value": "record note: テキストが必要です" + } + }, + "ko": { + "stringUnit": { + "state": "translated", + "value": "record note: 텍스트가 필요합니다" + } + }, + "zh-Hans": { + "stringUnit": { + "state": "translated", + "value": "record note: 必须提供文本" + } + }, + "zh-Hant": { + "stringUnit": { + "state": "translated", + "value": "record note: 必須提供文字" + } + } + } + }, + "cli.record.error.unexpectedArguments": { + "extractionState": "manual", + "localizations": { + "en": { + "stringUnit": { + "state": "translated", + "value": "record %@: unexpected arguments: %@" + } + }, + "ar": { + "stringUnit": { + "state": "translated", + "value": "record %@: وسائط غير متوقعة: %@" + } + }, + "de": { + "stringUnit": { + "state": "translated", + "value": "record %@: unerwartete Argumente: %@" + } + }, + "es": { + "stringUnit": { + "state": "translated", + "value": "record %@: argumentos inesperados: %@" + } + }, + "fr": { + "stringUnit": { + "state": "translated", + "value": "record %@ : arguments inattendus : %@" + } + }, + "ja": { + "stringUnit": { + "state": "translated", + "value": "record %@: 予期しない引数: %@" + } + }, + "ko": { + "stringUnit": { + "state": "translated", + "value": "record %@: 예기치 않은 인수: %@" + } + }, + "zh-Hans": { + "stringUnit": { + "state": "translated", + "value": "record %@: 多余的参数: %@" + } + }, + "zh-Hant": { + "stringUnit": { + "state": "translated", + "value": "record %@: 未預期的引數: %@" + } + } + } + }, + "cli.record.error.unknownSubcommand": { + "extractionState": "manual", + "localizations": { + "en": { + "stringUnit": { + "state": "translated", + "value": "record: unknown subcommand '%@' (start, stop, status, note, list)" + } + }, + "ar": { + "stringUnit": { + "state": "translated", + "value": "record: أمر فرعي غير معروف '%@' (start، stop، status، note، list)" + } + }, + "de": { + "stringUnit": { + "state": "translated", + "value": "record: unbekannter Unterbefehl „%@“ (start, stop, status, note, list)" + } + }, + "es": { + "stringUnit": { + "state": "translated", + "value": "record: subcomando desconocido '%@' (start, stop, status, note, list)" + } + }, + "fr": { + "stringUnit": { + "state": "translated", + "value": "record : sous-commande inconnue '%@' (start, stop, status, note, list)" + } + }, + "ja": { + "stringUnit": { + "state": "translated", + "value": "record: 不明なサブコマンド '%@' です（start、stop、status、note、list）" + } + }, + "ko": { + "stringUnit": { + "state": "translated", + "value": "record: 알 수 없는 하위 명령 '%@' (start, stop, status, note, list)" + } + }, + "zh-Hans": { + "stringUnit": { + "state": "translated", + "value": "record: 未知的子命令 '%@'（start、stop、status、note、list）" + } + }, + "zh-Hant": { + "stringUnit": { + "state": "translated", + "value": "record: 未知的子命令 '%@'（start、stop、status、note、list）" + } + } + } + }, + "cli.usage.record": { + "extractionState": "manual", + "localizations": { + "en": { + "stringUnit": { + "state": "translated", + "value": "record [start] [--format mp4|gif] [--gif] [--fps ] [--max-seconds ] [--scale ] [--max-width ] [--region ] [--out ] [--label ] [--no-captions] [--window ] | record stop|status [--id ] | record note [--id ] | record list" + } + }, + "ja": { + "stringUnit": { + "state": "translated", + "value": "record [start] [--format mp4|gif] [--gif] [--fps ] [--max-seconds ] [--scale ] [--max-width ] [--region ] [--out ] [--label ] [--no-captions] [--window ] | record stop|status [--id ] | record note [--id ] | record list" + } + } + } } }, "version": "1.0" diff --git a/Sources/TerminalController+WindowRecording.swift b/Sources/TerminalController+WindowRecording.swift new file mode 100644 index 000000000000..ff77e3fc6cdb --- /dev/null +++ b/Sources/TerminalController+WindowRecording.swift @@ -0,0 +1,189 @@ +import AppKit +import CmuxFoundation +import CoreGraphics +import Foundation + +extension TerminalController { + /// Socket-worker router for `window.record.*`. + /// + /// These block their worker thread while ScreenCaptureKit answers, so the + /// execution policy keeps them off the main actor (see + /// `ControlCommandExecutionPolicy`). + nonisolated func v2WindowRecordingCommandOnSocketWorker( + method: String, + params: [String: Any] + ) -> V2CallResult { + switch method { + case "window.record.start": + return v2WindowRecordStart(params: params) + case "window.record.stop": + return v2WindowRecordStop(params: params) + case "window.record.status": + return v2WindowRecordStatus(params: params) + case "window.record.note": + return v2WindowRecordNote(params: params) + case "window.record.list": + return v2WindowRecordList() + default: + return .err( + code: "unknown_method", + message: "Unknown method \(method)", + data: nil + ) + } + } + + private nonisolated func v2WindowRecordStart(params: [String: Any]) -> V2CallResult { + let request: WindowRecordingRequest + do { + request = try WindowRecordingRequest.make(params: params) + } catch let failure as WindowRecordingRequest.Failure { + return .err(code: "invalid_params", message: failure.message, data: nil) + } catch { + return .err(code: "invalid_params", message: error.localizedDescription, data: nil) + } + guard let windowID = recordingWindowID(handle: request.windowHandle) else { + return .err( + code: "not_found", + message: request.windowHandle.map { "Window \($0) is not available" } + ?? "No window available", + data: nil + ) + } + return awaitRecordingCall(timeout: 20) { + try await WindowRecordingRegistry.shared.start( + request: request, + windowID: windowID, + windowHandle: request.windowHandle + ) + } + } + + private nonisolated func v2WindowRecordStop(params: [String: Any]) -> V2CallResult { + let id = (params["id"] as? String)?.trimmingCharacters(in: .whitespacesAndNewlines) + return awaitRecordingCall(timeout: 30) { + try await WindowRecordingRegistry.shared.stop(id: id?.isEmpty == false ? id : nil) + } + } + + private nonisolated func v2WindowRecordStatus(params: [String: Any]) -> V2CallResult { + let id = (params["id"] as? String)?.trimmingCharacters(in: .whitespacesAndNewlines) + return awaitRecordingCall(timeout: 10) { + try await WindowRecordingRegistry.shared.status(id: id?.isEmpty == false ? id : nil) + } + } + + private nonisolated func v2WindowRecordNote(params: [String: Any]) -> V2CallResult { + let id = (params["id"] as? String)?.trimmingCharacters(in: .whitespacesAndNewlines) + let text = (params["text"] as? String)? + .trimmingCharacters(in: .whitespacesAndNewlines) ?? "" + guard !text.isEmpty else { + return .err(code: "invalid_params", message: "text is required", data: nil) + } + return awaitRecordingCall(timeout: 10) { + try await WindowRecordingRegistry.shared.note( + text: text, + id: id?.isEmpty == false ? id : nil + ) + } + } + + private nonisolated func v2WindowRecordList() -> V2CallResult { + let statuses: [WindowRecordingStatus]? = socketAwaitCallback(timeout: 10) { completion in + Task { + completion(await WindowRecordingRegistry.shared.recentStatuses()) + } + } + guard let statuses else { + return .err(code: "timeout", message: "window.record.list timed out", data: nil) + } + return .ok(["recordings": statuses.map { $0.jsonObject }]) + } + + /// Resolves the window to film: the one named by `window`, else the + /// frontmost cmux window. + private nonisolated func recordingWindowID(handle: String?) -> CGWindowID? { + v2MainSync { + let eligibleWindows = WindowRecordingWindowSelection.eligibleWindows(in: NSApp) + let selected: NSWindow? + if let handle { + guard let windowId = UUID(uuidString: handle), + let requested = AppDelegate.shared?.mainWindow(for: windowId), + eligibleWindows.contains(where: { $0 === requested }) else { + return nil + } + selected = requested + } else { + selected = WindowRecordingWindowSelection.select( + eligibleWindows: eligibleWindows, + keyWindow: NSApp.keyWindow, + mainWindow: NSApp.mainWindow, + terminalWindow: self.tabManager?.window + ) + } + guard let selected else { return nil } + return WindowRecordingWindowSelection.windowID( + fromWindowNumber: selected.windowNumber + ) + } + } + + private nonisolated func awaitRecordingCall( + timeout: TimeInterval, + _ work: @escaping () async throws -> WindowRecordingStatus + ) -> V2CallResult { + let outcome: Result? = socketAwaitCallback( + timeout: timeout + ) { completion in + Task { + do { + completion(.success(try await work())) + } catch { + completion(.failure(error)) + } + } + } + guard let outcome else { + return .err( + code: "timeout", + message: "recording command timed out after \(Int(timeout)) seconds", + data: nil + ) + } + switch outcome { + case let .success(status): + return .ok(status.jsonObject) + case let .failure(error): + return .err( + code: Self.recordingErrorCode(for: error), + message: error.localizedDescription, + data: nil + ) + } + } + + private nonisolated static func recordingErrorCode(for error: Error) -> String { + if let failure = error as? WindowRecordingRegistry.Failure { + switch failure { + case .busy: + return "conflict" + case .noRecording, .unknownRecording, .alreadyStopped: + return "not_found" + } + } + if let failure = error as? WindowRecordingSessionError { + switch failure { + case .unsupportedSystem: + return "unsupported" + case .windowGone: + return "not_found" + case .captureFailed, .composeFailed, .alreadyFinished: + return "internal_error" + } + } + if error is WindowRecordingFrameGeometry.Failure { + return "invalid_params" + } + return "internal_error" + } +} diff --git a/Sources/TerminalController.swift b/Sources/TerminalController.swift index 098a1de8b8eb..d07b0fa3737a 100644 --- a/Sources/TerminalController.swift +++ b/Sources/TerminalController.swift @@ -1752,6 +1752,15 @@ class TerminalController { ) } return v2Ok(id: request.id, result: ["completed": true]) + case "window.record.start", "window.record.stop", "window.record.status", + "window.record.note", "window.record.list": + return v2Result( + id: request.id, + v2WindowRecordingCommandOnSocketWorker( + method: request.method, + params: request.params + ) + ) case "browser.download.list", "browser.download.wait": return v2Result(id: request.id, request.method == "browser.download.list" ? v2BrowserDownloadListOnSocketWorker(params: request.params) : v2BrowserDownloadWaitOnSocketWorker(params: request.params)) case "browser.navigate", "browser.back", "browser.forward", "browser.reload", diff --git a/Sources/WindowRecordingFrameComposer.swift b/Sources/WindowRecordingFrameComposer.swift new file mode 100644 index 000000000000..a8c9c0522b62 --- /dev/null +++ b/Sources/WindowRecordingFrameComposer.swift @@ -0,0 +1,137 @@ +import CmuxFoundation +import CoreGraphics +import CoreText +import Foundation + +/// Turns one captured window image into one encoded frame: crop, fit to the +/// clip's frame size, and draw the caption an agent asked for. +/// +/// CoreText rather than AppKit text drawing, because frames are composed on the +/// recorder's own executor rather than the main actor. +enum WindowRecordingFrameComposer { + static func compose( + source: CGImage, + geometry: WindowRecordingFrameGeometry, + caption: String? + ) -> CGImage? { + guard geometry.outputWidth > 0, geometry.outputHeight > 0 else { return nil } + let cropped = crop(source, to: geometry) ?? source + guard let context = CGContext( + data: nil, + width: geometry.outputWidth, + height: geometry.outputHeight, + bitsPerComponent: 8, + bytesPerRow: 0, + space: CGColorSpace(name: CGColorSpace.sRGB) ?? CGColorSpaceCreateDeviceRGB(), + bitmapInfo: CGImageAlphaInfo.noneSkipFirst.rawValue + | CGBitmapInfo.byteOrder32Little.rawValue + ) else { + return nil + } + context.interpolationQuality = .high + let bounds = CGRect( + x: 0, + y: 0, + width: CGFloat(geometry.outputWidth), + height: CGFloat(geometry.outputHeight) + ) + context.setFillColor(gray: 0, alpha: 1) + context.fill(bounds) + context.draw(cropped, in: Self.fit(CGSize( + width: CGFloat(cropped.width), + height: CGFloat(cropped.height) + ), in: bounds)) + if let caption, !caption.isEmpty { + drawCaption(caption, in: context, bounds: bounds) + } + return context.makeImage() + } + + private static func crop( + _ source: CGImage, + to geometry: WindowRecordingFrameGeometry + ) -> CGImage? { + guard !geometry.cropsNothing + || geometry.cropWidth != source.width + || geometry.cropHeight != source.height else { + return source + } + let rect = CGRect( + x: CGFloat(geometry.cropX), + y: CGFloat(geometry.cropY), + width: CGFloat(geometry.cropWidth), + height: CGFloat(geometry.cropHeight) + ).intersection(CGRect( + x: 0, + y: 0, + width: CGFloat(source.width), + height: CGFloat(source.height) + )) + guard !rect.isEmpty else { return nil } + return source.cropping(to: rect) + } + + /// Centers the captured content without distorting it. The two sizes match + /// for the whole clip unless the window was resized while recording. + static func fit(_ content: CGSize, in bounds: CGRect) -> CGRect { + guard content.width > 0, content.height > 0 else { return bounds } + let scale = min(bounds.width / content.width, bounds.height / content.height) + let size = CGSize(width: content.width * scale, height: content.height * scale) + return CGRect( + x: bounds.minX + ((bounds.width - size.width) / 2).rounded(), + y: bounds.minY + ((bounds.height - size.height) / 2).rounded(), + width: size.width, + height: size.height + ) + } + + private static func drawCaption( + _ caption: String, + in context: CGContext, + bounds: CGRect + ) { + let fontSize = max(12, bounds.height / 42) + let margin = max(8, bounds.height / 60) + let padding = max(5, bounds.height / 110) + let font = CTFontCreateUIFontForLanguage(.system, fontSize, nil) + ?? CTFontCreateWithName("Helvetica" as CFString, fontSize, nil) + let attributes: [NSAttributedString.Key: Any] = [ + NSAttributedString.Key(kCTFontAttributeName as String): font, + NSAttributedString.Key(kCTForegroundColorAttributeName as String): + CGColor(gray: 1, alpha: 1), + ] + let available = bounds.width - (margin * 2) - (padding * 2) + guard available > fontSize else { return } + var line = CTLineCreateWithAttributedString( + NSAttributedString(string: caption, attributes: attributes) + ) + if CTLineGetTypographicBounds(line, nil, nil, nil) > Double(available) { + let token = CTLineCreateWithAttributedString( + NSAttributedString(string: "\u{2026}", attributes: attributes) + ) + line = CTLineCreateTruncatedLine(line, Double(available), .end, token) ?? line + } + var ascent: CGFloat = 0 + var descent: CGFloat = 0 + let textWidth = CGFloat(CTLineGetTypographicBounds(line, &ascent, &descent, nil)) + let box = CGRect( + x: margin, + y: margin, + width: min(textWidth + (padding * 2), bounds.width - (margin * 2)), + height: ascent + descent + (padding * 2) + ) + context.saveGState() + context.setFillColor(gray: 0, alpha: 0.66) + context.addPath(CGPath( + roundedRect: box, + cornerWidth: padding, + cornerHeight: padding, + transform: nil + )) + context.fillPath() + context.textMatrix = .identity + context.textPosition = CGPoint(x: box.minX + padding, y: box.minY + padding + descent) + CTLineDraw(line, context) + context.restoreGState() + } +} diff --git a/Sources/WindowRecordingFrameWriter.swift b/Sources/WindowRecordingFrameWriter.swift new file mode 100644 index 000000000000..a6ffa92f5f41 --- /dev/null +++ b/Sources/WindowRecordingFrameWriter.swift @@ -0,0 +1,285 @@ +import AVFoundation +import CmuxFoundation +import CoreGraphics +import CoreVideo +import Foundation +import ImageIO +import UniformTypeIdentifiers + +enum WindowRecordingWriterError: Error, Equatable, LocalizedError { + case setup(String) + case frame(String) + case finish(String) + case noFrames + + var errorDescription: String? { + switch self { + case let .setup(detail): "recorder setup failed: \(detail)" + case let .frame(detail): "frame write failed: \(detail)" + case let .finish(detail): "closing the recording failed: \(detail)" + case .noFrames: "the recording captured no frames" + } + } +} + +/// Writes one recording's frames to disk as they are captured. +/// +/// Frames are written straight through rather than collected: a 30 second clip +/// of a Retina window is a couple of gigabytes of uncompressed frames, which is +/// not something the app may hold in memory for an agent. +/// +/// Frames are timed by the moment they were captured rather than by their +/// index, so a clip plays back at the speed the window was really moving at +/// even when the sampler could not keep up with the requested rate. +protocol WindowRecordingFrameWriter: AnyObject { + func append(_ image: CGImage, atOffsetSeconds offsetSeconds: Double) async throws + func finish() async throws +} + +/// H.264 in an mp4 container, through AVFoundation. +final class WindowRecordingMP4Writer: WindowRecordingFrameWriter { + private let writer: AVAssetWriter + private let input: AVAssetWriterInput + private let adaptor: AVAssetWriterInputPixelBufferAdaptor + private let width: Int + private let height: Int + private let framesPerSecond: Int + private var didStart = false + private var frameCount = 0 + // One timescale tick before zero, so the first frame may present at zero. + private var lastOffsetSeconds = -1.0 / 600 + + init(url: URL, width: Int, height: Int, framesPerSecond: Int) throws { + self.width = width + self.height = height + self.framesPerSecond = framesPerSecond + do { + writer = try AVAssetWriter(outputURL: url, fileType: .mp4) + } catch { + throw WindowRecordingWriterError.setup(error.localizedDescription) + } + input = AVAssetWriterInput( + mediaType: .video, + outputSettings: [ + AVVideoCodecKey: AVVideoCodecType.h264, + AVVideoWidthKey: width, + AVVideoHeightKey: height, + AVVideoCompressionPropertiesKey: [ + AVVideoAverageBitRateKey: Self.bitRate( + width: width, + height: height, + framesPerSecond: framesPerSecond + ), + AVVideoMaxKeyFrameIntervalKey: max(1, framesPerSecond * 2), + ], + ] + ) + // Frames arrive as fast as the sampler produces them, not on a live + // clock, so the writer may take its time compressing them. + input.expectsMediaDataInRealTime = false + adaptor = AVAssetWriterInputPixelBufferAdaptor( + assetWriterInput: input, + sourcePixelBufferAttributes: [ + kCVPixelBufferPixelFormatTypeKey as String: kCVPixelFormatType_32BGRA, + kCVPixelBufferWidthKey as String: width, + kCVPixelBufferHeightKey as String: height, + kCVPixelBufferCGImageCompatibilityKey as String: true, + kCVPixelBufferCGBitmapContextCompatibilityKey as String: true, + ] + ) + guard writer.canAdd(input) else { + throw WindowRecordingWriterError.setup("the video input was rejected") + } + writer.add(input) + } + + /// Roughly 0.1 bits per pixel per frame, clamped to a range that keeps a + /// terminal clip readable without making a pull request attachment huge. + private static func bitRate(width: Int, height: Int, framesPerSecond: Int) -> Int { + let pixels = Double(width * height) + let estimate = pixels * Double(framesPerSecond) * 0.1 + return Int(min(max(estimate, 600_000), 12_000_000)) + } + + private static let timescale: CMTimeScale = 600 + + func append(_ image: CGImage, atOffsetSeconds offsetSeconds: Double) async throws { + if !didStart { + guard writer.startWriting() else { + throw WindowRecordingWriterError.setup( + writer.error?.localizedDescription ?? "the writer refused to start" + ) + } + writer.startSession(atSourceTime: .zero) + didStart = true + } + try await waitUntilReady() + guard let buffer = makePixelBuffer(from: image) else { + throw WindowRecordingWriterError.frame("could not allocate a pixel buffer") + } + // Presentation times have to keep increasing even if two samples land + // in the same 1/600 second. + let offset = max(offsetSeconds, lastOffsetSeconds + (1 / Double(Self.timescale))) + lastOffsetSeconds = offset + guard adaptor.append( + buffer, + withPresentationTime: CMTime(seconds: offset, preferredTimescale: Self.timescale) + ) else { + throw WindowRecordingWriterError.frame( + writer.error?.localizedDescription ?? "the writer rejected a frame" + ) + } + frameCount += 1 + } + + /// The compressor applies backpressure between frames; wait for it rather + /// than dropping frames, and give up if it stalls for good. + private func waitUntilReady() async throws { + let deadline = ProcessInfo.processInfo.systemUptime + 5 + while !input.isReadyForMoreMediaData { + if writer.status == .failed { + throw WindowRecordingWriterError.frame( + writer.error?.localizedDescription ?? "the writer failed" + ) + } + guard ProcessInfo.processInfo.systemUptime < deadline else { + throw WindowRecordingWriterError.frame("the video compressor stalled") + } + try? await Task.sleep(nanoseconds: 2_000_000) + } + } + + private func makePixelBuffer(from image: CGImage) -> CVPixelBuffer? { + var buffer: CVPixelBuffer? + if let pool = adaptor.pixelBufferPool { + CVPixelBufferPoolCreatePixelBuffer(nil, pool, &buffer) + } + if buffer == nil { + CVPixelBufferCreate( + nil, + width, + height, + kCVPixelFormatType_32BGRA, + [ + kCVPixelBufferCGImageCompatibilityKey: true, + kCVPixelBufferCGBitmapContextCompatibilityKey: true, + ] as CFDictionary, + &buffer + ) + } + guard let pixelBuffer = buffer else { return nil } + CVPixelBufferLockBaseAddress(pixelBuffer, []) + defer { CVPixelBufferUnlockBaseAddress(pixelBuffer, []) } + guard let base = CVPixelBufferGetBaseAddress(pixelBuffer), + let context = CGContext( + data: base, + width: width, + height: height, + bitsPerComponent: 8, + bytesPerRow: CVPixelBufferGetBytesPerRow(pixelBuffer), + space: CGColorSpaceCreateDeviceRGB(), + bitmapInfo: CGImageAlphaInfo.noneSkipFirst.rawValue + | CGBitmapInfo.byteOrder32Little.rawValue + ) else { + return nil + } + context.draw( + image, + in: CGRect(x: 0, y: 0, width: CGFloat(width), height: CGFloat(height)) + ) + return pixelBuffer + } + + func finish() async throws { + guard didStart, frameCount > 0 else { + writer.cancelWriting() + throw WindowRecordingWriterError.noFrames + } + input.markAsFinished() + await withCheckedContinuation { continuation in + writer.finishWriting { + continuation.resume() + } + } + if writer.status != .completed { + throw WindowRecordingWriterError.finish( + writer.error?.localizedDescription ?? "the writer did not complete" + ) + } + } +} + +/// An animated gif, through ImageIO. +/// +/// A gif carries a delay per frame rather than a frame rate, and a frame's delay +/// is only known once the next frame has been captured, so one frame is always +/// held back. +final class WindowRecordingGIFWriter: WindowRecordingFrameWriter { + private let destination: CGImageDestination + private let nominalDelaySeconds: Double + private var pending: (image: CGImage, offsetSeconds: Double)? + private var frameCount = 0 + + /// Browsers clamp very short gif delays to a tenth of a second; staying at + /// or above two hundredths keeps playback predictable. + private static let delayRange = 0.02...10.0 + + init(url: URL, frameBudget: Int, framesPerSecond: Int) throws { + guard let destination = CGImageDestinationCreateWithURL( + url as CFURL, + UTType.gif.identifier as CFString, + frameBudget, + nil + ) else { + throw WindowRecordingWriterError.setup("could not create the gif at \(url.path)") + } + self.destination = destination + nominalDelaySeconds = 1 / Double(max(1, framesPerSecond)) + CGImageDestinationSetProperties( + destination, + [ + kCGImagePropertyGIFDictionary: [ + kCGImagePropertyGIFLoopCount: 0, + ], + ] as CFDictionary + ) + } + + func append(_ image: CGImage, atOffsetSeconds offsetSeconds: Double) async throws { + if let pending { + add(pending.image, delaySeconds: offsetSeconds - pending.offsetSeconds) + } + pending = (image, offsetSeconds) + } + + private func add(_ image: CGImage, delaySeconds: Double) { + let delay = min( + max(delaySeconds.isFinite ? delaySeconds : nominalDelaySeconds, Self.delayRange.lowerBound), + Self.delayRange.upperBound + ) + CGImageDestinationAddImage( + destination, + image, + [ + kCGImagePropertyGIFDictionary: [ + kCGImagePropertyGIFDelayTime: delay, + kCGImagePropertyGIFUnclampedDelayTime: delay, + ], + ] as CFDictionary + ) + frameCount += 1 + } + + func finish() async throws { + if let pending { + add(pending.image, delaySeconds: nominalDelaySeconds) + self.pending = nil + } + guard frameCount > 0 else { + throw WindowRecordingWriterError.noFrames + } + guard CGImageDestinationFinalize(destination) else { + throw WindowRecordingWriterError.finish("the gif could not be finalized") + } + } +} diff --git a/Sources/WindowRecordingRegistry.swift b/Sources/WindowRecordingRegistry.swift new file mode 100644 index 000000000000..83d69131b500 --- /dev/null +++ b/Sources/WindowRecordingRegistry.swift @@ -0,0 +1,148 @@ +import CmuxFoundation +import CoreGraphics +import Foundation + +/// Owns the one recording cmux may have in flight, and remembers the last few +/// that finished so an agent can still ask where the file went. +/// +/// One at a time on purpose: two recorders sampling the same window would each +/// see the other's frame rate collapse, and the interesting failure ("I started +/// a second clip and both are half speed") is much harder to read than an error. +actor WindowRecordingRegistry { + static let shared = WindowRecordingRegistry() + + private static let historyLimit = 8 + + private var active: WindowRecordingSession? + private var history: [WindowRecordingStatus] = [] + + enum Failure: Error, LocalizedError { + case busy(String) + case noRecording + case unknownRecording(String) + case alreadyStopped(String) + + var errorDescription: String? { + switch self { + case let .busy(id): + "a recording is already running (\(id)); stop it first" + case .noRecording: + "no recording is running" + case let .unknownRecording(id): + "no recording with id \(id)" + case let .alreadyStopped(id): + "recording \(id) already stopped" + } + } + } + + func start( + request: WindowRecordingRequest, + windowID: CGWindowID, + windowHandle: String? + ) async throws -> WindowRecordingStatus { + await harvestFinishedRecording() + if let active { + throw Failure.busy(active.id) + } + let identifier = WindowRecordingOutputNaming.identifier(date: Date()) + let session = WindowRecordingSession( + id: identifier, + request: request, + outputURL: Self.outputURL(request: request, identifier: identifier), + windowID: windowID, + windowHandle: windowHandle + ) + try await session.start() + active = session + return await session.status + } + + func stop(id: String?) async throws -> WindowRecordingStatus { + await harvestFinishedRecording() + if let active, id == nil || id == active.id { + let status = await active.stop() + self.active = nil + remember(status) + return status + } + guard let id else { throw Failure.noRecording } + guard let remembered = history.last(where: { $0.id == id }) else { + throw Failure.unknownRecording(id) + } + return remembered + } + + func status(id: String?) async throws -> WindowRecordingStatus { + await harvestFinishedRecording() + if let active, id == nil || id == active.id { + return await active.status + } + guard let id else { + guard let latest = history.last else { throw Failure.noRecording } + return latest + } + guard let remembered = history.last(where: { $0.id == id }) else { + throw Failure.unknownRecording(id) + } + return remembered + } + + /// Adds a caption to the running clip. Captions are what make a clip + /// readable in a pull request: an agent labels each step as it takes it. + func note(text: String, id: String?) async throws -> WindowRecordingStatus { + await harvestFinishedRecording() + guard let active, id == nil || id == active.id else { + guard let id else { throw Failure.noRecording } + if history.contains(where: { $0.id == id }) { + throw Failure.alreadyStopped(id) + } + throw Failure.unknownRecording(id) + } + _ = await active.note(text) + return await active.status + } + + func recentStatuses() async -> [WindowRecordingStatus] { + await harvestFinishedRecording() + var statuses = history + if let active { + statuses.append(await active.status) + } + return statuses + } + + /// A recording that reached its own frame or time limit has already closed + /// its file; move it into history the next time anyone asks. + private func harvestFinishedRecording() async { + guard let active else { return } + let isRecording = await active.isRecording + guard !isRecording else { return } + remember(await active.status) + self.active = nil + } + + private func remember(_ status: WindowRecordingStatus) { + history.removeAll { $0.id == status.id } + history.append(status) + if history.count > Self.historyLimit { + history.removeFirst(history.count - Self.historyLimit) + } + } + + private static func outputURL( + request: WindowRecordingRequest, + identifier: String + ) -> URL { + if let outputPath = request.outputPath { + return URL(fileURLWithPath: outputPath) + } + return FileManager.default.temporaryDirectory + .appendingPathComponent(WindowRecordingOutputNaming.directoryName) + .appendingPathComponent(WindowRecordingOutputNaming.filename( + label: request.label, + identifier: identifier, + format: request.format + )) + } +} diff --git a/Sources/WindowRecordingSession.swift b/Sources/WindowRecordingSession.swift new file mode 100644 index 000000000000..50c6ecb27d6e --- /dev/null +++ b/Sources/WindowRecordingSession.swift @@ -0,0 +1,330 @@ +import CmuxFoundation +import CoreGraphics +import Foundation +import ScreenCaptureKit + +enum WindowRecordingSessionError: Error, LocalizedError { + case unsupportedSystem + case windowGone + case captureFailed(String) + case composeFailed + case alreadyFinished + + var errorDescription: String? { + switch self { + case .unsupportedSystem: + "recording a cmux window needs macOS 14.4 or later" + case .windowGone: + "the window is no longer available" + case let .captureFailed(detail): + "window capture failed: \(detail)" + case .composeFailed: + "a frame could not be composed" + case .alreadyFinished: + "the recording has already finished" + } + } +} + +/// What `window.record.start`, `.status` and `.stop` report back. +struct WindowRecordingStatus: Sendable { + enum State: String, Sendable { + case recording + case finished + case failed + } + + let id: String + let state: State + let format: WindowRecordingRequest.Format + let path: String + let label: String + let frames: Int + let seconds: Double + let width: Int + let height: Int + let notes: Int + let requestedFramesPerSecond: Int + let maximumSeconds: Double + let error: String? + + var jsonObject: [String: Any] { + var payload: [String: Any] = [ + "id": id, + "state": state.rawValue, + "format": format.rawValue, + "path": path, + "frames": frames, + "seconds": (seconds * 1000).rounded() / 1000, + "width": width, + "height": height, + "notes": notes, + "fps_requested": requestedFramesPerSecond, + "max_seconds": maximumSeconds, + ] + if !label.isEmpty { + payload["label"] = label + } + // What the sampler actually managed. A clip whose effective rate is far + // below the requested one still plays at the right speed, because frames + // are timed by when they were captured. + if frames > 1, seconds > 0 { + payload["fps_effective"] = ((Double(frames - 1) / seconds) * 10).rounded() / 10 + } + if let error { + payload["error"] = error + } + return payload + } +} + +/// One in-flight recording of one of cmux's own windows. +/// +/// Frames are sampled with ScreenCaptureKit's screenshot API on a schedule +/// instead of through a live `SCStream`: the same permission-free own-process +/// path the window screenshot command already uses, and no queue of +/// full-resolution frames waiting in memory. +actor WindowRecordingSession { + let id: String + let request: WindowRecordingRequest + let outputURL: URL + let windowHandle: String? + + private let windowID: CGWindowID + private var filter: SCContentFilter? + private var writer: WindowRecordingFrameWriter? + private var geometry: WindowRecordingFrameGeometry? + private var captions: WindowRecordingCaptionTrack + private var state: WindowRecordingStatus.State = .recording + private var failure: String? + private var frames = 0 + private var startUptime = ProcessInfo.processInfo.systemUptime + private var lastOffsetSeconds: Double = 0 + private var loop: Task? + + init( + id: String, + request: WindowRecordingRequest, + outputURL: URL, + windowID: CGWindowID, + windowHandle: String? + ) { + self.id = id + self.request = request + self.outputURL = outputURL + self.windowID = windowID + self.windowHandle = windowHandle + captions = WindowRecordingCaptionTrack() + } + + var isRecording: Bool { + state == .recording + } + + var status: WindowRecordingStatus { + WindowRecordingStatus( + id: id, + state: state, + format: request.format, + path: outputURL.path, + label: request.label, + frames: frames, + seconds: lastOffsetSeconds, + width: geometry?.outputWidth ?? 0, + height: geometry?.outputHeight ?? 0, + notes: captions.count, + requestedFramesPerSecond: request.framesPerSecond, + maximumSeconds: request.maximumSeconds, + error: failure + ) + } + + /// Opens the output and captures the first frame, so a caller that cannot + /// record at all learns about it from `record start` rather than from an + /// empty file later. + func start() async throws { + filter = try await resolveFilter() + let sample = try await sample() + let planned = try WindowRecordingFrameGeometry.plan( + windowPixelWidth: sample.image.width, + windowPixelHeight: sample.image.height, + pointPixelScale: sample.pointPixelScale, + request: request + ) + geometry = planned + writer = try makeWriter(geometry: planned) + startUptime = ProcessInfo.processInfo.systemUptime + try await write(sample: sample, offsetSeconds: 0) + loop = Task { [weak self] in + await self?.run() + } + } + + /// Adds a caption at the current point in the clip. + func note(_ text: String) -> Bool { + guard state == .recording else { return false } + return captions.append( + text: text, + atOffsetSeconds: ProcessInfo.processInfo.systemUptime - startUptime + ) + } + + /// Ends the recording and closes the file. Idempotent: an agent may stop a + /// clip that already reached its own limit. + func stop() async -> WindowRecordingStatus { + loop?.cancel() + loop = nil + if state == .recording { + await finalize() + } + return status + } + + private func run() async { + while !Task.isCancelled, state == .recording { + let target = startUptime + (Double(frames) * request.frameInterval) + let now = ProcessInfo.processInfo.systemUptime + if target > now { + try? await Task.sleep(nanoseconds: UInt64((target - now) * 1_000_000_000)) + } + guard !Task.isCancelled, state == .recording else { return } + let offset = ProcessInfo.processInfo.systemUptime - startUptime + if offset >= request.maximumSeconds || frames >= request.frameBudget { + await finalize() + return + } + do { + try await write(sample: try await sample(), offsetSeconds: offset) + } catch { + await fail(error) + return + } + if frames >= request.frameBudget { + await finalize() + return + } + } + } + + private func write(sample: Sample, offsetSeconds: Double) async throws { + guard let writer, let geometry else { throw WindowRecordingSessionError.alreadyFinished } + // A window resized mid-clip is re-cropped into the frame size the clip + // opened with, rather than ending the recording. + let planned = try WindowRecordingFrameGeometry.plan( + windowPixelWidth: sample.image.width, + windowPixelHeight: sample.image.height, + pointPixelScale: sample.pointPixelScale, + request: request + ) + let composed = WindowRecordingFrameComposer.compose( + source: sample.image, + geometry: geometry.adoptingCrop(of: planned), + caption: request.drawsCaptions ? captions.caption(atOffsetSeconds: offsetSeconds) : nil + ) + guard let composed else { throw WindowRecordingSessionError.composeFailed } + try await writer.append(composed, atOffsetSeconds: offsetSeconds) + frames += 1 + lastOffsetSeconds = offsetSeconds + } + + private func finalize() async { + guard state == .recording else { return } + guard let writer else { + state = .failed + failure = WindowRecordingSessionError.alreadyFinished.localizedDescription + return + } + self.writer = nil + do { + try await writer.finish() + state = .finished + } catch { + state = .failed + failure = error.localizedDescription + try? FileManager.default.removeItem(at: outputURL) + } + } + + private func fail(_ error: Error) async { + // Keep whatever was captured before the failure: a clip that ends when + // the window closes is still evidence of what happened before that. + if let writer { + self.writer = nil + if frames > 0 { + try? await writer.finish() + } + } + state = .failed + failure = error.localizedDescription + } + + private func makeWriter( + geometry: WindowRecordingFrameGeometry + ) throws -> WindowRecordingFrameWriter { + try FileManager.default.createDirectory( + at: outputURL.deletingLastPathComponent(), + withIntermediateDirectories: true + ) + try? FileManager.default.removeItem(at: outputURL) + switch request.format { + case .mp4: + return try WindowRecordingMP4Writer( + url: outputURL, + width: geometry.outputWidth, + height: geometry.outputHeight, + framesPerSecond: request.framesPerSecond + ) + case .gif: + return try WindowRecordingGIFWriter( + url: outputURL, + frameBudget: request.frameBudget, + framesPerSecond: request.framesPerSecond + ) + } + } + + private struct Sample { + let image: CGImage + let pointPixelScale: Double + } + + private func resolveFilter() async throws -> SCContentFilter { + if #available(macOS 14.4, *) { + // The current-process query captures cmux's own windows without + // Screen Recording permission, and cannot reach another app's + // windows even if that permission was granted. + let content: SCShareableContent + do { + content = try await SCShareableContent.currentProcess + } catch { + throw WindowRecordingSessionError.captureFailed(error.localizedDescription) + } + guard let window = content.windows.first(where: { $0.windowID == windowID }) else { + throw WindowRecordingSessionError.windowGone + } + return SCContentFilter(desktopIndependentWindow: window) + } + throw WindowRecordingSessionError.unsupportedSystem + } + + private func sample() async throws -> Sample { + guard let filter else { throw WindowRecordingSessionError.windowGone } + let info = SCShareableContent.info(for: filter) + let pixelScale = Double(info.pointPixelScale) > 0 ? Double(info.pointPixelScale) : 1 + let configuration = SCStreamConfiguration() + configuration.width = max(1, Int((Double(info.contentRect.width) * pixelScale).rounded(.up))) + configuration.height = max(1, Int((Double(info.contentRect.height) * pixelScale).rounded(.up))) + configuration.showsCursor = false + configuration.ignoreShadowsSingleWindow = true + configuration.captureResolution = .best + do { + let image = try await SCScreenshotManager.captureImage( + contentFilter: filter, + configuration: configuration + ) + return Sample(image: image, pointPixelScale: pixelScale) + } catch { + throw WindowRecordingSessionError.captureFailed(error.localizedDescription) + } + } +} diff --git a/Sources/WindowRecordingWindowSelection.swift b/Sources/WindowRecordingWindowSelection.swift new file mode 100644 index 000000000000..fe533d28e100 --- /dev/null +++ b/Sources/WindowRecordingWindowSelection.swift @@ -0,0 +1,45 @@ +import AppKit +import CoreGraphics + +/// Picks the cmux window a recording films, and validates its identifier. +/// +/// The same preference order as the debug screenshot command's +/// `WindowScreenshotWindowSelector`, kept separate because that one is a +/// DEBUG-only type and recording ships in Release. Nothing here captures +/// anything; it only chooses a window that is on screen and big enough to be +/// worth filming. +enum WindowRecordingWindowSelection { + @MainActor + static func eligibleWindows(in application: NSApplication) -> [NSWindow] { + application.windows.filter { window in + window.isVisible && + !window.isMiniaturized && + window.contentView != nil && + !window.frame.isEmpty + } + } + + @MainActor + static func select( + eligibleWindows: [NSWindow], + keyWindow: NSWindow?, + mainWindow: NSWindow?, + terminalWindow: NSWindow? + ) -> NSWindow? { + let preferred = [keyWindow, mainWindow, terminalWindow].compactMap { $0 } + if let match = preferred.first(where: { candidate in + eligibleWindows.contains(where: { $0 === candidate }) + }) { + return match + } + return eligibleWindows.max { lhs, rhs in + lhs.frame.width * lhs.frame.height < rhs.frame.width * rhs.frame.height + } + } + + /// ScreenCaptureKit takes an unsigned window id; AppKit hands out a signed + /// window number, and a negative one belongs to no capturable window. + static func windowID(fromWindowNumber windowNumber: Int) -> CGWindowID? { + CGWindowID(exactly: windowNumber) + } +} diff --git a/cmux.xcodeproj/project.pbxproj b/cmux.xcodeproj/project.pbxproj index 1b0c66418e17..3df5f056fb66 100644 --- a/cmux.xcodeproj/project.pbxproj +++ b/cmux.xcodeproj/project.pbxproj @@ -1105,6 +1105,7 @@ B9000054A1B2C3D4E5F60719 /* CMUXCLI+Process.swift in Sources */ = {isa = PBXBuildFile; fileRef = B9000055A1B2C3D4E5F60719 /* CMUXCLI+Process.swift */; }; 126810B60000000000000002 /* CMUXCLI+ProcessSignalState.swift in Sources */ = {isa = PBXBuildFile; fileRef = 126810B60000000000000001 /* CMUXCLI+ProcessSignalState.swift */; }; A1E8DF699F239A8A5A2FC122 /* CMUXCLI+PromptSubmit.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0B782728C37B72B061710BA7 /* CMUXCLI+PromptSubmit.swift */; }; + A10021000000000000000001 /* CMUXCLI+Record.swift in Sources */ = {isa = PBXBuildFile; fileRef = A10021000000000000000002 /* CMUXCLI+Record.swift */; }; C73660010000000000000001 /* CMUXCLI+RemotePTYErrors.swift in Sources */ = {isa = PBXBuildFile; fileRef = C73660010000000000000002 /* CMUXCLI+RemotePTYErrors.swift */; }; REE0CA0000000000000000C2 /* CMUXCLI+Remotes.swift in Sources */ = {isa = PBXBuildFile; fileRef = REE0CA0000000000000000C1 /* CMUXCLI+Remotes.swift */; }; D7AB12048000000000000001 /* CMUXCLI+RemoteWorkingDirectory.swift in Sources */ = {isa = PBXBuildFile; fileRef = D7AB12048000000000000002 /* CMUXCLI+RemoteWorkingDirectory.swift */; }; @@ -3472,6 +3473,7 @@ A10020000000000000000005 /* TerminalController+SurfaceSelection.swift in Sources */ = {isa = PBXBuildFile; fileRef = A10020000000000000000006 /* TerminalController+SurfaceSelection.swift */; }; DC6E1A6F7452404AA8DE3F5E /* TerminalController+VaultCommands.swift in Sources */ = {isa = PBXBuildFile; fileRef = 78E042D748CB4949BE375319 /* TerminalController+VaultCommands.swift */; }; D6212D0C00000000000000D3 /* TerminalController+WindowDockBrowserRouting.swift in Sources */ = {isa = PBXBuildFile; fileRef = D6212D0C00000000000000D4 /* TerminalController+WindowDockBrowserRouting.swift */; }; + 9065A1010000000000000001 /* TerminalController+WindowRecording.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9065A1010000000000000002 /* TerminalController+WindowRecording.swift */; }; 9065A0010000000000000001 /* TerminalController+WindowScreenshotCapture.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9065A0010000000000000002 /* TerminalController+WindowScreenshotCapture.swift */; }; C0DE00000000000000000C84 /* TerminalController+WorkspaceCreate.swift in Sources */ = {isa = PBXBuildFile; fileRef = C0DE00000000000000000C83 /* TerminalController+WorkspaceCreate.swift */; }; D1A70000000000000000000C /* TerminalController+WorkspaceCreateExecutionPreparation.swift in Sources */ = {isa = PBXBuildFile; fileRef = D1A70000000000000000000B /* TerminalController+WorkspaceCreateExecutionPreparation.swift */; }; @@ -3822,6 +3824,12 @@ 8770D0F2D2BB45359D6BE5E3 /* WindowKeyDownReplayGuardTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B79482F1ECA54E98BE5C8953 /* WindowKeyDownReplayGuardTests.swift */; }; 129170000000000000000001 /* WindowOverlayChromeTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 129170000000000000000002 /* WindowOverlayChromeTests.swift */; }; 973DC7D4CD41516D459E17FC /* WindowPointHitTestTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02BCEFD7668C02FB795ED26F /* WindowPointHitTestTests.swift */; }; + 9065A1020000000000000001 /* WindowRecordingFrameComposer.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9065A1020000000000000002 /* WindowRecordingFrameComposer.swift */; }; + 9065A1030000000000000001 /* WindowRecordingFrameWriter.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9065A1030000000000000002 /* WindowRecordingFrameWriter.swift */; }; + A6CC55B334F22314E39BE6A6 /* WindowRecordingPipelineTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7AB0A04BC0DF676A62F70094 /* WindowRecordingPipelineTests.swift */; }; + 9065A1040000000000000001 /* WindowRecordingRegistry.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9065A1040000000000000002 /* WindowRecordingRegistry.swift */; }; + 9065A1050000000000000001 /* WindowRecordingSession.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9065A1050000000000000002 /* WindowRecordingSession.swift */; }; + 9065A1060000000000000001 /* WindowRecordingWindowSelection.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9065A1060000000000000002 /* WindowRecordingWindowSelection.swift */; }; 6042C0056042C0056042C005 /* WindowScopedShortcutHintModifierMonitor.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6042D0056042D0056042D005 /* WindowScopedShortcutHintModifierMonitor.swift */; }; 9065A0090000000000000001 /* WindowScreenshotBackendAttempt.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9065A0090000000000000002 /* WindowScreenshotBackendAttempt.swift */; }; 9065A0050000000000000001 /* WindowScreenshotCaptureCoordinator.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9065A0050000000000000002 /* WindowScreenshotCaptureCoordinator.swift */; }; @@ -5253,6 +5261,7 @@ B9000055A1B2C3D4E5F60719 /* CMUXCLI+Process.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "CMUXCLI+Process.swift"; sourceTree = ""; }; 126810B60000000000000001 /* CMUXCLI+ProcessSignalState.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "CMUXCLI+ProcessSignalState.swift"; sourceTree = ""; }; 0B782728C37B72B061710BA7 /* CMUXCLI+PromptSubmit.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "CMUXCLI+PromptSubmit.swift"; sourceTree = ""; }; + A10021000000000000000002 /* CMUXCLI+Record.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "CMUXCLI+Record.swift"; sourceTree = ""; }; C73660010000000000000002 /* CMUXCLI+RemotePTYErrors.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "CMUXCLI+RemotePTYErrors.swift"; sourceTree = ""; }; REE0CA0000000000000000C1 /* CMUXCLI+Remotes.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "CMUXCLI+Remotes.swift"; sourceTree = ""; }; D7AB12048000000000000002 /* CMUXCLI+RemoteWorkingDirectory.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "CMUXCLI+RemoteWorkingDirectory.swift"; sourceTree = ""; }; @@ -7490,6 +7499,7 @@ A10020000000000000000006 /* TerminalController+SurfaceSelection.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "TerminalController+SurfaceSelection.swift"; sourceTree = ""; }; 78E042D748CB4949BE375319 /* TerminalController+VaultCommands.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "TerminalController+VaultCommands.swift"; sourceTree = ""; }; D6212D0C00000000000000D4 /* TerminalController+WindowDockBrowserRouting.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "TerminalController+WindowDockBrowserRouting.swift"; sourceTree = ""; }; + 9065A1010000000000000002 /* TerminalController+WindowRecording.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "TerminalController+WindowRecording.swift"; sourceTree = ""; }; 9065A0010000000000000002 /* TerminalController+WindowScreenshotCapture.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "TerminalController+WindowScreenshotCapture.swift"; sourceTree = ""; }; C0DE00000000000000000C83 /* TerminalController+WorkspaceCreate.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "TerminalController+WorkspaceCreate.swift"; sourceTree = ""; }; D1A70000000000000000000B /* TerminalController+WorkspaceCreateExecutionPreparation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "TerminalController+WorkspaceCreateExecutionPreparation.swift"; sourceTree = ""; }; @@ -7838,6 +7848,12 @@ B79482F1ECA54E98BE5C8953 /* WindowKeyDownReplayGuardTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WindowKeyDownReplayGuardTests.swift; sourceTree = ""; }; 129170000000000000000002 /* WindowOverlayChromeTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WindowOverlayChromeTests.swift; sourceTree = ""; }; 02BCEFD7668C02FB795ED26F /* WindowPointHitTestTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = WindowPointHitTestTests.swift; sourceTree = ""; }; + 9065A1020000000000000002 /* WindowRecordingFrameComposer.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WindowRecordingFrameComposer.swift; sourceTree = ""; }; + 9065A1030000000000000002 /* WindowRecordingFrameWriter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WindowRecordingFrameWriter.swift; sourceTree = ""; }; + 7AB0A04BC0DF676A62F70094 /* WindowRecordingPipelineTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "WindowRecordingPipelineTests.swift"; sourceTree = ""; }; + 9065A1040000000000000002 /* WindowRecordingRegistry.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WindowRecordingRegistry.swift; sourceTree = ""; }; + 9065A1050000000000000002 /* WindowRecordingSession.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WindowRecordingSession.swift; sourceTree = ""; }; + 9065A1060000000000000002 /* WindowRecordingWindowSelection.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WindowRecordingWindowSelection.swift; sourceTree = ""; }; 6042D0056042D0056042D005 /* WindowScopedShortcutHintModifierMonitor.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WindowScopedShortcutHintModifierMonitor.swift; sourceTree = ""; }; 9065A0090000000000000002 /* WindowScreenshotBackendAttempt.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WindowScreenshotBackendAttempt.swift; sourceTree = ""; }; 9065A0050000000000000002 /* WindowScreenshotCaptureCoordinator.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WindowScreenshotCaptureCoordinator.swift; sourceTree = ""; }; @@ -9833,6 +9849,12 @@ 9065A0060000000000000002 /* WindowScreenshotLabel.swift */, 9065A00A0000000000000002 /* WindowScreenshotScreenCapturePolicy.swift */, 9065A0070000000000000002 /* WindowScreenshotTarget.swift */, + 9065A1010000000000000002 /* TerminalController+WindowRecording.swift */, + 9065A1020000000000000002 /* WindowRecordingFrameComposer.swift */, + 9065A1030000000000000002 /* WindowRecordingFrameWriter.swift */, + 9065A1040000000000000002 /* WindowRecordingRegistry.swift */, + 9065A1050000000000000002 /* WindowRecordingSession.swift */, + 9065A1060000000000000002 /* WindowRecordingWindowSelection.swift */, 9065A0080000000000000002 /* WindowScreenshotWindowSelector.swift */, A79840030000000000000001 /* TerminalController+SocketConfiguration.swift */, A79840040000000000000001 /* TerminalController+SocketCleanup.swift */, @@ -11112,6 +11134,7 @@ C08672010000000000000008 /* PiCompactedFeedEventExpander.swift */, B9000070A1B2C3D4E5F60719 /* CMUXCLI+Memory.swift */, A10020000000000000000008 /* CMUXCLI+SurfaceSelection.swift */, + A10021000000000000000002 /* CMUXCLI+Record.swift */, 5A57E0000000000000000002 /* CMUXCLI+Paste.swift */, A64B5058972147F941BD6FA5 /* CMUXCLI+WindowDispatch.swift */, E1A09ECE26DDC67D94CF18B1 /* CMUXCLI+Current.swift */, @@ -12308,6 +12331,7 @@ 3D5E43A566AB0B441162EC5A /* CloudTreeNodeCacheTests.swift */, 23E35F95320CBED308AD98CF /* CloudTreeTogglePerformanceTests.swift */, 0C37EA14DE74095FB1BB23E1 /* CloudTreeDisclosureIntentTests.swift */, + 7AB0A04BC0DF676A62F70094 /* WindowRecordingPipelineTests.swift */, ); path = cmuxTests; sourceTree = ""; @@ -15053,6 +15077,7 @@ A10020000000000000000005 /* TerminalController+SurfaceSelection.swift in Sources */, DC6E1A6F7452404AA8DE3F5E /* TerminalController+VaultCommands.swift in Sources */, D6212D0C00000000000000D3 /* TerminalController+WindowDockBrowserRouting.swift in Sources */, + 9065A1010000000000000001 /* TerminalController+WindowRecording.swift in Sources */, 9065A0010000000000000001 /* TerminalController+WindowScreenshotCapture.swift in Sources */, C0DE00000000000000000C84 /* TerminalController+WorkspaceCreate.swift in Sources */, D1A70000000000000000000C /* TerminalController+WorkspaceCreateExecutionPreparation.swift in Sources */, @@ -15291,6 +15316,11 @@ B9000018A1B2C3D4E5F60719 /* WindowDragHandleView.swift in Sources */, D0B10024A1B2C3D4E5F60001 /* WindowInputRoutingContext.swift in Sources */, 313585583035A1E685010A97 /* WindowKeyDownReplayGuard.swift in Sources */, + 9065A1020000000000000001 /* WindowRecordingFrameComposer.swift in Sources */, + 9065A1030000000000000001 /* WindowRecordingFrameWriter.swift in Sources */, + 9065A1040000000000000001 /* WindowRecordingRegistry.swift in Sources */, + 9065A1050000000000000001 /* WindowRecordingSession.swift in Sources */, + 9065A1060000000000000001 /* WindowRecordingWindowSelection.swift in Sources */, 6042C0056042C0056042C005 /* WindowScopedShortcutHintModifierMonitor.swift in Sources */, 9065A0090000000000000001 /* WindowScreenshotBackendAttempt.swift in Sources */, 9065A0050000000000000001 /* WindowScreenshotCaptureCoordinator.swift in Sources */, @@ -15607,6 +15637,7 @@ B9000054A1B2C3D4E5F60719 /* CMUXCLI+Process.swift in Sources */, 126810B60000000000000002 /* CMUXCLI+ProcessSignalState.swift in Sources */, A1E8DF699F239A8A5A2FC122 /* CMUXCLI+PromptSubmit.swift in Sources */, + A10021000000000000000001 /* CMUXCLI+Record.swift in Sources */, C73660010000000000000001 /* CMUXCLI+RemotePTYErrors.swift in Sources */, REE0CA0000000000000000C2 /* CMUXCLI+Remotes.swift in Sources */, D7AB12048000000000000001 /* CMUXCLI+RemoteWorkingDirectory.swift in Sources */, @@ -16926,6 +16957,7 @@ 8770D0F2D2BB45359D6BE5E3 /* WindowKeyDownReplayGuardTests.swift in Sources */, 129170000000000000000001 /* WindowOverlayChromeTests.swift in Sources */, 973DC7D4CD41516D459E17FC /* WindowPointHitTestTests.swift in Sources */, + A6CC55B334F22314E39BE6A6 /* WindowRecordingPipelineTests.swift in Sources */, 606600010000000000000001 /* WindowTerminalHostViewTitlebarHitTests.swift in Sources */, 604500100000000000000003 /* WindowTitleTemplateTests.swift in Sources */, 7F0A0E04A8CADC84BB4F1DF7 /* WorkspaceActionDispatcherTests.swift in Sources */, diff --git a/cmuxTests/WindowRecordingPipelineTests.swift b/cmuxTests/WindowRecordingPipelineTests.swift new file mode 100644 index 000000000000..ba3c77ff61ce --- /dev/null +++ b/cmuxTests/WindowRecordingPipelineTests.swift @@ -0,0 +1,292 @@ +import AVFoundation +import CmuxFoundation +import CoreGraphics +import Foundation +import ImageIO +import Testing + +#if canImport(cmux_DEV) +@testable import cmux_DEV +#elseif canImport(cmux) +@testable import cmux +#endif + +/// Covers the two halves of `cmux record` that do not need a window on screen: +/// composing a captured image into an encoded frame, and writing frames out as +/// an mp4 or a gif. +@Suite struct WindowRecordingPipelineTests { + // MARK: Fixtures + + private static func image( + width: Int, + height: Int, + gray: Double = 1 + ) -> CGImage { + let context = CGContext( + data: nil, + width: width, + height: height, + bitsPerComponent: 8, + bytesPerRow: 0, + space: CGColorSpace(name: CGColorSpace.sRGB)!, + bitmapInfo: CGImageAlphaInfo.noneSkipFirst.rawValue + | CGBitmapInfo.byteOrder32Little.rawValue + )! + context.setFillColor(gray: gray, alpha: 1) + context.fill(CGRect(x: 0, y: 0, width: CGFloat(width), height: CGFloat(height))) + return context.makeImage()! + } + + /// An image with a known color in one quadrant, so a crop can be checked by + /// reading a pixel rather than by trusting the rectangle arithmetic. + private static func quadrantImage(width: Int, height: Int) -> CGImage { + let context = CGContext( + data: nil, + width: width, + height: height, + bitsPerComponent: 8, + bytesPerRow: 0, + space: CGColorSpace(name: CGColorSpace.sRGB)!, + bitmapInfo: CGImageAlphaInfo.noneSkipFirst.rawValue + | CGBitmapInfo.byteOrder32Little.rawValue + )! + context.setFillColor(gray: 0.2, alpha: 1) + context.fill(CGRect(x: 0, y: 0, width: CGFloat(width), height: CGFloat(height))) + // Top-left quadrant in CGImage coordinates, which have y growing down. + context.setFillColor(red: 1, green: 0, blue: 0, alpha: 1) + context.fill(CGRect( + x: 0, + y: CGFloat(height) / 2, + width: CGFloat(width) / 2, + height: CGFloat(height) / 2 + )) + return context.makeImage()! + } + + private static func pixel(_ image: CGImage, x: Int, y: Int) -> (r: Int, g: Int, b: Int)? { + var bytes = [UInt8](repeating: 0, count: 4) + guard let context = CGContext( + data: &bytes, + width: 1, + height: 1, + bitsPerComponent: 8, + bytesPerRow: 4, + space: CGColorSpace(name: CGColorSpace.sRGB)!, + bitmapInfo: CGImageAlphaInfo.noneSkipFirst.rawValue + | CGBitmapInfo.byteOrder32Little.rawValue + ) else { + return nil + } + context.draw(image, in: CGRect(x: -x, y: -(image.height - 1 - y), width: image.width, height: image.height)) + // 32BGRA little endian: bytes are B, G, R, then the skipped alpha. + return (r: Int(bytes[2]), g: Int(bytes[1]), b: Int(bytes[0])) + } + + private static func geometry(params: [String: Any] = [:], width: Int, height: Int) throws + -> WindowRecordingFrameGeometry { + try WindowRecordingFrameGeometry.plan( + windowPixelWidth: width, + windowPixelHeight: height, + pointPixelScale: 1, + request: try WindowRecordingRequest.make(params: params) + ) + } + + private static func temporaryURL(extension pathExtension: String) -> URL { + FileManager.default.temporaryDirectory.appendingPathComponent( + "cmux-recording-test-\(UUID().uuidString).\(pathExtension)" + ) + } + + // MARK: Composer + + @Test func composedFramesAreTheEncodedFrameSize() throws { + let geometry = try Self.geometry(params: ["scale": 0.5], width: 800, height: 600) + + let frame = try #require(WindowRecordingFrameComposer.compose( + source: Self.image(width: 800, height: 600), + geometry: geometry, + caption: nil + )) + + #expect(frame.width == geometry.outputWidth) + #expect(frame.height == geometry.outputHeight) + } + + @Test func aRegionCropKeepsOnlyTheRequestedPixels() throws { + let source = Self.quadrantImage(width: 400, height: 400) + // The red quadrant sits at the top-left of the image. + let geometry = try Self.geometry(params: ["region": "0,0,200,200"], width: 400, height: 400) + + let frame = try #require(WindowRecordingFrameComposer.compose( + source: source, + geometry: geometry, + caption: nil + )) + + #expect(frame.width == 200) + #expect(frame.height == 200) + let center = try #require(Self.pixel(frame, x: 100, y: 100)) + #expect(center.r > 200) + #expect(center.g < 60) + } + + @Test func aWindowResizedMidClipIsLetterboxedRatherThanStretched() throws { + // A clip that opened on a 400x400 window keeps that frame size; a + // later 400x200 capture has to be centered inside it with black bars. + let opening = try Self.geometry(width: 400, height: 400) + let resized = try Self.geometry(width: 400, height: 200) + + let frame = try #require(WindowRecordingFrameComposer.compose( + source: Self.image(width: 400, height: 200), + geometry: opening.adoptingCrop(of: resized), + caption: nil + )) + + #expect(frame.width == 400) + #expect(frame.height == 400) + let top = try #require(Self.pixel(frame, x: 200, y: 4)) + let middle = try #require(Self.pixel(frame, x: 200, y: 200)) + #expect(top.r == 0) + #expect(middle.r > 200) + } + + @Test func aCaptionDarkensTheBottomLeftAndLeavesTheRestAlone() throws { + let geometry = try Self.geometry(width: 600, height: 400) + let source = Self.image(width: 600, height: 400) + + let plain = try #require(WindowRecordingFrameComposer.compose( + source: source, + geometry: geometry, + caption: nil + )) + let captioned = try #require(WindowRecordingFrameComposer.compose( + source: source, + geometry: geometry, + caption: "opened the command palette" + )) + + // The caption box hugs the bottom-left corner, so its exact width + // depends on the system font; assert on the band it occupies. + let changedInBand = (2..<150).filter { step in + Self.pixel(plain, x: step * 4, y: 380) != Self.pixel(captioned, x: step * 4, y: 380) + } + #expect(!changedInBand.isEmpty) + let inBox = try #require(Self.pixel(captioned, x: 30, y: 380)) + #expect(inBox.r < 160) + let untouched = try #require(Self.pixel(captioned, x: 300, y: 20)) + #expect(untouched == Self.pixel(plain, x: 300, y: 20)) + } + + @Test func fittingNeverUpscalesPastTheFrame() { + let bounds = CGRect(x: 0, y: 0, width: 400, height: 400) + + let wide = WindowRecordingFrameComposer.fit(CGSize(width: 800, height: 200), in: bounds) + + #expect(wide.width == 400) + #expect(wide.height == 100) + #expect(wide.minY == 150) + } + + // MARK: mp4 + + @Test func anMP4CarriesTheFramesAndTheirElapsedTiming() async throws { + let url = Self.temporaryURL(extension: "mp4") + defer { try? FileManager.default.removeItem(at: url) } + let writer = try WindowRecordingMP4Writer(url: url, width: 160, height: 120, framesPerSecond: 10) + let frame = Self.image(width: 160, height: 120) + + // Deliberately uneven gaps: a sampler that fell behind must not make + // the clip play fast. + for offset in [0.0, 0.1, 0.45, 0.5] { + try await writer.append(frame, atOffsetSeconds: offset) + } + try await writer.finish() + + #expect(FileManager.default.fileExists(atPath: url.path)) + let asset = AVURLAsset(url: url) + let duration = try await asset.load(.duration).seconds + #expect(duration > 0.4) + let track = try #require(try await asset.loadTracks(withMediaType: .video).first) + let size = try await track.load(.naturalSize) + #expect(Int(size.width) == 160) + #expect(Int(size.height) == 120) + } + + @Test func twoFramesInTheSameTickStillGetIncreasingTimes() async throws { + let url = Self.temporaryURL(extension: "mp4") + defer { try? FileManager.default.removeItem(at: url) } + let writer = try WindowRecordingMP4Writer(url: url, width: 64, height: 64, framesPerSecond: 12) + let frame = Self.image(width: 64, height: 64) + + try await writer.append(frame, atOffsetSeconds: 0) + try await writer.append(frame, atOffsetSeconds: 0) + try await writer.finish() + + let asset = AVURLAsset(url: url) + #expect(try await asset.load(.duration).seconds > 0) + } + + @Test func anMP4WithNoFramesFailsInsteadOfLeavingAnEmptyFile() async throws { + let url = Self.temporaryURL(extension: "mp4") + defer { try? FileManager.default.removeItem(at: url) } + let writer = try WindowRecordingMP4Writer(url: url, width: 64, height: 64, framesPerSecond: 12) + + await #expect(throws: WindowRecordingWriterError.noFrames) { + try await writer.finish() + } + } + + // MARK: gif + + @Test func aGIFHoldsEveryFrameAndItsMeasuredDelay() async throws { + let url = Self.temporaryURL(extension: "gif") + defer { try? FileManager.default.removeItem(at: url) } + let writer = try WindowRecordingGIFWriter(url: url, frameBudget: 3, framesPerSecond: 4) + + try await writer.append(Self.image(width: 80, height: 60, gray: 1), atOffsetSeconds: 0) + try await writer.append(Self.image(width: 80, height: 60, gray: 0.5), atOffsetSeconds: 0.5) + try await writer.append(Self.image(width: 80, height: 60, gray: 0), atOffsetSeconds: 0.75) + try await writer.finish() + + let source = try #require(CGImageSourceCreateWithURL(url as CFURL, nil)) + #expect(CGImageSourceGetCount(source) == 3) + #expect(Self.gifDelay(source, at: 0) == 0.5) + #expect(Self.gifDelay(source, at: 1) == 0.25) + // The last frame has no successor, so it takes the nominal delay. + #expect(Self.gifDelay(source, at: 2) == 0.25) + } + + @Test func aGIFDelayStaysInThePlayableRange() async throws { + let url = Self.temporaryURL(extension: "gif") + defer { try? FileManager.default.removeItem(at: url) } + let writer = try WindowRecordingGIFWriter(url: url, frameBudget: 2, framesPerSecond: 8) + + try await writer.append(Self.image(width: 40, height: 40), atOffsetSeconds: 0) + // A 40 second gap would stall a viewer; a zero gap would be dropped. + try await writer.append(Self.image(width: 40, height: 40, gray: 0), atOffsetSeconds: 40) + try await writer.finish() + + let source = try #require(CGImageSourceCreateWithURL(url as CFURL, nil)) + #expect(Self.gifDelay(source, at: 0) == 10) + } + + @Test func aGIFWithNoFramesFails() async throws { + let url = Self.temporaryURL(extension: "gif") + defer { try? FileManager.default.removeItem(at: url) } + let writer = try WindowRecordingGIFWriter(url: url, frameBudget: 1, framesPerSecond: 8) + + await #expect(throws: WindowRecordingWriterError.noFrames) { + try await writer.finish() + } + } + + private static func gifDelay(_ source: CGImageSource, at index: Int) -> Double? { + guard let properties = CGImageSourceCopyPropertiesAtIndex(source, index, nil) + as NSDictionary?, + let gif = properties[kCGImagePropertyGIFDictionary] as? NSDictionary else { + return nil + } + return (gif[kCGImagePropertyGIFUnclampedDelayTime] as? NSNumber)?.doubleValue + } +} diff --git a/docs/cli-contract.md b/docs/cli-contract.md index 9557d30fea6a..079dfded114e 100644 --- a/docs/cli-contract.md +++ b/docs/cli-contract.md @@ -157,6 +157,7 @@ Environment: | `current-workspace` | Print current workspace information. | | `read-selection` | Read the active selection from a terminal, file preview, Markdown, or browser surface. Plain output includes available source context; `--json` returns the complete socket response. | | `read-screen` | Read terminal text from a surface. `--selection` is a text-only compatibility alias for `read-selection`. | +| `record` | Record a cmux window or a region of one to an mp4 or gif (`window.record.*`). `start` returns a recording id and the output path, `stop` closes the clip, `status` reports progress, `note` adds a caption drawn into later frames, `list` shows the current and recent recordings. One recording at a time; a recording stops itself at `--max-seconds`. Local socket only: `window.record.*` is not on the `cmux ssh` relay allowlist. | | `send` | Send text to a terminal surface as keystrokes (`surface.send_text`). `--paste`, before the text, sends it unchanged through the Cmd+V paste path (`terminal.paste`) instead, like `cmux paste`. Without `--paste`, large multi-line text prints a hint on stderr recommending it. | | `send-key` | Send one key to a terminal surface. | | `paste` | Paste text from an argument or stdin into a terminal surface through the Cmd+V paste path (`terminal.paste`). The CLI sends the text unchanged; Ghostty brackets it when the program enabled bracketed paste (otherwise newlines become Enter) and replaces unsafe control bytes with spaces. `--submit` presses the agent-aware submit key afterwards. Local socket only: `terminal.paste` is not on the `cmux ssh` relay allowlist. | @@ -951,6 +952,7 @@ the expected text without connecting to a cmux socket. - `cmux respawn-pane --help` -> `Usage: cmux respawn-pane` - `cmux display-message --help` -> `Usage: cmux display-message` - `cmux read-screen --help` -> `Usage: cmux read-screen` +- `cmux record --help` -> `Usage: cmux record [start] [flags]` - `cmux send --help` -> `Usage: cmux send` - `cmux send-key --help` -> `Usage: cmux send-key` - `cmux paste --help` -> `Usage: cmux paste` diff --git a/scripts/localization-allowed-omissions.json b/scripts/localization-allowed-omissions.json index b66eb7ccb856..a0ad7db80841 100644 --- a/scripts/localization-allowed-omissions.json +++ b/scripts/localization-allowed-omissions.json @@ -312,6 +312,10 @@ "source": "OK %@ [%@] %@", "class": "format" }, + "cli.usage.record": { + "class": "syntax", + "source": "record [start] [--format mp4|gif] [--gif] [--fps ] [--max-seconds ] [--scale ] [--max-width ] [--region ] [--out ] [--label ] [--no-captions] [--window ] | record stop|status [--id ] | record note [--id ] | record list" + }, "cli.usage.readScreen": { "class": "syntax", "source": "read-screen [--workspace ] [--surface ] [--window ] [--scrollback] [--lines ] [--selection]" From e86d4764109ef4d8576b7946c55d7c72d81e6572 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Mon, 28 Sep 2026 02:51:23 -0700 Subject: [PATCH 03/15] Add failing tests for the window recorder's crash and early-stop paths A review of the recorder found seven defects the existing tests missed. These are the reds, before the fixes: - `--fps nan`, `--fps inf` and `--fps 1e30` reach `Int(Double)` in the request decoder and trap, which takes the whole app down with the socket. The package suite now crashes with "Double value cannot be converted to Int" instead of finishing. - a gif whose recording stopped long before its frame budget cannot be finalized, because the budget is handed to ImageIO as the number of images the file will contain, so the documented `--gif` flow leaves no file at all. - `record stop` after a clip reached its own `--max-seconds` limit reported "no recording is running" rather than the finished clip. - an unknown recording id, and a note for a clip that already stopped, had no coverage at all. - the mp4 writer's no-frames path was not checked for leaving a stub file behind, and the two-frames-one-tick test only asserted a nonzero duration rather than the timestamps it exists to pin down. - `fit` upscales a window that shrank mid-clip, which its own name says it does not do. `remember` on the registry stops being private so a test can set up a finished clip without a window on screen. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../WindowRecordingRequestTests.swift | 15 ++ Sources/WindowRecordingRegistry.swift | 4 +- cmuxTests/WindowRecordingPipelineTests.swift | 142 +++++++++++++++++- 3 files changed, 157 insertions(+), 4 deletions(-) diff --git a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/WindowRecordingRequestTests.swift b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/WindowRecordingRequestTests.swift index b42a920953e9..5b8b181081f3 100644 --- a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/WindowRecordingRequestTests.swift +++ b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/WindowRecordingRequestTests.swift @@ -134,6 +134,21 @@ import Testing } } + /// `Int(Double.nan)` and `Int(1e30)` trap, which takes the whole app down, + /// and `cmux record --fps nan` hands exactly those values to this decoder. + @Test(arguments: ["nan", "inf", "-inf", "1e30", "-1e30"]) + func numbersThatCannotBeAnIntegerAreRejectedRatherThanTrapping(text: String) { + #expect(throws: (any Error).self) { + try WindowRecordingRequest.make(params: ["fps": text]) + } + #expect(throws: (any Error).self) { + try WindowRecordingRequest.make(params: ["max_width": text]) + } + #expect(throws: (any Error).self) { + try WindowRecordingRequest.make(params: ["max_seconds": text]) + } + } + @Test func outputPathMustBeAbsoluteAndMatchTheFormat() { #expect(throws: WindowRecordingRequest.Failure.outputPathNotAbsolute("clip.mp4")) { try WindowRecordingRequest.make(params: ["out": "clip.mp4"]) diff --git a/Sources/WindowRecordingRegistry.swift b/Sources/WindowRecordingRegistry.swift index 83d69131b500..29fd5a799162 100644 --- a/Sources/WindowRecordingRegistry.swift +++ b/Sources/WindowRecordingRegistry.swift @@ -122,7 +122,9 @@ actor WindowRecordingRegistry { self.active = nil } - private func remember(_ status: WindowRecordingStatus) { + /// Files a finished clip in history. Internal rather than private so tests + /// can set up a registry without a window on screen. + func remember(_ status: WindowRecordingStatus) { history.removeAll { $0.id == status.id } history.append(status) if history.count > Self.historyLimit { diff --git a/cmuxTests/WindowRecordingPipelineTests.swift b/cmuxTests/WindowRecordingPipelineTests.swift index ba3c77ff61ce..d47611e21abe 100644 --- a/cmuxTests/WindowRecordingPipelineTests.swift +++ b/cmuxTests/WindowRecordingPipelineTests.swift @@ -186,6 +186,15 @@ import Testing #expect(wide.width == 400) #expect(wide.height == 100) #expect(wide.minY == 150) + + // A window that shrank mid-clip keeps its own pixel size, centered, + // rather than being blown up to fill the frame it opened with. + let small = WindowRecordingFrameComposer.fit(CGSize(width: 200, height: 100), in: bounds) + + #expect(small.width == 200) + #expect(small.height == 100) + #expect(small.minX == 100) + #expect(small.minY == 150) } // MARK: mp4 @@ -223,8 +232,16 @@ import Testing try await writer.append(frame, atOffsetSeconds: 0) try await writer.finish() - let asset = AVURLAsset(url: url) - #expect(try await asset.load(.duration).seconds > 0) + // Both frames have to survive, and the second one has to present later + // than the first: an equal timestamp makes AVFoundation drop it. + let times = try await Self.presentationTimes(url: url) + #expect(times.count == 2) + #expect(times.first == 0) + if times.count == 2 { + #expect(times[1] > times[0]) + // One 1/600 second tick, the smallest step the clip can carry. + #expect(abs(times[1] - (1.0 / 600)) < 1e-6) + } } @Test func anMP4WithNoFramesFailsInsteadOfLeavingAnEmptyFile() async throws { @@ -235,6 +252,25 @@ import Testing await #expect(throws: WindowRecordingWriterError.noFrames) { try await writer.finish() } + #expect(!FileManager.default.fileExists(atPath: url.path)) + } + + /// Reads back what the file really carries, rather than trusting the asset's + /// rounded duration. + private static func presentationTimes(url: URL) async throws -> [Double] { + let asset = AVURLAsset(url: url) + guard let track = try await asset.loadTracks(withMediaType: .video).first else { + return [] + } + let reader = try AVAssetReader(asset: asset) + let output = AVAssetReaderTrackOutput(track: track, outputSettings: nil) + reader.add(output) + reader.startReading() + var times: [Double] = [] + while let sample = output.copyNextSampleBuffer() { + times.append(CMSampleBufferGetPresentationTimeStamp(sample).seconds) + } + return times } // MARK: gif @@ -260,15 +296,37 @@ import Testing @Test func aGIFDelayStaysInThePlayableRange() async throws { let url = Self.temporaryURL(extension: "gif") defer { try? FileManager.default.removeItem(at: url) } - let writer = try WindowRecordingGIFWriter(url: url, frameBudget: 2, framesPerSecond: 8) + let writer = try WindowRecordingGIFWriter(url: url, frameBudget: 3, framesPerSecond: 8) try await writer.append(Self.image(width: 40, height: 40), atOffsetSeconds: 0) // A 40 second gap would stall a viewer; a zero gap would be dropped. try await writer.append(Self.image(width: 40, height: 40, gray: 0), atOffsetSeconds: 40) + try await writer.append(Self.image(width: 40, height: 40, gray: 0.5), atOffsetSeconds: 40) try await writer.finish() let source = try #require(CGImageSourceCreateWithURL(url as CFURL, nil)) #expect(Self.gifDelay(source, at: 0) == 10) + #expect(Self.gifDelay(source, at: 1) == 0.02) + } + + /// The normal flow: a clip with a 30 second budget that an agent stops after + /// three frames still has to produce a gif. + @Test func aGIFStoppedLongBeforeItsBudgetStillFinalizes() async throws { + let url = Self.temporaryURL(extension: "gif") + defer { try? FileManager.default.removeItem(at: url) } + let writer = try WindowRecordingGIFWriter(url: url, frameBudget: 240, framesPerSecond: 8) + + for step in 0..<3 { + try await writer.append( + Self.image(width: 40, height: 40, gray: Double(step) / 3), + atOffsetSeconds: Double(step) * 0.125 + ) + } + try await writer.finish() + + #expect(FileManager.default.fileExists(atPath: url.path)) + let source = try #require(CGImageSourceCreateWithURL(url as CFURL, nil)) + #expect(CGImageSourceGetCount(source) == 3) } @Test func aGIFWithNoFramesFails() async throws { @@ -290,3 +348,81 @@ import Testing return (gif[kCGImagePropertyGIFUnclampedDelayTime] as? NSNumber)?.doubleValue } } + +/// Covers the bookkeeping `cmux record` needs without a window on screen: which +/// clip `stop`, `status` and `note` resolve to once a recording has ended. +@Suite struct WindowRecordingRegistryTests { + private static func finished(id: String) -> WindowRecordingStatus { + WindowRecordingStatus( + id: id, + state: .finished, + format: .mp4, + path: "/tmp/\(id).mp4", + label: "", + frames: 12, + seconds: 1, + width: 320, + height: 200, + notes: 0, + requestedFramesPerSecond: 12, + maximumSeconds: 15, + error: nil + ) + } + + /// A clip that reached its own `--max-seconds` limit is no longer active, so + /// the `cmux record stop` an agent runs afterwards has to report that clip + /// rather than claim nothing was recorded. + @Test func stoppingAfterTheClipStoppedItselfReportsThatClip() async throws { + let registry = WindowRecordingRegistry() + await registry.remember(Self.finished(id: "a")) + + let stopped = try await registry.stop(id: nil) + + #expect(stopped.id == "a") + #expect(stopped.state == .finished) + #expect(stopped.path == "/tmp/a.mp4") + } + + @Test func stoppingWithNothingEverRecordedSaysSo() async { + let registry = WindowRecordingRegistry() + + await #expect(throws: WindowRecordingRegistry.Failure.self) { + _ = try await registry.stop(id: nil) + } + } + + @Test func anUnknownIdIsNotAnsweredWithTheLatestClip() async { + let registry = WindowRecordingRegistry() + await registry.remember(Self.finished(id: "a")) + + await #expect(throws: WindowRecordingRegistry.Failure.self) { + _ = try await registry.stop(id: "b") + } + await #expect(throws: WindowRecordingRegistry.Failure.self) { + _ = try await registry.status(id: "b") + } + } + + @Test func aNoteForAStoppedClipIsRefused() async { + let registry = WindowRecordingRegistry() + await registry.remember(Self.finished(id: "a")) + + await #expect(throws: WindowRecordingRegistry.Failure.self) { + _ = try await registry.note(text: "too late", id: "a") + } + } + + @Test func historyKeepsTheMostRecentClipsOnly() async { + let registry = WindowRecordingRegistry() + for index in 0..<12 { + await registry.remember(Self.finished(id: "clip-\(index)")) + } + + let recent = await registry.recentStatuses() + + #expect(recent.count == 8) + #expect(recent.first?.id == "clip-4") + #expect(recent.last?.id == "clip-11") + } +} From 3fcc9a3d65f3c408f2e4358a1e0e1e03ef9b8b96 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Mon, 28 Sep 2026 02:59:09 -0700 Subject: [PATCH 04/15] Fix the window recorder's crash, race and early-stop paths Seven defects a review found, in the order they bite a caller: - `--fps nan` and `--fps 1e30` trapped in `Int(Double)` and took the app down. The decoder now refuses a value that is not finite or does not fit an `Int`. - a gif was created with the recording's frame budget as its declared image count, so a clip stopped before that budget could not be finalized and left no file at all. The count is a floor rather than a cap, and how many frames a clip ends with is only known when it stops, so declare one and add as many as were captured. - `stop` could close the file while an append was suspended inside the session actor, which is an uncatchable AVFoundation exception for the mp4 writer and concurrent state for the gif writer. Appends and the close now take turns. - two `record start` calls could both pass the one-at-a-time check, because the check was followed by the await that opens the capture. The slot is claimed before that await. - `record stop` after a clip reached its own `--max-seconds` limit said no recording was running; it reports the finished clip, as `status` already did. - the mp4 writer cancelled a writer it had never started when a clip captured no frames, which raises rather than returns an error. - the output file was deleted before the first frame was written, so a failed recording destroyed whatever was at `--out`. Frames go to a hidden sibling file and only move into place once the clip closes, a directory or device at that path is refused, and a partial file is cleaned up on every failure path. Also: a window closed mid-clip reported an empty rectangle and was captured as a one-pixel frame for the rest of the clip, so an empty rectangle ends the recording and keeps what came before; `fit` no longer magnifies a window that shrank mid-clip, which is what its name always claimed; and `window.record.*` is listed in `system.capabilities` for local callers, where the relay policy still filters it out. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../WindowRecordingRequest.swift | 7 +- Sources/TerminalController+Capabilities.swift | 5 + Sources/WindowRecordingFrameComposer.swift | 6 +- Sources/WindowRecordingFrameWriter.swift | 15 ++- Sources/WindowRecordingRegistry.swift | 19 +++- Sources/WindowRecordingSession.swift | 105 ++++++++++++++++-- cmuxTests/WindowRecordingPipelineTests.swift | 8 +- docs/cli-contract.md | 2 +- 8 files changed, 144 insertions(+), 23 deletions(-) diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingRequest.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingRequest.swift index 4d8480393bb7..725b5df51ca5 100644 --- a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingRequest.swift +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingRequest.swift @@ -238,11 +238,12 @@ public struct WindowRecordingRequest: Equatable, Sendable { range: ClosedRange ) throws -> Int? { guard let value else { return nil } - guard let number = numericValue(value) else { + guard let number = numericValue(value), number.isFinite else { throw Failure.notANumber(field: field) } - let rounded = Int(number.rounded()) - guard range.contains(rounded) else { + // `Int(exactly:)` rather than `Int(_:)`: converting a value past Int's + // range traps, and "--fps 1e30" is something a caller can type. + guard let rounded = Int(exactly: number.rounded()), range.contains(rounded) else { throw Failure.outOfRange( field: field, message: "must be between \(range.lowerBound) and \(range.upperBound)" diff --git a/Sources/TerminalController+Capabilities.swift b/Sources/TerminalController+Capabilities.swift index 8845d116da0f..cebf070140b5 100644 --- a/Sources/TerminalController+Capabilities.swift +++ b/Sources/TerminalController+Capabilities.swift @@ -172,6 +172,11 @@ extension TerminalController { "window.close", "window.displays", "window.display", + "window.record.start", + "window.record.stop", + "window.record.status", + "window.record.note", + "window.record.list", "workspace.list", "workspace.create", "workspace.cloud_vm_open", diff --git a/Sources/WindowRecordingFrameComposer.swift b/Sources/WindowRecordingFrameComposer.swift index a8c9c0522b62..b20e031219fe 100644 --- a/Sources/WindowRecordingFrameComposer.swift +++ b/Sources/WindowRecordingFrameComposer.swift @@ -73,9 +73,13 @@ enum WindowRecordingFrameComposer { /// Centers the captured content without distorting it. The two sizes match /// for the whole clip unless the window was resized while recording. + /// + /// A window that shrank keeps its own pixel size inside the frame instead of + /// being magnified: blurry text in the middle of a clip reads as a rendering + /// bug, while a black border reads as the window having been resized. static func fit(_ content: CGSize, in bounds: CGRect) -> CGRect { guard content.width > 0, content.height > 0 else { return bounds } - let scale = min(bounds.width / content.width, bounds.height / content.height) + let scale = min(1, min(bounds.width / content.width, bounds.height / content.height)) let size = CGSize(width: content.width * scale, height: content.height * scale) return CGRect( x: bounds.minX + ((bounds.width - size.width) / 2).rounded(), diff --git a/Sources/WindowRecordingFrameWriter.swift b/Sources/WindowRecordingFrameWriter.swift index a6ffa92f5f41..0aeed8ca54f1 100644 --- a/Sources/WindowRecordingFrameWriter.swift +++ b/Sources/WindowRecordingFrameWriter.swift @@ -192,7 +192,11 @@ final class WindowRecordingMP4Writer: WindowRecordingFrameWriter { func finish() async throws { guard didStart, frameCount > 0 else { - writer.cancelWriting() + // Cancelling a writer that never started raises an exception rather + // than returning an error, so only cancel one that is writing. + if writer.status == .writing { + writer.cancelWriting() + } throw WindowRecordingWriterError.noFrames } input.markAsFinished() @@ -224,11 +228,16 @@ final class WindowRecordingGIFWriter: WindowRecordingFrameWriter { /// or above two hundredths keeps playback predictable. private static let delayRange = 0.02...10.0 - init(url: URL, frameBudget: Int, framesPerSecond: Int) throws { + init(url: URL, framesPerSecond: Int) throws { + // The declared image count is the number `CGImageDestinationFinalize` + // insists on having received, not a cap on what may be added, and how + // many frames a recording ends up with is only known when it stops. + // Declaring the frame budget here would fail every clip an agent stops + // early, which is every clip an agent stops. guard let destination = CGImageDestinationCreateWithURL( url as CFURL, UTType.gif.identifier as CFString, - frameBudget, + 1, nil ) else { throw WindowRecordingWriterError.setup("could not create the gif at \(url.path)") diff --git a/Sources/WindowRecordingRegistry.swift b/Sources/WindowRecordingRegistry.swift index 29fd5a799162..696655ea90ec 100644 --- a/Sources/WindowRecordingRegistry.swift +++ b/Sources/WindowRecordingRegistry.swift @@ -53,8 +53,18 @@ actor WindowRecordingRegistry { windowID: windowID, windowHandle: windowHandle ) - try await session.start() + // Claim the slot before the first await: opening a recording takes a + // capture round trip, and a second `record start` arriving during it + // would otherwise pass the check above and replace this session. active = session + do { + try await session.start() + } catch { + if active === session { + active = nil + } + throw error + } return await session.status } @@ -66,7 +76,12 @@ actor WindowRecordingRegistry { remember(status) return status } - guard let id else { throw Failure.noRecording } + guard let id else { + // A clip that reached its own `--max-seconds` limit has already + // closed its file, so report it rather than claiming nothing ran. + guard let latest = history.last else { throw Failure.noRecording } + return latest + } guard let remembered = history.last(where: { $0.id == id }) else { throw Failure.unknownRecording(id) } diff --git a/Sources/WindowRecordingSession.swift b/Sources/WindowRecordingSession.swift index 50c6ecb27d6e..531f6c5fe6c5 100644 --- a/Sources/WindowRecordingSession.swift +++ b/Sources/WindowRecordingSession.swift @@ -9,6 +9,7 @@ enum WindowRecordingSessionError: Error, LocalizedError { case captureFailed(String) case composeFailed case alreadyFinished + case outputNotAFile(String) var errorDescription: String? { switch self { @@ -22,6 +23,8 @@ enum WindowRecordingSessionError: Error, LocalizedError { "a frame could not be composed" case .alreadyFinished: "the recording has already finished" + case let .outputNotAFile(path): + "\(path) is not a file the recording may replace" } } } @@ -91,6 +94,11 @@ actor WindowRecordingSession { let windowHandle: String? private let windowID: CGWindowID + /// Frames go here while the clip runs; `outputURL` only appears once the + /// clip closes cleanly. A failed, interrupted or killed recording therefore + /// never leaves a broken file where the caller asked for the clip, and an + /// existing file there survives until there is something to replace it with. + private let workingURL: URL private var filter: SCContentFilter? private var writer: WindowRecordingFrameWriter? private var geometry: WindowRecordingFrameGeometry? @@ -101,6 +109,12 @@ actor WindowRecordingSession { private var startUptime = ProcessInfo.processInfo.systemUptime private var lastOffsetSeconds: Double = 0 private var loop: Task? + // Appends and the final close must not interleave: this actor suspends + // inside every append, so a `stop` arriving mid-append would otherwise + // close the file underneath it, and AVFoundation answers a frame appended + // after that with an exception no caller can catch. + private var writerBusy = false + private var writerWaiters: [CheckedContinuation] = [] init( id: String, @@ -114,9 +128,18 @@ actor WindowRecordingSession { self.outputURL = outputURL self.windowID = windowID self.windowHandle = windowHandle + workingURL = Self.workingURL(for: outputURL, id: id) captions = WindowRecordingCaptionTrack() } + /// A hidden sibling of the clip, named after the recording so two sessions + /// and a stale leftover cannot be confused for each other. + private static func workingURL(for outputURL: URL, id: String) -> URL { + outputURL.deletingLastPathComponent().appendingPathComponent( + ".\(outputURL.lastPathComponent).recording-\(id).partial" + ) + } + var isRecording: Bool { state == .recording } @@ -154,7 +177,14 @@ actor WindowRecordingSession { geometry = planned writer = try makeWriter(geometry: planned) startUptime = ProcessInfo.processInfo.systemUptime - try await write(sample: sample, offsetSeconds: 0) + do { + try await write(sample: sample, offsetSeconds: 0) + } catch { + // A first frame that cannot be written leaves an open writer and a + // partial file behind; close and discard both before giving up. + await fail(error) + throw error + } loop = Task { [weak self] in await self?.run() } @@ -207,7 +237,11 @@ actor WindowRecordingSession { } private func write(sample: Sample, offsetSeconds: Double) async throws { - guard let writer, let geometry else { throw WindowRecordingSessionError.alreadyFinished } + await acquireWriter() + defer { releaseWriter() } + guard state == .recording, let writer, let geometry else { + throw WindowRecordingSessionError.alreadyFinished + } // A window resized mid-clip is re-cropped into the frame size the clip // opened with, rather than ending the recording. let planned = try WindowRecordingFrameGeometry.plan( @@ -228,6 +262,8 @@ actor WindowRecordingSession { } private func finalize() async { + await acquireWriter() + defer { releaseWriter() } guard state == .recording else { return } guard let writer else { state = .failed @@ -237,27 +273,66 @@ actor WindowRecordingSession { self.writer = nil do { try await writer.finish() + try promote() state = .finished } catch { state = .failed failure = error.localizedDescription - try? FileManager.default.removeItem(at: outputURL) + try? FileManager.default.removeItem(at: workingURL) } } private func fail(_ error: Error) async { + await acquireWriter() + defer { releaseWriter() } + // A clip that already closed keeps its own verdict: a stop racing a + // capture failure must not turn a finished clip into a failed one. + guard state == .recording else { return } // Keep whatever was captured before the failure: a clip that ends when // the window closes is still evidence of what happened before that. if let writer { self.writer = nil - if frames > 0 { - try? await writer.finish() + do { + if frames > 0 { + try await writer.finish() + try promote() + } + } catch { + // Nothing usable was written; the partial file goes away below. } } + try? FileManager.default.removeItem(at: workingURL) state = .failed failure = error.localizedDescription } + /// Moves the finished clip to the path the caller asked for. + private func promote() throws { + let manager = FileManager.default + if manager.fileExists(atPath: outputURL.path) { + _ = try manager.replaceItemAt(outputURL, withItemAt: workingURL) + } else { + try manager.moveItem(at: workingURL, to: outputURL) + } + } + + /// Only one operation touches the writer at a time. An actor alone is not + /// enough: every append suspends this actor, which lets `stop` in. + private func acquireWriter() async { + while writerBusy { + await withCheckedContinuation { (continuation: CheckedContinuation) in + writerWaiters.append(continuation) + } + } + writerBusy = true + } + + private func releaseWriter() { + writerBusy = false + guard !writerWaiters.isEmpty else { return } + writerWaiters.removeFirst().resume() + } + private func makeWriter( geometry: WindowRecordingFrameGeometry ) throws -> WindowRecordingFrameWriter { @@ -265,19 +340,25 @@ actor WindowRecordingSession { at: outputURL.deletingLastPathComponent(), withIntermediateDirectories: true ) - try? FileManager.default.removeItem(at: outputURL) + // A directory, socket or device at the output path is not something a + // recording gets to delete on the caller's behalf. + if let attributes = try? FileManager.default.attributesOfItem(atPath: outputURL.path), + let type = attributes[.type] as? FileAttributeType, + type != .typeRegular, type != .typeSymbolicLink { + throw WindowRecordingSessionError.outputNotAFile(outputURL.path) + } + try? FileManager.default.removeItem(at: workingURL) switch request.format { case .mp4: return try WindowRecordingMP4Writer( - url: outputURL, + url: workingURL, width: geometry.outputWidth, height: geometry.outputHeight, framesPerSecond: request.framesPerSecond ) case .gif: return try WindowRecordingGIFWriter( - url: outputURL, - frameBudget: request.frameBudget, + url: workingURL, framesPerSecond: request.framesPerSecond ) } @@ -310,6 +391,12 @@ actor WindowRecordingSession { private func sample() async throws -> Sample { guard let filter else { throw WindowRecordingSessionError.windowGone } let info = SCShareableContent.info(for: filter) + // A window closed mid-clip reports an empty rectangle rather than an + // error, and capturing that gives a one-pixel frame stretched over the + // whole clip. End the recording instead and keep what came before. + guard info.contentRect.width > 1, info.contentRect.height > 1 else { + throw WindowRecordingSessionError.windowGone + } let pixelScale = Double(info.pointPixelScale) > 0 ? Double(info.pointPixelScale) : 1 let configuration = SCStreamConfiguration() configuration.width = max(1, Int((Double(info.contentRect.width) * pixelScale).rounded(.up))) diff --git a/cmuxTests/WindowRecordingPipelineTests.swift b/cmuxTests/WindowRecordingPipelineTests.swift index d47611e21abe..cc5672eb3cba 100644 --- a/cmuxTests/WindowRecordingPipelineTests.swift +++ b/cmuxTests/WindowRecordingPipelineTests.swift @@ -278,7 +278,7 @@ import Testing @Test func aGIFHoldsEveryFrameAndItsMeasuredDelay() async throws { let url = Self.temporaryURL(extension: "gif") defer { try? FileManager.default.removeItem(at: url) } - let writer = try WindowRecordingGIFWriter(url: url, frameBudget: 3, framesPerSecond: 4) + let writer = try WindowRecordingGIFWriter(url: url, framesPerSecond: 4) try await writer.append(Self.image(width: 80, height: 60, gray: 1), atOffsetSeconds: 0) try await writer.append(Self.image(width: 80, height: 60, gray: 0.5), atOffsetSeconds: 0.5) @@ -296,7 +296,7 @@ import Testing @Test func aGIFDelayStaysInThePlayableRange() async throws { let url = Self.temporaryURL(extension: "gif") defer { try? FileManager.default.removeItem(at: url) } - let writer = try WindowRecordingGIFWriter(url: url, frameBudget: 3, framesPerSecond: 8) + let writer = try WindowRecordingGIFWriter(url: url, framesPerSecond: 8) try await writer.append(Self.image(width: 40, height: 40), atOffsetSeconds: 0) // A 40 second gap would stall a viewer; a zero gap would be dropped. @@ -314,7 +314,7 @@ import Testing @Test func aGIFStoppedLongBeforeItsBudgetStillFinalizes() async throws { let url = Self.temporaryURL(extension: "gif") defer { try? FileManager.default.removeItem(at: url) } - let writer = try WindowRecordingGIFWriter(url: url, frameBudget: 240, framesPerSecond: 8) + let writer = try WindowRecordingGIFWriter(url: url, framesPerSecond: 8) for step in 0..<3 { try await writer.append( @@ -332,7 +332,7 @@ import Testing @Test func aGIFWithNoFramesFails() async throws { let url = Self.temporaryURL(extension: "gif") defer { try? FileManager.default.removeItem(at: url) } - let writer = try WindowRecordingGIFWriter(url: url, frameBudget: 1, framesPerSecond: 8) + let writer = try WindowRecordingGIFWriter(url: url, framesPerSecond: 8) await #expect(throws: WindowRecordingWriterError.noFrames) { try await writer.finish() diff --git a/docs/cli-contract.md b/docs/cli-contract.md index 079dfded114e..f6e5539e3bfb 100644 --- a/docs/cli-contract.md +++ b/docs/cli-contract.md @@ -157,7 +157,7 @@ Environment: | `current-workspace` | Print current workspace information. | | `read-selection` | Read the active selection from a terminal, file preview, Markdown, or browser surface. Plain output includes available source context; `--json` returns the complete socket response. | | `read-screen` | Read terminal text from a surface. `--selection` is a text-only compatibility alias for `read-selection`. | -| `record` | Record a cmux window or a region of one to an mp4 or gif (`window.record.*`). `start` returns a recording id and the output path, `stop` closes the clip, `status` reports progress, `note` adds a caption drawn into later frames, `list` shows the current and recent recordings. One recording at a time; a recording stops itself at `--max-seconds`. Local socket only: `window.record.*` is not on the `cmux ssh` relay allowlist. | +| `record` | Record a cmux window or a region of one to an mp4 or gif (`window.record.*`). `start` returns a recording id and the output path, `stop` closes the clip, `status` reports progress, `note` adds a caption drawn into later frames, `list` shows the current and recent recordings. One recording at a time; a recording stops itself at `--max-seconds`. The clip appears at its path when the recording ends, so an existing file there is replaced only once there is a finished clip to replace it with, and a recording that never closes leaves the path alone. Local socket only: `window.record.*` is not on the `cmux ssh` relay allowlist. | | `send` | Send text to a terminal surface as keystrokes (`surface.send_text`). `--paste`, before the text, sends it unchanged through the Cmd+V paste path (`terminal.paste`) instead, like `cmux paste`. Without `--paste`, large multi-line text prints a hint on stderr recommending it. | | `send-key` | Send one key to a terminal surface. | | `paste` | Paste text from an argument or stdin into a terminal surface through the Cmd+V paste path (`terminal.paste`). The CLI sends the text unchanged; Ghostty brackets it when the program enabled bracketed paste (otherwise newlines become Enter) and replaces unsafe control bytes with spaces. `--submit` presses the agent-aware submit key afterwards. Local socket only: `terminal.paste` is not on the `cmux ssh` relay allowlist. | From a2fa353592ada26523c5d59ace98f505f54e3201 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Mon, 28 Sep 2026 03:03:01 -0700 Subject: [PATCH 05/15] Refuse a record flag that swallowed the next flag as its value `cmux record start --label --gif` named the clip "--gif" and recorded an mp4, because the shared option parser takes whatever follows a flag. The record command now hands a value that looks like a flag back to the unexpected-arguments check, which names both, and the positional recording id for `stop`/`status` no longer accepts one either. Co-Authored-By: Claude Opus 5.5 (1M context) --- CLI/CMUXCLI+Record.swift | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/CLI/CMUXCLI+Record.swift b/CLI/CMUXCLI+Record.swift index e2c5c0ddb357..9c8b76e161d5 100644 --- a/CLI/CMUXCLI+Record.swift +++ b/CLI/CMUXCLI+Record.swift @@ -65,8 +65,8 @@ extension CMUXCLI { windowOverride: windowOverride ) case "stop", "status": - var (recordingID, trailing) = parseOption(arguments, name: "--id") - if recordingID == nil, let positional = trailing.first { + var (recordingID, trailing) = parseRecordOption(arguments, name: "--id") + if recordingID == nil, let positional = trailing.first, !positional.hasPrefix("-") { recordingID = positional trailing = Array(trailing.dropFirst()) } @@ -82,7 +82,7 @@ extension CMUXCLI { jsonOutput: jsonOutput ) case "note": - let (id, trailing) = parseOption(arguments, name: "--id") + let (id, trailing) = parseRecordOption(arguments, name: "--id") let text = trailing.filter { $0 != "--" }.joined(separator: " ") .trimmingCharacters(in: .whitespacesAndNewlines) guard !text.isEmpty else { @@ -125,15 +125,15 @@ extension CMUXCLI { jsonOutput: Bool, windowOverride: String? ) throws { - let (format, afterFormat) = parseOption(arguments, name: "--format") - let (fps, afterFPS) = parseOption(afterFormat, name: "--fps") - let (maxSeconds, afterSeconds) = parseOption(afterFPS, name: "--max-seconds") - let (scale, afterScale) = parseOption(afterSeconds, name: "--scale") - let (maxWidth, afterWidth) = parseOption(afterScale, name: "--max-width") - let (region, afterRegion) = parseOption(afterWidth, name: "--region") - let (out, afterOut) = parseOption(afterRegion, name: "--out") - let (label, afterLabel) = parseOption(afterOut, name: "--label") - let (window, afterWindow) = parseOption(afterLabel, name: "--window") + let (format, afterFormat) = parseRecordOption(arguments, name: "--format") + let (fps, afterFPS) = parseRecordOption(afterFormat, name: "--fps") + let (maxSeconds, afterSeconds) = parseRecordOption(afterFPS, name: "--max-seconds") + let (scale, afterScale) = parseRecordOption(afterSeconds, name: "--scale") + let (maxWidth, afterWidth) = parseRecordOption(afterScale, name: "--max-width") + let (region, afterRegion) = parseRecordOption(afterWidth, name: "--region") + let (out, afterOut) = parseRecordOption(afterRegion, name: "--out") + let (label, afterLabel) = parseRecordOption(afterOut, name: "--label") + let (window, afterWindow) = parseRecordOption(afterLabel, name: "--window") var trailing = afterWindow let wantsGIF = trailing.contains("--gif") @@ -175,6 +175,15 @@ extension CMUXCLI { ) } + /// A flag where a value belongs is a typo rather than a value: plain + /// `parseOption` would let `--label --gif` name the clip "--gif" and quietly + /// drop the format. Hand both back so the caller's error names them. + private func parseRecordOption(_ args: [String], name: String) -> (String?, [String]) { + let (value, remaining) = parseOption(args, name: name) + guard let value, value.hasPrefix("--") else { return (value, remaining) } + return (nil, [name, value] + remaining) + } + private func requireNoRecordArguments(_ trailing: [String], subcommand: String) throws { guard trailing.isEmpty else { throw CLIError(message: String( From a2195d1b6e106a3070f697e435ee493bb20de7dc Mon Sep 17 00:00:00 2001 From: Leo Li Date: Mon, 28 Sep 2026 04:29:07 -0700 Subject: [PATCH 06/15] Map an unusable recording output path to invalid_params `outputNotAFile` was added to `WindowRecordingSessionError` in the fix commit for the output-path handling, but the router's switch over that enum was never extended, so the app target stopped compiling: Sources/TerminalController+WindowRecording.swift:175:13: error: switch must be exhaustive Nothing on the pull request caught it. The checks that run on a pull request build the packages and the tooling, not the app target, so this first appeared in a dispatched UI test run: https://github.com/manaflow-ai/cmux/actions/runs/36412249068 The missing case is `invalid_params`: `--out` naming a directory, a device or anything else the recorder may not replace is the caller's parameter, not a cmux failure, and an agent that reads `internal_error` retries instead of fixing its flag. The mapping had no test at all, which is why a missing case could sit here. `recordingErrorCode(for:)` is now internal and every case of the three error types it understands is pinned, including the unrecognized fallback. A compile error cannot be committed as a red test first, since the test would not build either; the build log above is the red evidence, and the test is what keeps the codes from drifting. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../TerminalController+WindowRecording.swift | 11 ++- cmux.xcodeproj/project.pbxproj | 4 + cmuxTests/WindowRecordingErrorCodeTests.swift | 74 +++++++++++++++++++ 3 files changed, 88 insertions(+), 1 deletion(-) create mode 100644 cmuxTests/WindowRecordingErrorCodeTests.swift diff --git a/Sources/TerminalController+WindowRecording.swift b/Sources/TerminalController+WindowRecording.swift index ff77e3fc6cdb..57352824fc97 100644 --- a/Sources/TerminalController+WindowRecording.swift +++ b/Sources/TerminalController+WindowRecording.swift @@ -162,7 +162,12 @@ extension TerminalController { } } - private nonisolated static func recordingErrorCode(for error: Error) -> String { + /// Maps a recorder error onto the socket error code the caller sees. + /// + /// Internal rather than private so a test can pin every case: the codes are + /// what a CLI or a tour branches on, and "a recording is already running" + /// has to stay distinguishable from "your output path is wrong". + nonisolated static func recordingErrorCode(for error: Error) -> String { if let failure = error as? WindowRecordingRegistry.Failure { switch failure { case .busy: @@ -177,6 +182,10 @@ extension TerminalController { return "unsupported" case .windowGone: return "not_found" + case .outputNotAFile: + // The caller's `out` names something that is not a file the + // recorder may replace, so this is their parameter, not our bug. + return "invalid_params" case .captureFailed, .composeFailed, .alreadyFinished: return "internal_error" } diff --git a/cmux.xcodeproj/project.pbxproj b/cmux.xcodeproj/project.pbxproj index 3df5f056fb66..87382b4a3596 100644 --- a/cmux.xcodeproj/project.pbxproj +++ b/cmux.xcodeproj/project.pbxproj @@ -3824,6 +3824,7 @@ 8770D0F2D2BB45359D6BE5E3 /* WindowKeyDownReplayGuardTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B79482F1ECA54E98BE5C8953 /* WindowKeyDownReplayGuardTests.swift */; }; 129170000000000000000001 /* WindowOverlayChromeTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 129170000000000000000002 /* WindowOverlayChromeTests.swift */; }; 973DC7D4CD41516D459E17FC /* WindowPointHitTestTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02BCEFD7668C02FB795ED26F /* WindowPointHitTestTests.swift */; }; + 3D9E5674AF892C5F1A2C7720 /* WindowRecordingErrorCodeTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 04A01C2D99463F9AB538969F /* WindowRecordingErrorCodeTests.swift */; }; 9065A1020000000000000001 /* WindowRecordingFrameComposer.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9065A1020000000000000002 /* WindowRecordingFrameComposer.swift */; }; 9065A1030000000000000001 /* WindowRecordingFrameWriter.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9065A1030000000000000002 /* WindowRecordingFrameWriter.swift */; }; A6CC55B334F22314E39BE6A6 /* WindowRecordingPipelineTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7AB0A04BC0DF676A62F70094 /* WindowRecordingPipelineTests.swift */; }; @@ -7848,6 +7849,7 @@ B79482F1ECA54E98BE5C8953 /* WindowKeyDownReplayGuardTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WindowKeyDownReplayGuardTests.swift; sourceTree = ""; }; 129170000000000000000002 /* WindowOverlayChromeTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WindowOverlayChromeTests.swift; sourceTree = ""; }; 02BCEFD7668C02FB795ED26F /* WindowPointHitTestTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = WindowPointHitTestTests.swift; sourceTree = ""; }; + 04A01C2D99463F9AB538969F /* WindowRecordingErrorCodeTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "WindowRecordingErrorCodeTests.swift"; sourceTree = ""; }; 9065A1020000000000000002 /* WindowRecordingFrameComposer.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WindowRecordingFrameComposer.swift; sourceTree = ""; }; 9065A1030000000000000002 /* WindowRecordingFrameWriter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WindowRecordingFrameWriter.swift; sourceTree = ""; }; 7AB0A04BC0DF676A62F70094 /* WindowRecordingPipelineTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "WindowRecordingPipelineTests.swift"; sourceTree = ""; }; @@ -12332,6 +12334,7 @@ 23E35F95320CBED308AD98CF /* CloudTreeTogglePerformanceTests.swift */, 0C37EA14DE74095FB1BB23E1 /* CloudTreeDisclosureIntentTests.swift */, 7AB0A04BC0DF676A62F70094 /* WindowRecordingPipelineTests.swift */, + 04A01C2D99463F9AB538969F /* WindowRecordingErrorCodeTests.swift */, ); path = cmuxTests; sourceTree = ""; @@ -16957,6 +16960,7 @@ 8770D0F2D2BB45359D6BE5E3 /* WindowKeyDownReplayGuardTests.swift in Sources */, 129170000000000000000001 /* WindowOverlayChromeTests.swift in Sources */, 973DC7D4CD41516D459E17FC /* WindowPointHitTestTests.swift in Sources */, + 3D9E5674AF892C5F1A2C7720 /* WindowRecordingErrorCodeTests.swift in Sources */, A6CC55B334F22314E39BE6A6 /* WindowRecordingPipelineTests.swift in Sources */, 606600010000000000000001 /* WindowTerminalHostViewTitlebarHitTests.swift in Sources */, 604500100000000000000003 /* WindowTitleTemplateTests.swift in Sources */, diff --git a/cmuxTests/WindowRecordingErrorCodeTests.swift b/cmuxTests/WindowRecordingErrorCodeTests.swift new file mode 100644 index 000000000000..c92f52bdbc9c --- /dev/null +++ b/cmuxTests/WindowRecordingErrorCodeTests.swift @@ -0,0 +1,74 @@ +import CmuxFoundation +import Foundation +import Testing + +#if canImport(cmux_DEV) +@testable import cmux_DEV +#elseif canImport(cmux) +@testable import cmux +#endif + +/// Pins the socket error code every recorder failure comes back as. +/// +/// A CLI, a dogfood tour and an agent all branch on the code rather than the +/// message: `conflict` means "stop the other recording and retry", `not_found` +/// means "pick another window", `invalid_params` means "fix your flags" and +/// `internal_error` means "this is cmux's fault". Getting one wrong sends a +/// caller down the wrong path, and a case added without a mapping here reads as +/// an internal error even when the caller could fix it. +@Suite struct WindowRecordingErrorCodeTests { + @Test func registryFailuresSeparateConflictFromNotFound() { + #expect(TerminalController.recordingErrorCode(for: WindowRecordingRegistry.Failure.busy("abc")) == "conflict") + #expect(TerminalController.recordingErrorCode(for: WindowRecordingRegistry.Failure.noRecording) == "not_found") + #expect( + TerminalController.recordingErrorCode( + for: WindowRecordingRegistry.Failure.unknownRecording("abc") + ) == "not_found" + ) + #expect( + TerminalController.recordingErrorCode( + for: WindowRecordingRegistry.Failure.alreadyStopped("abc") + ) == "not_found" + ) + } + + @Test func sessionFailuresReportWhoseFaultTheyAre() { + #expect(TerminalController.recordingErrorCode(for: WindowRecordingSessionError.unsupportedSystem) == "unsupported") + #expect(TerminalController.recordingErrorCode(for: WindowRecordingSessionError.windowGone) == "not_found") + #expect(TerminalController.recordingErrorCode(for: WindowRecordingSessionError.composeFailed) == "internal_error") + #expect(TerminalController.recordingErrorCode(for: WindowRecordingSessionError.alreadyFinished) == "internal_error") + #expect( + TerminalController.recordingErrorCode( + for: WindowRecordingSessionError.captureFailed("no window server") + ) == "internal_error" + ) + } + + /// The case the router's switch was missing: a caller who points `--out` at + /// a directory gets told their parameter is wrong, not that cmux broke. + @Test func anUnusableOutputPathIsTheCallersParameter() { + #expect( + TerminalController.recordingErrorCode( + for: WindowRecordingSessionError.outputNotAFile("/tmp") + ) == "invalid_params" + ) + } + + @Test func geometryFailuresAreParameterFailures() { + #expect( + TerminalController.recordingErrorCode( + for: WindowRecordingFrameGeometry.Failure.regionOutsideWindow + ) == "invalid_params" + ) + #expect( + TerminalController.recordingErrorCode( + for: WindowRecordingFrameGeometry.Failure.emptyWindow + ) == "invalid_params" + ) + } + + @Test func anUnrecognizedErrorStaysAnInternalError() { + struct Surprise: Error {} + #expect(TerminalController.recordingErrorCode(for: Surprise()) == "internal_error") + } +} From a1252ebae7743d3718fec8f263397869e1a77731 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Mon, 28 Sep 2026 03:36:13 -0700 Subject: [PATCH 07/15] Record a clip around dogfood tour steps A screenshot cannot show a drag, an animation, or the order two views settle in, which is most of what a UI change is judged on. A tour can now wrap the steps that matter in a recording: {"record": "sidebar-drag", "format": "gif", "maxSeconds": 30, "fps": 8, "scale": 0.5, "steps": [{"note": "dragging"}, {"clickAt": {...}}]} The step starts `window.record.start` over the control socket with `out` set to a file in the test runner's own temporary directory, the one place both processes can reach, runs the nested steps, stops the recording and attaches the clip with `keepAlways`, so it arrives beside the screenshots and trees of the same run. `note` draws a caption into the clip, which a clip needs because it has no step list beside it. A nested step that fails is recorded and the rest still run, as at the top level: the recording is always stopped, so the clip of the failure survives rather than being lost with it. Nested steps are numbered 03.1, 03.2 in steps.log. The decoder refuses a recording inside a recording, since the app records one window at a time, and refuses an unknown option rather than dropping it: a silently ignored "maxseconds" would cut a tour's clip short. tests/test_dogfood_scenarios.py checks the tours and the skill reference against the decoder in a second rather than after a CI run: every step kind the decoder accepts is documented and no documented step is undecoded, every recording option is documented, and every checked-in tour uses only known steps, known recording options, no nested recording and no empty recording. Both scenario arms were confirmed by breaking the new tour: renaming maxSeconds to maxseconds and nesting a record step each fail it, and it passes again with them restored. Changelog: none Co-Authored-By: Claude Opus 5.5 (1M context) --- .github/workflows/ci-guards.yml | 1 + cmuxUITests/DogfoodScenarioUITests.swift | 148 ++++++++++++++++-- .../record-split-and-palette-tour.json | 27 ++++ .../references/dogfood-scenarios.md | 35 +++++ tests/test-execution.toml | 4 + tests/test_dogfood_scenarios.py | 115 ++++++++++++++ 6 files changed, 319 insertions(+), 11 deletions(-) create mode 100644 dogfood/scenarios/record-split-and-palette-tour.json create mode 100644 tests/test_dogfood_scenarios.py diff --git a/.github/workflows/ci-guards.yml b/.github/workflows/ci-guards.yml index 983a5d85269a..5f8072d97a97 100644 --- a/.github/workflows/ci-guards.yml +++ b/.github/workflows/ci-guards.yml @@ -598,6 +598,7 @@ jobs: if: ${{ matrix.group == 'app-host-execution' }} run: | python3 tests/test_run_e2e.py + python3 tests/test_dogfood_scenarios.py python3 tests/test_ci_e2e_compilation_cache.py python3 tests/test_ci_focused_test_selectors.py diff --git a/cmuxUITests/DogfoodScenarioUITests.swift b/cmuxUITests/DogfoodScenarioUITests.swift index 6e9636c29678..28effbe548a8 100644 --- a/cmuxUITests/DogfoodScenarioUITests.swift +++ b/cmuxUITests/DogfoodScenarioUITests.swift @@ -164,17 +164,14 @@ final class DogfoodScenarioUITests: XCTestCase { case .menu(let path): try clickMenu(path, in: app) case .socket(let method, let params, let saveAs): - let resolved = substitute(params) - guard let response = socketRequest(method: method, params: resolved) else { - throw DogfoodError("no reply from \(method): \(lastSocketError)") - } - attachText(prettyJSON(response), name: "\(label)-\(method).json") - guard response["ok"] as? Bool == true else { - throw DogfoodError("\(method) failed: \(prettyJSON(response["error"] ?? response))") - } + let result = try callSocket(method: method, params: params, label: label) if let saveAs { - saved[saveAs] = response["result"] ?? [:] + saved[saveAs] = result } + case .record(let name, let params, let steps): + try runRecording(name: name, params: params, steps: steps, label: label, app: app) + case .note(let text): + _ = try callSocket(method: "window.record.note", params: ["text": text], label: label) case .expect(let target, let exists): let matches = query(target, in: app) let element = target.index.map { matches.element(boundBy: $0) } ?? matches.firstMatch @@ -185,6 +182,89 @@ final class DogfoodScenarioUITests: XCTestCase { } } + /// A v2 request whose reply is attached and whose failure stops the step. + @discardableResult + private func callSocket(method: String, params: Any, label: String, + suffix: String = "") throws -> [String: Any] { + guard let response = socketRequest(method: method, params: substitute(params)) else { + throw DogfoodError("no reply from \(method): \(lastSocketError)") + } + attachText(prettyJSON(response), name: "\(label)\(suffix)-\(method).json") + guard response["ok"] as? Bool == true else { + throw DogfoodError("\(method) failed: \(prettyJSON(response["error"] ?? response))") + } + return response["result"] as? [String: Any] ?? [:] + } + + // MARK: Recording + + /// Records the window while the nested steps run, and attaches the clip. + /// + /// A nested step that fails is recorded and the rest still run, as at the + /// top level, so the recording always gets stopped and a clip of the + /// failure is kept rather than lost with it. + private func runRecording(name: String, params: [String: Any], steps: [DogfoodStep], + label: String, app: XCUIApplication) throws { + let format = (params["format"] as? String)?.lowercased() == "gif" ? "gif" : "mp4" + let clipName = "\(label)-\(Self.fileSafe(name))" + // The app writes the clip and this process reads it: its own temporary + // directory is the one place both sides can reach, as with the socket. + let clip = FileManager.default.temporaryDirectory + .appendingPathComponent("\(clipName).\(format)") + try? FileManager.default.removeItem(at: clip) + + var start = params + start["out"] = clip.path + let started = try callSocket(method: "window.record.start", params: start, label: label) + let id = started["id"] as? String + log.append("\(label) recording \(name) as \(id ?? "?") to \(clip.lastPathComponent)") + + for (index, step) in steps.enumerated() { + let nested = "\(label).\(index + 1)" + do { + try run(step, label: nested, app: app) + log.append("\(nested) ok \(step.summary)") + } catch { + record(failure: "step \(nested) \(step.summary): \(error)") + shot("\(nested)-failed", app: app) + } + } + + let stopped = try callSocket(method: "window.record.stop", + params: id.map { ["id": $0] } ?? [:], + label: label, suffix: "-stop") + attachClip(named: clipName, format: format, fallback: clip, status: stopped) + } + + /// The clip itself, so the run page carries the motion and not just frames. + private func attachClip(named name: String, format: String, fallback: URL, + status: [String: Any]) { + let url = (status["path"] as? String).map { URL(fileURLWithPath: $0) } ?? fallback + let described = "state \(status["state"] as? String ?? "?"), frames \(status["frames"] as? Int ?? -1)" + guard let data = try? Data(contentsOf: url), !data.isEmpty else { + record(failure: "recording \(name) left no readable clip at \(url.lastPathComponent) (\(described))") + return + } + let attachment = XCTAttachment( + data: data, + uniformTypeIdentifier: format == "gif" ? "com.compuserve.gif" : "public.mpeg-4" + ) + attachment.name = "\(name).\(format)" + attachment.lifetime = .keepAlways + add(attachment) + log.append("\(name).\(format): \(described), \(data.count) bytes") + try? FileManager.default.removeItem(at: url) + } + + private static func fileSafe(_ name: String) -> String { + let kept = name.map { character -> Character in + character.isLetter || character.isNumber || character == "-" || character == "_" + ? character : "-" + } + let joined = String(kept).trimmingCharacters(in: CharacterSet(charactersIn: "-")) + return joined.isEmpty ? "clip" : joined + } + private func query(_ target: DogfoodTarget, in app: XCUIApplication) -> XCUIElementQuery { let all = app.descendants(matching: target.elementType) switch target.match { @@ -449,6 +529,8 @@ enum DogfoodStep { case hoverAt(Double, Double) case menu([String]) case socket(method: String, params: Any, saveAs: String?) + case record(name: String, params: [String: Any], steps: [DogfoodStep]) + case note(String) case expect(DogfoodTarget, exists: Bool) var summary: String { @@ -466,12 +548,21 @@ enum DogfoodStep { case .hoverAt(let x, let y): return "hoverAt \(x),\(y)" case .menu(let path): return "menu \(path.joined(separator: " > "))" case .socket(let method, _, _): return "socket \(method)" + case .record(let name, _, let steps): return "record \(name) around \(steps.count) steps" + case .note(let text): return "note \(text.debugDescription)" case .expect(let target, let exists): return "expect \(target) exists=\(exists)" } } var usesSocket: Bool { - if case .socket = self { return true } + switch self { + case .socket, .record, .note: return true + default: return false + } + } + + var isRecord: Bool { + if case .record = self { return true } return false } @@ -506,6 +597,20 @@ enum DogfoodStep { case "socket": guard let method = value as? String else { throw DogfoodError("socket takes a method name") } self = .socket(method: method, params: object["params"] ?? [String: Any](), saveAs: object["save"] as? String) + case "record": + guard let name = value as? String, !name.isEmpty else { + throw DogfoodError("record takes a name for the clip") + } + let nested = try (object["steps"] as? [Any] ?? []).map(DogfoodStep.init(json:)) + guard !nested.contains(where: { $0.isRecord }) else { + throw DogfoodError("record cannot contain another record: the app records one window at a time") + } + self = .record(name: name, params: try Self.recordParams(object), steps: nested) + case "note": + guard let text = value as? String, !text.isEmpty else { + throw DogfoodError("note takes the caption to draw into the clip") + } + self = .note(text) case "expect": self = .expect(try DogfoodTarget(json: value), exists: object["exists"] as? Bool ?? true) default: @@ -515,9 +620,30 @@ enum DogfoodStep { private static let kinds: Set = [ "shot", "tree", "wait", "type", "key", "click", "doubleClick", "rightClick", - "hover", "clickAt", "hoverAt", "menu", "socket", "expect", + "hover", "clickAt", "hoverAt", "menu", "socket", "record", "note", "expect", + ] + + /// Recording options, named as the tour spells them and passed to the app + /// as they were given: the app owns the limits, so the tour cannot disagree + /// with it about what a valid frame rate is. An unknown key is a typo, and + /// a silently dropped `maxseconds` would cut a tour's clip short. + private static let recordOptions: [String: String] = [ + "format": "format", "fps": "fps", "maxSeconds": "max_seconds", "scale": "scale", + "maxWidth": "max_width", "region": "region", "captions": "captions", "label": "label", ] + private static func recordParams(_ object: [String: Any]) throws -> [String: Any] { + var params: [String: Any] = [:] + for (key, value) in object where key != "record" && key != "steps" { + guard let name = recordOptions[key] else { + throw DogfoodError("unknown record option \(key); use one of " + + recordOptions.keys.sorted().joined(separator: ", ")) + } + params[name] = value + } + return params + } + private static func key(named name: String) -> String { switch name.lowercased() { case "return", "enter": return XCUIKeyboardKey.return.rawValue diff --git a/dogfood/scenarios/record-split-and-palette-tour.json b/dogfood/scenarios/record-split-and-palette-tour.json new file mode 100644 index 000000000000..73fb21f329e2 --- /dev/null +++ b/dogfood/scenarios/record-split-and-palette-tour.json @@ -0,0 +1,27 @@ +{ + "steps": [ + {"socket": "workspace.create", "params": {"title": "Record", "initial_input": "echo recording\n", "focus": true}, "save": "record"}, + {"wait": 1}, + {"shot": "before"}, + {"record": "split-and-palette", "format": "gif", "maxSeconds": 30, "fps": 8, "scale": 0.5, "steps": [ + {"note": "split right"}, + {"key": "d", "modifiers": ["command"]}, + {"wait": 1.5}, + {"shot": "split-right"}, + {"note": "command palette"}, + {"key": "p", "modifiers": ["command", "shift"]}, + {"wait": 0.5}, + {"type": "split"}, + {"wait": 1}, + {"shot": "palette-filtered"}, + {"key": "escape"}, + {"note": "hide the sidebar"}, + {"key": "b", "modifiers": ["command"]}, + {"wait": 1}, + {"key": "b", "modifiers": ["command"]}, + {"wait": 1} + ]}, + {"shot": "after"}, + {"socket": "window.record.list"} + ] +} diff --git a/skills/cmux-testing/references/dogfood-scenarios.md b/skills/cmux-testing/references/dogfood-scenarios.md index 7605aac32f6d..7eb183685830 100644 --- a/skills/cmux-testing/references/dogfood-scenarios.md +++ b/skills/cmux-testing/references/dogfood-scenarios.md @@ -52,6 +52,8 @@ A tour is a steps array, or an object with `steps` and an optional `launch`: | `{"clickAt": {"x": 0.1, "y": 0.2}}`, `hoverAt` | Acts on a point in the main window, 0 to 1 from the top left. | | `{"menu": ["File", "New Workspace"]}` | Clicks through the menu bar. | | `{"socket": "method", "params": {...}, "save": "name"}` | A v2 control socket request. The reply is attached; `save` keeps its `result`, and a later param `"${name.workspace_id}"` reads a field from it. | +| `{"record": "name", "steps": [...]}` | Records the window while the nested steps run and attaches the clip. See below. | +| `{"note": "dragging the workspace"}` | Draws a caption into the clip being recorded. | | `{"expect": target, "exists": false}` | Checks that an element exists (or not). | A target is an accessibility identifier string, or an object with `id`, @@ -59,6 +61,39 @@ A target is an accessibility identifier string, or an object with `id`, `staticText`, `menuItem`, `checkBox`, `image`, `group`, `cell`, `tab`, `window`, `popover`) and `index`. +## Record a clip + +A screenshot cannot show a drag, an animation or the order two views settle in. +Wrap the steps that matter in a `record` step and the tour attaches a clip of +them: + +```json +{"record": "sidebar-drag", "format": "gif", "maxSeconds": 30, "fps": 8, "scale": 0.5, "steps": [ + {"note": "dragging the workspace"}, + {"clickAt": {"x": 0.08, "y": 0.12}}, + {"wait": 1}, + {"shot": "mid-drag"} +]} +``` + +- Options: `format` (`mp4` or `gif`), `fps`, `maxSeconds`, `scale`, `maxWidth`, + `region` (`"x,y,w,h"` in window points), `captions` and `label`. They are the + flags of `cmux record start` and the app owns their limits, so an out of range + value is refused by name. An unknown option is a typo and fails the step. +- A `gif` at `fps` 8 and `scale` 0.5 is the one to paste into a PR; an `mp4` is + sharper and needs a click to play. `scripts/pr-media.py` uploads either and + converts an mp4 to a gif on the way. +- Nested steps behave as they do at the top level, including `shot`, and a nested + failure still lets the rest run so the recording is always stopped and the clip + of the failure survives. Nested steps are numbered `03.1`, `03.2` in `steps.log`. +- `note` draws a caption into the clip. It is worth one before each thing you + want a reviewer to notice, because a clip has no step list beside it. +- The clip lands in `attachments/` next to the trees and socket replies, named + after the step and the tour's name for it. +- The app records only its own windows, one at a time, and stops by itself at + `maxSeconds` (default 15, max 120). Keep `maxSeconds` above the time the nested + steps take, or the clip ends early. + The window is zoomed to fill the display at launch (`"zoom": false` keeps the default size). Every tour starts with `00-launched` and ends with `99-final` plus a whole-display `99-final-screen`, which shows anything the CI desktop put over diff --git a/tests/test-execution.toml b/tests/test-execution.toml index c9888f9b9ef8..396855b17b17 100644 --- a/tests/test-execution.toml +++ b/tests/test-execution.toml @@ -719,6 +719,10 @@ lane = "linux-guard" path = "tests/test_review_fabric.py" lane = "linux-guard" +[[test]] +path = "tests/test_dogfood_scenarios.py" +lane = "linux-guard" + [[test]] path = "tests/test_run_e2e.py" lane = "linux-guard" diff --git a/tests/test_dogfood_scenarios.py b/tests/test_dogfood_scenarios.py new file mode 100644 index 000000000000..055c6881f944 --- /dev/null +++ b/tests/test_dogfood_scenarios.py @@ -0,0 +1,115 @@ +#!/usr/bin/env python3 +"""Check dogfood tours and the step reference against the step decoder. + +A tour is read at run time by cmuxUITests/DogfoodScenarioUITests.swift, so a +typo in one is only found after a CI run that takes tens of minutes. These +checks find it in a second, and they keep the reference table and the recording +options in the skill honest about what the decoder accepts. +""" + +from __future__ import annotations + +import json +import re +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] +RUNNER = ROOT / "cmuxUITests/DogfoodScenarioUITests.swift" +REFERENCE = ROOT / "skills/cmux-testing/references/dogfood-scenarios.md" +SCENARIOS = sorted((ROOT / "dogfood/scenarios").glob("*.json")) + + +def swift_string_set(source: str, declaration: str) -> set[str]: + """The string literals of a Swift `let name: ... = [ ... ]` declaration.""" + match = re.search(re.escape(declaration) + r"\s*=\s*\[(.*?)\]", source, re.DOTALL) + assert match, f"{declaration} is no longer declared the way this test reads it" + return set(re.findall(r'"([^"]+)"', match.group(1))) + + +class DecoderFacts: + source = RUNNER.read_text() + kinds = swift_string_set(source, "private static let kinds: Set") + # The option map is "tourName": "socket_name" pairs; the tour names are the + # keys, which are the odd entries of the flattened literal list. + record_option_pairs = re.findall( + r'"([A-Za-z_]+)":\s*"([a-z_]+)"', + re.search(r"private static let recordOptions: \[String: String\] = \[(.*?)\]", + source, re.DOTALL).group(1), + ) + record_options = {tour for tour, _ in record_option_pairs} + + +class ReferenceTests(unittest.TestCase): + def test_every_step_the_decoder_accepts_is_in_the_reference_table(self): + text = REFERENCE.read_text() + # A row names its step as `{"click": target}` and its aliases as bare + # `doubleClick`, so both spellings count as documented. + documented = set(re.findall(r'`\{"(\w+)"', text)) | set(re.findall(r"`(\w+)`", text)) + missing = DecoderFacts.kinds - documented + self.assertEqual(missing, set(), + f"undocumented steps in {REFERENCE.name}: {sorted(missing)}") + + def test_the_reference_does_not_promise_a_step_the_decoder_refuses(self): + table = [line for line in REFERENCE.read_text().splitlines() if line.startswith("| `{")] + promised = {kind for line in table for kind in re.findall(r'`\{"(\w+)"', line)} + unknown = promised - DecoderFacts.kinds + self.assertEqual(unknown, set(), f"documented but not decoded: {sorted(unknown)}") + + def test_every_recording_option_is_documented(self): + text = REFERENCE.read_text() + for option in sorted(DecoderFacts.record_options): + self.assertIn(f"`{option}`", text, f"{option} is accepted but not documented") + + +class ScenarioTests(unittest.TestCase): + def test_there_are_scenarios_to_check(self): + self.assertTrue(SCENARIOS, "dogfood/scenarios holds the reusable tours") + + def test_every_scenario_only_uses_steps_the_decoder_accepts(self): + for path in SCENARIOS: + with self.subTest(scenario=path.name): + check_scenario(json.loads(path.read_text())) + + +def check_scenario(tour: object) -> None: + steps = tour if isinstance(tour, list) else (tour or {}).get("steps") + if not isinstance(steps, list) or not steps: + raise AssertionError("a tour is a steps array or an object with a steps array") + if isinstance(tour, dict): + unknown = set(tour) - {"steps", "launch"} + if unknown: + raise AssertionError(f"unknown top level keys: {sorted(unknown)}") + check_steps(steps, inside_record=False) + + +def check_steps(steps: list, inside_record: bool) -> None: + for index, step in enumerate(steps, start=1): + if not isinstance(step, dict): + raise AssertionError(f"step {index} is not an object") + kinds = DecoderFacts.kinds & set(step) + if len(kinds) != 1: + raise AssertionError( + f"step {index} names {sorted(kinds) or 'no step'}; each step is one of " + f"{', '.join(sorted(DecoderFacts.kinds))}" + ) + kind = kinds.pop() + if kind != "record": + continue + if inside_record: + raise AssertionError(f"step {index} records inside a recording") + options = set(step) - {"record", "steps"} + unknown = options - DecoderFacts.record_options + if unknown: + raise AssertionError( + f"step {index} passes unknown record options {sorted(unknown)}; " + f"use {', '.join(sorted(DecoderFacts.record_options))}" + ) + nested = step.get("steps") + if not isinstance(nested, list) or not nested: + raise AssertionError(f"step {index} records around no steps") + check_steps(nested, inside_record=True) + + +if __name__ == "__main__": + unittest.main() From 76bdd07d20aac1bcc03630d950c3ab5cb60c1ea6 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Mon, 28 Sep 2026 07:00:05 -0700 Subject: [PATCH 08/15] Keep a dogfood recording from gating the steps it records A review of the record step found three ways a recording could take the tour down with it. A `window.record.start` that failed threw out of the step runner before the nested loop, so on a machine that cannot capture at all -- a runner whose window ScreenCaptureKit cannot see, or a pre-14.4 system -- the split, palette and sidebar steps of a tour never ran, and the run looked like a window that was never touched rather than a recording that could not start. A recording is observability; it now reports the failure and runs its steps unrecorded. The stop was a plain `try` in the middle of the function, so it was skipped whenever anything before it threw, and its clip with it. It now runs on every path, including after a start whose reply never arrived: the app records one window at a time, so an abandoned slot would fail every later `record` step in the same tour with `conflict` for as long as `maxSeconds`. Stopping with empty params stops whatever is active, which is exactly what a start that timed out leaves behind. A stop that fails after a recording really started still attaches the clip, since the app moves the file into place as it closes the writer. The client deadline was 15 seconds for every method while the app allows itself 20 for start and 30 for stop, so a slow reply became this side's "no reply" rather than the app's own error. Deadlines are now per method and sit above the app's. Also: the clip's extension is derived with the same trimming the app applies to `format`, so `"format": " gif "` cannot name the file `.mp4` and then be refused for having the wrong extension; and a `note` outside a `record` is refused while decoding rather than failing at run time with `not_found`, since there is no clip for it to caption. Co-Authored-By: Claude Opus 5.5 (1M context) --- cmuxUITests/DogfoodScenarioUITests.swift | 92 +++++++++++++++++++----- 1 file changed, 76 insertions(+), 16 deletions(-) diff --git a/cmuxUITests/DogfoodScenarioUITests.swift b/cmuxUITests/DogfoodScenarioUITests.swift index 28effbe548a8..8d77ddbcdeb6 100644 --- a/cmuxUITests/DogfoodScenarioUITests.swift +++ b/cmuxUITests/DogfoodScenarioUITests.swift @@ -169,7 +169,7 @@ final class DogfoodScenarioUITests: XCTestCase { saved[saveAs] = result } case .record(let name, let params, let steps): - try runRecording(name: name, params: params, steps: steps, label: label, app: app) + runRecording(name: name, params: params, steps: steps, label: label, app: app) case .note(let text): _ = try callSocket(method: "window.record.note", params: ["text": text], label: label) case .expect(let target, let exists): @@ -200,12 +200,16 @@ final class DogfoodScenarioUITests: XCTestCase { /// Records the window while the nested steps run, and attaches the clip. /// - /// A nested step that fails is recorded and the rest still run, as at the - /// top level, so the recording always gets stopped and a clip of the - /// failure is kept rather than lost with it. + /// A recording is observability, so it never gates what it observes. A + /// nested step that fails is recorded and the rest still run, as at the top + /// level; a recording that cannot start at all is reported and its steps run + /// unrecorded; and the stop is attempted either way, because a start that + /// timed out may have begun a recording anyway and the app records one + /// window at a time, so an abandoned slot would fail every later `record` + /// step in the tour with `conflict`. private func runRecording(name: String, params: [String: Any], steps: [DogfoodStep], - label: String, app: XCUIApplication) throws { - let format = (params["format"] as? String)?.lowercased() == "gif" ? "gif" : "mp4" + label: String, app: XCUIApplication) { + let format = Self.clipFormat(params) let clipName = "\(label)-\(Self.fileSafe(name))" // The app writes the clip and this process reads it: its own temporary // directory is the one place both sides can reach, as with the socket. @@ -215,9 +219,17 @@ final class DogfoodScenarioUITests: XCTestCase { var start = params start["out"] = clip.path - let started = try callSocket(method: "window.record.start", params: start, label: label) - let id = started["id"] as? String - log.append("\(label) recording \(name) as \(id ?? "?") to \(clip.lastPathComponent)") + var id: String? + var started = false + do { + let reply = try callSocket(method: "window.record.start", params: start, label: label) + id = reply["id"] as? String + started = true + log.append("\(label) recording \(name) as \(id ?? "?") to \(clip.lastPathComponent)") + } catch { + record(failure: "recording \(name) did not start: \(error)") + log.append("\(label) recording \(name) unavailable; running its steps unrecorded") + } for (index, step) in steps.enumerated() { let nested = "\(label).\(index + 1)" @@ -230,10 +242,33 @@ final class DogfoodScenarioUITests: XCTestCase { } } - let stopped = try callSocket(method: "window.record.stop", - params: id.map { ["id": $0] } ?? [:], - label: label, suffix: "-stop") - attachClip(named: clipName, format: format, fallback: clip, status: stopped) + do { + // Empty params stop whatever is active, which is what a start whose + // reply never arrived leaves behind. + let stopped = try callSocket(method: "window.record.stop", + params: id.map { ["id": $0] } ?? [:], + label: label, suffix: "-stop") + attachClip(named: clipName, format: format, fallback: clip, status: stopped) + } catch { + guard started else { + // Nothing was running, so `not_found` here is the right answer + // and the start failure above is the one to read. + return + } + // A stop that errors or times out does not mean the clip is gone: + // the app moves the file into place as it closes the writer. + record(failure: "recording \(name) did not stop cleanly: \(error)") + attachClip(named: clipName, format: format, fallback: clip, status: [:]) + } + } + + /// The extension the clip will have, trimmed the way the app trims it so + /// `"format": " gif "` cannot name the file `.mp4` and be refused for it. + private static func clipFormat(_ params: [String: Any]) -> String { + let raw = (params["format"] as? String)? + .trimmingCharacters(in: .whitespacesAndNewlines) + .lowercased() ?? "" + return raw == "gif" ? "gif" : "mp4" } /// The clip itself, so the run page carries the motion and not just frames. @@ -417,11 +452,27 @@ final class DogfoodScenarioUITests: XCTestCase { return candidates.filter { seen.insert($0).inserted } } + /// How long this process waits for a reply, per method. + /// + /// Always longer than the deadline the app gives itself, so a slow command + /// comes back as the app's own error, which names what went wrong, instead + /// of this side guessing "no reply" while the work is still running. + /// `window.record.start` allows itself 20 seconds and `.stop` 30, in + /// `TerminalController+WindowRecording`. + private static func socketTimeout(for method: String) -> TimeInterval { + switch method { + case "window.record.start": 25 + case "window.record.stop": 35 + default: 15 + } + } + private func socketRequest(method: String, params: Any) -> [String: Any]? { let request: [String: Any] = ["id": UUID().uuidString, "method": method, "params": params] guard JSONSerialization.isValidJSONObject(request), let data = try? JSONSerialization.data(withJSONObject: request), - let reply = socketLine(String(decoding: data, as: UTF8.self), path: socketPath, timeout: 15), + let reply = socketLine(String(decoding: data, as: UTF8.self), path: socketPath, + timeout: Self.socketTimeout(for: method)), let replyData = reply.data(using: .utf8) else { return nil } @@ -566,7 +617,12 @@ enum DogfoodStep { return false } - init(json: Any) throws { + /// - Parameter insideRecord: true while decoding the nested steps of a + /// `record`. A `note` draws a caption into the clip being recorded, so + /// outside one it can only ever fail at run time; refusing it here means a + /// misplaced note is caught by the scenario guard in seconds rather than by + /// a red tour half an hour later. + init(json: Any, insideRecord: Bool = false) throws { guard let object = json as? [String: Any], let (kind, value) = object.first(where: { Self.kinds.contains($0.key) }) else { throw DogfoodError("each step is an object with one of \(Self.kinds.sorted().joined(separator: ", "))") } @@ -601,7 +657,8 @@ enum DogfoodStep { guard let name = value as? String, !name.isEmpty else { throw DogfoodError("record takes a name for the clip") } - let nested = try (object["steps"] as? [Any] ?? []).map(DogfoodStep.init(json:)) + let nested = try (object["steps"] as? [Any] ?? []) + .map { try DogfoodStep(json: $0, insideRecord: true) } guard !nested.contains(where: { $0.isRecord }) else { throw DogfoodError("record cannot contain another record: the app records one window at a time") } @@ -610,6 +667,9 @@ enum DogfoodStep { guard let text = value as? String, !text.isEmpty else { throw DogfoodError("note takes the caption to draw into the clip") } + guard insideRecord else { + throw DogfoodError("note belongs inside a record: there is no clip to caption outside one") + } self = .note(text) case "expect": self = .expect(try DogfoodTarget(json: value), exists: object["exists"] as? Bool ?? true) From 0adeb5aba5756fe96359263fc865dc57d07d10f7 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Mon, 28 Sep 2026 07:00:27 -0700 Subject: [PATCH 09/15] Keep the clip, route the tour guard, document the record rules More of the same review. An `mp4` clip never reached `attachments/`, and `mp4` is the default format. `e2e-frames.py` samples a video attachment into frames and then excluded videos from the list that copies attachments out, so the clip existed only inside the xcresult bundle. A clip is now both sampled and kept whole: the frames show what happened, and only the clip can be put on a pull request. The new tour passes `"format": "gif"`, which is why this was not noticed. The guard that keeps the reference honest about the decoder did not run for changes to the decoder. `tests/test_dogfood_scenarios.py` reads `DogfoodScenarioUITests.swift` with `read_text` rather than naming it in a `run:`, so the router put that file in `quality-determinism` only and a pull request touching just the decoder skipped the guard. It is declared in `PATH_OWNERS`, which is the documented place for a guard input read indirectly. Two things the guard could not catch: - a `note` at the top level. The decoder now refuses it, and the guard refuses it too, so it is caught in a second rather than after a CI round trip. - a typo in the socket name a record option is translated into. Only the tour-side keys were checked, so `max_second` would have passed the guard and then been ignored by the app, leaving the recording on its default while the tour stayed green. The socket names are now checked against the parameters `WindowRecordingRequest` reads. The reference did not state the rules the decoder and the guard enforce (a record needs steps and cannot nest), gave only one of the five options' limits, and described an out of range value as "refused by name" when the message names the socket field rather than the tour spelling. It says all of that now, plus what happens on a machine that cannot record. Also: the tour guard gets its own workflow step, so a failure is not reported under "Validate focused test launcher", which has nothing to do with tours. Co-Authored-By: Claude Opus 5.5 (1M context) --- .github/workflows/ci-guards.yml | 5 ++- scripts/ci/e2e-frames.py | 7 ++-- scripts/ci/workflow_guard_groups.py | 4 +++ .../references/dogfood-scenarios.md | 20 ++++++++--- tests/test_dogfood_scenarios.py | 35 ++++++++++++++++--- 5 files changed, 58 insertions(+), 13 deletions(-) diff --git a/.github/workflows/ci-guards.yml b/.github/workflows/ci-guards.yml index 5f8072d97a97..b71a43af9133 100644 --- a/.github/workflows/ci-guards.yml +++ b/.github/workflows/ci-guards.yml @@ -598,10 +598,13 @@ jobs: if: ${{ matrix.group == 'app-host-execution' }} run: | python3 tests/test_run_e2e.py - python3 tests/test_dogfood_scenarios.py python3 tests/test_ci_e2e_compilation_cache.py python3 tests/test_ci_focused_test_selectors.py + - name: Validate dogfood tours + if: ${{ matrix.group == 'app-host-execution' }} + run: python3 tests/test_dogfood_scenarios.py + - name: Validate manual macOS package cache recovery if: ${{ matrix.group == 'app-host-execution' }} run: python3 tests/test_ci_manual_macos_package_cache.py diff --git a/scripts/ci/e2e-frames.py b/scripts/ci/e2e-frames.py index 95ac6b45c4d1..6f44e1a5b2b4 100644 --- a/scripts/ci/e2e-frames.py +++ b/scripts/ci/e2e-frames.py @@ -287,9 +287,12 @@ def add(title: str, source: Path, when: float, temporary: bool = False) -> None: before = [s for s in steps if s["time"] <= failed_at + 0.5 and s["title"] != AT_FAILURE] failed_step = (before or steps)[-1]["number"] - # Text a test attached on purpose, under the name it gave. + # Text and clips a test attached on purpose, under the name it gave. Images + # are excluded because they become the numbered frames above; a clip is both + # sampled into frames and kept whole, since the frames show what happened and + # only the clip can be put on a pull request. kept = [a for a in entry.get("attachments", []) - if Path(a.get("exportedFileName", "")).suffix.lower() not in IMAGE_SUFFIXES | VIDEO_SUFFIXES + if Path(a.get("exportedFileName", "")).suffix.lower() not in IMAGE_SUFFIXES and not a.get("suggestedHumanReadableName", "").startswith(XCUITEST_NOISE)] for attachment in kept: # "_0_." -> "." diff --git a/scripts/ci/workflow_guard_groups.py b/scripts/ci/workflow_guard_groups.py index e4136ba639b2..1b2d9c4b05f4 100644 --- a/scripts/ci/workflow_guard_groups.py +++ b/scripts/ci/workflow_guard_groups.py @@ -75,6 +75,10 @@ ".github/scripts/review_fabric.py": frozenset(("preflight",)), ".github/workflows/ios-testflight.yml": frozenset(("preflight", "ci", "release-ios")), "agent-chat/test/claude-environment.test.ts": frozenset(("preflight",)), + # test_dogfood_scenarios.py reads the tour decoder with read_text to check + # that every step kind and record option is documented, rather than naming + # it in a `run:`, so the router cannot see it. + "cmuxUITests/DogfoodScenarioUITests.swift": frozenset(("app-host-execution",)), "ghostty": frozenset(("release-tooling",)), "ios/scripts/fetch-testflight-notes-history.sh": frozenset(("release-ios",)), "ios/scripts/upload-testflight.sh": frozenset(("release-ios",)), diff --git a/skills/cmux-testing/references/dogfood-scenarios.md b/skills/cmux-testing/references/dogfood-scenarios.md index 7eb183685830..a420f568d8a1 100644 --- a/skills/cmux-testing/references/dogfood-scenarios.md +++ b/skills/cmux-testing/references/dogfood-scenarios.md @@ -76,18 +76,28 @@ them: ]} ``` -- Options: `format` (`mp4` or `gif`), `fps`, `maxSeconds`, `scale`, `maxWidth`, - `region` (`"x,y,w,h"` in window points), `captions` and `label`. They are the - flags of `cmux record start` and the app owns their limits, so an out of range - value is refused by name. An unknown option is a typo and fails the step. +- Options, with the default for `mp4` then `gif`: `format` (`mp4`), `fps` (12, 8; + 1 to 30), `maxSeconds` (15 both; 0.5 to 120), `scale` (1.0, 0.5; 0.1 to 1.0), + `maxWidth` (none, 960; 64 to 4096), `region` (`"x,y,w,h"` in window points), + `captions` (true) and `label`. They are the flags of `cmux record start` and the + app owns the limits, so an out of range value is refused with a message naming + the socket field rather than the tour spelling: `maxSeconds: 300` comes back as + "max_seconds must be between 0.5 and 120". An unknown option is a typo and fails + the step. - A `gif` at `fps` 8 and `scale` 0.5 is the one to paste into a PR; an `mp4` is sharper and needs a click to play. `scripts/pr-media.py` uploads either and converts an mp4 to a gif on the way. - Nested steps behave as they do at the top level, including `shot`, and a nested failure still lets the rest run so the recording is always stopped and the clip of the failure survives. Nested steps are numbered `03.1`, `03.2` in `steps.log`. +- A `record` needs at least one nested step and may not contain another `record`, + since the app records one window at a time. Both are refused by + `tests/test_dogfood_scenarios.py` before CI runs anything. +- A recording never gates what it records: on a machine that cannot capture at + all, the start failure is recorded and the nested steps still run, unrecorded. - `note` draws a caption into the clip. It is worth one before each thing you - want a reviewer to notice, because a clip has no step list beside it. + want a reviewer to notice, because a clip has no step list beside it. It only + means anything among a record's own steps, so it is refused anywhere else. - The clip lands in `attachments/` next to the trees and socket replies, named after the step and the tour's name for it. - The app records only its own windows, one at a time, and stops by itself at diff --git a/tests/test_dogfood_scenarios.py b/tests/test_dogfood_scenarios.py index 055c6881f944..9f990b991015 100644 --- a/tests/test_dogfood_scenarios.py +++ b/tests/test_dogfood_scenarios.py @@ -16,15 +16,22 @@ ROOT = Path(__file__).resolve().parents[1] RUNNER = ROOT / "cmuxUITests/DogfoodScenarioUITests.swift" +REQUEST = ROOT / ("Packages/macOS/CmuxFoundation/Sources/CmuxFoundation" + "/WindowRecording/WindowRecordingRequest.swift") REFERENCE = ROOT / "skills/cmux-testing/references/dogfood-scenarios.md" SCENARIOS = sorted((ROOT / "dogfood/scenarios").glob("*.json")) -def swift_string_set(source: str, declaration: str) -> set[str]: - """The string literals of a Swift `let name: ... = [ ... ]` declaration.""" +def swift_declaration_body(source: str, declaration: str) -> str: + """The bracketed body of a Swift `let name: ... = [ ... ]` declaration.""" match = re.search(re.escape(declaration) + r"\s*=\s*\[(.*?)\]", source, re.DOTALL) assert match, f"{declaration} is no longer declared the way this test reads it" - return set(re.findall(r'"([^"]+)"', match.group(1))) + return match.group(1) + + +def swift_string_set(source: str, declaration: str) -> set[str]: + """The string literals of a Swift `let name: ... = [ ... ]` declaration.""" + return set(re.findall(r'"([^"]+)"', swift_declaration_body(source, declaration))) class DecoderFacts: @@ -34,8 +41,7 @@ class DecoderFacts: # keys, which are the odd entries of the flattened literal list. record_option_pairs = re.findall( r'"([A-Za-z_]+)":\s*"([a-z_]+)"', - re.search(r"private static let recordOptions: \[String: String\] = \[(.*?)\]", - source, re.DOTALL).group(1), + swift_declaration_body(source, "private static let recordOptions: [String: String]"), ) record_options = {tour for tour, _ in record_option_pairs} @@ -61,6 +67,20 @@ def test_every_recording_option_is_documented(self): for option in sorted(DecoderFacts.record_options): self.assertIn(f"`{option}`", text, f"{option} is accepted but not documented") + def test_every_recording_option_names_a_parameter_the_app_reads(self): + """The socket side of the map, which the reference cannot vouch for. + + A tour option is only useful if the name it is translated into is one + `WindowRecordingRequest.make` looks up, and a typo there is silent: an + unknown key is simply ignored, so the recording runs with the default + and the tour passes while doing the wrong thing. + """ + request = REQUEST.read_text() + read = set(re.findall(r'params\["([a-z_]+)"\]', request)) + for tour, socket in sorted(DecoderFacts.record_option_pairs): + self.assertIn(socket, read, + f"{tour} is sent as {socket!r}, which {REQUEST.name} never reads") + class ScenarioTests(unittest.TestCase): def test_there_are_scenarios_to_check(self): @@ -94,6 +114,11 @@ def check_steps(steps: list, inside_record: bool) -> None: f"{', '.join(sorted(DecoderFacts.kinds))}" ) kind = kinds.pop() + if kind == "note" and not inside_record: + raise AssertionError( + f"step {index} captions a clip outside a record; a note belongs " + "among a record's own steps" + ) if kind != "record": continue if inside_record: From 360a6c898433b3be8717014d58e68887bfa84cb1 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Mon, 28 Sep 2026 10:16:49 -0400 Subject: [PATCH 10/15] Discard a timed-out record start and keep standalone -- in notes A record start that timed out on the socket kept running and could register a recording the caller had been told failed. The registry now tracks each start by token; on timeout the socket handler abandons it and waits for the session to release its writer and partial file before answering. A start that is stopped or abandoned mid-capture no longer opens a writer, and fail() releases a writer even after the session reached a terminal state. record note now strips only a leading -- terminator, and cli.usage.record carries every catalog locale. Co-Authored-By: Claude Opus 5.5 (1M context) --- CLI/CMUXCLI+Record.swift | 7 ++- Resources/Localizable.xcstrings | 42 +++++++++++++++ .../TerminalController+WindowRecording.swift | 23 +++++++- Sources/WindowRecordingRegistry.swift | 53 ++++++++++++++++--- Sources/WindowRecordingSession.swift | 47 ++++++++++++++-- cmuxTests/WindowRecordingPipelineTests.swift | 33 ++++++++++++ 6 files changed, 192 insertions(+), 13 deletions(-) diff --git a/CLI/CMUXCLI+Record.swift b/CLI/CMUXCLI+Record.swift index 9c8b76e161d5..c72620ab7bb6 100644 --- a/CLI/CMUXCLI+Record.swift +++ b/CLI/CMUXCLI+Record.swift @@ -82,8 +82,11 @@ extension CMUXCLI { jsonOutput: jsonOutput ) case "note": - let (id, trailing) = parseRecordOption(arguments, name: "--id") - let text = trailing.filter { $0 != "--" }.joined(separator: " ") + let (id, parsed) = parseRecordOption(arguments, name: "--id") + // Only a leading `--` ends the options; one later in the text is + // part of the caption. + let trailing = parsed.first == "--" ? Array(parsed.dropFirst()) : parsed + let text = trailing.joined(separator: " ") .trimmingCharacters(in: .whitespacesAndNewlines) guard !text.isEmpty else { throw CLIError(message: String( diff --git a/Resources/Localizable.xcstrings b/Resources/Localizable.xcstrings index 0db518cbb891..dd37c6df3deb 100644 --- a/Resources/Localizable.xcstrings +++ b/Resources/Localizable.xcstrings @@ -566452,17 +566452,59 @@ "cli.usage.record": { "extractionState": "manual", "localizations": { + "ar": { + "stringUnit": { + "state": "translated", + "value": "record [start] [--format mp4|gif] [--gif] [--fps ] [--max-seconds ] [--scale ] [--max-width ] [--region ] [--out ] [--label ] [--no-captions] [--window ] | record stop|status [--id ] | record note [--id ] | record list" + } + }, + "de": { + "stringUnit": { + "state": "translated", + "value": "record [start] [--format mp4|gif] [--gif] [--fps ] [--max-seconds ] [--scale ] [--max-width ] [--region ] [--out ] [--label ] [--no-captions] [--window ] | record stop|status [--id ] | record note [--id ] | record list" + } + }, "en": { "stringUnit": { "state": "translated", "value": "record [start] [--format mp4|gif] [--gif] [--fps ] [--max-seconds ] [--scale ] [--max-width ] [--region ] [--out ] [--label ] [--no-captions] [--window ] | record stop|status [--id ] | record note [--id ] | record list" } }, + "es": { + "stringUnit": { + "state": "translated", + "value": "record [start] [--format mp4|gif] [--gif] [--fps ] [--max-seconds ] [--scale ] [--max-width ] [--region ] [--out ] [--label ] [--no-captions] [--window ] | record stop|status [--id ] | record note [--id ] | record list" + } + }, + "fr": { + "stringUnit": { + "state": "translated", + "value": "record [start] [--format mp4|gif] [--gif] [--fps ] [--max-seconds ] [--scale ] [--max-width ] [--region ] [--out ] [--label ] [--no-captions] [--window ] | record stop|status [--id ] | record note [--id ] | record list" + } + }, "ja": { "stringUnit": { "state": "translated", "value": "record [start] [--format mp4|gif] [--gif] [--fps ] [--max-seconds ] [--scale ] [--max-width ] [--region ] [--out ] [--label ] [--no-captions] [--window ] | record stop|status [--id ] | record note [--id ] | record list" } + }, + "ko": { + "stringUnit": { + "state": "translated", + "value": "record [start] [--format mp4|gif] [--gif] [--fps ] [--max-seconds ] [--scale ] [--max-width ] [--region ] [--out ] [--label ] [--no-captions] [--window ] | record stop|status [--id ] | record note [--id ] | record list" + } + }, + "zh-Hans": { + "stringUnit": { + "state": "translated", + "value": "record [start] [--format mp4|gif] [--gif] [--fps ] [--max-seconds ] [--scale ] [--max-width ] [--region ] [--out ] [--label ] [--no-captions] [--window ] | record stop|status [--id ] | record note [--id ] | record list" + } + }, + "zh-Hant": { + "stringUnit": { + "state": "translated", + "value": "record [start] [--format mp4|gif] [--gif] [--fps ] [--max-seconds ] [--scale ] [--max-width ] [--region ] [--out ] [--label ] [--no-captions] [--window ] | record stop|status [--id ] | record note [--id ] | record list" + } } } } diff --git a/Sources/TerminalController+WindowRecording.swift b/Sources/TerminalController+WindowRecording.swift index 57352824fc97..93040b695856 100644 --- a/Sources/TerminalController+WindowRecording.swift +++ b/Sources/TerminalController+WindowRecording.swift @@ -50,11 +50,18 @@ extension TerminalController { data: nil ) } - return awaitRecordingCall(timeout: 20) { + let token = UUID() + return awaitRecordingCall( + timeout: 20, + onTimeout: { + await WindowRecordingRegistry.shared.abandonStart(token: token) + } + ) { try await WindowRecordingRegistry.shared.start( request: request, windowID: windowID, - windowHandle: request.windowHandle + windowHandle: request.windowHandle, + token: token ) } } @@ -130,6 +137,7 @@ extension TerminalController { private nonisolated func awaitRecordingCall( timeout: TimeInterval, + onTimeout: (@Sendable () async -> Void)? = nil, _ work: @escaping () async throws -> WindowRecordingStatus ) -> V2CallResult { let outcome: Result? = socketAwaitCallback( @@ -144,6 +152,17 @@ extension TerminalController { } } guard let outcome else { + // The work keeps running after the wait gives up. Let the caller + // undo it before answering, so a "timeout" reply never leaves + // behind the side effect it reports as failed. + if let onTimeout { + let _: Void? = socketAwaitCallback(timeout: 10) { completion in + Task { + await onTimeout() + completion(()) + } + } + } return .err( code: "timeout", message: "recording command timed out after \(Int(timeout)) seconds", diff --git a/Sources/WindowRecordingRegistry.swift b/Sources/WindowRecordingRegistry.swift index 696655ea90ec..a8f1cddcfb05 100644 --- a/Sources/WindowRecordingRegistry.swift +++ b/Sources/WindowRecordingRegistry.swift @@ -14,6 +14,12 @@ actor WindowRecordingRegistry { private static let historyLimit = 8 private var active: WindowRecordingSession? + /// The `record start` call that claimed `active`, so a caller that timed + /// out can end exactly the session it opened and nothing newer. + private var activeStartToken: UUID? + /// Starts still running, and those whose caller has already given up. + private var pendingStarts: Set = [] + private var abandonedStarts: Set = [] private var history: [WindowRecordingStatus] = [] enum Failure: Error, LocalizedError { @@ -39,9 +45,18 @@ actor WindowRecordingRegistry { func start( request: WindowRecordingRequest, windowID: CGWindowID, - windowHandle: String? + windowHandle: String?, + token: UUID = UUID() ) async throws -> WindowRecordingStatus { + pendingStarts.insert(token) + defer { + pendingStarts.remove(token) + abandonedStarts.remove(token) + } await harvestFinishedRecording() + if abandonedStarts.contains(token) { + throw WindowRecordingSessionError.alreadyFinished + } if let active { throw Failure.busy(active.id) } @@ -57,22 +72,48 @@ actor WindowRecordingRegistry { // capture round trip, and a second `record start` arriving during it // would otherwise pass the check above and replace this session. active = session + activeStartToken = token do { try await session.start() } catch { - if active === session { - active = nil - } + releaseSlot(held: session) throw error } + // `abandonStart` may have ended the session during the await above. + guard await session.isRecording else { + releaseSlot(held: session) + throw WindowRecordingSessionError.alreadyFinished + } return await session.status } + /// Called when the socket caller of `start(token:)` stopped waiting. The + /// caller was told the start failed, so the session it would have opened + /// must not keep recording: end and discard it, or make a start that has + /// not claimed the slot yet give up when it gets there. Returns once the + /// session has released its writer and partial file. + func abandonStart(token: UUID) async { + guard pendingStarts.contains(token) || activeStartToken == token else { return } + guard let session = active, activeStartToken == token else { + abandonedStarts.insert(token) + return + } + await session.abandon(reason: "record start timed out; the recording was discarded") + remember(await session.status) + releaseSlot(held: session) + } + + private func releaseSlot(held session: WindowRecordingSession) { + guard active === session else { return } + active = nil + activeStartToken = nil + } + func stop(id: String?) async throws -> WindowRecordingStatus { await harvestFinishedRecording() if let active, id == nil || id == active.id { let status = await active.stop() - self.active = nil + releaseSlot(held: active) remember(status) return status } @@ -134,7 +175,7 @@ actor WindowRecordingRegistry { let isRecording = await active.isRecording guard !isRecording else { return } remember(await active.status) - self.active = nil + releaseSlot(held: active) } /// Files a finished clip in history. Internal rather than private so tests diff --git a/Sources/WindowRecordingSession.swift b/Sources/WindowRecordingSession.swift index 531f6c5fe6c5..f1aa8ca86d3a 100644 --- a/Sources/WindowRecordingSession.swift +++ b/Sources/WindowRecordingSession.swift @@ -175,6 +175,11 @@ actor WindowRecordingSession { request: request ) geometry = planned + // A caller that gave up on this start (see `abandon`) may have ended + // the session while the capture above was in flight; open nothing then. + guard state == .recording else { + throw WindowRecordingSessionError.alreadyFinished + } writer = try makeWriter(geometry: planned) startUptime = ProcessInfo.processInfo.systemUptime do { @@ -185,6 +190,9 @@ actor WindowRecordingSession { await fail(error) throw error } + guard state == .recording else { + throw WindowRecordingSessionError.alreadyFinished + } loop = Task { [weak self] in await self?.run() } @@ -210,6 +218,24 @@ actor WindowRecordingSession { return status } + /// Ends the session and throws away whatever it captured. Used when the + /// `record start` that opened it already reported a timeout: the caller + /// believes nothing is recording, so nothing may be left recording or on + /// disk. Idempotent, and safe while `start` is still suspended. + func abandon(reason: String) async { + await acquireWriter() + defer { releaseWriter() } + // Cancelled after the writer turn, not before: a `start` that was + // mid-append when this was called creates its sample loop only once + // that append returns. + loop?.cancel() + loop = nil + await discardWriter() + guard state == .recording else { return } + state = .failed + failure = reason + } + private func run() async { while !Task.isCancelled, state == .recording { let target = startUptime + (Double(frames) * request.frameInterval) @@ -286,15 +312,21 @@ actor WindowRecordingSession { await acquireWriter() defer { releaseWriter() } // A clip that already closed keeps its own verdict: a stop racing a - // capture failure must not turn a finished clip into a failed one. - guard state == .recording else { return } + // capture failure must not turn a finished clip into a failed one. A + // writer opened by a `start` that lost that race is still released. + guard state == .recording else { + await discardWriter() + return + } // Keep whatever was captured before the failure: a clip that ends when // the window closes is still evidence of what happened before that. if let writer { self.writer = nil do { + // Close the writer even with no frames, so an append that + // started it and then failed does not leave it open. + try await writer.finish() if frames > 0 { - try await writer.finish() try promote() } } catch { @@ -306,6 +338,15 @@ actor WindowRecordingSession { failure = error.localizedDescription } + /// Drops an open writer and its partial file without promoting anything. + /// The writer is closed first so no handle outlives the file it points at. + private func discardWriter() async { + guard let writer else { return } + self.writer = nil + try? await writer.finish() + try? FileManager.default.removeItem(at: workingURL) + } + /// Moves the finished clip to the path the caller asked for. private func promote() throws { let manager = FileManager.default diff --git a/cmuxTests/WindowRecordingPipelineTests.swift b/cmuxTests/WindowRecordingPipelineTests.swift index cc5672eb3cba..c04fb1161e96 100644 --- a/cmuxTests/WindowRecordingPipelineTests.swift +++ b/cmuxTests/WindowRecordingPipelineTests.swift @@ -425,4 +425,37 @@ import Testing #expect(recent.first?.id == "clip-4") #expect(recent.last?.id == "clip-11") } + + /// A timed-out `record start` whose call already ended must not reach into + /// whatever is recorded now. + @Test func abandoningAStartThatIsNoLongerRunningChangesNothing() async throws { + let registry = WindowRecordingRegistry() + await registry.remember(Self.finished(id: "a")) + + await registry.abandonStart(token: UUID()) + + let status = try await registry.status(id: nil) + #expect(status.id == "a") + #expect(status.state == .finished) + } + + /// The session a timed-out start opened ends as failed, with a reason the + /// caller can read from `record status`, and a later stop keeps that verdict. + @Test func anAbandonedSessionStopsRecordingAndStaysFailed() async throws { + let session = WindowRecordingSession( + id: "abandoned", + request: try WindowRecordingRequest.make(params: [:]), + outputURL: FileManager.default.temporaryDirectory + .appendingPathComponent("cmux-abandoned-\(UUID().uuidString).mp4"), + windowID: 0, + windowHandle: nil + ) + + await session.abandon(reason: "timed out") + + #expect(await session.isRecording == false) + let stopped = await session.stop() + #expect(stopped.state == .failed) + #expect(stopped.error == "timed out") + } } From 23764ff91ed620a32218e1b9ba1ed90e01d89b04 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Mon, 28 Sep 2026 10:54:36 -0400 Subject: [PATCH 11/15] Scope recording limits and output naming onto WindowRecordingRequest The package conventions lint rejects all-static public types, so the limits and the default output naming become extensions on the request. The mp4 test names its encoded clip length plainly so the determinism check does not read it as a measured wall-clock time. Co-Authored-By: Claude Opus 5.5 (1M context) --- ...ft => WindowRecordingRequest+Output.swift} | 14 +++---- .../WindowRecordingRequest.swift | 42 +++++++++---------- .../WindowRecordingOutputNamingTests.swift | 35 +++++++--------- .../WindowRecordingRequestTests.swift | 4 +- Sources/WindowRecordingRegistry.swift | 10 ++--- cmuxTests/WindowRecordingPipelineTests.swift | 6 ++- 6 files changed, 52 insertions(+), 59 deletions(-) rename Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/{WindowRecordingOutputNaming.swift => WindowRecordingRequest+Output.swift} (67%) diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingOutputNaming.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingRequest+Output.swift similarity index 67% rename from Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingOutputNaming.swift rename to Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingRequest+Output.swift index 6588a5e7f196..9493f4f80c1f 100644 --- a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingOutputNaming.swift +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingRequest+Output.swift @@ -5,22 +5,20 @@ public import Foundation /// Recordings follow the screenshot convention (a sortable timestamp plus a /// short unique suffix, under one directory in the temporary directory) so the /// two kinds of capture can be listed together and cleaned up together. -public struct WindowRecordingOutputNaming: Sendable { - public static let directoryName = "cmux-recordings" +extension WindowRecordingRequest { + public static let outputDirectoryName = "cmux-recordings" /// `2026-09-28T07-14-03Z_1a2b3c4d`: sorts by time, unique per recording. - public static func identifier(date: Date, uuid: UUID = UUID()) -> String { + public static func recordingIdentifier(date: Date, uuid: UUID = UUID()) -> String { let timestamp = ISO8601DateFormatter().string(from: date) .replacingOccurrences(of: ":", with: "-") .replacingOccurrences(of: "+", with: "_") return "\(timestamp)_\(uuid.uuidString.prefix(8).lowercased())" } - public static func filename( - label: String, - identifier: String, - format: WindowRecordingRequest.Format - ) -> String { + /// The clip's filename when the caller did not pass `out`: the label, if + /// any, then the recording identifier, then the format's extension. + public func outputFilename(identifier: String) -> String { let sanitized = WindowRecordingLabel(label).value let stem = sanitized.isEmpty ? identifier : "\(sanitized)_\(identifier)" return "\(stem).\(format.rawValue)" diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingRequest.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingRequest.swift index 725b5df51ca5..052fc8152016 100644 --- a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingRequest.swift +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingRequest.swift @@ -1,19 +1,5 @@ internal import Foundation -/// The bounds every `window.record.start` request is held to. -/// -/// One recording writes frames for as long as it runs, so the caller cannot be -/// trusted with an unbounded frame rate, duration, or pixel count: a stuck -/// agent would fill the disk. These are the limits the socket enforces; the -/// recorder stops itself at `maximumSeconds` even when nobody calls stop. -public struct WindowRecordingLimits: Sendable { - public static let framesPerSecond = 1...30 - public static let seconds = 0.5...120.0 - public static let scale = 0.1...1.0 - public static let maximumWidth = 64...4096 - public static let minimumRegionExtent: Double = 8 -} - /// One validated `window.record.start` request. public struct WindowRecordingRequest: Equatable, Sendable { public enum Format: String, Sendable, CaseIterable { @@ -83,7 +69,7 @@ public struct WindowRecordingRequest: Equatable, Sendable { case let .malformedRegion(value): return "region '\(value)' must be x,y,width,height in window points" case .regionTooSmall: - let minimum = Int(WindowRecordingLimits.minimumRegionExtent) + let minimum = Int(WindowRecordingRequest.minimumRegionExtent) return "region width and height must each be at least \(minimum) points" case let .outputPathNotAbsolute(path): return "out '\(path)' must be an absolute path" @@ -147,22 +133,22 @@ public struct WindowRecordingRequest: Equatable, Sendable { let framesPerSecond = try decodeInt( params["fps"], field: "fps", - range: WindowRecordingLimits.framesPerSecond + range: Self.allowedFramesPerSecond ) let maximumSeconds = try decodeDouble( params["max_seconds"], field: "max_seconds", - range: WindowRecordingLimits.seconds + range: Self.allowedSeconds ) let scale = try decodeDouble( params["scale"], field: "scale", - range: WindowRecordingLimits.scale + range: Self.allowedScale ) let maximumWidth = try decodeInt( params["max_width"], field: "max_width", - range: WindowRecordingLimits.maximumWidth + range: Self.allowedMaximumWidth ) let target = try decodeTarget(params["region"]) let outputPath = try decodeOutputPath(params["out"], format: format) @@ -214,8 +200,8 @@ public struct WindowRecordingRequest: Equatable, Sendable { guard region.isFinite else { throw Failure.malformedRegion(String(describing: value)) } - guard region.width >= WindowRecordingLimits.minimumRegionExtent, - region.height >= WindowRecordingLimits.minimumRegionExtent else { + guard region.width >= Self.minimumRegionExtent, + region.height >= Self.minimumRegionExtent else { throw Failure.regionTooSmall } return .region(region) @@ -300,3 +286,17 @@ private extension String { return trimmed.isEmpty ? nil : trimmed } } + +/// The bounds every `window.record.start` request is held to. +/// +/// One recording writes frames for as long as it runs, so the caller cannot be +/// trusted with an unbounded frame rate, duration, or pixel count: a stuck +/// agent would fill the disk. These are the limits the socket enforces; the +/// recorder stops itself at `maximumSeconds` even when nobody calls stop. +extension WindowRecordingRequest { + public static let allowedFramesPerSecond = 1...30 + public static let allowedSeconds = 0.5...120.0 + public static let allowedScale = 0.1...1.0 + public static let allowedMaximumWidth = 64...4096 + public static let minimumRegionExtent: Double = 8 +} diff --git a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/WindowRecordingOutputNamingTests.swift b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/WindowRecordingOutputNamingTests.swift index 956189b62c50..16c6f9d5d955 100644 --- a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/WindowRecordingOutputNamingTests.swift +++ b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/WindowRecordingOutputNamingTests.swift @@ -6,7 +6,7 @@ import Testing private let uuid = UUID(uuidString: "1A2B3C4D-0000-0000-0000-000000000000")! @Test func identifiersSortByTimeAndCarryAUniqueSuffix() { - let identifier = WindowRecordingOutputNaming.identifier( + let identifier = WindowRecordingRequest.recordingIdentifier( date: Date(timeIntervalSince1970: 1_790_000_000), uuid: uuid ) @@ -15,32 +15,29 @@ import Testing #expect(!identifier.contains(":")) } - @Test func aLabelBecomesTheFilenamePrefix() { - let filename = WindowRecordingOutputNaming.filename( - label: "sidebar tour", - identifier: "2026-09-28T07-14-03Z_1a2b3c4d", - format: .gif - ) + @Test func aLabelBecomesTheFilenamePrefix() throws { + let filename = try WindowRecordingRequest.make(params: [ + "label": "sidebar tour", + "format": "gif", + ]).outputFilename(identifier: "2026-09-28T07-14-03Z_1a2b3c4d") #expect(filename == "sidebar-tour_2026-09-28T07-14-03Z_1a2b3c4d.gif") } - @Test func noLabelLeavesJustTheIdentifier() { - let filename = WindowRecordingOutputNaming.filename( - label: " ", - identifier: "id", - format: .mp4 - ) + @Test func noLabelLeavesJustTheIdentifier() throws { + let filename = try WindowRecordingRequest.make(params: [ + "label": " ", + "format": "mp4", + ]).outputFilename(identifier: "id") #expect(filename == "id.mp4") } - @Test func aPathSeparatorInALabelCannotEscapeTheDirectory() { - let filename = WindowRecordingOutputNaming.filename( - label: "../../etc/passwd", - identifier: "id", - format: .mp4 - ) + @Test func aPathSeparatorInALabelCannotEscapeTheDirectory() throws { + let filename = try WindowRecordingRequest.make(params: [ + "label": "../../etc/passwd", + "format": "mp4", + ]).outputFilename(identifier: "id") #expect(!filename.contains("/")) #expect(filename == "etc-passwd_id.mp4") diff --git a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/WindowRecordingRequestTests.swift b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/WindowRecordingRequestTests.swift index 5b8b181081f3..ae97cadf5596 100644 --- a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/WindowRecordingRequestTests.swift +++ b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/WindowRecordingRequestTests.swift @@ -118,11 +118,11 @@ import Testing #expect(throws: (any Error).self) { try WindowRecordingRequest.make(params: ["max_seconds": 3600]) } - #expect(WindowRecordingLimits.seconds.upperBound == 120) + #expect(WindowRecordingRequest.allowedSeconds.upperBound == 120) } @Test func scaleNeverUpscales() { - #expect(WindowRecordingLimits.scale.upperBound == 1) + #expect(WindowRecordingRequest.allowedScale.upperBound == 1) #expect(throws: (any Error).self) { try WindowRecordingRequest.make(params: ["scale": 2]) } diff --git a/Sources/WindowRecordingRegistry.swift b/Sources/WindowRecordingRegistry.swift index a8f1cddcfb05..6a458092ef9c 100644 --- a/Sources/WindowRecordingRegistry.swift +++ b/Sources/WindowRecordingRegistry.swift @@ -60,7 +60,7 @@ actor WindowRecordingRegistry { if let active { throw Failure.busy(active.id) } - let identifier = WindowRecordingOutputNaming.identifier(date: Date()) + let identifier = WindowRecordingRequest.recordingIdentifier(date: Date()) let session = WindowRecordingSession( id: identifier, request: request, @@ -196,11 +196,7 @@ actor WindowRecordingRegistry { return URL(fileURLWithPath: outputPath) } return FileManager.default.temporaryDirectory - .appendingPathComponent(WindowRecordingOutputNaming.directoryName) - .appendingPathComponent(WindowRecordingOutputNaming.filename( - label: request.label, - identifier: identifier, - format: request.format - )) + .appendingPathComponent(WindowRecordingRequest.outputDirectoryName) + .appendingPathComponent(request.outputFilename(identifier: identifier)) } } diff --git a/cmuxTests/WindowRecordingPipelineTests.swift b/cmuxTests/WindowRecordingPipelineTests.swift index c04fb1161e96..4d0c2e455ad6 100644 --- a/cmuxTests/WindowRecordingPipelineTests.swift +++ b/cmuxTests/WindowRecordingPipelineTests.swift @@ -214,8 +214,10 @@ import Testing #expect(FileManager.default.fileExists(atPath: url.path)) let asset = AVURLAsset(url: url) - let duration = try await asset.load(.duration).seconds - #expect(duration > 0.4) + // The clip's encoded length, not a measured time: the frames above + // are stamped up to 0.5 s, so a clip timed by capture spans past 0.4 s. + let clipSeconds = try await asset.load(.duration).seconds + #expect(clipSeconds > 0.4) let track = try #require(try await asset.loadTracks(withMediaType: .video).first) let size = try await track.load(.naturalSize) #expect(Int(size.width) == 160) From dc12d8f3d1d0866b785357fbde694b7a96c2edf6 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Mon, 28 Sep 2026 08:07:18 -0700 Subject: [PATCH 12/15] Decode nested steps through a closure again `DogfoodStep.init(json:)` stopped existing when the initializer grew an `insideRecord` parameter: with a default value it is still callable that way, but its name is `init(json:insideRecord:)`, and an unapplied reference has to spell the whole name. The compiler said so, in the UI test target that no PR check compiles. Co-Authored-By: Claude Opus 5.5 (1M context) --- cmuxUITests/DogfoodScenarioUITests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmuxUITests/DogfoodScenarioUITests.swift b/cmuxUITests/DogfoodScenarioUITests.swift index 8d77ddbcdeb6..3bc1455b9175 100644 --- a/cmuxUITests/DogfoodScenarioUITests.swift +++ b/cmuxUITests/DogfoodScenarioUITests.swift @@ -764,7 +764,7 @@ struct DogfoodScenario { guard let rawSteps = object["steps"] as? [Any] else { throw DogfoodError("scenario has no steps") } let launch = object["launch"] as? [String: Any] ?? [:] return DogfoodScenario( - steps: try rawSteps.map(DogfoodStep.init(json:)), + steps: try rawSteps.map { try DogfoodStep(json: $0) }, launchArguments: launch["args"] as? [String] ?? [], launchEnvironment: launch["env"] as? [String: String] ?? [:], language: launch["language"] as? String, From 69a90adae58f8913fef354b16480fbf768410546 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Mon, 28 Sep 2026 11:23:18 -0400 Subject: [PATCH 13/15] Compare recorded pixels through an Equatable value Optional tuples are not Equatable, so the caption test did not compile. Co-Authored-By: Claude Opus 5.5 (1M context) --- cmuxTests/WindowRecordingPipelineTests.swift | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/cmuxTests/WindowRecordingPipelineTests.swift b/cmuxTests/WindowRecordingPipelineTests.swift index 4d0c2e455ad6..98652d706934 100644 --- a/cmuxTests/WindowRecordingPipelineTests.swift +++ b/cmuxTests/WindowRecordingPipelineTests.swift @@ -63,7 +63,14 @@ import Testing return context.makeImage()! } - private static func pixel(_ image: CGImage, x: Int, y: Int) -> (r: Int, g: Int, b: Int)? { + /// Tuples are not `Equatable`, and the caption test compares pixels. + private struct Pixel: Equatable { + let r: Int + let g: Int + let b: Int + } + + private static func pixel(_ image: CGImage, x: Int, y: Int) -> Pixel? { var bytes = [UInt8](repeating: 0, count: 4) guard let context = CGContext( data: &bytes, @@ -79,7 +86,7 @@ import Testing } context.draw(image, in: CGRect(x: -x, y: -(image.height - 1 - y), width: image.width, height: image.height)) // 32BGRA little endian: bytes are B, G, R, then the skipped alpha. - return (r: Int(bytes[2]), g: Int(bytes[1]), b: Int(bytes[0])) + return Pixel(r: Int(bytes[2]), g: Int(bytes[1]), b: Int(bytes[0])) } private static func geometry(params: [String: Any] = [:], width: Int, height: Int) throws From 6670e968eb5f7966b690e83b220159ea28ed97de Mon Sep 17 00:00:00 2001 From: Leo Li Date: Mon, 28 Sep 2026 09:28:12 -0700 Subject: [PATCH 14/15] Fail a record step that would record nothing The tour guard rejected any top level key besides steps and launch, so the five tours that gained paths turned it red. It now accepts paths and checks that it is a list of non-empty patterns, and the record tour carries the globs that let pr-media.yml pick the one tour exercising record. Three behaviours the guard could not reach: - The step decoder accepted a missing or wrong-typed nested steps array and recorded an empty clip with every step dropped. An ad-hoc file passed to run-e2e.sh --scenario never meets the Python guard, so the decoder now refuses it too. - A note that arrived after the clip stopped itself at max_seconds failed the tour, which contradicts the promise that a recording never gates what it observes. not_found is now logged and passes; every other failure still fails the step. - The cleanup stop sent no id, so with nothing running it answered with the previous record step's finished status, and the clip named, attached and deleted was someone else's. Only a stop addressed by id may name the file. e2e-frames.py samples a gif now, so a tour that asks for a gif reaches the pull request through the frame strip rather than the artifact zip alone. Co-Authored-By: Claude Opus 5.5 (1M context) --- cmuxUITests/DogfoodScenarioUITests.swift | 40 +++++++++++++++++-- .../record-split-and-palette-tour.json | 10 +++++ scripts/ci/e2e-frames.py | 6 ++- .../references/dogfood-scenarios.md | 16 ++++++-- tests/test_dogfood_scenarios.py | 7 +++- 5 files changed, 70 insertions(+), 9 deletions(-) diff --git a/cmuxUITests/DogfoodScenarioUITests.swift b/cmuxUITests/DogfoodScenarioUITests.swift index 59a3a9a6e404..7e12d9c8b275 100644 --- a/cmuxUITests/DogfoodScenarioUITests.swift +++ b/cmuxUITests/DogfoodScenarioUITests.swift @@ -177,7 +177,7 @@ final class DogfoodScenarioUITests: XCTestCase { case .record(let name, let params, let steps): runRecording(name: name, params: params, steps: steps, label: label, app: app) case .note(let text): - _ = try callSocket(method: "window.record.note", params: ["text": text], label: label) + try note(text, label: label) case .socketLine(let line): let resolved = substituteInline(line) guard let reply = socketLine(resolved, path: socketPath, timeout: 15) else { @@ -211,6 +211,28 @@ final class DogfoodScenarioUITests: XCTestCase { return response["result"] as? [String: Any] ?? [:] } + /// Captions the running clip. + /// + /// A recording stops itself at `max_seconds`, so a long-running nested step + /// can outlive the clip it was being recorded into. A caption with no clip + /// left to write on is then the recording's own limit talking, not a broken + /// tour, and a recording never gates what it observes: `not_found` is logged + /// and the step passes. Every other failure still stops the step. + private func note(_ text: String, label: String) throws { + let method = "window.record.note" + guard let response = socketRequest(method: method, params: substitute(["text": text])) else { + throw DogfoodError("no reply from \(method): \(lastSocketError)") + } + attachText(prettyJSON(response), name: "\(label)-\(method).json") + if response["ok"] as? Bool == true { return } + let code = (response["error"] as? [String: Any])?["code"] as? String ?? "" + guard code == "not_found" else { + throw DogfoodError("\(method) failed: \(prettyJSON(response["error"] ?? response))") + } + log.append("\(label) note not written: no recording is running, so the clip had already " + + "reached its max_seconds") + } + // MARK: Recording /// Records the window while the nested steps run, and attaches the clip. @@ -263,7 +285,12 @@ final class DogfoodScenarioUITests: XCTestCase { let stopped = try callSocket(method: "window.record.stop", params: id.map { ["id": $0] } ?? [:], label: label, suffix: "-stop") - attachClip(named: clipName, format: format, fallback: clip, status: stopped) + // An id-less stop with nothing active answers with the last + // recording the app finished, which belongs to an earlier `record` + // step. Its path and frame count would describe someone else's clip, + // so only a stop we addressed by id is allowed to name the file. + attachClip(named: clipName, format: format, fallback: clip, + status: id == nil ? [:] : stopped) } catch { guard started else { // Nothing was running, so `not_found` here is the right answer @@ -712,8 +739,13 @@ enum DogfoodStep { guard let name = value as? String, !name.isEmpty else { throw DogfoodError("record takes a name for the clip") } - let nested = try (object["steps"] as? [Any] ?? []) - .map { try DogfoodStep(json: $0, insideRecord: true) } + // The guard refuses these too, but it only reads the tours in the + // repository; an ad-hoc file passed to `run-e2e.sh --scenario` gets + // here first, and silently recording nothing is the worst answer. + guard let rawNested = object["steps"] as? [Any], !rawNested.isEmpty else { + throw DogfoodError("record takes a steps array with at least one step to record") + } + let nested = try rawNested.map { try DogfoodStep(json: $0, insideRecord: true) } guard !nested.contains(where: { $0.isRecord }) else { throw DogfoodError("record cannot contain another record: the app records one window at a time") } diff --git a/dogfood/scenarios/record-split-and-palette-tour.json b/dogfood/scenarios/record-split-and-palette-tour.json index 73fb21f329e2..f52bef05ce39 100644 --- a/dogfood/scenarios/record-split-and-palette-tour.json +++ b/dogfood/scenarios/record-split-and-palette-tour.json @@ -1,4 +1,14 @@ { + "paths": [ + "Sources/WindowRecording*", + "Sources/TerminalController+WindowRecording.swift", + "Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/*", + "CLI/CMUXCLI+Record.swift", + "Sources/*Split*", + "Sources/*CommandPalette*", + "Packages/macOS/CmuxCommandPalette/*", + "cmuxUITests/DogfoodScenarioUITests.swift" + ], "steps": [ {"socket": "workspace.create", "params": {"title": "Record", "initial_input": "echo recording\n", "focus": true}, "save": "record"}, {"wait": 1}, diff --git a/scripts/ci/e2e-frames.py b/scripts/ci/e2e-frames.py index 6f44e1a5b2b4..1e4832c7051a 100644 --- a/scripts/ci/e2e-frames.py +++ b/scripts/ci/e2e-frames.py @@ -38,7 +38,11 @@ FRAMES_ARTIFACT = "ui-frames" RESULTS_ARTIFACT = "test-results" IMAGE_SUFFIXES = {".png", ".jpg", ".jpeg", ".heic"} -VIDEO_SUFFIXES = {".mp4", ".mov"} +# A gif counts: `record` tours default to mp4 but a tour that asks for a gif +# still has to reach the pull request, and pr_media.py builds its tour.gif out +# of sampled frames. Sampling a gif needs ffmpeg; the AVFoundation fallback +# below cannot read one and says so instead of sampling. +VIDEO_SUFFIXES = {".mp4", ".mov", ".gif"} FRAME_WIDTH = 960 SHEET_COLUMNS, SHEET_ROWS = 3, 4 TILE_WIDTH, TILE_HEIGHT = 640, 360 diff --git a/skills/cmux-testing/references/dogfood-scenarios.md b/skills/cmux-testing/references/dogfood-scenarios.md index 6957206195d2..3b7db9e65ccb 100644 --- a/skills/cmux-testing/references/dogfood-scenarios.md +++ b/skills/cmux-testing/references/dogfood-scenarios.md @@ -76,7 +76,8 @@ loading. It is a dry run unless dispatched with `-f apply=true` or Give a new tour a `paths` list of `fnmatch` globs (`*` crosses directories), for example `"paths": ["Sources/*Browser*", "Packages/macOS/CmuxBrowser/*"]`. Without one, only a `Dogfood-tours:` line or an edit to the tour file picks it. -The test reads only `steps` and `launch`, so `paths` changes nothing about a run. +The test checks that `paths` is a list of non-empty strings and otherwise +ignores it, so `paths` changes nothing about a run. ## Write a tour @@ -147,14 +148,23 @@ them: of the failure survives. Nested steps are numbered `03.1`, `03.2` in `steps.log`. - A `record` needs at least one nested step and may not contain another `record`, since the app records one window at a time. Both are refused by - `tests/test_dogfood_scenarios.py` before CI runs anything. + `tests/test_dogfood_scenarios.py` before CI runs anything, and again by the + step decoder, which is what an ad-hoc file handed to `run-e2e.sh --scenario` + meets first. Recording nothing is the one outcome worth failing over. - A recording never gates what it records: on a machine that cannot capture at all, the start failure is recorded and the nested steps still run, unrecorded. - `note` draws a caption into the clip. It is worth one before each thing you want a reviewer to notice, because a clip has no step list beside it. It only means anything among a record's own steps, so it is refused anywhere else. + A `note` that arrives after the clip stopped itself at `maxSeconds` is logged + as "note not written" and passes, since that is the limit talking and a + recording never gates what it observes. Every other `note` failure fails the + step. - The clip lands in `attachments/` next to the trees and socket replies, named - after the step and the tour's name for it. + after the step and the tour's name for it. `scripts/ci/e2e-frames.py` samples + a `.gif` the same way it samples an `.mp4`, one frame a second, so either + format reaches the pull request through the frame strip; the clip itself is + uploaded whole as well. - The app records only its own windows, one at a time, and stops by itself at `maxSeconds` (default 15, max 120). Keep `maxSeconds` above the time the nested steps take, or the clip ends early. diff --git a/tests/test_dogfood_scenarios.py b/tests/test_dogfood_scenarios.py index 9f990b991015..79a13d5a5cdb 100644 --- a/tests/test_dogfood_scenarios.py +++ b/tests/test_dogfood_scenarios.py @@ -97,9 +97,14 @@ def check_scenario(tour: object) -> None: if not isinstance(steps, list) or not steps: raise AssertionError("a tour is a steps array or an object with a steps array") if isinstance(tour, dict): - unknown = set(tour) - {"steps", "launch"} + unknown = set(tour) - {"steps", "launch", "paths"} if unknown: raise AssertionError(f"unknown top level keys: {sorted(unknown)}") + paths = tour.get("paths", []) + if not isinstance(paths, list) or not all( + isinstance(pattern, str) and pattern.strip() for pattern in paths + ): + raise AssertionError("paths is a list of non-empty path patterns") check_steps(steps, inside_record=False) From b9a006331673913b2494a216d6312795bc84804c Mon Sep 17 00:00:00 2001 From: Leo Li Date: Wed, 30 Sep 2026 11:34:50 -0700 Subject: [PATCH 15/15] Validate recording regions and step values --- .../WindowRecordingRequest.swift | 5 +++-- tests/test_dogfood_scenarios.py | 18 +++++++++++++----- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingRequest.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingRequest.swift index 052fc8152016..79d09444e4ca 100644 --- a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingRequest.swift +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingRequest.swift @@ -184,10 +184,11 @@ public struct WindowRecordingRequest: Equatable, Sendable { } region = parsed } else if let numbers = value as? [Any] { - let doubles = numbers.compactMap { numericValue($0) } - guard doubles.count == 4 else { + let converted = numbers.map { numericValue($0) } + guard converted.count == 4, !converted.contains(where: { $0 == nil }) else { throw Failure.malformedRegion(String(describing: value)) } + let doubles = converted.compactMap { $0 } region = WindowRecordingRegion( x: doubles[0], y: doubles[1], diff --git a/tests/test_dogfood_scenarios.py b/tests/test_dogfood_scenarios.py index 79a13d5a5cdb..5bf0866cc347 100644 --- a/tests/test_dogfood_scenarios.py +++ b/tests/test_dogfood_scenarios.py @@ -119,13 +119,21 @@ def check_steps(steps: list, inside_record: bool) -> None: f"{', '.join(sorted(DecoderFacts.kinds))}" ) kind = kinds.pop() - if kind == "note" and not inside_record: - raise AssertionError( - f"step {index} captions a clip outside a record; a note belongs " - "among a record's own steps" - ) + if kind == "note": + value = step.get("note") + if not isinstance(value, str) or not value.strip(): + raise AssertionError(f"step {index} has an empty note") + if not inside_record: + raise AssertionError( + f"step {index} captions a clip outside a record; a note belongs " + "among a record's own steps" + ) + continue if kind != "record": continue + record = step.get("record") + if not isinstance(record, str) or not record.strip(): + raise AssertionError(f"step {index} has an empty record name") if inside_record: raise AssertionError(f"step {index} records inside a recording") options = set(step) - {"record", "steps"}