feat: add --headless mode + Unix-socket IPC for programmatic control#674
Open
Karanjot786 wants to merge 6 commits into
Open
feat: add --headless mode + Unix-socket IPC for programmatic control#674Karanjot786 wants to merge 6 commits into
Karanjot786 wants to merge 6 commits into
Conversation
… activate + window-all-closed
- Parse argv via electron/cli.ts at the earliest top-level statement and
set process.env.HEADLESS=true when --headless is present.
- Switch ./windows import to a dynamic import inside app.whenReady so the
HEADLESS env var is set BEFORE windows.ts evaluates its top-level
`const HEADLESS = process.env["HEADLESS"] === "true"`. Static imports
get inlined above main.ts's top-level code by the Rollup bundler, so a
dynamic import is required for env-var sequencing to work.
- Add headless boot path in app.whenReady that skips dock.show / tray /
application menu / IPC handler registration, eagerly creates the HUD
(which respects HEADLESS=true and stays hidden) so the renderer is
alive to receive future cli-start-recording IPC, logs a clear
"openscreen --headless: renderer loaded, ipc-path=..." line, and
early-returns. TODO marker left for T3.3 (IpcSocketServer).
- Gate app.on('window-all-closed') so headless mode keeps the app alive.
- Gate app.on('activate') so dock-click does not re-show a hidden window
in headless mode.
Verified via dev-mode launch with --headless --ipc-path: main process
stays alive, renderer process loaded (HUD with backgroundThrottling:
false), no Dock icon visible, log line emitted with correct ipc-path,
and global shortcut/tray code paths confirmed skipped.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- electron/recorder-bridge.ts: bridge over cli-recording-started / cli-recording-finalized renderer channels; exposes start/stop/status/ cleanup with promise lifecycle and disk hydration for bytes/click_count - electron/__tests__/recorder-bridge.test.ts: 10 tests covering all states, timeout, cleanup, and cursor-sidecar parsing (all pass) - electron/preload.ts: expose onCliStartRecording, notifyCliRecordingStarted, notifyCliRecordingFinalized on window.electronAPI contextBridge - electron/electron-env.d.ts: TypeScript declarations for the three new API surface entries - src/hooks/useScreenRecorder.ts: listen for cli-start-recording; call notifyCliRecordingStarted after renderer enters recording state; notifyCliRecordingFinalized after store-recorded-session resolves; add startRecordingRef to avoid stale closure in tray-style useEffect - electron/main.ts: wire RecorderBridge + IpcSocketServer in headless path; register recorder.start/stop/status/cleanup over NDJSON Unix socket; auto-select first screen via selectSource JS bridge; register permission + display-media handlers; SIGINT/SIGTERM shutdown store-recorded-session is ipcMain.handle (invoke-style), so the bridge cannot use ipcOnce on it. Renderer instead calls notifyCliRecordingFinalized which sends cli-recording-finalized back to main after the session is saved. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…aScript with direct setSelectedDesktopSource call Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughAdds complete headless CLI recording support: parses command-line arguments, establishes a Unix socket IPC server, implements a RecorderBridge state machine managing recording lifecycle, boots the app without GUI when headless, orchestrates recording commands through IPC, and integrates the renderer hook to respond to CLI triggers. ChangesHeadless CLI Recording System
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b53b4efaea
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Comment on lines
+411
to
+414
| if (cliInitiatedRecordingIds.current.has(activeRecordingId)) { | ||
| cliInitiatedRecordingIds.current.delete(activeRecordingId); | ||
| const cursorTelemetryPath = result.path ? `${result.path}.cursor.json` : undefined; | ||
| window.electronAPI?.notifyCliRecordingFinalized?.({ |
There was a problem hiding this comment.
When native capture is available on macOS or Windows, recorder.stop sends stop-recording-from-tray, which takes the native finalizers instead of the browser finalizeRecording path. Since cli-recording-finalized is only emitted here, the native stop can save the file and switch state locally but RecorderBridge.stop() never sees the completion and waits until its 30s timeout, making the headless start/stop cycle fail on those platforms. Please emit the same CLI finalization notification from the native finalize paths as well.
Useful? React with 👍 / 👎.
| ipcMain.once(channel, wrapped); | ||
| return () => ipcMain.removeListener(channel, wrapped); | ||
| }, | ||
| recordingsDir: RECORDINGS_DIR, |
There was a problem hiding this comment.
When launched with --out-dir=/some/path, parseArgs stores the value but the headless recorder is still wired to RECORDINGS_DIR, and the registered recording handlers also continue using the app user-data recordings directory. That means recorder.start reports/saves/cleans up under the default directory regardless of the flag, so callers cannot direct headless output as the CLI option implies. Please thread cliOpts.outDir through the recordings directory used by both the bridge and the storage handlers.
Useful? React with 👍 / 👎.
Comment on lines
+720
to
+726
| const mode = payload?.cursorCaptureMode; | ||
| if (mode === "editable-overlay" || mode === "system" || mode === "none") { | ||
| setCursorCaptureMode(mode as CursorCaptureMode); | ||
| } | ||
| void (async () => { | ||
| try { | ||
| await startRecordingRef.current(); |
There was a problem hiding this comment.
For recorder.start calls that pass cursorCaptureMode: "system" or "none", this state update is asynchronous and startRecordingRef.current() runs immediately with the previous render's cursorCaptureMode value, usually the default editable-overlay. As a result the API accepts the parameter but starts with the wrong cursor capture behavior for that first recording. Please pass the requested mode directly into the start path or keep it in a ref that is updated before starting.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/hooks/useScreenRecorder.ts (1)
522-530:⚠️ Potential issue | 🟠 Major | ⚡ Quick winNative finalize paths never notify the CLI bridge.
The web-media finalize flow emits
notifyCliRecordingFinalized, but the native Windows/macOS finalize flows skip it and go straight toswitchToEditor(). In headless mode that meansRecorderBridge.stop()can wait the full 30s and then fail even though the recording actually finished.Also applies to: 622-629
🤖 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 522 - 530, The native finalize paths (in the block that calls clearNativeRecordingState(), window.electronAPI.setCurrentRecordingSession / setCurrentVideoPath, then window.electronAPI.switchToEditor()) do not emit the same notifyCliRecordingFinalized event used by the web-media flow; update both native finalize blocks (the one using clearNativeRecordingState() and the second similar block later) to call the CLI bridge notifier (e.g. window.electronAPI.notifyCliRecordingFinalized or the existing notifyCliRecordingFinalized helper) with the finalized session/path before calling switchToEditor(), ensuring the CLI is informed in headless mode.
🤖 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/__tests__/recorder-bridge.test.ts`:
- Around line 179-191: The test named "start times out if renderer never acks"
is misleading because it acknowledges the start request via
fakes.emit("cli-recording-started", ..., { recordingId: 7 }) and never exercises
the timeout branch; update the test to either rename it to reflect the actual
behavior (e.g., "start sends start message and resolves on ack") or change the
implementation to exercise the timeout path by using fake timers and advancing
them so RecorderBridge.start(...) hits its internal timeout; locate the test
using the RecorderBridge instance and the bridge.start call and either rename
the it(...) description or wrap the test with vitest/jest fake timers and
advance time to trigger the timeout instead of calling fakes.emit.
In `@electron/ipc-socket-server.ts`:
- Around line 28-31: The current listen() unconditionally unlinks this.sockPath
which may remove arbitrary files; instead, first lstat(this.sockPath) and only
unlink if the entry exists and stats.isSocket() is true. Modify the listen()
implementation (reference: function listen and this.sockPath) to: call fs.lstat
or fs.promises.lstat on this.sockPath, if lstat rejects with ENOENT return
silently, if it succeeds check stats.isSocket() and only then call fs.unlink;
for any other lstat errors rethrow or log so we don’t silently delete non-socket
files.
In `@electron/ipc/handlers.ts`:
- Around line 381-392: The setter setSelectedDesktopSource currently only
updates selectedDesktopSource and leaves selectedSource stale when passed null;
update the function so that when source === null you also clear/reset
selectedSource (e.g., set selectedSource = null or reset its fields:
id/name/display_id to empty strings and thumbnail/appIcon to null) so helper
readers like get-selected-source, getSelectedDisplay, and
getSelectedSourceBounds reflect the cleared state. Target the
setSelectedDesktopSource function and the selectedDesktopSource/selectedSource
symbols when making this change.
In `@electron/main.ts`:
- Around line 25-28: The main boot currently only checks cliOpts.headless from
parseArgs, so setting HEADLESS=true in the environment is ignored; update
main.ts to derive the effective headless flag from both sources (e.g., const
isHeadless = cliOpts.headless || process.env.HEADLESS === "true") and use
isHeadless for all headless branches instead of cliOpts.headless (or set
cliOpts.headless from process.env if you prefer), ensuring any code paths that
reference cliOpts.headless (created by parseArgs) are switched to the unified
isHeadless variable so environment-only HEADLESS=true is honored.
- Around line 595-607: The CLI flags --out-dir and --retention-hours are parsed
but not passed into the recorder; replace hardcoded RECORDINGS_DIR usage when
constructing RecorderBridge with the parsed CLI option (e.g., cliOpts.outDir or
outDir) and add a retentionHours (or retention_hours) property set from
cliOpts.retentionHours so the bridge knows where to store recordings and how
long to retain them; also update the other headless boot path that still uses
RECORDINGS_DIR (the second RecorderBridge/recording initialization referenced
around lines 625-638) to accept and forward the same outDir and retentionHours
values instead of the constant.
- Around line 589-638: The socket is opened before the HUD renderer is
guaranteed to be ready, allowing clients to call RecorderBridge.start() before
the renderer has mounted; delay calling IpcSocketServer.listen() (and emitting
the "ipc listening" log) until the HUD renderer signals readiness—e.g. await
mainWindow.webContents to finish loading with await new Promise(resolve =>
mainWindow.webContents.once('did-finish-load', resolve)) or wait for a
renderer-side IPC like ipcMain.once('hud.ready', ...)—then call ipc.listen() and
the console.log; keep all existing ipc.register(...) calls as-is but move the
final await ipc.listen() and log after the renderer-ready await so
RecorderBridge.start() (bridge.start) can't race the renderer mount created by
createHudOverlayWindow().
In `@electron/recorder-bridge.ts`:
- Around line 164-247: The stop method currently ignores StopParams.session_id;
update stop(_params: StopParams = {}) to validate the optional session_id: if
params.session_id is provided and this.state === "idle" then only return
this.latestStopResult when its session_id equals params.session_id, otherwise
throw (same RPC error shape); if params.session_id is provided and this.state
!== "idle" ensure it matches this.currentSessionId before proceeding and throw
if not; use the validated session id when computing sessionId for the StopResult
(fall back to this.currentSessionId when params.session_id is undefined) and
keep existing behavior for clearing this.currentSessionId, pendingStop and
setting latestStopResult. References: stop, StopParams, _params.session_id,
latestStopResult, currentSessionId, pendingStop.
In `@src/hooks/useScreenRecorder.ts`:
- Around line 719-727: The CLI callback should use the provided
payload.cursorCaptureMode directly when starting to avoid a stale
cursorCaptureMode; instead of relying on setCursorCaptureMode to synchronously
update state before invoking startRecordingRef.current, pass the mode into the
start call (or call a variant like startRecordingWithMode) so startRecording
uses the CLI mode immediately; update the onCliStartRecording handler in
useScreenRecorder.ts (the cliCleanup callback that currently calls
setCursorCaptureMode and startRecordingRef.current) to pass the mode into
startRecordingRef.current (or adapt startRecordingRef/current implementation to
accept an explicit CursorCaptureMode) while still calling setCursorCaptureMode
for UI state.
---
Outside diff comments:
In `@src/hooks/useScreenRecorder.ts`:
- Around line 522-530: The native finalize paths (in the block that calls
clearNativeRecordingState(), window.electronAPI.setCurrentRecordingSession /
setCurrentVideoPath, then window.electronAPI.switchToEditor()) do not emit the
same notifyCliRecordingFinalized event used by the web-media flow; update both
native finalize blocks (the one using clearNativeRecordingState() and the second
similar block later) to call the CLI bridge notifier (e.g.
window.electronAPI.notifyCliRecordingFinalized or the existing
notifyCliRecordingFinalized helper) with the finalized session/path before
calling switchToEditor(), ensuring the CLI is informed in headless mode.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4471d18e-09a3-47e3-bd1f-afa40084fe37
📒 Files selected for processing (12)
.gitignoreelectron/__tests__/cli.test.tselectron/__tests__/ipc-socket-server.test.tselectron/__tests__/recorder-bridge.test.tselectron/cli.tselectron/electron-env.d.tselectron/ipc-socket-server.tselectron/ipc/handlers.tselectron/main.tselectron/preload.tselectron/recorder-bridge.tssrc/hooks/useScreenRecorder.ts
Comment on lines
+179
to
+191
| it("start times out if renderer never acks", async () => { | ||
| fakes = makeFakes(); | ||
| const bridge = new RecorderBridge(fakes.deps); | ||
| // Patch the bridge's internal timeout via a fast race: invoke start with a | ||
| // stubbed internal that triggers timeout by manipulating the listener map. | ||
| // Simpler: just verify the message is sent. Timeout path is exercised by | ||
| // production code; mocking timers here would require vitest fake timers. | ||
| const p = bridge.start({}).catch((e) => e); | ||
| expect(fakes.sent[0]?.channel).toBe("cli-start-recording"); | ||
| // Make this test fast by rejecting via finalize-shaped channel won't help; | ||
| // just resolve to keep test non-flaky. | ||
| fakes.emit("cli-recording-started", {}, { recordingId: 7 }); | ||
| await p; |
Contributor
There was a problem hiding this comment.
This test name is lying a bit.
It says timeout, but it acks the start request on Line 190 and never exercises the timeout branch. Either rename it or switch to fake timers so the real timeout path is covered.
🤖 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/__tests__/recorder-bridge.test.ts` around lines 179 - 191, The test
named "start times out if renderer never acks" is misleading because it
acknowledges the start request via fakes.emit("cli-recording-started", ..., {
recordingId: 7 }) and never exercises the timeout branch; update the test to
either rename it to reflect the actual behavior (e.g., "start sends start
message and resolves on ack") or change the implementation to exercise the
timeout path by using fake timers and advancing them so
RecorderBridge.start(...) hits its internal timeout; locate the test using the
RecorderBridge instance and the bridge.start call and either rename the it(...)
description or wrap the test with vitest/jest fake timers and advance time to
trigger the timeout instead of calling fakes.emit.
Comment on lines
+28
to
+31
| async listen(): Promise<void> { | ||
| await unlink(this.sockPath).catch(() => { | ||
| /* not present, fine */ | ||
| }); |
Contributor
There was a problem hiding this comment.
Only unlink an existing socket, not whatever lives at sockPath.
This deletes any pre-existing filesystem entry at that path before bind. If --ipc-path is mistyped to a real file, we silently remove it, which is kinda cursed for a CLI flag.
🤖 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-socket-server.ts` around lines 28 - 31, The current listen()
unconditionally unlinks this.sockPath which may remove arbitrary files; instead,
first lstat(this.sockPath) and only unlink if the entry exists and
stats.isSocket() is true. Modify the listen() implementation (reference:
function listen and this.sockPath) to: call fs.lstat or fs.promises.lstat on
this.sockPath, if lstat rejects with ENOENT return silently, if it succeeds
check stats.isSocket() and only then call fs.unlink; for any other lstat errors
rethrow or log so we don’t silently delete non-socket files.
Comment on lines
+381
to
+392
| export function setSelectedDesktopSource(source: DesktopCapturerSource | null): void { | ||
| selectedDesktopSource = source; | ||
| if (source !== null) { | ||
| selectedSource = { | ||
| id: source.id, | ||
| name: source.name, | ||
| display_id: source.display_id ?? "", | ||
| thumbnail: null, | ||
| appIcon: null, | ||
| }; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Clear both source caches on null.
Passing null only clears selectedDesktopSource; selectedSource stays stale. That leaves get-selected-source, getSelectedDisplay(), and getSelectedSourceBounds() out of sync with the actual selection state. lowkey risky state leak for a helper that explicitly accepts null.
nit: cleaner fix
export function setSelectedDesktopSource(source: DesktopCapturerSource | null): void {
selectedDesktopSource = source;
- if (source !== null) {
- selectedSource = {
- id: source.id,
- name: source.name,
- display_id: source.display_id ?? "",
- thumbnail: null,
- appIcon: null,
- };
- }
+ if (source === null) {
+ selectedSource = null;
+ return;
+ }
+ selectedSource = {
+ id: source.id,
+ name: source.name,
+ display_id: source.display_id ?? "",
+ thumbnail: null,
+ appIcon: null,
+ };
}🤖 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 - 392, The setter
setSelectedDesktopSource currently only updates selectedDesktopSource and leaves
selectedSource stale when passed null; update the function so that when source
=== null you also clear/reset selectedSource (e.g., set selectedSource = null or
reset its fields: id/name/display_id to empty strings and thumbnail/appIcon to
null) so helper readers like get-selected-source, getSelectedDisplay, and
getSelectedSourceBounds reflect the cleared state. Target the
setSelectedDesktopSource function and the selectedDesktopSource/selectedSource
symbols when making this change.
Comment on lines
+25
to
+28
| const cliOpts = parseArgs(process.argv.slice(2)); | ||
| if (cliOpts.headless) { | ||
| process.env.HEADLESS = "true"; | ||
| } |
Contributor
There was a problem hiding this comment.
Honor HEADLESS=true in main boot too.
Right now every headless branch in this file keys off cliOpts.headless, so setting only the env var never takes the app down the headless path. The window module may see process.env.HEADLESS, but main.ts itself still behaves like GUI mode.
🤖 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/main.ts` around lines 25 - 28, The main boot currently only checks
cliOpts.headless from parseArgs, so setting HEADLESS=true in the environment is
ignored; update main.ts to derive the effective headless flag from both sources
(e.g., const isHeadless = cliOpts.headless || process.env.HEADLESS === "true")
and use isHeadless for all headless branches instead of cliOpts.headless (or set
cliOpts.headless from process.env if you prefer), ensuring any code paths that
reference cliOpts.headless (created by parseArgs) are switched to the unified
isHeadless variable so environment-only HEADLESS=true is honored.
Comment on lines
+589
to
+638
| mainWindow = createHudOverlayWindow(); | ||
|
|
||
| const { IpcSocketServer } = await import("./ipc-socket-server.js"); | ||
| const { RecorderBridge } = await import("./recorder-bridge.js"); | ||
| const { desktopCapturer } = await import("electron"); | ||
|
|
||
| const bridge = new RecorderBridge({ | ||
| webContents: mainWindow.webContents, | ||
| ipcOn: (channel, listener) => { | ||
| ipcMain.on(channel, (event, ...args) => listener(event, ...args)); | ||
| }, | ||
| ipcOnce: (channel, listener) => { | ||
| const wrapped = (event: Electron.IpcMainEvent, ...args: unknown[]) => | ||
| listener(event, ...args); | ||
| ipcMain.once(channel, wrapped); | ||
| return () => ipcMain.removeListener(channel, wrapped); | ||
| }, | ||
| recordingsDir: RECORDINGS_DIR, | ||
| }); | ||
| bridgeRef = bridge; | ||
|
|
||
| // Auto-pick the first screen source before any cli-start-recording fires. | ||
| // Sets module-level state in handlers.ts directly so that | ||
| // setDisplayMediaRequestHandler resolves the correct source without | ||
| // going through executeJavaScript (which would inject an OS-controlled | ||
| // string into an eval, risking injection and fragility). | ||
| async function autoSelectFirstScreen(): Promise<void> { | ||
| const sources = await desktopCapturer.getSources({ | ||
| types: ["screen"], | ||
| thumbnailSize: { width: 0, height: 0 }, | ||
| }); | ||
| const first = sources[0]; | ||
| if (!first) throw new Error("no screen sources available"); | ||
| setSelectedDesktopSource(first); | ||
| } | ||
|
|
||
| const ipc = new IpcSocketServer(cliOpts.ipcPath); | ||
| ipc.register("recorder.start", async (params) => { | ||
| await autoSelectFirstScreen(); | ||
| return bridge.start((params as Parameters<typeof bridge.start>[0]) ?? {}); | ||
| }); | ||
| ipc.register("recorder.stop", (params) => | ||
| bridge.stop((params as Parameters<typeof bridge.stop>[0]) ?? {}), | ||
| ); | ||
| ipc.register("recorder.status", () => Promise.resolve(bridge.status())); | ||
| ipc.register("recorder.cleanup", (params) => | ||
| bridge.cleanup(params as Parameters<typeof bridge.cleanup>[0]), | ||
| ); | ||
| await ipc.listen(); | ||
| console.log(`openscreen --headless: renderer loaded, ipc listening on ${cliOpts.ipcPath}`); |
Contributor
There was a problem hiding this comment.
Don’t open the socket until the renderer is actually ready.
RecorderBridge.start() depends on the HUD renderer having mounted and subscribed to onCliStartRecording, but we start listening immediately after creating the window. A client that connects as soon as the “ipc listening” log appears can race this and hit the 10s start timeout for no real reason.
🤖 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/main.ts` around lines 589 - 638, The socket is opened before the HUD
renderer is guaranteed to be ready, allowing clients to call
RecorderBridge.start() before the renderer has mounted; delay calling
IpcSocketServer.listen() (and emitting the "ipc listening" log) until the HUD
renderer signals readiness—e.g. await mainWindow.webContents to finish loading
with await new Promise(resolve => mainWindow.webContents.once('did-finish-load',
resolve)) or wait for a renderer-side IPC like ipcMain.once('hud.ready',
...)—then call ipc.listen() and the console.log; keep all existing
ipc.register(...) calls as-is but move the final await ipc.listen() and log
after the renderer-ready await so RecorderBridge.start() (bridge.start) can't
race the renderer mount created by createHudOverlayWindow().
Comment on lines
+595
to
+607
| const bridge = new RecorderBridge({ | ||
| webContents: mainWindow.webContents, | ||
| ipcOn: (channel, listener) => { | ||
| ipcMain.on(channel, (event, ...args) => listener(event, ...args)); | ||
| }, | ||
| ipcOnce: (channel, listener) => { | ||
| const wrapped = (event: Electron.IpcMainEvent, ...args: unknown[]) => | ||
| listener(event, ...args); | ||
| ipcMain.once(channel, wrapped); | ||
| return () => ipcMain.removeListener(channel, wrapped); | ||
| }, | ||
| recordingsDir: RECORDINGS_DIR, | ||
| }); |
Contributor
There was a problem hiding this comment.
--out-dir and --retention-hours are parsed, but never applied.
Headless recording is still hardwired to RECORDINGS_DIR on Line 606, and nothing in this boot path uses cliOpts.retentionHours at all. So the CLI surface advertised by this PR isn’t actually honored yet.
Also applies to: 625-638
🤖 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/main.ts` around lines 595 - 607, The CLI flags --out-dir and
--retention-hours are parsed but not passed into the recorder; replace hardcoded
RECORDINGS_DIR usage when constructing RecorderBridge with the parsed CLI option
(e.g., cliOpts.outDir or outDir) and add a retentionHours (or retention_hours)
property set from cliOpts.retentionHours so the bridge knows where to store
recordings and how long to retain them; also update the other headless boot path
that still uses RECORDINGS_DIR (the second RecorderBridge/recording
initialization referenced around lines 625-638) to accept and forward the same
outDir and retentionHours values instead of the constant.
Comment on lines
+164
to
+247
| async stop(_params: StopParams = {}): Promise<StopResult> { | ||
| if (this.state === "idle") { | ||
| if (this.latestStopResult) return this.latestStopResult; | ||
| throw Object.assign(new Error("no active recording"), { code: -32000 }); | ||
| } | ||
| if (this.pendingStop) { | ||
| throw Object.assign(new Error("a stop is already in flight"), { code: -32000 }); | ||
| } | ||
|
|
||
| this.state = "encoding"; | ||
|
|
||
| const stopPromise = new Promise<StopRecordedPayload>((resolve, reject) => { | ||
| const timer = setTimeout(() => { | ||
| this.pendingStop = null; | ||
| this.state = "idle"; | ||
| this.currentSessionId = null; | ||
| reject(new Error("timed out waiting for renderer to finalize recording")); | ||
| }, 30_000); | ||
| this.pendingStop = { resolve, reject, timer }; | ||
| }); | ||
|
|
||
| this.deps.webContents.send("stop-recording-from-tray"); | ||
|
|
||
| const payload = await stopPromise; | ||
|
|
||
| const sessionId = this.currentSessionId ?? "unknown"; | ||
| const screenVideoPath = | ||
| payload.screenVideoPath ?? | ||
| path.join(this.deps.recordingsDir, `${RECORDING_FILE_PREFIX}${sessionId}${VIDEO_EXT}`); | ||
| const cursorLogPath = | ||
| payload.cursorTelemetryPath ?? `${screenVideoPath}${CURSOR_SIDECAR_SUFFIX}`; | ||
| const durationMs = payload.durationMs ?? (this.startedAt ? this.now() - this.startedAt : 0); | ||
|
|
||
| const result: StopResult = { | ||
| session_id: sessionId, | ||
| path: screenVideoPath, | ||
| duration_ms: durationMs, | ||
| cursor_log_path: cursorLogPath, | ||
| click_count: 0, | ||
| bytes: 0, | ||
| }; | ||
|
|
||
| // Hydrate bytes from disk. | ||
| try { | ||
| const st = await stat(screenVideoPath); | ||
| result.bytes = st.size; | ||
| } catch { | ||
| /* ignore — file may not exist if discard path was taken */ | ||
| } | ||
|
|
||
| // Hydrate click_count from cursor.json sidecar. | ||
| try { | ||
| const raw = await readFile(cursorLogPath, "utf8"); | ||
| const parsed = JSON.parse(raw); | ||
| if (Array.isArray(parsed)) { | ||
| result.click_count = parsed.filter( | ||
| (s: unknown) => | ||
| typeof s === "object" && | ||
| s !== null && | ||
| "type" in (s as Record<string, unknown>) && | ||
| (s as Record<string, unknown>).type === "click", | ||
| ).length; | ||
| } else if (parsed && typeof parsed === "object") { | ||
| const obj = parsed as { clicks?: unknown; samples?: unknown }; | ||
| if (Array.isArray(obj.clicks)) { | ||
| result.click_count = obj.clicks.length; | ||
| } else if (Array.isArray(obj.samples)) { | ||
| result.click_count = (obj.samples as unknown[]).filter( | ||
| (s) => | ||
| typeof s === "object" && | ||
| s !== null && | ||
| "type" in (s as Record<string, unknown>) && | ||
| (s as Record<string, unknown>).type === "click", | ||
| ).length; | ||
| } | ||
| } | ||
| } catch { | ||
| /* ignore — sidecar may not exist */ | ||
| } | ||
|
|
||
| this.latestStopResult = result; | ||
| this.currentSessionId = null; | ||
| this.startedAt = null; | ||
| return result; |
Contributor
There was a problem hiding this comment.
Validate session_id here instead of ignoring it.
StopParams advertises session scoping, but this implementation never checks it. In practice that means a caller can stop whichever session happens to be active, or get back latestStopResult for a different session once idle. That's a pretty rough contract for programmatic control.
🤖 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/recorder-bridge.ts` around lines 164 - 247, The stop method
currently ignores StopParams.session_id; update stop(_params: StopParams = {})
to validate the optional session_id: if params.session_id is provided and
this.state === "idle" then only return this.latestStopResult when its session_id
equals params.session_id, otherwise throw (same RPC error shape); if
params.session_id is provided and this.state !== "idle" ensure it matches
this.currentSessionId before proceeding and throw if not; use the validated
session id when computing sessionId for the StopResult (fall back to
this.currentSessionId when params.session_id is undefined) and keep existing
behavior for clearing this.currentSessionId, pendingStop and setting
latestStopResult. References: stop, StopParams, _params.session_id,
latestStopResult, currentSessionId, pendingStop.
Comment on lines
+719
to
+727
| cliCleanup = window.electronAPI.onCliStartRecording((payload) => { | ||
| const mode = payload?.cursorCaptureMode; | ||
| if (mode === "editable-overlay" || mode === "system" || mode === "none") { | ||
| setCursorCaptureMode(mode as CursorCaptureMode); | ||
| } | ||
| void (async () => { | ||
| try { | ||
| await startRecordingRef.current(); | ||
| const id = recordingId.current; |
Contributor
There was a problem hiding this comment.
Use the CLI-provided cursor mode directly for this start.
setCursorCaptureMode(mode) won’t synchronously update the cursorCaptureMode captured by startRecording, so this immediate call can still start with the old mode. If IPC asks for "system" right after the UI was on "editable-overlay", we’ll likely record with the stale setting.
🤖 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 719 - 727, The CLI callback
should use the provided payload.cursorCaptureMode directly when starting to
avoid a stale cursorCaptureMode; instead of relying on setCursorCaptureMode to
synchronously update state before invoking startRecordingRef.current, pass the
mode into the start call (or call a variant like startRecordingWithMode) so
startRecording uses the CLI mode immediately; update the onCliStartRecording
handler in useScreenRecorder.ts (the cliCleanup callback that currently calls
setCursorCaptureMode and startRecordingRef.current) to pass the mode into
startRecordingRef.current (or adapt startRecordingRef/current implementation to
accept an explicit CursorCaptureMode) while still calling setCursorCaptureMode
for UI state.
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
Adds a
--headlessmode that boots openscreen without showing a window, plus a Unix-domain-socket NDJSON IPC server in the main process. Enables programmatic control by external tools — e.g. AI agent screen recording via MCP.How it works
IPC methods exposed
recorder.start— start a new recording (auto-picks first screen source)recorder.stop— stop + return path to.webm+.cursor.jsonsidecarrecorder.status— poll current staterecorder.cleanup— delete session filesWhy
Closes #290 (CLI request). AI agents and automation tools can now drive openscreen headlessly without any GUI interaction.
Companion project: openscreen-mcp — an open-source Model Context Protocol server that exposes these 4 methods as MCP tools for Claude Code, Claude Desktop, and Cursor.
Files changed
electron/cli.ts--headless,--ipc-path,--out-dir,--retention-hours. Pure, no Electron imports. Unit-tested.electron/ipc-socket-server.tselectron/recorder-bridge.tselectron/preload.tsonCliStartRecording+ notification helpers for the headless recording flowelectron/main.ts--headlessCLI dispatch, headless boot path, and IPC server startupsrc/hooks/useScreenRecorder.tscli-start-recordingeffect + notification callbacks for the main-process bridgeelectron/ipc/handlers.tssetSelectedDesktopSourcefor headless source selectionTotal net additions: ~400 LOC + unit tests. No changes to existing recorder, editor, or renderer UI. Existing GUI path is completely unchanged — the headless flag adds a parallel boot path.
Test plan
npm run devopens window, records, exports as before--headlessflag: process starts with no Dock icon, no visible windowrecorder.statusreturns{"state":"idle"}vianc -UNotes
HEADLESS=trueenv var also works (fornpm run devduring development).Co-authored-by: Claude Opus 4.7 noreply@anthropic.com
Summary by CodeRabbit
Release Notes
New Features
Tests