Record a clip around dogfood tour steps by azooz2003-bit · Pull Request #17774 · manaflow-ai/cmux

azooz2003-bit · 2026-10-06T20:08:54Z

Stacked on #15277: the first two commits are that PR's, and only 1a56b664393 is new. Review that one, and merge this after #15277.

A dogfood 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 the workspace"},
  {"clickAt": {"x": 0.08, "y": 0.12}},
  {"wait": 1},
  {"shot": "mid-drag"}
]}

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 (the same reason the control socket lives there), 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 next to it.

A nested step that fails is recorded and the rest still run, as at the top level, so the recording always gets stopped and the clip of the failure survives instead of being lost with it. Nested steps are numbered 03.1, 03.2 in steps.log.

The decoder refuses a recording inside a recording, because the app records one window at a time, and refuses an unknown option instead of dropping it: a silently ignored maxseconds would cut a tour's clip short.

dogfood/scenarios/record-split-and-palette-tour.json is a tour that uses it.

Testing

tests/test_dogfood_scenarios.py (new, linux-guard lane) checks the tours and the skill reference against the decoder in a second rather than after a CI run of tens of minutes: every step kind the decoder accepts is documented, no documented step is undecoded, every recording option is documented, and every checked-in tour uses only known steps and options with no nested or empty recording. Both scenario arms were confirmed by breaking the new tour, renaming maxSeconds to maxseconds and nesting a record step; each fails the guard, and it passes again with them restored.

python3 scripts/verify-local.py --affected origin/main: 14/14 passed. The recording path itself needs a window server and is exercised by running the new tour on a CI runner, which is the dogfood evidence for this PR.

Changelog

none

🤖 Generated with Claude Code

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

Summary by cubic

Adds cmux record so an agent working in its own cmux can capture a window, or a region of one, to an mp4 or gif for a pull request, and lets dogfood tours wrap steps in a recording so a drag, an animation, or the order views settle in can be shown as a clip.

Recording

cmux record start|stop|status|note|list captures through ScreenCaptureKit's own-process content, so no Screen Recording permission is involved.
A recording stops itself at --max-seconds, only one runs at a time, and frames go straight to disk instead of being held in memory.
Output moves into place only when the clip is finished, so a failed recording no longer destroys whatever was at --out; a malformed --region, or one outside the window, is refused.

Dogfood tours

A record step runs nested steps under a recording and attaches the clip; note draws the caption into it, and nested steps are numbered 03.1, 03.2.
A failed record start runs the steps unrecorded instead of gating them; the decoder refuses a recording inside a recording, unknown options, a top-level note, and a record step with missing or empty steps.
A note sent after the clip self-stopped at max-seconds logs and passes, and only a stop addressed by id may name the file it closes.
tests/test_dogfood_scenarios.py checks every tour and the step reference against the decoder in seconds; it accepts the paths key tours use, and the guard also runs when the decoder itself changes.
Clips are kept whole in attachments as well as sampled into frames, gifs included; an mp4 clip previously never left the xcresult bundle.

^{Written for commit b9a0063. Summary will update on new commits.}

Summary by CodeRabbit

New Features
- Record a cmux window or selected region as an MP4 or GIF using cmux record. Start and stop recordings, check their status, add captions, and list recent recordings.
- Choose recording quality, frame rate, duration, output path, and label. Recordings stop automatically when the requested duration is reached.
Documentation
- Added localized command help and updated CLI and dogfood scenario documentation.

Migrated from #15320 after correcting the PR author identity. The head branch and commit history are preserved.

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 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) <noreply@anthropic.com>

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) <noreply@anthropic.com>

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) <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>

