feat(cli): headless record / export / info commands - #176
Open
PeterTakahashi wants to merge 2 commits into
Open
Conversation
Adds a CLI so scripts, CI pipelines, and AI coding agents can drive OpenScreen end-to-end without a visible window: openscreen record --duration 20 --project demo.openscreen --json openscreen export demo.openscreen -o demo.mp4 --audio voice.m4a --json openscreen info demo.openscreen --json Design: CLI mode boots Electron without HUD/tray/menu and drives a hidden runner window (windowType=cli-export|cli-record) that reuses the existing pipelines unchanged — VideoExporter/GifExporter for export (the approach already proven by the HEADLESS=true e2e test) and useScreenRecorder for recording (native SCK/WGC helpers, browser fallback). registerIpcHandlers is reused with inert window callbacks, so the 2900-line handler module is untouched. - electron/cli/args.ts: pure argv parser + unit tests; skips leading Chromium switches (AppImage --no-sandbox) before subcommand detection - electron/cli/cliMain.ts: stdio protocol (NDJSON with --json, TTY-aware progress otherwise), SIGINT/SIGTERM/stdin stop, EPIPE-safe writes, exit codes 0/1/2, console rerouted to stderr, SwiftShader fallback - src/cli/CliExportRunner.tsx: loads .openscreen via the native bridge, rebuilds the editor's exporter config deterministically (1280x720 reference preview box, overridable via --preview-size), optional voiceover mix (--audio/--audio-mode/--audio-offset) that copies video packets and re-renders audio offline via OfflineAudioContext+mediabunny - src/cli/CliRecordRunner.tsx: source pick by display index or window title, mic device by label, --duration/signal/stdin stop, optional ready-to-export --project output - useScreenRecorder: expose startRecordingImmediately() (skips countdown) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019ceTJjoPJYa5BdmXZdDc5J
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (8)
📝 WalkthroughWalkthroughAdds a headless Electron CLI for recording, inspecting projects, and exporting to GIF or MP4 with NDJSON output and optional voiceover mixing. It includes argument parsing, hidden renderer runners, IPC contracts, recording controls, export integration, tests, and documentation. ChangesHeadless CLI
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant ElectronMain
participant HiddenRunner
participant RecorderOrExporter
participant FileSystem
CLI->>ElectronMain: invoke record or export
ElectronMain->>HiddenRunner: load cli-record or cli-export window
HiddenRunner->>RecorderOrExporter: request and execute workflow
RecorderOrExporter-->>ElectronMain: progress, logs, and completion
ElectronMain->>FileSystem: write project or export output
ElectronMain-->>CLI: NDJSON events and exit status
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (7)
src/cli/CliExportRunner.tsx (1)
42-57: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNo timeout on metadata probe — a stalled load hangs the CLI forever.
onerrorcovers decode failures, but a media element that never fires either event (unreachable/locked file, stalled reader) leaves the promise pending, and the CLI has no export-side deadline to fall back on. A bounded wait keepsopenscreen exportfrom hanging headlessly.♻️ Proposed fix
function probeVideoDimensions(url: string): Promise<{ width: number; height: number }> { return new Promise((resolve, reject) => { const video = document.createElement("video"); video.preload = "metadata"; video.muted = true; + const timer = setTimeout(() => { + video.removeAttribute("src"); + video.load(); + reject(new Error(`Timed out reading video metadata: ${url}`)); + }, 30_000); video.onloadedmetadata = () => { + clearTimeout(timer); const width = video.videoWidth; const height = video.videoHeight; video.removeAttribute("src"); video.load(); resolve({ width, height }); }; - video.onerror = () => reject(new Error(`Failed to load video metadata: ${url}`)); + video.onerror = () => { + clearTimeout(timer); + reject(new Error(`Failed to load video metadata: ${url}`)); + }; video.src = url; }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cli/CliExportRunner.tsx` around lines 42 - 57, Update probeVideoDimensions to enforce a bounded timeout for video metadata loading, rejecting the promise when neither onloadedmetadata nor onerror fires before the deadline. Ensure successful metadata loads and errors clear the timeout and perform the existing video cleanup so the CLI cannot remain pending indefinitely.electron/cli/args.test.ts (1)
4-8: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPath assertions are POSIX-only.
resolvePathdelegates topath.resolve, so on a Windows runnerparse(["export","demo.openscreen"])yields\work\demo.openscreenand every hard-coded/work/...expectation fails. Build expectations withpath.resolve(CWD, …)(orpath.join) so the suite is platform-agnostic.♻️ Suggested approach
+import path from "node:path"; import { describe, expect, it } from "vitest"; import { parseCliArgs } from "./args"; const CWD = "/work"; +const at = (name: string) => path.resolve(CWD, name);Then use
projectPath: at("demo.openscreen")instead of the literal.Also applies to: 17-20
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@electron/cli/args.test.ts` around lines 4 - 8, Update path expectations in the tests around parse and the projectPath assertions to use a platform-aware helper such as at, backed by path.resolve(CWD, ...) or path.join(CWD, ...), instead of hard-coded /work/... strings. Ensure all affected expected paths, including demo.openscreen, match resolvePath on Windows and POSIX systems.src/lib/exporter/voiceoverMix.ts (1)
105-135: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winCancel the partially started Output if remux fails.
After
output.start()fails once adding packets oraudioSource.add(mixedAudio)throws, thefinallyonly disposes the input;outputremains started and unfinalized, andBufferTarget.bufferwill not be the remux output. Track whetheroutput.start()has resolved and awaitoutput.cancel()in the catch path before rethrowing.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/exporter/voiceoverMix.ts` around lines 105 - 135, The remux cleanup must cancel a started but unfinalized Output when packet or audio processing fails. In the surrounding flow that creates output and calls output.start(), track successful startup, catch subsequent errors, await output.cancel() when startup completed, then rethrow; retain input.dispose() in finally and avoid canceling if output.start() never resolved.electron/preload.ts (1)
309-317: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueType these against the CLI contracts instead of
unknown.
electron-env.d.tsalready declaresCliProgressEvent/CliDoneResult; typing the preload the same way keeps the two sides from drifting.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@electron/preload.ts` around lines 309 - 317, Update the cliProgress and cliDone methods in the preload API to use the existing CliProgressEvent and CliDoneResult types from electron-env.d.ts instead of unknown, while preserving their current IPC behavior and cliLog signature.electron/cli/cliMain.ts (1)
245-252: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
JSON.stringifyon a circular arg throws outsidesafeWrite.The
args.map(...)runs beforesafeWrite, so any log call carrying a circular object (common with Electron/DOM-ish objects) throws fromconsole.log, escaping into theuncaughtExceptionhandler and killing the CLI run.♻️ Proposed fix
+ const stringify = (a: unknown) => { + if (typeof a === "string") return a; + try { + return JSON.stringify(a); + } catch { + return String(a); + } + }; for (const level of ["log", "info", "warn", "error", "debug"] as const) { console[level] = (...args: unknown[]) => { - safeWrite( - process.stderr, - `${args.map((a) => (typeof a === "string" ? a : JSON.stringify(a))).join(" ")}\n`, - ); + safeWrite(process.stderr, `${args.map(stringify).join(" ")}\n`); }; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@electron/cli/cliMain.ts` around lines 245 - 252, Update the console method overrides in the level loop so argument serialization cannot throw before safeWrite, including for circular or otherwise non-serializable objects. Preserve string arguments and the existing space-separated output, while using a safe fallback representation for values that JSON.stringify cannot handle.src/cli/CliRecordRunner.tsx (2)
122-126: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRef writes during render.
toggleRecordingRef/recordingRefare mutated in the render body; React may discard or replay that work. Move both into an effect.♻️ Proposed fix
- const toggleRecordingRef = useRef(toggleRecording); - toggleRecordingRef.current = toggleRecording; - const recordingRef = useRef(recording); - recordingRef.current = recording; + const toggleRecordingRef = useRef(toggleRecording); + const recordingRef = useRef(recording); + useEffect(() => { + toggleRecordingRef.current = toggleRecording; + recordingRef.current = recording; + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cli/CliRecordRunner.tsx` around lines 122 - 126, Move the current-value assignments for toggleRecordingRef and recordingRef out of the render body and into a React effect in the surrounding component. Keep both refs initialized with their corresponding values and update their .current fields after committed renders so the stop/finish effects continue using the latest values.Source: Linters/SAST tools
98-108: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo test accompanies this runner.
The cohort only adds
electron/cli/args.test.ts; the record runner's bootstrap/stop/completion state machine is untested. As per coding guidelines, "Add a test for every new behavior in the same package as the code under test." Happy to draft a test that stubswindow.electronAPIand asserts the start/stop/cliDonetransitions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cli/CliRecordRunner.tsx` around lines 98 - 108, Add tests in the same package for CliRecordRunner’s bootstrap and recording state machine, stubbing window.electronAPI as needed. Cover recorder start, stop, and cliDone completion transitions, including the requestReady bootstrap path.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/cli.md`:
- Around line 7-9: Add a language identifier to the fenced code block containing
the record-to-export workflow in docs/cli.md, using text for descriptive content
or bash only if it is intended to be executable.
In `@electron/cli/args.ts`:
- Around line 183-191: Update the --preview-size parsing in the argument switch
to reject zero-valued width or height after matching the dimensions. Preserve
the existing format validation and assignment for positive dimensions, while
throwing the same style of descriptive error when either parsed value is zero.
In `@electron/cli/cliMain.ts`:
- Around line 212-225: Update the JSON and human-readable output branches in the
CLI summary writer to use the existing safeWrite helper instead of direct
process.stdout.write calls. Ensure both writes handle closed-pipe EPIPE errors
before runCli installs its stdout error guards, preserving the current output
content and formatting.
- Around line 283-287: Hoist the finished flag to the surrounding function scope
and remove the inner declaration near the deferred exit logic. In the
app.on("window-all-closed") handler, return immediately when finished is set
before logging the unexpected closure or calling app.exit(1), while preserving
the existing failure behavior for unfinished runs.
In `@src/cli/CliRecordRunner.tsx`:
- Around line 226-234: Update the stop-signal effect around onCliStopRecording
to retain a stop request received during initialization instead of ignoring it.
Use the existing stopRequestedRef, then check it immediately after
startRecordingImmediately() resolves and abort or stop recording so the CLI
always reaches completion; preserve the current stopping behavior when recording
is already active.
In `@src/hooks/useScreenRecorder.ts`:
- Around line 57-58: Update startRecordingImmediately and the underlying
startRecording flow to report startup success or failure instead of always
resolving; return true only after recording is enabled and false from catch or
early-return paths. In CliRecordRunner, inspect that result and call fail(...)
when startup returns false so CLI startup failures terminate rather than waiting
for completion.
In `@src/lib/exporter/voiceoverMix.ts`:
- Around line 53-73: Update the mix path around voiceoverNode and gainNode so
the default mix mode attenuates the original audio below unity before combining
it with the voiceover, preventing both sources from playing at full gain.
Preserve explicitly supplied originalGain values and replace-mode behavior.
- Around line 100-103: Materialize the video data lazily in the voiceover mixing
flow: avoid calling videoBlob.arrayBuffer() before renderMixedAudio when replace
mode does not use it. Update renderMixedAudio or its caller so the ArrayBuffer
is obtained only inside the mix branch, while preserving existing replace and
mix behavior.
---
Nitpick comments:
In `@electron/cli/args.test.ts`:
- Around line 4-8: Update path expectations in the tests around parse and the
projectPath assertions to use a platform-aware helper such as at, backed by
path.resolve(CWD, ...) or path.join(CWD, ...), instead of hard-coded /work/...
strings. Ensure all affected expected paths, including demo.openscreen, match
resolvePath on Windows and POSIX systems.
In `@electron/cli/cliMain.ts`:
- Around line 245-252: Update the console method overrides in the level loop so
argument serialization cannot throw before safeWrite, including for circular or
otherwise non-serializable objects. Preserve string arguments and the existing
space-separated output, while using a safe fallback representation for values
that JSON.stringify cannot handle.
In `@electron/preload.ts`:
- Around line 309-317: Update the cliProgress and cliDone methods in the preload
API to use the existing CliProgressEvent and CliDoneResult types from
electron-env.d.ts instead of unknown, while preserving their current IPC
behavior and cliLog signature.
In `@src/cli/CliExportRunner.tsx`:
- Around line 42-57: Update probeVideoDimensions to enforce a bounded timeout
for video metadata loading, rejecting the promise when neither onloadedmetadata
nor onerror fires before the deadline. Ensure successful metadata loads and
errors clear the timeout and perform the existing video cleanup so the CLI
cannot remain pending indefinitely.
In `@src/cli/CliRecordRunner.tsx`:
- Around line 122-126: Move the current-value assignments for toggleRecordingRef
and recordingRef out of the render body and into a React effect in the
surrounding component. Keep both refs initialized with their corresponding
values and update their .current fields after committed renders so the
stop/finish effects continue using the latest values.
- Around line 98-108: Add tests in the same package for CliRecordRunner’s
bootstrap and recording state machine, stubbing window.electronAPI as needed.
Cover recorder start, stop, and cliDone completion transitions, including the
requestReady bootstrap path.
In `@src/lib/exporter/voiceoverMix.ts`:
- Around line 105-135: The remux cleanup must cancel a started but unfinalized
Output when packet or audio processing fails. In the surrounding flow that
creates output and calls output.start(), track successful startup, catch
subsequent errors, await output.cancel() when startup completed, then rethrow;
retain input.dispose() in finally and avoid canceling if output.start() never
resolved.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d3dba1c5-2e28-4090-8d2f-2e52c6aea3d2
📒 Files selected for processing (16)
README.mddocs/cli.mdelectron/cli/args.test.tselectron/cli/args.tselectron/cli/cliMain.tselectron/electron-env.d.tselectron/main.tselectron/preload.tselectron/windows.tspackage.jsonsrc/App.tsxsrc/cli/CliExportRunner.tsxsrc/cli/CliRecordRunner.tsxsrc/hooks/useScreenRecorder.tssrc/lib/cliContracts.tssrc/lib/exporter/voiceoverMix.ts
Comment on lines
+57
to
+58
| /** Starts recording with no countdown overlay. Used by the headless CLI runner. */ | ||
| startRecordingImmediately: () => Promise<void>; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
startRecordingImmediately never rejects, so CLI start failures hang the run.
startRecording catches everything internally (toast + state reset, Lines 1449-1465). The CLI runner awaits this in a try/catch (src/cli/CliRecordRunner.tsx Lines 190-196) expecting a throw; instead it resolves, recording stays false, recordingStartedAtRef stays null, the completion effect early-returns, and cli-done is never sent — the CLI blocks until the user kills it.
Return a success signal (or rethrow) from this entrypoint and have the runner fail on it.
🔧 Sketch
- /** Starts recording with no countdown overlay. Used by the headless CLI runner. */
- startRecordingImmediately: () => Promise<void>;
+ /** Starts recording with no countdown overlay. Resolves false when start failed. Used by the headless CLI runner. */
+ startRecordingImmediately: () => Promise<boolean>;Have startRecording return false from its catch/early-return paths and true once setRecording(true) has run, then in CliRecordRunner treat false as a failure and call fail(...).
Also applies to: 1733-1733
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/hooks/useScreenRecorder.ts` around lines 57 - 58, Update
startRecordingImmediately and the underlying startRecording flow to report
startup success or failure instead of always resolving; return true only after
recording is enabled and false from catch or early-return paths. In
CliRecordRunner, inspect that result and call fail(...) when startup returns
false so CLI startup failures terminate rather than waiting for completion.
Actionable findings: - voiceoverMix: duck the original bed to 0.4 gain by default in "mix" mode so the unity-gain sum cannot hard-clip; skip buffering the whole MP4 in "replace" mode; cancel the mediabunny Output on remux failure - CliRecordRunner: a stop signal arriving before capture starts is now remembered and applied the moment recording begins; add a 30s watchdog so a silently failed start no longer hangs the CLI; move ref sync out of render into an effect - cliMain: guard window-all-closed with the finished flag so teardown after a successful run can't report a false failure; route info-command output through safeWrite; harden console rerouting against circular args - args: reject zero dimensions in --preview-size - CliExportRunner: 30s timeout on the video metadata probe - preload: type the CLI bridge methods against cliContracts - args.test: platform-agnostic path assertions (win32-safe) - docs: fence language, document mix-mode ducking Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019ceTJjoPJYa5BdmXZdDc5J
Author
|
Addressed the CodeRabbit review in cc64b11 — all 8 actionable findings plus 6 of the nitpicks: Actionable
Nitpicks taken: metadata-probe 30s timeout, Re-verified after the changes: full unit suite (439 tests), repo lint, 🤖 Generated with Claude Code |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds a headless CLI to OpenScreen so scripts, CI pipelines, and AI coding agents can produce polished demos end-to-end — record, edit the project JSON programmatically, render — with no visible windows and machine-readable output:
Full docs in
docs/cli.md(commands, NDJSON protocol, exit codes, an end-to-end agent example).Why
.openscreenprojects are plain JSON and the export pipeline is already proven headless by theHEADLESS=truee2e test — the missing piece was a way to drive both without the GUI. With this CLI, an AI coding agent can demo the product it just built: record it, zoom into the interesting parts, narrate with TTS, and ship an MP4.Design — deliberately minimal footprint
The CLI reuses the existing pipelines unchanged via the hidden-window pattern your
tests/e2e/gif-export.spec.tsalready established:electron/cli/args.ts— pure argv parser (14 unit tests). Skips leading Chromium switches soOpenscreen.AppImage --no-sandbox export …works.electron/cli/cliMain.ts— headless boot: no HUD/tray/menu/dock, reusesregisterIpcHandlerswith inert window callbacks (the big handler module is untouched), NDJSON/--jsonprotocol on stdout, diagnostics on stderr, SIGINT/SIGTERM/stdinstop, EPIPE-safe writes, exit codes 0/1/2, SwiftShader fallback for GPU-less environments.src/cli/CliExportRunner.tsx— loads the project via the existingload-project-file-from-pathbridge action, rebuilds the same exporter config the editor's dialog builds, runsVideoExporter/GifExporter. DOM-derived preview dimensions are replaced by a deterministic 1280×720 reference box (--preview-sizeto override).src/cli/CliRecordRunner.tsx— drivesuseScreenRecorder(source by display index or window-title substring, mic by device label, system audio, cursor modes,--duration), then optionally writes a ready-to-export project file.--audiovoiceover mixing (src/lib/exporter/voiceoverMix.ts): copies the exported video packets without re-encoding (mediabunnyInput/EncodedPacketSink→EncodedVideoPacketSource) and renders the audio offline withOfflineAudioContext(mixorreplace,--audio-offset).Only three GUI-side touches: the
main.tsCLI branch, new preload IPC methods, and exposingstartRecordingImmediately()(countdown-less start) fromuseScreenRecorder.Tested (macOS 26.6, Apple Silicon)
record --duration 4/5→ MP4 +.cursor.json+ session manifest; stop via SIGINT and via--duration;--projectoutput loads in the GUI editorexportMP4 (zooms, annotations, padding, multi-track audio fixture) and GIF (magic bytes + dimensions verified); output verified with ffprobe--audiowith a realsay-generated TTS track (volumedetect confirms audible narration)openscreen export | head(EPIPE) no longer crashes or shows Electron's error dialog; renderer console errors are forwarded to stderrnpm run lintclean,tscclean, all 441 vitest unit tests pass (14 new), GUI mode boots unaffectedPlatform caveats (honest disclosure)
Windows and Linux paths were statically reviewed, not device-tested (I only have macOS hardware). The review-driven fixes are included: leading-switch subcommand detection, stdin guard moved inside try (Windows GUI-subsystem stdio edge case), and platform notes in
docs/cli.md(Windows: no SIGTERM — use stdinstop/--duration; Linux Wayland: PipeWire portal may prompt and headless sessions can't record). Happy to iterate if maintainers can test on those platforms.🤖 Generated with Claude Code
Summary by CodeRabbit
Summary by CodeRabbit
record,export, andinfo, including NDJSON/TTY progress output and scripted stop controls.