M1: Recording Core + UI (Routing Off)

[Jasonnnz]

Summary

• Add IPC contract types for recording lifecycle: RecordingOptions, RecordingStatus (client-to-server), RecordingStart and RecordingStop (server-to-client)
• Create ScreenRecorder — app-agnostic screen recorder using ScreenCaptureKit + AVAssetWriter (H.264 video, optional AAC audio, 30fps)
• Create RecordingManager — centralized orchestration with single-active recording guard, state machine, and IPC status reporting
• Create source picker UI (RecordingSourcePickerView, RecordingSourcePickerViewModel, RecordingSourcePickerWindow) with display/window selection and audio toggle
• Create recording HUD (RecordingHUDWindow) with animated red dot, elapsed timer, and stop button
• Wire recording_start and recording_stop IPC callbacks through DaemonClient → DaemonMessageRouter → AppDelegate
• Add permission checking with actionable guidance when screen recording access is denied
• Add forceStop() on app termination to clean up active recordings

Routing is not wired in this milestone. Part of #8672, closes #8673.

Test plan

• Verify TypeScript compiles: cd assistant && bunx tsc --noEmit (pre-existing errors only)
• Verify IPC codegen produces valid Swift types
• Verify Swift builds: cd clients/macos && ./build.sh
• Verify IPCRecordingStart, IPCRecordingStop, IPCRecordingStatus, IPCRecordingOptions exist in generated Swift
• Verify ServerMessage enum has recordingStart and recordingStop cases
• Verify DaemonClient has onRecordingStart and onRecordingStop callbacks
• Verify RecordingManager enforces single-active guard (returns false if already recording)
• Verify source picker UI renders with design system tokens

🤖 Generated with Claude Code

---

[Jasonnnz]

@codex review

[Jasonnnz]

Addressed in #8686

[devin-ai-integration]

Devin Review found 4 new potential issues.

View 13 additional findings in Devin Review.

[devin-ai-integration]

🟡 Hardcoded 2x Retina multiplier produces incorrect capture resolution on non-Retina displays

The ScreenRecorder.start() method hardcodes * 2 when computing the capture dimensions from SCDisplay.width/height and SCWindow.frame, assuming all displays are 2x Retina.

Root Cause and Impact

Apple's documentation states that SCDisplay.width and SCDisplay.height return dimensions in points, not pixels. On a 2x Retina display (e.g., built-in MacBook Pro), points × 2 = pixels, so the hardcoded multiplier happens to be correct. However, on a non-Retina external monitor (common with 1080p or 1440p displays connected via HDMI/USB-C), the point dimensions equal the pixel dimensions (1x scale factor).

For a 1920×1080 non-Retina monitor:

• targetDisplay.width = 1920 (points = pixels at 1x)
• Config is set to 3840×2160 (double the actual pixel resolution)
• ScreenCaptureKit delivers upscaled 3840×2160 frames
• H.264 encoding runs at 4K resolution unnecessarily, wasting CPU/GPU and producing files ~4x larger than needed

The same issue affects window capture at ScreenRecorder.swift:105-106.

The correct approach is to query the backing scale factor for the target display (e.g., via NSScreen.backingScaleFactor) instead of assuming 2x.

Prompt for agents

In clients/macos/vellum-assistant/ComputerUse/ScreenRecorder.swift, replace the hardcoded `* 2` Retina multiplier on lines 105-106 and 122-123 with a dynamic scale factor lookup. For display capture (lines 122-123), find the matching NSScreen for the SCDisplay and use its backingScaleFactor. For window capture (lines 105-106), similarly resolve the appropriate screen. Example approach:

1. Add a helper function that finds the NSScreen matching an SCDisplay by comparing display IDs: `private static func scaleFactor(for display: SCDisplay) -> Int`
2. For display capture, use `Int(targetDisplay.width) * scaleFactor` instead of `Int(targetDisplay.width) * 2`
3. For window capture, determine which screen the window is on (or default to NSScreen.main) and use its backingScaleFactor
4. Fallback to 2 if no matching screen is found, to preserve current behavior on unknown configurations

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

🟡 sendStatus in single-active guard sends stale attachToConversationId from the existing recording

When RecordingManager.start() is called while another recording is already active, the "already active" guard at line 66 rejects the new request and calls sendStatus to report a failure. However, sendStatus (line 200) reads self.attachToConversationId, which at that point belongs to the currently active recording — not the new request that just failed.

Root Cause

The guard fires before self.attachToConversationId is updated (lines 72-73 only execute after the guard passes). So the recording_status message sent to the daemon for the new rejected session carries the old recording's attachToConversationId.

For example:

