feat(cli): headless record / export / info commands - #176
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughChangesHeadless CLI
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CLIProcess
participant ElectronMain
participant HiddenRunner
participant MediaPipeline
CLIProcess->>ElectronMain: parse and start command
ElectronMain->>HiddenRunner: load command-specific runner
HiddenRunner->>MediaPipeline: record, export, enumerate, or caption
MediaPipeline-->>HiddenRunner: result and progress
HiddenRunner-->>ElectronMain: completion payload
ElectronMain-->>CLIProcess: NDJSON or text output
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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
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
|
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 |
|
Pushed 6dcd778 with four more commands that close the remaining gaps between the CLI and GUI workflows (docs updated in
All verified on macOS; 443 unit tests (7 new), lint and tsc clean. 🤖 Generated with Claude Code |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
electron/cli/cliMain.ts (1)
541-543: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winCaptions rewrites the user's project in place — consider a temp-file + rename.
A crash or partial write during
writeProjectFileleaves the only copy of the project truncated. Writing to<path>.tmpandfs.rename-ing over the original makes the update atomic on the same filesystem.🤖 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 541 - 543, Update the captions success path around writeProjectFile to write result.projectData to a temporary file beside command.projectPath, then atomically rename the temporary file over the original on the same filesystem. Ensure cleanup of the temporary file on write or rename failure, while preserving the existing success condition and destination path.src/cli/CliSourcesRunner.tsx (1)
57-59: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConstant status strings held in
useStatein both CLI runners. Neither runner ever updatesstatus, so the state hook (and its unused setter) adds nothing over a plain constant.
src/cli/CliSourcesRunner.tsx#L57-L59: replaceconst [status] = useState("Enumerating sources…")with a plainconstand dropuseStatefrom the import.src/cli/CliCaptionsRunner.tsx#L138-L140: replaceconst [status] = useState("Generating captions…")with a plainconstand dropuseStatefrom the import.🤖 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/CliSourcesRunner.tsx` around lines 57 - 59, Replace the unused useState status tuple with a plain constant in CliSourcesRunner.tsx lines 57-59 and remove useState from that file’s imports; make the same change in CliCaptionsRunner.tsx lines 138-140 for the “Generating captions…” status and remove its useState import.
🤖 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 77-86: Update the `openscreen sources --json` documentation
example to show valid parseable JSON, replacing the unquoted `displays`,
`windows`, and `microphones` placeholders with representative quoted keys and
JSON-compatible values.
- Around line 208-209: Update the step-4 heading in the CLI documentation to
accurately describe the selected --audio-mode replace behavior as replacing the
recording audio, or change the command to --audio-mode mix if the intended
behavior is to mix voiceover with existing audio; keep the heading and command
consistent.
In `@electron/cli/args.ts`:
- Around line 381-437: Add coverage in electron/cli/args.test.ts for the missing
rejection edge cases in parsePack and parseCaptions, including invalid or
incomplete --out/word-count arguments and extra positional arguments, while
preserving existing happy-path and unknown-option tests. Also add equivalent
edge-case tests for the sources parser using its visible parser entry point and
expected CLI errors.
---
Nitpick comments:
In `@electron/cli/cliMain.ts`:
- Around line 541-543: Update the captions success path around writeProjectFile
to write result.projectData to a temporary file beside command.projectPath, then
atomically rename the temporary file over the original on the same filesystem.
Ensure cleanup of the temporary file on write or rename failure, while
preserving the existing success condition and destination path.
In `@src/cli/CliSourcesRunner.tsx`:
- Around line 57-59: Replace the unused useState status tuple with a plain
constant in CliSourcesRunner.tsx lines 57-59 and remove useState from that
file’s imports; make the same change in CliCaptionsRunner.tsx lines 138-140 for
the “Generating captions…” status and remove its useState import.
🪄 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: 534a9821-c46d-4ef7-b317-7cc47c06faef
📒 Files selected for processing (11)
.gitignoredocs/cli.mdelectron/cli/args.test.tselectron/cli/args.tselectron/cli/cliMain.tselectron/ipc/handlers.tssrc/App.tsxsrc/cli/CliCaptionsRunner.tsxsrc/cli/CliExportRunner.tsxsrc/cli/CliSourcesRunner.tsxsrc/lib/cliContracts.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/cli/CliExportRunner.tsx
- electron/cli/args.test.ts
|
Addressed the second review round — all three findings:
Lint and the full unit suite pass. 🤖 Generated with Claude Code |
|
This one needs a bit more than a rebase. The model layer you build on survived —
The design holds — it's the plumbing that moved. Happy to sketch the mapping if that helps. |
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
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
Four additions that close the gap between CLI and GUI workflows:
- export --auto-zoom: runs the editor's cursor-dwell suggestion engine
(timeline/zoomSuggestionUtils) over the recording's telemetry and adds
focus-following zoom regions before rendering; never overlaps regions
already in the project
- sources: lists capturable displays, windows and microphones (same
enumeration as the GUI picker) so scripts can pick --display/--window/
--mic-device values without trial and error
- pack <project> --out <dir>: copies the project plus referenced media
and the cursor-telemetry sidecar into one portable folder; the media
approval path (getApprovedProjectSession) and the export runner gain a
same-basename sibling fallback so a packed folder keeps working after
being moved to another location or machine
- captions <project>: transcribes the project's audio with the bundled
on-device Whisper worker (mirrors VideoEditor.generateAutoCaptions:
leading-silence trim, retry on empty, word grouping) and writes
auto-caption annotations into the project; re-running replaces earlier
auto-captions and preserves manual annotations
Also: console rerouting now serializes Error objects properly instead of
printing "{}".
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019ceTJjoPJYa5BdmXZdDc5J
- docs: real JSON payload example for `sources --json`; align the demo narration wording with --audio-mode replace - args.test: edge-case coverage for sources/pack/captions parsers (missing paths, unknown options, extra positionals, non-integer and zero word counts) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019ceTJjoPJYa5BdmXZdDc5J
Resolving the preload conflict kept both sides but missed the opening marker, so the file didn't parse. Both the stt block and the CLI bridge members are intended -- they collided only because each side appended to the same object.
|
Pushed the rebase to Two things I dropped as leftovers from the old What's left is six symbols:
The last row is the real work: the native exporters take an |
…eline
Completes the six-symbol port on top of contrib/pr-176-rebased:
- CliExportRunner now migrates the .openscreen project to an AxcutDocument
(migrateProjectDataToAxcutDocument + applyProbedDuration — without the
probe the clip has no duration and the export is a single frame) and
drives exportMultiNative/exportGifNative, mirroring the v4 ExportDialog:
same clip list, same scene JSON, same crop-aware smallest-clip sizing,
same gif dimension capping. Progress is synthesized from the native
frame-count push (export:native-progress) so the CLI's NDJSON contract
(percentage/currentFrame/totalFrames/ETA + done{outputPath,...}) is
unchanged. --preview-size becomes an accepted no-op (annotation geometry
is percentage-based in the scene); --audio now reads the natively
written file back, mixes the voiceover, and overwrites outPath.
- Auto-zoom repointed to @/lib/ai-edition/timeline/zoom-suggestions and
zoom-scale; suggestions append straight to document.zoomRanges.
- Deleted helpers vendored under src/cli/vendor/ with provenance notes:
captionSegmentsToAnnotationRegions (the CLI still writes caption
annotations into projects — the v2 route the migration preserves),
trimLeadingSilenceMono16k, clampZoomFocus, hasNativeCursorRecordingData.
- Cursor plumbing dropped from the runner: the native compositor discovers
the <video>.cursor.json sidecar by filename convention.
Known limitation carried over: the native pipeline has no cancel and no
extra-audio-track concept; kill mid-export leaves the worker running until
process exit, and --audio costs one extra read/write of the output.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019ceTJjoPJYa5BdmXZdDc5J
- captions: the whisper.cpp stt:transcribe handler is registered in the GUI boot path, so CLI mode registered nothing and the renderer adapter failed with "No handler registered" — register registerSttIpc in runCli like set-locale. - docs/cli.md: one-shot dev build block (renderer, capture helpers, ffmpeg + Rust compositor, whisper STT server), --audio read-back behavior, --preview-size no-op, no-cancel caveat, whisper.cpp model auto-download. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019ceTJjoPJYa5BdmXZdDc5J
|
@EtienneLescot Thank you for the rebase branch and the symbol map — that table saved hours. The branch is now ported on top of The six symbols:
The Also registered Verified on macOS (Apple Silicon): export MP4/GIF through the Rust compositor, One setup note that may bite other contributors: on an Apple Silicon machine with a Rosetta-installed rustup (default host #190 gets the same treatment next (multi-window mode re-added to the split 🤖 Generated with Claude Code |
36a2fff to
a297cee
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
electron/cli/cliMain.ts (2)
73-100: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe progress label says
Exportingfor every command.
createOutputis shared by the record, captions, and export runners. A captions run printsExporting 42%. Derive the label from the command kind, or use a neutral verb such asProgress.🤖 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 73 - 100, Update the progress text construction in createOutput’s progress handler so the label is not always “Exporting” for record and captions commands. Derive the verb from the command kind available to createOutput, or use the neutral “Progress” label, while preserving the existing percentage, frame, ETA, phase, TTY, and JSON output behavior.
442-621: 🩺 Stability & Availability | 🔵 TrivialConsider a global run timeout for non-record commands.
cli-done,did-fail-load, andrender-process-gonecover the known exits. If the renderer stalls without any of these events, the CLI process waits forever, which blocks scripted and CI callers. An optional--timeoutthat callsapp.exit(1)would bound the run.🤖 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 442 - 621, add an optional timeout for non-record CLI commands in the app.whenReady flow, starting it after the runner window is launched and clearing it when cli-done completes or an early renderer failure exits. When the timeout expires, report the failure through output.error and call app.exit(1); preserve unlimited waiting for record commands and avoid exiting after completion.electron/ipc/handlers.ts (1)
381-418: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe fallback keeps the security guards intact.
path.basenamestrips traversal segments, and the sibling candidate stays inside the project directory, which is already intrustedDirs.approveReadableVideoPathstill enforces the extension and trusted-directory checks. The behavior on a missing sibling preserves the original error path.One optional cleanup:
resolveSourceinelectron/cli/cliMain.ts(Lines 234-247) implements the same sibling-lookup rule with a throwing tail. Extract one shared helper so the pack command and the loader cannot drift apart.🤖 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/ipc/handlers.ts` around lines 381 - 418, Extract the shared sibling-lookup behavior from resolveWithSiblingFallback and resolveSource into one reusable helper, then update both callers to use it. Preserve basename sanitization, project-directory resolution, missing-sibling fallback to the original path, and each caller’s existing error behavior.
🤖 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 `@electron/cli/args.ts`:
- Around line 144-278: In parseExport, move the audio-format conflict validation
to after the request.outPath extension logic resolves request.format. Validate
the final request.format so --audio combined with a .gif output is rejected even
when --format is omitted, while preserving the existing format-extension
conflict checks.
In `@electron/cli/cliMain.ts`:
- Around line 252-267: Update copyIn to detect when the basename-derived
destination is already present in copied and generate a de-duplicated
destination with a suffix before copying. Ensure screen and webcam files with
identical basenames receive distinct project paths while preserving the existing
same-source skip behavior.
- Around line 370-380: Update runCli so the help and error output paths use
safeWrite instead of direct process.stdout.write/process.stderr.write calls,
ensuring broken-pipe handling is applied before app.exit. Preserve the existing
CLI_USAGE, error message formatting, exit codes, and returns.
- Around line 227-286: Update the media validation near the start of the flow to
guard with media != null and verify media.screenVideoPath directly, then assign
screenVideoPath from the narrowed media object. Keep the later
media.webcamVideoPath access and packedProject construction unchanged, relying
on the guard to establish media is defined.
---
Nitpick comments:
In `@electron/cli/cliMain.ts`:
- Around line 73-100: Update the progress text construction in createOutput’s
progress handler so the label is not always “Exporting” for record and captions
commands. Derive the verb from the command kind available to createOutput, or
use the neutral “Progress” label, while preserving the existing percentage,
frame, ETA, phase, TTY, and JSON output behavior.
- Around line 442-621: add an optional timeout for non-record CLI commands in
the app.whenReady flow, starting it after the runner window is launched and
clearing it when cli-done completes or an early renderer failure exits. When the
timeout expires, report the failure through output.error and call app.exit(1);
preserve unlimited waiting for record commands and avoid exiting after
completion.
In `@electron/ipc/handlers.ts`:
- Around line 381-418: Extract the shared sibling-lookup behavior from
resolveWithSiblingFallback and resolveSource into one reusable helper, then
update both callers to use it. Preserve basename sanitization, project-directory
resolution, missing-sibling fallback to the original path, and each caller’s
existing error behavior.
🪄 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: a14b94f5-594d-4149-94c2-5b36fd0c1ebe
📒 Files selected for processing (8)
README.mddocs/cli.mdelectron/cli/args.test.tselectron/cli/args.tselectron/cli/cliMain.tselectron/electron-env.d.tselectron/ipc/handlers.tselectron/main.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- electron/cli/args.test.ts
- docs/cli.md
- README.md
- electron/electron-env.d.ts
- electron/main.ts
- parseExport validated --audio against a format that --out had not resolved yet, so `--audio v.m4a -o out.gif` slipped through; check after inference. - pack derived the destination from the basename alone, so a webcam video sharing a name with the screen video overwrote it and both project paths pointed at one file. - help/error output bypassed safeWrite, so `openscreen help | head` could throw before the EPIPE guards are installed.
src/cli/vendor/captionRegions.ts was annotationsFromCaptions.ts copied whole (~460 identical lines) for the sake of the annotation converter that the 1.8 line did delete. The copy had already lost dedupeAdjacentCaptionRepeats and finalizeCaptionSegmentsForPlayback, so CLI captions were drifting from the editor's. Only the converter is CLI-specific, so only it stays — in src/cli, not in a vendor folder, importing the grouping/dedupe helpers from the live module. Output is unchanged: the shared code was byte-identical, and the dropped reconcileAutoCaptionTimelineGaps was a no-op (its gap constant is 0). Also drops the unused hasNativeCursorRecordingData from vendor/zoomHelpers.
|
Pushed two commits to this branch (thanks for
Verified on Windows: Three things left before I'd merge:
Also noted, not blocking: |
`pack` and `info` only touch the project file and its media, but they sat in cliMain.ts behind its `electron` import, so nothing could test them — including the basename-collision fix from 17fde1a. Moved to electron/cli/projectCommands.ts with the writer injected, and covered: colliding screen/webcam basenames, the cursor sidecar, the stale-path sibling fallback, missing media, and info's exit codes. Verified by ablation — reverting the de-dup turns the first test red. An e2e would have been the obvious home for this, but ci.yml runs no Playwright job, so it would be a test nobody runs; vitest is what the Test job executes. --preview-size is gone: it was a documented no-op on the native pipeline, kept for compatibility by a CLI that has never shipped.
|
Tests. I did not add a Playwright e2e:
Verified locally on Windows: One thing left that is yours rather than mine: the PR description still describes Note for whoever merges: CI has never actually run on this branch. The run on every head sits at |
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
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
2 participants
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