From 0b3169ffff9657e133e9032afd32494cba9c6171 Mon Sep 17 00:00:00 2001 From: webadderall <131426131+webadderall@users.noreply.github.com> Date: Fri, 17 Apr 2026 21:11:58 +1000 Subject: [PATCH] fix: address ipc review follow-ups --- electron/ipc/cursor/bounds.ts | 45 ++++++++++++++++------------- electron/ipc/cursor/interaction.ts | 4 +-- electron/ipc/cursor/monitor.ts | 33 +++++++++++++++------ electron/ipc/cursor/telemetry.ts | 4 --- electron/ipc/export/native-video.ts | 8 +++-- electron/ipc/project/manager.ts | 15 +++++----- electron/ipc/recording/ffmpeg.ts | 9 +++--- electron/ipc/recording/mac.ts | 5 ++-- electron/ipc/recording/windows.ts | 5 ++-- electron/ipc/register/project.ts | 15 ++++++---- electron/ipc/register/recording.ts | 26 +++++++++++------ electron/ipc/register/sources.ts | 33 +++++++++++++++++---- electron/ipc/state.ts | 4 +-- 13 files changed, 131 insertions(+), 75 deletions(-) diff --git a/electron/ipc/cursor/bounds.ts b/electron/ipc/cursor/bounds.ts index 6cf638db..02fb3c75 100644 --- a/electron/ipc/cursor/bounds.ts +++ b/electron/ipc/cursor/bounds.ts @@ -29,29 +29,33 @@ export async function getNativeMacWindowSources(options?: { maxAgeMs?: number }) return cachedNativeMacWindowSources; } - const binaryPath = await ensureNativeWindowListBinary(); - const { stdout } = await execFileAsync(binaryPath, [], { - timeout: 30000, - maxBuffer: 10 * 1024 * 1024, - }); + try { + const binaryPath = await ensureNativeWindowListBinary(); + const { stdout } = await execFileAsync(binaryPath, [], { + timeout: 30000, + maxBuffer: 10 * 1024 * 1024, + }); - const parsed = JSON.parse(stdout); - if (!Array.isArray(parsed)) { - return [] as NativeMacWindowSource[]; - } - - const entries = parsed.filter((entry: unknown): entry is NativeMacWindowSource => { - if (!entry || typeof entry !== "object") { - return false; + const parsed = JSON.parse(stdout); + if (!Array.isArray(parsed)) { + return [] as NativeMacWindowSource[]; } - const candidate = entry as Partial; - return typeof candidate.id === "string" && typeof candidate.name === "string"; - }); + const entries = parsed.filter((entry: unknown): entry is NativeMacWindowSource => { + if (!entry || typeof entry !== "object") { + return false; + } - setCachedNativeMacWindowSources(entries); - setCachedNativeMacWindowSourcesAtMs(now); - return entries; + const candidate = entry as Partial; + return typeof candidate.id === "string" && typeof candidate.name === "string"; + }); + + setCachedNativeMacWindowSources(entries); + setCachedNativeMacWindowSourcesAtMs(now); + return entries; + } catch { + return cachedNativeMacWindowSources ?? ([] as NativeMacWindowSource[]); + } } export function getWindowBoundsFromNativeSource( @@ -180,8 +184,9 @@ export async function resolveWindowsWindowBounds(source: SelectedSource): Promis "if ($windowId) {", " $handle = [Int64]$windowId", "}", + "$escapedWindowTitle = if ($windowTitle) { [WildcardPattern]::Escape($windowTitle) } else { $null }", "if ($handle -le 0 -and $windowTitle) {", - ' $matchingProcess = Get-Process | Where-Object { $_.MainWindowTitle -eq $windowTitle -or $_.MainWindowTitle -like "*$windowTitle*" } | Select-Object -First 1', + ' $matchingProcess = Get-Process | Where-Object { $_.MainWindowTitle -eq $windowTitle -or ($escapedWindowTitle -and $_.MainWindowTitle -like "*$escapedWindowTitle*") } | Select-Object -First 1', " if ($matchingProcess) {", " $handle = $matchingProcess.MainWindowHandle.ToInt64()", " }", diff --git a/electron/ipc/cursor/interaction.ts b/electron/ipc/cursor/interaction.ts index f5ae15ca..e0b501e1 100644 --- a/electron/ipc/cursor/interaction.ts +++ b/electron/ipc/cursor/interaction.ts @@ -187,8 +187,6 @@ export async function startInteractionCapture() { hook.on("mousemove", onMouseMove); } - hook.start(); - setInteractionCaptureCleanup(() => { try { if (typeof hook.off === "function") { @@ -216,6 +214,8 @@ export async function startInteractionCapture() { // ignore hook shutdown errors } }); + + hook.start(); } catch (error) { if (!hasLoggedInteractionHookFailure) { setHasLoggedInteractionHookFailure(true); diff --git a/electron/ipc/cursor/monitor.ts b/electron/ipc/cursor/monitor.ts index 0c39909a..a076713e 100644 --- a/electron/ipc/cursor/monitor.ts +++ b/electron/ipc/cursor/monitor.ts @@ -112,20 +112,35 @@ export async function startNativeCursorMonitor() { } setNativeCursorMonitorProcess(proc as Parameters[0]); - - proc.once("error", (error) => { - console.warn("Native cursor monitor process error:", error); + const spawned = proc; + if (!spawned) { setNativeCursorMonitorProcess(null); - setNativeCursorMonitorOutputBuffer(""); setCurrentCursorVisualType("arrow"); + return; + } + + spawned.once("error", (error) => { + console.warn("Native cursor monitor process error:", error); + if (nativeCursorMonitorProcess === spawned) { + setNativeCursorMonitorProcess(null); + setNativeCursorMonitorOutputBuffer(""); + setCurrentCursorVisualType("arrow"); + } }); - if (proc.stdout) proc.stdout.on("data", handleCursorMonitorStdout); + if (spawned.stdout) spawned.stdout.on("data", handleCursorMonitorStdout); + if (spawned.stderr) { + spawned.stderr.on("data", () => { + // Drain stderr so helper logging cannot block the process. + }); + } - proc.once("close", () => { - setNativeCursorMonitorProcess(null); - setNativeCursorMonitorOutputBuffer(""); - setCurrentCursorVisualType("arrow"); + spawned.once("close", () => { + if (nativeCursorMonitorProcess === spawned) { + setNativeCursorMonitorProcess(null); + setNativeCursorMonitorOutputBuffer(""); + setCurrentCursorVisualType("arrow"); + } }); } catch (error) { console.warn("Failed to start native cursor monitor:", error); diff --git a/electron/ipc/cursor/telemetry.ts b/electron/ipc/cursor/telemetry.ts index f7eb5eaf..51b901cd 100644 --- a/electron/ipc/cursor/telemetry.ts +++ b/electron/ipc/cursor/telemetry.ts @@ -117,10 +117,6 @@ export function pushCursorSample( export function sampleCursorPoint() { const point = getNormalizedCursorPoint(); - if (!point) { - return; - } - pushCursorSample(point.cx, point.cy, Date.now() - cursorCaptureStartTimeMs, "move"); } diff --git a/electron/ipc/export/native-video.ts b/electron/ipc/export/native-video.ts index ccfe1486..9b2472bf 100644 --- a/electron/ipc/export/native-video.ts +++ b/electron/ipc/export/native-video.ts @@ -326,7 +326,10 @@ export async function resolveNativeVideoEncoder( ffmpegPath: string, encodingMode: NativeExportEncodingMode, ) { - if (cachedNativeVideoEncoder?.ffmpegPath === ffmpegPath) { + if ( + cachedNativeVideoEncoder?.ffmpegPath === ffmpegPath && + cachedNativeVideoEncoder?.encodingMode === encodingMode + ) { return cachedNativeVideoEncoder.encoderName; } @@ -341,7 +344,7 @@ export async function resolveNativeVideoEncoder( } if (await probeNativeVideoEncoder(ffmpegPath, encoderName, encodingMode)) { - setCachedNativeVideoEncoder({ ffmpegPath, encoderName }); + setCachedNativeVideoEncoder({ ffmpegPath, encodingMode, encoderName }); return encoderName; } } @@ -451,7 +454,6 @@ export async function muxExportedVideoAudioBuffer( } finally { await Promise.allSettled([ removeTemporaryExportFile(tempVideoPath), - removeTemporaryExportFile(`${tempVideoPath}.muxed.mp4`), removeTemporaryExportFile( path.join( path.dirname(tempVideoPath), diff --git a/electron/ipc/project/manager.ts b/electron/ipc/project/manager.ts index faabccfe..9859db0b 100644 --- a/electron/ipc/project/manager.ts +++ b/electron/ipc/project/manager.ts @@ -51,11 +51,12 @@ export function isPathInsideDirectory(candidatePath: string, directoryPath: stri export function isAllowedLocalReadPath(candidatePath: string) { const allowedPrefixes = [RECORDINGS_DIR, USER_DATA_PATH, getAssetRootPath(), app.getPath("temp")]; + const normalizedCandidatePath = normalizePath(candidatePath); return ( - existsSync(candidatePath) || - allowedPrefixes.some((prefix) => isPathInsideDirectory(candidatePath, prefix)) || - approvedLocalReadPaths.has(candidatePath) + existsSync(normalizedCandidatePath) || + allowedPrefixes.some((prefix) => isPathInsideDirectory(normalizedCandidatePath, prefix)) || + approvedLocalReadPaths.has(normalizedCandidatePath) ); } @@ -308,9 +309,6 @@ export async function loadProjectFromPath(projectPath: string) { message: mediaSources.message, }; } - - setCurrentProjectPath(normalizedPath); - setCurrentVideoPath(mediaSources.videoPath); const projectObj = project as Record; const editorObj = projectObj?.editor as Record | undefined; const audioTracks = editorObj?.audioTracks as { sourcePath?: unknown }[] | undefined; @@ -326,12 +324,15 @@ export async function loadProjectFromPath(projectPath: string) { } } await replaceApprovedSessionLocalReadPaths(approvedProjectPaths); + await rememberRecentProject(normalizedPath); + + setCurrentProjectPath(normalizedPath); + setCurrentVideoPath(mediaSources.videoPath); setCurrentRecordingSession({ videoPath: mediaSources.videoPath, webcamPath: mediaSources.webcamPath, timeOffsetMs: 0, } as RecordingSessionData); - await rememberRecentProject(normalizedPath); return { success: true, diff --git a/electron/ipc/recording/ffmpeg.ts b/electron/ipc/recording/ffmpeg.ts index 39dbc5dc..2d486d16 100644 --- a/electron/ipc/recording/ffmpeg.ts +++ b/electron/ipc/recording/ffmpeg.ts @@ -4,7 +4,7 @@ import type { SelectedSource } from "../types"; import { ffmpegCaptureOutputBuffer, } from "../state"; -import { getScreen } from "../utils"; +import { getScreen, parseWindowId } from "../utils"; import { resolveWindowsCaptureDisplay } from "../windowsCaptureSelection"; import { resolveLinuxWindowBounds } from "../cursor/bounds"; @@ -32,12 +32,13 @@ export async function buildFfmpegCaptureArgs(source: SelectedSource, outputPath: if (process.platform === "win32") { if (source?.id?.startsWith("window:")) { + const windowId = parseWindowId(source.id); const windowTitle = typeof source.windowTitle === "string" ? source.windowTitle.trim() : source.name.trim(); - if (!windowTitle) { - throw new Error("Missing window title for FFmpeg window capture"); + if (!windowId && !windowTitle) { + throw new Error("Missing window identifier for FFmpeg window capture"); } return [ @@ -49,7 +50,7 @@ export async function buildFfmpegCaptureArgs(source: SelectedSource, outputPath: "-draw_mouse", "0", "-i", - `title=${windowTitle}`, + windowId ? `hwnd=${windowId}` : `title=${windowTitle}`, ...commonOutputArgs, ]; } diff --git a/electron/ipc/recording/mac.ts b/electron/ipc/recording/mac.ts index b7cb8747..d340b8e1 100644 --- a/electron/ipc/recording/mac.ts +++ b/electron/ipc/recording/mac.ts @@ -52,9 +52,10 @@ export function waitForNativeCaptureStart(process: ChildProcessWithoutNullStream reject(new Error("Timed out waiting for ScreenCaptureKit recorder to start")); }, 12000); + let stdoutBuffer = ""; const onStdout = (chunk: Buffer) => { - const text = chunk.toString(); - if (text.includes("Recording started")) { + stdoutBuffer += chunk.toString(); + if (stdoutBuffer.includes("Recording started")) { cleanup(); resolve(); } diff --git a/electron/ipc/recording/windows.ts b/electron/ipc/recording/windows.ts index 32c39cb4..312f31e1 100644 --- a/electron/ipc/recording/windows.ts +++ b/electron/ipc/recording/windows.ts @@ -53,9 +53,10 @@ export function waitForWindowsCaptureStart(proc: ChildProcessWithoutNullStreams) reject(new Error("Timed out waiting for native Windows capture to start")); }, 12000); + let stdoutBuffer = ""; const onStdout = (chunk: Buffer) => { - const text = chunk.toString(); - if (text.includes("Recording started")) { + stdoutBuffer += chunk.toString(); + if (stdoutBuffer.includes("Recording started")) { cleanup(); resolve(); } diff --git a/electron/ipc/register/project.ts b/electron/ipc/register/project.ts index e5028802..2e9836d9 100644 --- a/electron/ipc/register/project.ts +++ b/electron/ipc/register/project.ts @@ -16,7 +16,7 @@ import { setCurrentRecordingSession, } from "../state"; import { normalizeVideoSourcePath } from "../utils"; -import { replaceApprovedSessionLocalReadPaths } from "../project/manager"; +import { isPathInsideDirectory, replaceApprovedSessionLocalReadPaths } from "../project/manager"; import { getTelemetryPathForVideo, isAutoRecordingPath, @@ -351,14 +351,19 @@ export function registerProjectHandlers() { ipcMain.handle('delete-recording-file', async (_, filePath: string) => { try { - if (!filePath || !isAutoRecordingPath(filePath)) { + if (!filePath) { return { success: false, error: 'Only auto-generated recordings can be deleted' }; } - await fs.unlink(filePath); + const resolvedPath = await fs.realpath(filePath).catch(() => path.resolve(filePath)); + const recordingsDir = await getRecordingsDir(); + if (!isPathInsideDirectory(resolvedPath, recordingsDir) || !isAutoRecordingPath(resolvedPath)) { + return { success: false, error: 'Only auto-generated recordings can be deleted' }; + } + await fs.unlink(resolvedPath); // Also delete the cursor telemetry sidecar if it exists - const telemetryPath = getTelemetryPathForVideo(filePath); + const telemetryPath = getTelemetryPathForVideo(resolvedPath); await fs.unlink(telemetryPath).catch(() => {}); - if (currentVideoPath === filePath) { + if (currentVideoPath === resolvedPath) { setCurrentVideoPath(null); setCurrentRecordingSession(null); } diff --git a/electron/ipc/register/recording.ts b/electron/ipc/register/recording.ts index 5be6495a..83d4b27b 100644 --- a/electron/ipc/register/recording.ts +++ b/electron/ipc/register/recording.ts @@ -1040,17 +1040,25 @@ export function registerRecordingHandlers( ipcMain.handle('get-recorded-video-path', async () => { try { const recordingsDir = await getRecordingsDir() - const files = await fs.readdir(recordingsDir) - const videoFiles = files.filter(file => /\.(webm|mov|mp4)$/i.test(file)) - - if (videoFiles.length === 0) { + const entries = await fs.readdir(recordingsDir, { withFileTypes: true }) + const candidates = await Promise.all( + entries + .filter((entry) => entry.isFile() && /^recording-\d+\.(webm|mov|mp4)$/i.test(entry.name)) + .map(async (entry) => { + const fullPath = path.join(recordingsDir, entry.name) + const stat = await fs.stat(fullPath).catch(() => null) + return stat ? { path: fullPath, mtimeMs: stat.mtimeMs } : null + }), + ) + const latestVideo = candidates + .filter((candidate): candidate is { path: string; mtimeMs: number } => candidate !== null) + .sort((left, right) => right.mtimeMs - left.mtimeMs)[0] + + if (!latestVideo) { return { success: false, message: 'No recorded video found' } } - - const latestVideo = videoFiles.sort().reverse()[0] - const videoPath = path.join(recordingsDir, latestVideo) - - return { success: true, path: videoPath } + + return { success: true, path: latestVideo.path } } catch (error) { console.error('Failed to get video path:', error) return { success: false, message: 'Failed to get video path', error: String(error) } diff --git a/electron/ipc/register/sources.ts b/electron/ipc/register/sources.ts index 81eb82da..cb24341b 100644 --- a/electron/ipc/register/sources.ts +++ b/electron/ipc/register/sources.ts @@ -283,12 +283,23 @@ export function registerSourceHandlers({ // ── 1. Bring window to front ── if (isWindow && process.platform === "darwin") { - const appName = source.appName || source.name?.split(" — ")[0]?.trim(); + const rawAppName = source.appName || source.name?.split(" — ")[0]?.trim(); + const appName = + rawAppName && /^[\w .&()+'-]{1,64}$/.test(rawAppName) ? rawAppName : null; if (appName) { try { await execFileAsync( "osascript", - ["-e", `tell application "${appName}" to activate`], + [ + "-e", + "on run argv", + "-e", + "tell application (item 1 of argv) to activate", + "-e", + "end run", + "--", + appName, + ], { timeout: 2000 }, ); await new Promise((resolve) => setTimeout(resolve, 350)); @@ -332,13 +343,23 @@ export function registerSourceHandlers({ bounds = getDisplayBoundsForSource(source); } + if (!bounds || bounds.width <= 0 || bounds.height <= 0) { + const primaryBounds = getScreen().getPrimaryDisplay().bounds; + if (primaryBounds.width <= 0 || primaryBounds.height <= 0) { + return { success: false }; + } + bounds = primaryBounds; + } + + const resolvedBounds = bounds; + // ── 3. Show traveling wave highlight ── const pad = 6; const highlightWin = new BrowserWindow({ - x: bounds.x - pad, - y: bounds.y - pad, - width: bounds.width + pad * 2, - height: bounds.height + pad * 2, + x: resolvedBounds.x - pad, + y: resolvedBounds.y - pad, + width: resolvedBounds.width + pad * 2, + height: resolvedBounds.height + pad * 2, frame: false, transparent: true, alwaysOnTop: true, diff --git a/electron/ipc/state.ts b/electron/ipc/state.ts index 75b83644..7fa98176 100644 --- a/electron/ipc/state.ts +++ b/electron/ipc/state.ts @@ -90,7 +90,7 @@ export let cachedNativeMacWindowSources: import("./types").NativeMacWindowSource export let cachedNativeMacWindowSourcesAtMs = 0; // ── Native video export ─────────────────────────────────────────────────────── -export let cachedNativeVideoEncoder: { ffmpegPath: string; encoderName: string } | null = null; +export let cachedNativeVideoEncoder: { ffmpegPath: string; encodingMode: string; encoderName: string } | null = null; // ── Native helper migration ─────────────────────────────────────────────────── export let nativeHelperMigrationPromise: Promise | null = null; @@ -164,6 +164,6 @@ export function setWindowBoundsCaptureInterval(v: NodeJS.Timeout | null) { windo export function setCachedNativeMacWindowSources(v: import("./types").NativeMacWindowSource[] | null) { cachedNativeMacWindowSources = v; } export function setCachedNativeMacWindowSourcesAtMs(v: number) { cachedNativeMacWindowSourcesAtMs = v; } -export function setCachedNativeVideoEncoder(v: { ffmpegPath: string; encoderName: string } | null) { cachedNativeVideoEncoder = v; } +export function setCachedNativeVideoEncoder(v: { ffmpegPath: string; encodingMode: string; encoderName: string } | null) { cachedNativeVideoEncoder = v; } export function setNativeHelperMigrationPromise(v: Promise | null) { nativeHelperMigrationPromise = v; }