Repository navigation
Add cmux record for capturing a cmux window to mp4 or gif - #17756
azooz2003-bit wants to merge 14 commits into
Conversation
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
`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) <noreply@anthropic.com>
`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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
# Conflicts: # cmux.xcodeproj/project.pbxproj
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) <noreply@anthropic.com>
Optional tuples are not Equatable, so the caption test did not compile. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
CGImageDestinationFinalize only succeeds when the destination received exactly the image count it was created with, so a gif opened with a count of one could not take a second frame. The gif writer now keeps each frame as PNG data and builds the gif in finish. The mp4 timing test skips the empty marker buffers the asset reader returns. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
# Conflicts: # cmux.xcodeproj/project.pbxproj
Four defects from the review: - `--region 0,0,1e19,1e19` killed the app. `Int(_: Double)` traps past Int's range, and only fps, scale and max-width were bounded. A region is bounded now, and the geometry clamps rather than traps, so a request built in code cannot take the app down either. - `--out` on a read-only volume answered `internal_error`. The writer's own failures had no mapping, and opening the file is the caller's parameter. - A stop that lands while the start is still opening the clip reported "the recording has already finished", an event that never happened, and the start answered `internal_error`. Both say stopped-while-starting now, with the code `conflict`. - The backpressure wait spun once its task was cancelled, which is exactly when a stop is waiting behind it. It uses a delay a cancelled task still waits out. The contract also says where the frames go while a clip is open, since quitting cmux mid-recording leaves that file behind. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 52 minutes. View limit detailsLimit details: You’ve used all 10 included reviews currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (32)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Note Pull Request opener @azooz2003-bit is not an author or co-author of any commit in this PR (commit identities: All contributors have signed the CLA ✍️ ✅ |
Summary
An agent working inside cmux can say what it changed but it cannot show it. Screenshots exist (
cmux screenshot), yet anything with motion, a drag, a resize, a sidebar animation, a terminal filling up, has to be described in prose and taken on faith. This addscmux record, which films a cmux window and writes an mp4 or a gif that can go straight into a pull request or a bug report.startprints<id> <state> <frames> <path>, or the whole response with--json.--region x,y,w,hrecords part of the window in window points,--window <id|ref>picks a window other than the frontmost one, and--format,--fps,--scale,--max-width,--outand--labelshape the output.record notewrites a caption into the frames from that moment on, so a reader can follow what was being done;--no-captionsturns that off.record statusandrecord listreport on a clip in flight.Mechanism:
SCShareableContent.currentProcess, so only cmux's own windows are reachable. No Screen Recording permission is requested or needed, and no other application can be filmed. Frames are sampled on a schedule rather than through a stream delegate, which keeps a slow encode from stalling the window it is filming.CGImageDestinationfor gif. Frame times come from when each frame was actually sampled, so playback matches what happened rather than assuming an even frame rate. mp4 presentation times are strictly increasing at timescale 600; the gif encoder holds one frame back so each delay is the measured gap to the next frame.WindowRecordingSessionis an actor that owns its filter, writer, geometry and captions;WindowRecordingRegistryis an actor that allows one recording at a time. A recording stops itself at--max-seconds(0.5 to 120, default 15), so an agent that goes away cannot leave a capture running.Socket policy, since this adds v2 methods:
window.record.start|stop|status|note|listaresocketWorker(mainThreadCallable: false). They must not run inline on the main thread, because the window being filmed has to keep drawing while the sampler runs.cmux sshrelay allowlist, which stays default-deny. A clip is local screen content, not an object of the peer's workspace: a relay peer is authorized for one workspace's objects, and whatever the local user happens to have on display is not among them. There is no remote flow these verbs would serve, so nothing is allowlisted andRemoteRelayCoreRPCPolicyTestsasserts each verb is denied both bare and with aworkspace_idfor the owning workspace. The methods carry no command-bearing parameters;--outis a local file path validated to be absolute and to match the chosen format, and it is only reachable from the local socket.Open design question for review: whether whole-window capture should be available in Release at all, or stay a debug-only capability with only region capture shipping. The screenshot helpers next door are
#if DEBUG; this PR ships in Release and addsSources/WindowRecordingWindowSelection.swift(about 12 lines) rather than un-gating them, so the decision stays reversible either way. Say the word and the gate goes on.Depends on #15265. That PR teaches
scripts/localize-changesto read Swift multi-linedefaultValue:literals; the 31-linecmux recordhelp text is the first of those, and without the fix the helper silently blanks the English catalog value. CI does not runlocalize-changesitself, so this PR is green without it, but the catalog here was produced with that fix applied and editing this help text later needs it landed.Testing
Added and executed:
CmuxFoundationTestscovering the model layer: request parsing and limits, region parsing and rejection, output naming, caption tracks, and frame geometry including the resize and crop paths. Run green in a package-only harness (swift test, 39 tests passed); in CI they run in the CmuxFoundation package lane.cmuxTests/WindowRecordingPipelineTests.swift, a new suite over composition and encoding: composed frames match the encoded frame size, a region crop keeps only the requested pixels, a window resized mid-clip is letterboxed rather than stretched, a caption darkens the bottom left and leaves the rest alone, fitting never upscales past the frame, an mp4 carries the frames and their uneven elapsed timing, two frames in the same tick still get increasing times, an empty mp4 fails instead of leaving a zero-frame file, a gif holds every frame and its measured delay, gif delays stay in the playable range, and an empty gif fails. These need AVFoundation and ImageIO, so they run in the app test lane in CI, not here.ControlCommandExecutionPolicyTests: the fivewindow.record.*methods are worker-lane and not main-thread callable.RemoteRelayCoreRPCPolicyTests: each of the five is denied, with and without an owningworkspace_id, and never appears inpermittedMethods.Executed locally:
python3 scripts/verify-local.pyselected 8 checks against this diff and all 8 passed (swift-syntax, xcstrings, localization parity, project normalization, app-source wiring, test wiring, package groups, feature flags). Native compilation, app tests and app launch were not run here by policy; they run in CI.Not yet verified: live capture. No frame has been recorded from a running window by me, because builds and dogfooding for this work happen on the fleet rather than locally. A fleet dogfood is queued and its clip will be attached below before merge, which is also the first end-to-end proof: the demo video for this PR is meant to be recorded by the feature itself.
Localization audited: 6 new keys in
Resources/Localizable.xcstrings.cli.help.recordand the fourcli.record.error.*keys are translated into all 9 macOS locales with line breaks matching the English source;cli.usage.recordis pure flag syntax and is registered inscripts/localization-allowed-omissions.jsonwithclass: "syntax".scripts/localization_catalog.py checkand./scripts/localize-changesboth report 0 parity errors.docs/cli-contract.mdgains therecordrow and its--helpprobe line.Changelog
Added:
cmux recordcaptures a cmux window or a region of one to an mp4 or gif, withcmux record notecaptions, for pasting into a pull request or bug reportDemo Video
Fleet dogfood queued; the clip will be attached here (recorded with
cmux recorditself) before merge.Checklist
cmux ssh: not allowlisted, and the reasoning is above🤖 Generated with Claude Code
Summary by cubic
Adds
cmux record, which films a cmux window (or a region of one) to an mp4 or gif for pasting into a pull request or bug report. Agents could already show stills viacmux screenshot, but anything with motion — a drag, a resize, a terminal filling up — had to be described in prose. Thewindow.record.*verbs run on the socket worker and are not main-thread callable (the filmed window has to keep drawing while the sampler runs), stay off thecmux sshrelay allowlist as default-deny local screen content, and depend on #15265 soscripts/localize-changescan read the multiline help text.New Features
cmux record start|stop|status|note|listdrives a capture;noteburns captions into frames andstartprints id, state, frame count, and output path (or JSON with--json).--max-seconds(0.5–120, default 15) so an abandoned agent can't leave a capture running.Bug Fixes
--fpsand--regionvalues that are not finite or don't fit anInt(nan,inf,1e30,1e19) crashed the app; they are now refused or clamped.stopafter--max-secondsreports the finished clip instead of "no recording".record startthat timed out on the socket no longer leaves a recording the caller was told failed; it is abandoned and its writer and partial file are released before the socket answers.conflicton both sides, and the backpressure wait no longer spins once its task is cancelled.fitno longer upscales a window that shrank.--outfiles are only replaced by a finished clip: frames go to a hidden sibling file, cleaned up on every failure path, and a path that is a directory, device, or read-only volume is refused asinvalid_params.--label --gifis refused rather than swallowed as the flag's value, andrecord notestrips only a leading--terminator so a note may start with--.Written for commit f690d16. Summary will update on new commits.
Summary by CodeRabbit
recordcommand.Migrated from #15277 after correcting the PR author identity. The head branch and commit history are preserved.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.