fix(preview): improve browser recording quality#8839

🟡 Medium preview/Manager.ts:3151

A failed startRecording can stop a newer recording: its Effect.onError unconditionally calls stopFrameCapture(tabId, "recording"), which removes whichever recording lease currently exists. If the first start fails after a second start succeeds, the second caller has a source ID but its capture lease and background-throttling protection are released. Associate cleanup with this invocation’s lease or serialize start/stop operations.

fixed in d876adc0e by serializing start and stop through the existing per-tab lifecycle lock. the new regression test holds the first measurement open, starts a second recording, fails the first, and verifies the second lease remains active through stop.

🟠 High preview/Manager.ts:3142

startRecording can return a media sourceId for a detached guest after the tab has registered a replacement webview, causing recording to target the old page or fail to acquire the source. executeJavaScript yields before wc.getMediaSourceId(requestWebContents), but the captured wc is never revalidated against the tab's current webContentsId; re-check that identity before obtaining the source ID (or serialize recording startup with webview replacement).

fixed in d876adc0e by serializing source acquisition with webview registration and revalidating the current WebContents after the async size read. the replacement-race test verifies registration cannot replace the guest while the source is being acquired.

This guard is a pure validation failure, but it is expressed by throwing a bare Error inside attempt, so the only failure that reaches callers is a generic PreviewOperationError ("Desktop preview operation failed: recording.measureSource") whose real reason exists solely as a manufactured cause. Convention here is to define the failure with Schema.TaggedErrorClass and structured attributes and derive message from them, rather than fabricating an Error just to populate cause (a validation error created without an underlying failure needs no cause).

Consider adding a dedicated error next to the other preview errors, e.g.

and failing with return yield* new PreviewRecordingSourceSizeUnavailableError({ tabId, webContentsId: wc.id }) after a plain (non-attempt) shape check, keeping attempt/attemptPromise for the calls that can actually throw (executeJavaScript, getMediaSourceId).

^{Posted via Macroscope — Effect Service Conventions}

fixed in d876adc0e with PreviewRecordingSourceSizeUnavailableError, a structured Schema.TaggedErrorClass carrying tabId and webContentsId. invalid dimensions now fail directly without a manufactured cause, with focused coverage.

Recording uses stale webview source

Medium Severity

startRecording keeps the WebContents from requireWebContents across executeJavaScript and then calls getMediaSourceId on that snapshot. A guest swap in that window, or later in registerWebview, leaves the renderer stream on the old sourceId, so the recording can capture a destroyed tab.

^{Reviewed by Cursor Bugbot for commit 7c0541f366aec0bfcd2004564d5d0d389f37a8c0. Configure here.}

This is a pure domain condition (the recorder never reported an output format), but it is expressed as a bare Error inside the try that maps everything to operation: "save-artifact". Callers therefore only ever see Browser recording operation save-artifact failed for tab X., with the real reason surviving solely as a manufactured cause — the convention is to model such failures with Schema.TaggedErrorClass and structured attributes and derive message from them, not to fabricate an Error to populate cause.

Consider resolving the mime type before the try, and failing with a dedicated error, e.g. add beside the other error classes:

and then throw new BrowserRecordingFormatUnavailableError({ tabId }) outside the save-artifact wrapper so the structured failure reaches callers unwrapped.

^{Posted via Macroscope — Effect Service Conventions}

fixed in 08bbfe04e with BrowserRecordingFormatUnavailableError, raised before the save-artifact wrapper so the structured failure reaches callers unchanged. the new test verifies the error, skipped save, and recording-slot cleanup.

🟠 High browser/browserRecording.ts:213

When the renderer is hidden or minimized, startBrowserRecording never settles, and a subsequent stop times out while retaining the active recording and native capture lease indefinitely. The two requestAnimationFrame callbacks are paused for hidden documents, so this wait needs a bounded timer fallback that cancels the pending frame work when it settles.

fixed in 5f9370b20. compositor warmup now races the two frame callbacks against a 250 ms fallback and cancels any still-pending frame callbacks on either path. the focused test covers a renderer where requestAnimationFrame never fires.

Hidden webviews show through chrome

Medium Severity

Hidden renderingActive guests now sit at the viewport origin with visibility: visible and z-index: -1. That stacks them above the root background, so glass and other translucent chrome reveal the guest and the wrapper’s bg-muted/35 for the whole recording, picture-in-picture, or background-activity lifetime.

^{Reviewed by Cursor Bugbot for commit 5f9370b201281b43121c29ce814ca65292d90a3f. Configure here.}

fixed in 14cb6ffa3. hidden guests are back at the original offscreen position, so they cannot bleed through translucent chrome. native recording now primes the guest with one non-encoded capturePage call before requesting its media source; the steady-state recording remains the native stream.

Background recording warmup can fail

Medium Severity

startRecording now requires a one-shot capturePage warmup, and that call is fatal. It runs inside startScreencast before the renderer applies visibility: visible or waits for compositor frames, while hidden guests stay at HIDDEN_BROWSER_WEBVIEW_OFFSET. A transient compositor miss on a never-shown tab fails the whole start, and the later paint wait no longer composites an on-screen guest.

^{Reviewed by Cursor Bugbot for commit 14cb6ffa38975f6a1282c256ccc5911e8d02f0f0. Configure here.}

fixed in 4db19c241. the renderer now waits for the hidden surface to receive two paint opportunities before requesting the native source. the one-shot capture warmup retries once and is best-effort, so a transient or permanent capturePage rejection no longer aborts native acquisition. focused tests cover paint-before-source ordering, paused-frame fallback, and warmup rejection.

🟠 High browser/browserRecording.ts:393

startBrowserRecording() can remain stuck indefinitely at captureTabMediaStream() when the getUserMedia() permission prompt is ignored, so its finally never runs and the active-recording slot plus surface-activity lease remain held. Add a timeout/cancellation path that releases the recording and lease, and stops any stream that resolves after cancellation.

fixed in ff7e4b9d0. native stream acquisition now shares the existing five-second startup bound. timeout runs the normal failed-start cleanup, which stops the desktop capture lease and releases the browser surface/activity slot. if getUserMedia resolves after timeout, every late track is stopped immediately. the regression test covers the timeout, released state, and late-stream disposal.

The acquisition timeout is a pure domain condition (no underlying failure exists), but it is expressed by manufacturing an Error whose message is the only record of what happened. The call site then wraps it as BrowserRecordingOperationError({ operation: "capture-media-stream" }), whose message is derived solely from operation/tabId, so a stalled getUserMedia and a rejected getUserMedia become indistinguishable to callers except through the fabricated cause string.

Consider modeling it the same way this PR already models the missing output format: a Schema.TaggedErrorClass (e.g. BrowserRecordingCaptureTimeoutError with tabId and timeoutMs, no cause) thrown from this helper, and let the capture-media-stream catch block pass an already-structured recording error through instead of re-wrapping it.

^{Posted via Macroscope — Effect Service Conventions}

fixed in 34406a298. capture timeout is now BrowserRecordingCaptureTimeoutError with structured tabId and timeoutMs fields and no fabricated cause. after the existing failed-start cleanup succeeds, the typed timeout passes through unchanged; a real cleanup failure still preserves its cause in BrowserRecordingOperationError. the focused timeout test now asserts the tagged error shape.