1. Recording A starts with attachToConversationId = "conv-A"
2. Recording B is requested with attachToConversationId = "conv-B"
3. Guard fires at line 66, sendStatus is called at line 68
4. sendStatus at line 200 reads self.attachToConversationId which is still "conv-A"
5. Daemon receives a failure status for session B but with attachToConversationId = "conv-A"

Impact: The daemon receives an incorrect attachToConversationId on the failure status for the rejected recording request, which could cause incorrect state tracking or misattributed failures.

Suggested change

	guard !state.isActive else {
	log.warning("Cannot start recording — already active (state=\(String(describing: self.state)), owner=\(self.ownerSessionId ?? "nil"))")
	sendStatus(sessionId: sessionId, status: "failed", error: "Another recording is already active")
	return false
	}
	guard !state.isActive else {
	log.warning("Cannot start recording — already active (state=\(String(describing: self.state)), owner=\(self.ownerSessionId ?? "nil"))")
	sendStatus(sessionId: sessionId, status: "failed", error: "Another recording is already active", overrideConversationId: attachToConversationId)
	return false
	}

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

🟡 RecordingSourcePickerWindow.dismiss() silently drops pending cancel callback

When RecordingSourcePickerWindow.show() is called while the picker is already open (e.g., a second recording_start arrives with promptForSource: true), the internal dismiss() at line 25 tears down the old window without firing its onCancelCallback. The daemon never receives a failure notification for the first recording request.

Root Cause

dismiss() at RecordingSourcePickerWindow.swift:73-78 sets window?.delegate = nil before closing, which prevents windowWillClose from firing. It also doesn't call fireCancel() itself. Then show() overwrites onCancelCallback at line 27 with the new callback, permanently losing the old one.

Sequence:

1. recording_start feat: initialize Next.js app in /web directory #1 arrives with promptForSource: true → picker shown, onCancelCallback set to notify daemon about request feat: initialize Next.js app in /web directory #1
2. recording_start feat: add platform terraform for GKE deployment #2 arrives with promptForSource: true → show() calls dismiss() (no cancel fired), then sets onCancelCallback to notify daemon about request feat: add platform terraform for GKE deployment #2
3. The daemon never learns that request feat: initialize Next.js app in /web directory #1 was abandoned

Impact: The daemon would continue waiting for a status update on the first recording request indefinitely. This is an edge case requiring two rapid recording_start messages with promptForSource: true.

Suggested change

	func show(onStart: @escaping (IPCRecordingOptions) -> Void, onCancel: @escaping () -> Void) {
	// Dismiss any existing picker window
	dismiss()
	func show(onStart: @escaping (IPCRecordingOptions) -> Void, onCancel: @escaping () -> Void) {
	// Fire any pending cancel callback before tearing down the old window
	fireCancel()
	// Dismiss any existing picker window
	dismiss()

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

🟡 ScreenRecorder.start() leaks resources when startCapture() fails — no cleanup of writer, stream, or delegate

When SCStream.startCapture() throws in ScreenRecorder.start(), the method re-throws without calling cleanUpWriter(). This leaves stale references in assetWriter, videoInput, audioInput, stream, and outputDelegate — none of which are cleaned up.

Root Cause

At ScreenRecorder.swift:190-196, all instance state is set before attempting to start the capture:

self.stream = captureStream       // line 190
do {
    try await captureStream.startCapture()  // line 194 — can throw
} catch {
    throw RecorderError.streamStartFailed(error.localizedDescription)  // line 196 — leaks!
}

By this point, self.assetWriter (line 177), self.videoInput (line 161), self.audioInput (line 174), self.outputDelegate (line 181), and self.stream (line 190) are all populated. The thrown error propagates to RecordingManager.start() which catches it, but the ScreenRecorder's internal state remains stale.

Because isRecordingActive is never set to true (that happens at line 199), both stop() and cancelRecording() will early-return on their isRecordingActive guard, leaving these resources stranded until the next successful start() overwrites them.

Impact: Leaked AVAssetWriter, SCStream, and delegate objects persist in memory. The StreamOutputDelegate holds a strong reference back to ScreenRecorder (ScreenRecorder.swift:332), creating a retain cycle that prevents deallocation if the recorder is ever released. In practice the recorder lives as long as the app, so the impact is limited to inconsistent internal state and minor memory waste.

Suggested change

	do {
	try await captureStream.startCapture()
	} catch {
	throw RecorderError.streamStartFailed(error.localizedDescription)
	}
	do {
	try await captureStream.startCapture()
	} catch {
	cleanUpWriter()
	throw RecorderError.streamStartFailed(error.localizedDescription)
	}

---

Was this helpful? React with 👍 or 👎 to provide feedback.