`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) <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>

Both sides added a step kind to the tour runner, so the step enum, its summary, its decoder and the kind list keep both `socketLine` and `record`/`note`. usesSocket is true for all four. The scenario table in the testing skill lists them in the same order. Brings in the recorder's Equatable Pixel, which is what the last dogfood dispatch failed to compile. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

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) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

cursor · 2026-10-06T20:09:00Z

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

coderabbitai · 2026-10-06T20:09:07Z

Warning

Review limit reached

You'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 51 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Advanced
Run ID: 68948b03-cecd-4857-8a60-3dfda7781504

📥 Commits

Reviewing files that changed from the base of the PR and between a8c4861 and b9a0063.

📒 Files selected for processing (40)

.github/workflows/ci-guards.yml
CLI/CMUXCLI+CommandSuggestions.swift
CLI/CMUXCLI+Record.swift
CLI/CMUXCLI+TaskHelp.swift
CLI/CMUXCLI+WindowDispatch.swift
CLI/cmux.swift
Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Wire/ControlCommandExecutionPolicy.swift
Packages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/ControlCommandExecutionPolicyTests.swift
Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingCaptionTrack.swift
Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingFrameGeometry.swift
Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingLabel.swift
Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingRegion.swift
Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingRequest+Output.swift
Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/WindowRecording/WindowRecordingRequest.swift
Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/WindowRecordingCaptionTrackTests.swift
Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/WindowRecordingFrameGeometryTests.swift
Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/WindowRecordingOutputNamingTests.swift
Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/WindowRecordingRequestTests.swift
Packages/macOS/CmuxRemoteWorkspace/Tests/CmuxRemoteWorkspaceTests/RemoteRelayCoreRPCPolicyTests.swift
Resources/Localizable.xcstrings
Sources/TerminalController+Capabilities.swift
Sources/TerminalController+WindowRecording.swift
Sources/TerminalController.swift
Sources/WindowRecordingFrameComposer.swift
Sources/WindowRecordingFrameWriter.swift
Sources/WindowRecordingRegistry.swift
Sources/WindowRecordingSession.swift
Sources/WindowRecordingWindowSelection.swift
cmux.xcodeproj/project.pbxproj
cmuxTests/WindowRecordingErrorCodeTests.swift
cmuxTests/WindowRecordingPipelineTests.swift
cmuxUITests/DogfoodScenarioUITests.swift
docs/cli-contract.md
dogfood/scenarios/record-split-and-palette-tour.json
scripts/ci/e2e-frames.py
scripts/ci/workflow_guard_groups.py
scripts/localization-allowed-omissions.json
skills/cmux-testing/references/dogfood-scenarios.md
tests/test-execution.toml
tests/test_dogfood_scenarios.py

Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

github-actions · 2026-10-06T20:09:11Z

Note

Pull Request opener @azooz2003-bit is not an author or co-author of any commit in this PR (commit identities: teamleaderleo, claude). The CLA check will still proceed and requires every listed identity plus @azooz2003-bit to have signed.

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

teamleaderleo and others added 18 commits September 28, 2026 02:12

Merge remote-tracking branch 'upstream/main' into HEAD

4b992e1

# Conflicts: # cmux.xcodeproj/project.pbxproj

Compare recorded pixels through an Equatable value

69a90ad

Optional tuples are not Equatable, so the caption test did not compile. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Merge origin/main into feat/dogfood-record-step

f032d90

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Validate recording regions and step values

b9a0063

azooz2003-bit requested a review from lawrencecchen as a code owner October 6, 2026 20:08

azooz2003-bit closed this Oct 6, 2026

Navigation Menu

Repository navigation

Record a clip around dogfood tour steps - #17774

Record a clip around dogfood tour steps#17774
azooz2003-bit wants to merge 18 commits into
mainfrom
feat/dogfood-record-step

azooz2003-bit commented Oct 6, 2026

Uh oh!

cursor Bot commented Oct 6, 2026

Uh oh!

coderabbitai Bot commented Oct 6, 2026

Review limit reached

Uh oh!

github-actions Bot commented Oct 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

azooz2003-bit commented Oct 6, 2026

Testing

Changelog

Summary by cubic

Summary by CodeRabbit

Uh oh!

cursor Bot commented Oct 6, 2026

Bugbot is paused — on-demand spend limit reached

Uh oh!

coderabbitai Bot commented Oct 6, 2026

Review limit reached

Uh oh!

github-actions Bot commented Oct 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants