From 0e8e57e5d4b7beee08c1e1f5bc7c29566ffd13c0 Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Sun, 1 Nov 2026 18:55:32 +0100 Subject: [PATCH 1/8] fix(updater): retain the app when Windows elevation is declined --- electron/auto-updater.test.ts | 83 +++++++++++++++++++++-- electron/auto-updater.ts | 18 +++-- electron/windows-update.test.ts | 114 ++++++++++++++++++++++++++++++++ electron/windows-update.ts | 71 ++++++++++++++++++++ 4 files changed, 278 insertions(+), 8 deletions(-) create mode 100644 electron/windows-update.test.ts create mode 100644 electron/windows-update.ts diff --git a/electron/auto-updater.test.ts b/electron/auto-updater.test.ts index 6dfa89e4c..d6059c405 100644 --- a/electron/auto-updater.test.ts +++ b/electron/auto-updater.test.ts @@ -24,10 +24,12 @@ const mocks = vi.hoisted(() => ({ quitAndInstall: vi.fn(), }, app: { isPackaged: true, getVersion: vi.fn(() => "1.9.2") }, + installWindowsUpdate: vi.fn(), })); vi.mock("electron", () => ({ app: mocks.app })); vi.mock("electron-updater", () => ({ autoUpdater: mocks.autoUpdater })); +vi.mock("./windows-update", () => ({ installWindowsUpdate: mocks.installWindowsUpdate })); // `reg query` finds no install, so the handover below is the same on every platform. vi.mock("node:child_process", () => ({ spawnSync: vi.fn(() => ({ status: 1 })) })); @@ -84,6 +86,7 @@ describe("self-update flow", () => { mocks.autoUpdater.checkForUpdates.mockReset(); mocks.autoUpdater.downloadUpdate.mockReset(); mocks.autoUpdater.quitAndInstall.mockReset(); + mocks.installWindowsUpdate.mockReset(); }); it("never touches the updater on a channel a package manager owns", async () => { @@ -155,11 +158,20 @@ describe("self-update flow", () => { expect(result).toMatchObject({ error: { message: "connection reset" } }); }); - // isSilent=true so Windows shows only UAC, not the NSIS wizard (#1437). isForceRunAfter=true - // so the app comes back. - it("hands over to the installer silently and relaunches", async () => { - await installSelfUpdate(); + // Other platforms retain electron-updater's own quit/relaunch handoff. + it("retains quitAndInstall on macOS and Linux", async () => { + const platform = Object.getOwnPropertyDescriptor(process, "platform"); + try { + for (const value of ["darwin", "linux"]) { + Object.defineProperty(process, "platform", { value }); + await installSelfUpdate(); + } + } finally { + if (platform) Object.defineProperty(process, "platform", platform); + } + expect(mocks.autoUpdater.quitAndInstall).toHaveBeenCalledTimes(2); expect(mocks.autoUpdater.quitAndInstall).toHaveBeenCalledWith(true, true); + expect(mocks.installWindowsUpdate).not.toHaveBeenCalled(); }); it("hands over to the wizard on Windows when both installs answer", async () => { @@ -173,6 +185,69 @@ describe("self-update flow", () => { vi.mocked(spawnSync).mockReturnValue({ status: 1 } as ReturnType); } expect(mocks.autoUpdater.quitAndInstall).toHaveBeenCalledWith(false, true); + expect(mocks.installWindowsUpdate).not.toHaveBeenCalled(); + }); + + it("waits for the Windows installer without quitting before the UAC decision", async () => { + const platform = Object.getOwnPropertyDescriptor(process, "platform"); + let finish: (() => void) | undefined; + mocks.installWindowsUpdate.mockImplementation(async (download: () => Promise) => { + await download(); + await new Promise((resolve) => { + finish = resolve; + }); + }); + mocks.autoUpdater.downloadUpdate.mockResolvedValue(["verified-installer.exe"]); + vi.mocked(spawnSync) + .mockReturnValueOnce({ status: 0 } as ReturnType) + .mockReturnValueOnce({ status: 1 } as ReturnType) + .mockReturnValue({ status: 0 } as ReturnType); + Object.defineProperty(process, "platform", { value: "win32" }); + try { + let settled = false; + const install = installSelfUpdate().then(() => { + settled = true; + }); + await vi.waitFor(() => expect(mocks.autoUpdater.downloadUpdate).toHaveBeenCalledOnce()); + expect(settled).toBe(false); + expect(mocks.installWindowsUpdate).toHaveBeenCalledWith(expect.any(Function)); + expect(mocks.autoUpdater.quitAndInstall).not.toHaveBeenCalled(); + finish?.(); + await install; + } finally { + finish?.(); + if (platform) Object.defineProperty(process, "platform", platform); + vi.mocked(spawnSync).mockReturnValue({ status: 1 } as ReturnType); + } + }); + + it("does not request elevation for a per-user Windows update", async () => { + const platform = Object.getOwnPropertyDescriptor(process, "platform"); + Object.defineProperty(process, "platform", { value: "win32" }); + try { + await installSelfUpdate(); + } finally { + if (platform) Object.defineProperty(process, "platform", platform); + } + expect(mocks.autoUpdater.quitAndInstall).toHaveBeenCalledWith(true, true); + expect(mocks.installWindowsUpdate).not.toHaveBeenCalled(); + }); + + it("does not force elevation when the machine install cannot be confirmed", async () => { + const platform = Object.getOwnPropertyDescriptor(process, "platform"); + Object.defineProperty(process, "platform", { value: "win32" }); + vi.mocked(spawnSync) + .mockReturnValueOnce({ status: null } as ReturnType) + .mockReturnValueOnce({ status: 1 } as ReturnType) + .mockReturnValue({ status: null } as ReturnType); + try { + await installSelfUpdate(); + } finally { + if (platform) Object.defineProperty(process, "platform", platform); + vi.mocked(spawnSync).mockReturnValue({ status: 1 } as ReturnType); + } + expect(mocks.autoUpdater.quitAndInstall).toHaveBeenCalledWith(true, true); + expect(mocks.installWindowsUpdate).not.toHaveBeenCalled(); }); }); diff --git a/electron/auto-updater.ts b/electron/auto-updater.ts index 41c17584a..6b26e5a51 100644 --- a/electron/auto-updater.ts +++ b/electron/auto-updater.ts @@ -87,12 +87,16 @@ export async function downloadSelfUpdate(): Promise { * under HKLM per machine and HKCU per user. auto-updater.test.ts pins it to the appId. */ export const NSIS_INSTALL_KEY = "Software\\5b412b41-8d8b-5faf-b557-de4fe681e5fc"; -function hasNsisInstall(hive: "HKLM" | "HKCU"): boolean { +function nsisInstallStatus(hive: "HKLM" | "HKCU"): number | null { const key = `${hive}\\${NSIS_INSTALL_KEY}`; const args = ["query", key, "/v", "InstallLocation", "/reg:64"]; + return spawnSync("reg", args, { windowsHide: true }).status; +} + +function hasNsisInstall(hive: "HKLM" | "HKCU"): boolean { // Exit 1 is reg's "key not found". Anything else, reg itself failing to run included, counts // as present, so a doubt keeps the wizard rather than risking the wrong copy. - return spawnSync("reg", args, { windowsHide: true }).status !== 1; + return nsisInstallStatus(hive) !== 1; } /** Whether the installer can run with /S. Without it the assisted NSIS installer shows its whole @@ -107,9 +111,15 @@ export function installsSilently( return platform !== "win32" || !(hasInstall("HKLM") && hasInstall("HKCU")); } -/** Quit and hand over to the installer. Callers MUST have checked `blockedFromInstalling`. */ +/** Hand over to the installer. Callers MUST have checked `blockedFromInstalling`. */ export async function installSelfUpdate(): Promise { const autoUpdater = await getUpdater(); + const silent = installsSilently(); + if (process.platform === "win32" && silent && nsisInstallStatus("HKLM") === 0) { + const { installWindowsUpdate } = await import("./windows-update"); + await installWindowsUpdate(() => autoUpdater.downloadUpdate()); + return; + } // isForceRunAfter=true so the app comes back up. - autoUpdater.quitAndInstall(installsSilently(), true); + autoUpdater.quitAndInstall(silent, true); } diff --git a/electron/windows-update.test.ts b/electron/windows-update.test.ts new file mode 100644 index 000000000..465a7c07d --- /dev/null +++ b/electron/windows-update.test.ts @@ -0,0 +1,114 @@ +import { type ChildProcess, spawn } from "node:child_process"; +import { EventEmitter } from "node:events"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { installWindowsUpdate } from "./windows-update"; + +const mocks = vi.hoisted(() => ({ + quit: vi.fn(), + window: { + isEnabled: vi.fn(() => true), + setEnabled: vi.fn(), + isDestroyed: vi.fn(() => false), + }, +})); +vi.mock("electron", () => ({ + app: { quit: mocks.quit }, + BrowserWindow: { getAllWindows: () => [mocks.window] }, +})); +vi.mock("node:child_process", () => ({ spawn: vi.fn() })); + +describe("installWindowsUpdate", () => { + let child: EventEmitter & { unref: ReturnType }; + let download: ReturnType Promise>>; + beforeEach(() => { + vi.clearAllMocks(); + mocks.window.isEnabled.mockReturnValue(true); + mocks.window.isDestroyed.mockReturnValue(false); + child = Object.assign(new EventEmitter(), { unref: vi.fn() }); + vi.mocked(spawn).mockReturnValue(child as unknown as ChildProcess); + download = vi.fn(async () => ["C:\\update\\installer.exe"]); + }); + + it("retains the app and controls after No, and permits retry", async () => { + const install = installWindowsUpdate(download); + await vi.waitFor(() => expect(spawn).toHaveBeenCalledOnce()); + expect(mocks.quit).not.toHaveBeenCalled(); + expect(mocks.window.setEnabled.mock.calls).toEqual([[false]]); + expect(child.unref).toHaveBeenCalledOnce(); + child.emit("exit", 1223); + await install; + expect(mocks.quit).not.toHaveBeenCalled(); + expect(mocks.window.setEnabled.mock.calls).toEqual([[false], [true]]); + const retry = installWindowsUpdate(download); + await vi.waitFor(() => expect(spawn).toHaveBeenCalledTimes(2)); + child.emit("exit", 1223); + await retry; + }); + + it("quits normally only after elevation succeeds, with controls disabled until quit", async () => { + const install = installWindowsUpdate(download); + await vi.waitFor(() => expect(spawn).toHaveBeenCalledOnce()); + expect(mocks.quit).not.toHaveBeenCalled(); + child.emit("exit", 0); + await install; + expect(mocks.quit).toHaveBeenCalledOnce(); + expect(mocks.window.setEnabled.mock.calls).toEqual([[false]]); + }); + + it("passes the verified installer as a literal without shell interpolation", async () => { + download.mockResolvedValue(["C:\\update\\it's $a`b.exe"]); + const install = installWindowsUpdate(download); + await vi.waitFor(() => expect(spawn).toHaveBeenCalledOnce()); + const [executable, args, options] = vi.mocked(spawn).mock.calls[0]; + expect(executable).toMatch(/powershell\.exe$/); + expect(args).toEqual([ + "-NoLogo", + "-NoProfile", + "-NonInteractive", + "-EncodedCommand", + expect.any(String), + ]); + const script = Buffer.from((args as string[])[4], "base64").toString("utf16le"); + expect(script).toContain("-FilePath 'C:\\update\\it''s $a`b.exe'"); + expect(script).toContain("@('--updated', '/S', '--force-run') -Verb RunAs"); + expect(script).toContain("NativeErrorCode -eq 1223"); + expect(options).toEqual({ detached: true, stdio: "ignore", windowsHide: true }); + child.emit("exit", 1223); + await install; + }); + + it("restores an already disabled window after a spawn error", async () => { + mocks.window.isEnabled.mockReturnValue(false); + const install = installWindowsUpdate(download); + const rejection = expect(install).rejects.toThrow("spawn failed"); + await vi.waitFor(() => expect(spawn).toHaveBeenCalledOnce()); + child.emit("error", new Error("spawn failed")); + await rejection; + expect(mocks.window.setEnabled.mock.calls).toEqual([[false], [false]]); + expect(mocks.quit).not.toHaveBeenCalled(); + }); + + it("restores the app if verifying the cached download fails", async () => { + download.mockRejectedValue(new Error("checksum mismatch")); + await expect(installWindowsUpdate(download)).rejects.toThrow("checksum mismatch"); + expect(spawn).not.toHaveBeenCalled(); + expect(mocks.window.setEnabled.mock.calls).toEqual([[false], [true]]); + }); + + it("refuses to launch without a verified installer", async () => { + download.mockResolvedValue([]); + await expect(installWindowsUpdate(download)).rejects.toThrow("No downloaded"); + expect(spawn).not.toHaveBeenCalled(); + }); + + it("reports failure without quitting or touching a closed window", async () => { + const install = installWindowsUpdate(download); + const rejection = expect(install).rejects.toThrow("failed (1)"); + await vi.waitFor(() => expect(spawn).toHaveBeenCalledOnce()); + mocks.window.isDestroyed.mockReturnValue(true); + child.emit("exit", 1); + await rejection; + expect(mocks.window.setEnabled.mock.calls).toEqual([[false]]); + expect(mocks.quit).not.toHaveBeenCalled(); + }); +}); diff --git a/electron/windows-update.ts b/electron/windows-update.ts new file mode 100644 index 000000000..6f6603561 --- /dev/null +++ b/electron/windows-update.ts @@ -0,0 +1,71 @@ +import { spawn } from "node:child_process"; +import path from "node:path"; +import { app, BrowserWindow } from "electron"; + +/** Elevate the verified per-machine installer before quitting (#1461). Start-Process + * returns after ShellExecute's UAC decision, without waiting for the installation. + * The prompt names the installer itself; no elevated shell or probe is launched. */ +export async function installWindowsUpdate( + downloadInstaller: () => Promise, +): Promise { + const windows = BrowserWindow.getAllWindows().map((window) => ({ + window, + enabled: window.isEnabled(), + })); + let handedOver = false; + try { + // Prevent a new take while elevation is pending, including the gap after accepting it. + for (const { window } of windows) window.setEnabled(false); + // The public API reuses and verifies the cached installer. + const [installerPath] = await downloadInstaller(); + if (!installerPath) throw new Error("No downloaded Windows update installer"); + const quotedPath = `'${installerPath.replace(/'/g, "''")}'`; + const script = ` +$ErrorActionPreference = 'Stop' +try { + Start-Process -FilePath ${quotedPath} -ArgumentList @('--updated', '/S', '--force-run') -Verb RunAs -WindowStyle Hidden -PassThru | Out-Null + exit 0 +} catch { + $errorCause = $_.Exception + while ($errorCause -and $errorCause -isnot [System.ComponentModel.Win32Exception]) { + $errorCause = $errorCause.InnerException + } + if ($errorCause -and $errorCause.NativeErrorCode -eq 1223) { exit 1223 } + exit 1 +}`; + const powershell = path.join( + process.env.SystemRoot ?? "C:\\Windows", + "System32", + "WindowsPowerShell", + "v1.0", + "powershell.exe", + ); + const exitCode = await new Promise((resolve, reject) => { + const child = spawn( + powershell, + [ + "-NoLogo", + "-NoProfile", + "-NonInteractive", + "-EncodedCommand", + Buffer.from(script, "utf16le").toString("base64"), + ], + { detached: true, stdio: "ignore", windowsHide: true }, + ); + child.once("error", reject); + child.once("exit", resolve); + child.unref(); + }); + if (exitCode === 1223) return; // User declined: retain the app and ready update badge. + if (exitCode !== 0) throw new Error(`Windows update elevation failed (${exitCode})`); + // Preserve normal before-quit cleanup; NSIS is already elevated and waiting for files. + app.quit(); + handedOver = true; + } finally { + if (!handedOver) { + for (const { window, enabled } of windows) { + if (!window.isDestroyed()) window.setEnabled(enabled); + } + } + } +} From 109feee4cc49e25d0adabdff25d8a30c9aab3819 Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Sun, 1 Nov 2026 19:10:04 +0100 Subject: [PATCH 2/8] fix(updater): wait for the PowerShell elevation result --- electron/windows-update.test.ts | 4 ++-- electron/windows-update.ts | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/electron/windows-update.test.ts b/electron/windows-update.test.ts index 465a7c07d..3863ff183 100644 --- a/electron/windows-update.test.ts +++ b/electron/windows-update.test.ts @@ -34,7 +34,7 @@ describe("installWindowsUpdate", () => { await vi.waitFor(() => expect(spawn).toHaveBeenCalledOnce()); expect(mocks.quit).not.toHaveBeenCalled(); expect(mocks.window.setEnabled.mock.calls).toEqual([[false]]); - expect(child.unref).toHaveBeenCalledOnce(); + expect(child.unref).not.toHaveBeenCalled(); child.emit("exit", 1223); await install; expect(mocks.quit).not.toHaveBeenCalled(); @@ -72,7 +72,7 @@ describe("installWindowsUpdate", () => { expect(script).toContain("-FilePath 'C:\\update\\it''s $a`b.exe'"); expect(script).toContain("@('--updated', '/S', '--force-run') -Verb RunAs"); expect(script).toContain("NativeErrorCode -eq 1223"); - expect(options).toEqual({ detached: true, stdio: "ignore", windowsHide: true }); + expect(options).toEqual({ detached: false, stdio: "ignore", windowsHide: true }); child.emit("exit", 1223); await install; }); diff --git a/electron/windows-update.ts b/electron/windows-update.ts index 6f6603561..8445e7a05 100644 --- a/electron/windows-update.ts +++ b/electron/windows-update.ts @@ -50,11 +50,12 @@ try { "-EncodedCommand", Buffer.from(script, "utf16le").toString("base64"), ], - { detached: true, stdio: "ignore", windowsHide: true }, + // Windows PowerShell 5.1 can exit 0 without running the command when detached. + // The bridge must return its actual result; ShellExecute starts NSIS independently. + { detached: false, stdio: "ignore", windowsHide: true }, ); child.once("error", reject); child.once("exit", resolve); - child.unref(); }); if (exitCode === 1223) return; // User declined: retain the app and ready update badge. if (exitCode !== 0) throw new Error(`Windows update elevation failed (${exitCode})`); From 5e34328f19665dff2d766df71b5845701180441b Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Mon, 2 Nov 2026 07:59:11 +0100 Subject: [PATCH 3/8] fix(updater): await the bundled elevation helper --- electron/windows-update.test.ts | 64 ++++++++++++++++++++++----------- electron/windows-update.ts | 59 +++++++++++------------------- 2 files changed, 64 insertions(+), 59 deletions(-) diff --git a/electron/windows-update.test.ts b/electron/windows-update.test.ts index 3863ff183..8514ecf3c 100644 --- a/electron/windows-update.test.ts +++ b/electron/windows-update.test.ts @@ -1,6 +1,8 @@ import { type ChildProcess, spawn } from "node:child_process"; import { EventEmitter } from "node:events"; -import { beforeEach, describe, expect, it, vi } from "vitest"; +import path from "node:path"; +import { PassThrough } from "node:stream"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { installWindowsUpdate } from "./windows-update"; const mocks = vi.hoisted(() => ({ @@ -18,16 +20,18 @@ vi.mock("electron", () => ({ vi.mock("node:child_process", () => ({ spawn: vi.fn() })); describe("installWindowsUpdate", () => { - let child: EventEmitter & { unref: ReturnType }; + let child: EventEmitter & { stderr: PassThrough; unref: ReturnType }; let download: ReturnType Promise>>; beforeEach(() => { vi.clearAllMocks(); + vi.stubGlobal("process", { ...process, resourcesPath: "C:\\OpenScreen\\resources" }); mocks.window.isEnabled.mockReturnValue(true); mocks.window.isDestroyed.mockReturnValue(false); - child = Object.assign(new EventEmitter(), { unref: vi.fn() }); + child = Object.assign(new EventEmitter(), { stderr: new PassThrough(), unref: vi.fn() }); vi.mocked(spawn).mockReturnValue(child as unknown as ChildProcess); download = vi.fn(async () => ["C:\\update\\installer.exe"]); }); + afterEach(() => vi.unstubAllGlobals()); it("retains the app and controls after No, and permits retry", async () => { const install = installWindowsUpdate(download); @@ -35,13 +39,16 @@ describe("installWindowsUpdate", () => { expect(mocks.quit).not.toHaveBeenCalled(); expect(mocks.window.setEnabled.mock.calls).toEqual([[false]]); expect(child.unref).not.toHaveBeenCalled(); - child.emit("exit", 1223); + child.stderr.write("C:\\update\\installer.exe could not be launched: 12"); + child.stderr.write("23\r\n"); + child.emit("close", 1); await install; expect(mocks.quit).not.toHaveBeenCalled(); expect(mocks.window.setEnabled.mock.calls).toEqual([[false], [true]]); const retry = installWindowsUpdate(download); await vi.waitFor(() => expect(spawn).toHaveBeenCalledTimes(2)); - child.emit("exit", 1223); + child.stderr.write("C:\\update\\installer.exe could not be launched: 1223\r\n"); + child.emit("close", 1); await retry; }); @@ -50,6 +57,8 @@ describe("installWindowsUpdate", () => { await vi.waitFor(() => expect(spawn).toHaveBeenCalledOnce()); expect(mocks.quit).not.toHaveBeenCalled(); child.emit("exit", 0); + expect(mocks.quit).not.toHaveBeenCalled(); + child.emit("close", 0); await install; expect(mocks.quit).toHaveBeenCalledOnce(); expect(mocks.window.setEnabled.mock.calls).toEqual([[false]]); @@ -60,23 +69,33 @@ describe("installWindowsUpdate", () => { const install = installWindowsUpdate(download); await vi.waitFor(() => expect(spawn).toHaveBeenCalledOnce()); const [executable, args, options] = vi.mocked(spawn).mock.calls[0]; - expect(executable).toMatch(/powershell\.exe$/); - expect(args).toEqual([ - "-NoLogo", - "-NoProfile", - "-NonInteractive", - "-EncodedCommand", - expect.any(String), - ]); - const script = Buffer.from((args as string[])[4], "base64").toString("utf16le"); - expect(script).toContain("-FilePath 'C:\\update\\it''s $a`b.exe'"); - expect(script).toContain("@('--updated', '/S', '--force-run') -Verb RunAs"); - expect(script).toContain("NativeErrorCode -eq 1223"); - expect(options).toEqual({ detached: false, stdio: "ignore", windowsHide: true }); - child.emit("exit", 1223); + expect(executable).toBe(path.join(process.resourcesPath, "elevate.exe")); + expect(args).toEqual(["C:\\update\\it's $a`b.exe", "--updated", "/S", "--force-run"]); + expect(options).toEqual({ + detached: true, + stdio: ["ignore", "ignore", "pipe"], + windowsHide: true, + }); + child.emit("close", 0); await install; }); + it.each([ + [1, ""], + [1, "installer.exe could not be launched: 12230\r\n"], + [1223, "installer.exe could not be launched: 1223\r\n"], + [null, "installer.exe could not be launched: 1223\r\n"], + ])("reports an unrecognised helper result (%s, %s)", async (exitCode, stderr) => { + const install = installWindowsUpdate(download); + const rejection = expect(install).rejects.toThrow("Windows update elevation failed"); + await vi.waitFor(() => expect(spawn).toHaveBeenCalledOnce()); + child.stderr.write(stderr); + child.emit("close", exitCode); + await rejection; + expect(mocks.quit).not.toHaveBeenCalled(); + expect(mocks.window.setEnabled.mock.calls).toEqual([[false], [true]]); + }); + it("restores an already disabled window after a spawn error", async () => { mocks.window.isEnabled.mockReturnValue(false); const install = installWindowsUpdate(download); @@ -103,10 +122,13 @@ describe("installWindowsUpdate", () => { it("reports failure without quitting or touching a closed window", async () => { const install = installWindowsUpdate(download); - const rejection = expect(install).rejects.toThrow("failed (1)"); + const rejection = expect(install).rejects.toThrow( + "failed (1): installer.exe could not be launched: 3", + ); await vi.waitFor(() => expect(spawn).toHaveBeenCalledOnce()); mocks.window.isDestroyed.mockReturnValue(true); - child.emit("exit", 1); + child.stderr.write("installer.exe could not be launched: 3\r\n"); + child.emit("close", 1); await rejection; expect(mocks.window.setEnabled.mock.calls).toEqual([[false]]); expect(mocks.quit).not.toHaveBeenCalled(); diff --git a/electron/windows-update.ts b/electron/windows-update.ts index 8445e7a05..32aece9b0 100644 --- a/electron/windows-update.ts +++ b/electron/windows-update.ts @@ -2,8 +2,8 @@ import { spawn } from "node:child_process"; import path from "node:path"; import { app, BrowserWindow } from "electron"; -/** Elevate the verified per-machine installer before quitting (#1461). Start-Process - * returns after ShellExecute's UAC decision, without waiting for the installation. +/** Elevate the verified per-machine installer before quitting (#1461). The bundled + * helper returns after ShellExecute's UAC decision, without waiting for installation. * The prompt names the installer itself; no elevated shell or probe is launched. */ export async function installWindowsUpdate( downloadInstaller: () => Promise, @@ -19,46 +19,29 @@ export async function installWindowsUpdate( // The public API reuses and verifies the cached installer. const [installerPath] = await downloadInstaller(); if (!installerPath) throw new Error("No downloaded Windows update installer"); - const quotedPath = `'${installerPath.replace(/'/g, "''")}'`; - const script = ` -$ErrorActionPreference = 'Stop' -try { - Start-Process -FilePath ${quotedPath} -ArgumentList @('--updated', '/S', '--force-run') -Verb RunAs -WindowStyle Hidden -PassThru | Out-Null - exit 0 -} catch { - $errorCause = $_.Exception - while ($errorCause -and $errorCause -isnot [System.ComponentModel.Win32Exception]) { - $errorCause = $errorCause.InnerException - } - if ($errorCause -and $errorCause.NativeErrorCode -eq 1223) { exit 1223 } - exit 1 -}`; - const powershell = path.join( - process.env.SystemRoot ?? "C:\\Windows", - "System32", - "WindowsPowerShell", - "v1.0", - "powershell.exe", - ); - const exitCode = await new Promise((resolve, reject) => { + const { exitCode, stderr } = await new Promise<{ + exitCode: number | null; + stderr: string; + }>((resolve, reject) => { const child = spawn( - powershell, - [ - "-NoLogo", - "-NoProfile", - "-NonInteractive", - "-EncodedCommand", - Buffer.from(script, "utf16le").toString("base64"), - ], - // Windows PowerShell 5.1 can exit 0 without running the command when detached. - // The bridge must return its actual result; ShellExecute starts NSIS independently. - { detached: false, stdio: "ignore", windowsHide: true }, + path.join(process.resourcesPath, "elevate.exe"), + [installerPath, "--updated", "/S", "--force-run"], + { detached: true, stdio: ["ignore", "ignore", "pipe"], windowsHide: true }, ); + let stderr = ""; + child.stderr.setEncoding("utf8"); + child.stderr.on("data", (chunk: string) => { + stderr += chunk; + }); child.once("error", reject); - child.once("exit", resolve); + // Wait for close so the helper's error output has drained too; do not unref it. + child.once("close", (exitCode) => resolve({ exitCode, stderr })); }); - if (exitCode === 1223) return; // User declined: retain the app and ready update badge. - if (exitCode !== 0) throw new Error(`Windows update elevation failed (${exitCode})`); + // electron-builder's bundled helper exits 1 and prints the Win32 error on failure. + if (exitCode === 1 && stderr.trimEnd().endsWith(" could not be launched: 1223")) return; + if (exitCode !== 0) { + throw new Error(`Windows update elevation failed (${exitCode}): ${stderr.trim()}`); + } // Preserve normal before-quit cleanup; NSIS is already elevated and waiting for files. app.quit(); handedOver = true; From 0832d0afae6414c93593a7dc3e3edd2cf3ef7cb8 Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Mon, 2 Nov 2026 09:40:09 +0100 Subject: [PATCH 4/8] fix(updater): serialize elevation and block new takes --- electron/auto-updater.test.ts | 21 +++++++- electron/auto-updater.ts | 12 +++-- electron/ipc/handlers.ts | 13 +++++ electron/main.ts | 3 ++ electron/mcp/app-recorder.test.ts | 15 +++++- electron/mcp/app-recorder.ts | 8 +++- electron/windows-update.test.ts | 79 ++++++++++++++++++++++++++++--- electron/windows-update.ts | 38 ++++++++++++--- 8 files changed, 170 insertions(+), 19 deletions(-) diff --git a/electron/auto-updater.test.ts b/electron/auto-updater.test.ts index d6059c405..1b1defee1 100644 --- a/electron/auto-updater.test.ts +++ b/electron/auto-updater.test.ts @@ -25,11 +25,15 @@ const mocks = vi.hoisted(() => ({ }, app: { isPackaged: true, getVersion: vi.fn(() => "1.9.2") }, installWindowsUpdate: vi.fn(), + assertWindowsUpdateIdle: vi.fn(), })); vi.mock("electron", () => ({ app: mocks.app })); vi.mock("electron-updater", () => ({ autoUpdater: mocks.autoUpdater })); -vi.mock("./windows-update", () => ({ installWindowsUpdate: mocks.installWindowsUpdate })); +vi.mock("./windows-update", () => ({ + installWindowsUpdate: mocks.installWindowsUpdate, + assertWindowsUpdateIdle: mocks.assertWindowsUpdateIdle, +})); // `reg query` finds no install, so the handover below is the same on every platform. vi.mock("node:child_process", () => ({ spawnSync: vi.fn(() => ({ status: 1 })) })); @@ -233,6 +237,21 @@ describe("self-update flow", () => { expect(mocks.installWindowsUpdate).not.toHaveBeenCalled(); }); + it("does not enter a legacy Windows handoff while another elevation is pending", async () => { + const platform = Object.getOwnPropertyDescriptor(process, "platform"); + Object.defineProperty(process, "platform", { value: "win32" }); + mocks.assertWindowsUpdateIdle.mockImplementationOnce(() => { + throw new Error("A Windows update is pending"); + }); + try { + await expect(installSelfUpdate()).rejects.toThrow("Windows update is pending"); + } finally { + if (platform) Object.defineProperty(process, "platform", platform); + } + expect(mocks.autoUpdater.quitAndInstall).not.toHaveBeenCalled(); + expect(mocks.installWindowsUpdate).not.toHaveBeenCalled(); + }); + it("does not force elevation when the machine install cannot be confirmed", async () => { const platform = Object.getOwnPropertyDescriptor(process, "platform"); Object.defineProperty(process, "platform", { value: "win32" }); diff --git a/electron/auto-updater.ts b/electron/auto-updater.ts index 6b26e5a51..3b14b3e40 100644 --- a/electron/auto-updater.ts +++ b/electron/auto-updater.ts @@ -115,10 +115,14 @@ export function installsSilently( export async function installSelfUpdate(): Promise { const autoUpdater = await getUpdater(); const silent = installsSilently(); - if (process.platform === "win32" && silent && nsisInstallStatus("HKLM") === 0) { - const { installWindowsUpdate } = await import("./windows-update"); - await installWindowsUpdate(() => autoUpdater.downloadUpdate()); - return; + if (process.platform === "win32") { + const { installWindowsUpdate, assertWindowsUpdateIdle } = await import("./windows-update"); + // A concurrent check must not fall back to quitting while another UAC is pending. + assertWindowsUpdateIdle(); + if (silent && nsisInstallStatus("HKLM") === 0) { + await installWindowsUpdate(() => autoUpdater.downloadUpdate()); + return; + } } // isForceRunAfter=true so the app comes back up. autoUpdater.quitAndInstall(silent, true); diff --git a/electron/ipc/handlers.ts b/electron/ipc/handlers.ts index 75c3a23df..255fc9c6c 100644 --- a/electron/ipc/handlers.ts +++ b/electron/ipc/handlers.ts @@ -162,6 +162,11 @@ import { } from "../recording-source-settings"; import { getSttManager } from "../stt"; import { createRunnerWindow } from "../windows"; +import { + assertWindowsUpdateIdle, + isWindowsUpdatePending, + WINDOWS_UPDATE_PENDING_MESSAGE, +} from "../windows-update"; import { getSourcesByType, shouldEnumerateSourceTypesSeparately } from "./desktopSourceTypes"; import { registerNativeBridgeHandlers } from "./nativeBridge"; import { createNativeMacMidCaptureErrorWatch } from "./nativeMacMidCaptureErrorWatch"; @@ -2338,6 +2343,9 @@ export function registerIpcHandlers( // Asked before every take starts. A disk that fills up mid-take used to be found only // when the take was saved, and the take was lost with it. ipcMain.handle("check-recording-storage", async () => { + if (isWindowsUpdatePending()) { + return { success: false, error: WINDOWS_UPDATE_PENDING_MESSAGE }; + } const problem = await recordingStorageProblem(RECORDINGS_DIR); if (!problem) return { success: true }; if (problem.kind === "unavailable") { @@ -2514,6 +2522,7 @@ export function registerIpcHandlers( /** The editor's Record button: over to the HUD, which starts the take by itself. */ function startNewRecording() { + assertWindowsUpdateIdle(); _switchToHud?.(); const hudWindow = getMainWindow(); if (hudWindow && !hudWindow.isDestroyed()) { @@ -2895,6 +2904,9 @@ export function registerIpcHandlers( ipcMain.handle( "start-native-windows-recording", async (_, request: NativeWindowsRecordingRequest) => { + if (isWindowsUpdatePending()) { + return { success: false, error: WINDOWS_UPDATE_PENDING_MESSAGE }; + } try { if (!isWindowsGraphicsCaptureOsSupported()) { return { @@ -3047,6 +3059,7 @@ export function registerIpcHandlers( pendingCursorRecordingData = null; } + assertWindowsUpdateIdle(); const proc = spawn(helperPath, [JSON.stringify(config)], { cwd: RECORDINGS_DIR, stdio: ["pipe", "pipe", "pipe"], diff --git a/electron/main.ts b/electron/main.ts index b0a7d38c2..255782c4a 100644 --- a/electron/main.ts +++ b/electron/main.ts @@ -88,6 +88,7 @@ import { createNotesWindow, createSourceSelectorWindow, } from "./windows"; +import { isWindowsUpdatePending } from "./windows-update"; const __dirname = path.dirname(fileURLToPath(import.meta.url)); @@ -188,6 +189,7 @@ const defaultTrayIcon = getTrayIcon("openscreen.png", trayIconSize); const recordingTrayIcon = getTrayIcon("rec-button.png", trayIconSize); function createWindow() { + if (isWindowsUpdatePending()) return; if (mainWindow && !mainWindow.isDestroyed()) { return; } @@ -1476,6 +1478,7 @@ appReady?.then(async () => { await ensureRecordingsDir(); function switchToHudWrapper() { + if (isWindowsUpdatePending()) return; if (mainWindow) { isForceClosing = true; mainWindow.close(); diff --git a/electron/mcp/app-recorder.test.ts b/electron/mcp/app-recorder.test.ts index 78b76be4e..f552a8426 100644 --- a/electron/mcp/app-recorder.test.ts +++ b/electron/mcp/app-recorder.test.ts @@ -2,7 +2,7 @@ // hand, and the editor is a document behind a revision. import path from "node:path"; -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { type AxcutDocument, createEmptyDocument, @@ -113,6 +113,19 @@ async function started(recorder: ReturnType["recorder"], calls: st } describe("an MCP take in the app", () => { + it("restores settings and permits retry when the handoff rejects a take", async () => { + const startTake = vi.fn().mockImplementationOnce(() => { + throw new Error("A Windows update is pending"); + }); + const { recorder, calls } = setup({ startTake }); + await expect(recorder.start(REQUEST)).rejects.toThrow("Windows update is pending"); + expect(calls).toEqual(["prepare My App", "restore"]); + const retry = recorder.start(REQUEST); + await vi.waitFor(() => expect(startTake).toHaveBeenCalledTimes(2)); + recorder.recordingChanged(true); + expect((await retry).startedAt).toBeGreaterThan(0); + }); + it("goes through the recording bar, and hands back the project the editor opened", async () => { const { recorder, calls, editor } = setup(); const starting = recorder.start(REQUEST); diff --git a/electron/mcp/app-recorder.ts b/electron/mcp/app-recorder.ts index 6f2134147..586ccd15f 100644 --- a/electron/mcp/app-recorder.ts +++ b/electron/mcp/app-recorder.ts @@ -248,7 +248,13 @@ export function createAppRecorder(deps: AppRecorderDeps): AppRecorder { }, }; current = take; - deps.startTake(); + try { + deps.startTake(); + } catch (error) { + current = null; + take.restore(); + throw error; + } const startedAt = await within(take.started.promise, timing.startMs); if (startedAt === null) { // A take that starts after this is the user's: nothing here waits for it. diff --git a/electron/windows-update.test.ts b/electron/windows-update.test.ts index 8514ecf3c..4a0d73c1e 100644 --- a/electron/windows-update.test.ts +++ b/electron/windows-update.test.ts @@ -3,10 +3,15 @@ import { EventEmitter } from "node:events"; import path from "node:path"; import { PassThrough } from "node:stream"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { installWindowsUpdate } from "./windows-update"; + +let installWindowsUpdate: typeof import("./windows-update").installWindowsUpdate; +let isWindowsUpdatePending: typeof import("./windows-update").isWindowsUpdatePending; +let assertWindowsUpdateIdle: typeof import("./windows-update").assertWindowsUpdateIdle; const mocks = vi.hoisted(() => ({ quit: vi.fn(), + spawn: vi.fn(), + getAllWindows: vi.fn(), window: { isEnabled: vi.fn(() => true), setEnabled: vi.fn(), @@ -15,16 +20,21 @@ const mocks = vi.hoisted(() => ({ })); vi.mock("electron", () => ({ app: { quit: mocks.quit }, - BrowserWindow: { getAllWindows: () => [mocks.window] }, + BrowserWindow: { getAllWindows: mocks.getAllWindows }, })); -vi.mock("node:child_process", () => ({ spawn: vi.fn() })); +vi.mock("node:child_process", () => ({ spawn: mocks.spawn })); describe("installWindowsUpdate", () => { let child: EventEmitter & { stderr: PassThrough; unref: ReturnType }; let download: ReturnType Promise>>; - beforeEach(() => { + beforeEach(async () => { + vi.resetModules(); vi.clearAllMocks(); vi.stubGlobal("process", { ...process, resourcesPath: "C:\\OpenScreen\\resources" }); + ({ installWindowsUpdate, isWindowsUpdatePending, assertWindowsUpdateIdle } = await import( + "./windows-update" + )); + mocks.getAllWindows.mockReturnValue([mocks.window]); mocks.window.isEnabled.mockReturnValue(true); mocks.window.isDestroyed.mockReturnValue(false); child = Object.assign(new EventEmitter(), { stderr: new PassThrough(), unref: vi.fn() }); @@ -33,6 +43,53 @@ describe("installWindowsUpdate", () => { }); afterEach(() => vi.unstubAllGlobals()); + it("keeps download interactive, then snapshots current windows for elevation", async () => { + let finishDownload: ((paths: string[]) => void) | undefined; + download.mockImplementation( + () => + new Promise((resolve) => { + finishDownload = resolve; + }), + ); + const install = installWindowsUpdate(download); + expect(mocks.window.setEnabled).not.toHaveBeenCalled(); + expect(spawn).not.toHaveBeenCalled(); + expect(isWindowsUpdatePending()).toBe(true); + expect(assertWindowsUpdateIdle).toThrow("Windows update is pending"); + await expect(installWindowsUpdate(download)).rejects.toThrow("Windows update is pending"); + expect(download).toHaveBeenCalledOnce(); + const newWindow = { + isEnabled: vi.fn(() => false), + setEnabled: vi.fn(), + isDestroyed: vi.fn(() => false), + }; + mocks.getAllWindows.mockReturnValue([mocks.window, newWindow]); + finishDownload?.(["C:\\update\\installer.exe"]); + await vi.waitFor(() => expect(spawn).toHaveBeenCalledOnce()); + expect(mocks.window.setEnabled.mock.calls).toEqual([[false]]); + expect(newWindow.setEnabled.mock.calls).toEqual([[false]]); + child.stderr.write("installer.exe could not be launched: 1223\r\n"); + child.emit("close", 1); + await install; + expect(mocks.window.setEnabled.mock.calls).toEqual([[false], [true]]); + expect(newWindow.setEnabled.mock.calls).toEqual([[false], [false]]); + expect(isWindowsUpdatePending()).toBe(false); + expect(assertWindowsUpdateIdle).not.toThrow(); + }); + + it("rejects a second elevation without changing the first window snapshot", async () => { + const install = installWindowsUpdate(download); + await vi.waitFor(() => expect(spawn).toHaveBeenCalledOnce()); + await expect(installWindowsUpdate(download)).rejects.toThrow("Windows update is pending"); + expect(spawn).toHaveBeenCalledOnce(); + expect(mocks.window.setEnabled.mock.calls).toEqual([[false]]); + child.stderr.write("installer.exe could not be launched: 1223\r\n"); + child.emit("close", 1); + await install; + expect(mocks.window.setEnabled.mock.calls).toEqual([[false], [true]]); + expect(isWindowsUpdatePending()).toBe(false); + }); + it("retains the app and controls after No, and permits retry", async () => { const install = installWindowsUpdate(download); await vi.waitFor(() => expect(spawn).toHaveBeenCalledOnce()); @@ -62,6 +119,8 @@ describe("installWindowsUpdate", () => { await install; expect(mocks.quit).toHaveBeenCalledOnce(); expect(mocks.window.setEnabled.mock.calls).toEqual([[false]]); + expect(isWindowsUpdatePending()).toBe(true); + await expect(installWindowsUpdate(download)).rejects.toThrow("Windows update is pending"); }); it("passes the verified installer as a literal without shell interpolation", async () => { @@ -70,7 +129,13 @@ describe("installWindowsUpdate", () => { await vi.waitFor(() => expect(spawn).toHaveBeenCalledOnce()); const [executable, args, options] = vi.mocked(spawn).mock.calls[0]; expect(executable).toBe(path.join(process.resourcesPath, "elevate.exe")); - expect(args).toEqual(["C:\\update\\it's $a`b.exe", "--updated", "/S", "--force-run"]); + expect(args).toEqual([ + "C:\\update\\it's $a`b.exe", + "--updated", + "/S", + "--force-run", + "/allusers", + ]); expect(options).toEqual({ detached: true, stdio: ["ignore", "ignore", "pipe"], @@ -105,13 +170,15 @@ describe("installWindowsUpdate", () => { await rejection; expect(mocks.window.setEnabled.mock.calls).toEqual([[false], [false]]); expect(mocks.quit).not.toHaveBeenCalled(); + expect(isWindowsUpdatePending()).toBe(false); }); it("restores the app if verifying the cached download fails", async () => { download.mockRejectedValue(new Error("checksum mismatch")); await expect(installWindowsUpdate(download)).rejects.toThrow("checksum mismatch"); expect(spawn).not.toHaveBeenCalled(); - expect(mocks.window.setEnabled.mock.calls).toEqual([[false], [true]]); + expect(mocks.window.setEnabled).not.toHaveBeenCalled(); + expect(isWindowsUpdatePending()).toBe(false); }); it("refuses to launch without a verified installer", async () => { diff --git a/electron/windows-update.ts b/electron/windows-update.ts index 32aece9b0..7c1982978 100644 --- a/electron/windows-update.ts +++ b/electron/windows-update.ts @@ -2,30 +2,55 @@ import { spawn } from "node:child_process"; import path from "node:path"; import { app, BrowserWindow } from "electron"; +let updatePending = false; +export const WINDOWS_UPDATE_PENDING_MESSAGE = + "A Windows update is pending. Try again after the update completes or is cancelled."; + +export function isWindowsUpdatePending(): boolean { + return updatePending; +} + +export function assertWindowsUpdateIdle(): void { + if (updatePending) throw new Error(WINDOWS_UPDATE_PENDING_MESSAGE); +} + /** Elevate the verified per-machine installer before quitting (#1461). The bundled * helper returns after ShellExecute's UAC decision, without waiting for installation. * The prompt names the installer itself; no elevated shell or probe is launched. */ export async function installWindowsUpdate( downloadInstaller: () => Promise, ): Promise { + assertWindowsUpdateIdle(); + updatePending = true; + let handedOver = false; + try { + // A missing cache can require a network download: keep the windows interactive. + const [installerPath] = await downloadInstaller(); + if (!installerPath) throw new Error("No downloaded Windows update installer"); + handedOver = await elevateInstaller(installerPath); + } finally { + // An accepted handoff remains pending through asynchronous before-quit cleanup. + if (!handedOver) updatePending = false; + } +} + +async function elevateInstaller(installerPath: string): Promise { const windows = BrowserWindow.getAllWindows().map((window) => ({ window, enabled: window.isEnabled(), })); let handedOver = false; try { - // Prevent a new take while elevation is pending, including the gap after accepting it. + // Main-process guards also prevent takes and new HUDs outside these windows. for (const { window } of windows) window.setEnabled(false); - // The public API reuses and verifies the cached installer. - const [installerPath] = await downloadInstaller(); - if (!installerPath) throw new Error("No downloaded Windows update installer"); const { exitCode, stderr } = await new Promise<{ exitCode: number | null; stderr: string; }>((resolve, reject) => { const child = spawn( path.join(process.resourcesPath, "elevate.exe"), - [installerPath, "--updated", "/S", "--force-run"], + // Pin the confirmed machine copy even if an HKCU install appears during UAC. + [installerPath, "--updated", "/S", "--force-run", "/allusers"], { detached: true, stdio: ["ignore", "ignore", "pipe"], windowsHide: true }, ); let stderr = ""; @@ -38,13 +63,14 @@ export async function installWindowsUpdate( child.once("close", (exitCode) => resolve({ exitCode, stderr })); }); // electron-builder's bundled helper exits 1 and prints the Win32 error on failure. - if (exitCode === 1 && stderr.trimEnd().endsWith(" could not be launched: 1223")) return; + if (exitCode === 1 && stderr.trimEnd().endsWith(" could not be launched: 1223")) return false; if (exitCode !== 0) { throw new Error(`Windows update elevation failed (${exitCode}): ${stderr.trim()}`); } // Preserve normal before-quit cleanup; NSIS is already elevated and waiting for files. app.quit(); handedOver = true; + return true; } finally { if (!handedOver) { for (const { window, enabled } of windows) { From 3a1c2379259da0d2b2f7b37244df14495bbb81f0 Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Mon, 2 Nov 2026 10:27:25 +0100 Subject: [PATCH 5/8] fix(updater): recheck readiness and reserve browser starts --- electron/auto-updater.test.ts | 11 +++- electron/auto-updater.ts | 8 ++- electron/electron-env.d.ts | 3 +- electron/ipc/handlers.ts | 23 +++++++-- electron/main.ts | 25 +++++++--- electron/preload.ts | 9 +++- electron/windows-update.test.ts | 33 ++++++++++++ electron/windows-update.ts | 6 +++ .../useScreenRecorder.restartFailure.test.tsx | 50 +++++++++++++++++-- src/hooks/useScreenRecorder.ts | 23 +++++++++ 10 files changed, 171 insertions(+), 20 deletions(-) diff --git a/electron/auto-updater.test.ts b/electron/auto-updater.test.ts index 1b1defee1..154c06291 100644 --- a/electron/auto-updater.test.ts +++ b/electron/auto-updater.test.ts @@ -56,6 +56,12 @@ describe("blockedFromInstalling", () => { } }); + it("blocks a Windows handoff with unsaved editor changes", () => { + expect(blockedFromInstalling(state({ platform: "win32", editorHasUnsavedChanges: true }))).toBe( + "unsaved-changes", + ); + }); + // App Translocation: a quarantined app runs from a read-only image where Squirrel cannot // replace the bundle. Pinned per-platform because CI is Linux-only and an unpinned branch // would be green here and wrong on the one platform it applies to. @@ -194,6 +200,7 @@ describe("self-update flow", () => { it("waits for the Windows installer without quitting before the UAC decision", async () => { const platform = Object.getOwnPropertyDescriptor(process, "platform"); + const canInstall = vi.fn(() => true); let finish: (() => void) | undefined; mocks.installWindowsUpdate.mockImplementation(async (download: () => Promise) => { await download(); @@ -209,12 +216,12 @@ describe("self-update flow", () => { Object.defineProperty(process, "platform", { value: "win32" }); try { let settled = false; - const install = installSelfUpdate().then(() => { + const install = installSelfUpdate(canInstall).then(() => { settled = true; }); await vi.waitFor(() => expect(mocks.autoUpdater.downloadUpdate).toHaveBeenCalledOnce()); expect(settled).toBe(false); - expect(mocks.installWindowsUpdate).toHaveBeenCalledWith(expect.any(Function)); + expect(mocks.installWindowsUpdate).toHaveBeenCalledWith(expect.any(Function), canInstall); expect(mocks.autoUpdater.quitAndInstall).not.toHaveBeenCalled(); finish?.(); await install; diff --git a/electron/auto-updater.ts b/electron/auto-updater.ts index 3b14b3e40..a2c108b0a 100644 --- a/electron/auto-updater.ts +++ b/electron/auto-updater.ts @@ -23,6 +23,8 @@ export type UpdateOutcome = * all" because these are transient — the answer changes as the user works. */ export interface InstallReadiness { recording: boolean; + /** A Windows handoff cannot wait on the editor's close prompt with input disabled. */ + editorHasUnsavedChanges?: boolean; /** macOS only. Gatekeeper runs a quarantined app from a read-only image (App Translocation), * where Squirrel cannot replace the bundle; worse, an app can BECOME translocated after an * update and then never update again. */ @@ -33,6 +35,7 @@ export interface InstallReadiness { /** Pure so the veto can be tested without an updater, a Mac, or a recording. */ export function blockedFromInstalling(state: InstallReadiness): string | null { if (state.recording) return "recording"; + if (state.platform === "win32" && state.editorHasUnsavedChanges) return "unsaved-changes"; if (state.platform === "darwin" && !state.inApplicationsFolder) return "not-in-applications"; return null; } @@ -112,7 +115,7 @@ export function installsSilently( } /** Hand over to the installer. Callers MUST have checked `blockedFromInstalling`. */ -export async function installSelfUpdate(): Promise { +export async function installSelfUpdate(canInstall: () => boolean = () => true): Promise { const autoUpdater = await getUpdater(); const silent = installsSilently(); if (process.platform === "win32") { @@ -120,10 +123,11 @@ export async function installSelfUpdate(): Promise { // A concurrent check must not fall back to quitting while another UAC is pending. assertWindowsUpdateIdle(); if (silent && nsisInstallStatus("HKLM") === 0) { - await installWindowsUpdate(() => autoUpdater.downloadUpdate()); + await installWindowsUpdate(() => autoUpdater.downloadUpdate(), canInstall); return; } } + if (!canInstall()) return; // isForceRunAfter=true so the app comes back up. autoUpdater.quitAndInstall(silent, true); } diff --git a/electron/electron-env.d.ts b/electron/electron-env.d.ts index b85d616ce..4cf2dbacf 100644 --- a/electron/electron-env.d.ts +++ b/electron/electron-env.d.ts @@ -149,7 +149,8 @@ interface Window { recording: boolean, recordingId?: number, cursorCaptureMode?: import("../src/lib/recordingSession").CursorCaptureMode, - ) => Promise; + reserveOnly?: boolean, + ) => Promise<{ success: boolean; error?: string }>; isNativeWindowsCaptureAvailable: () => Promise<{ success: boolean; available: boolean; diff --git a/electron/ipc/handlers.ts b/electron/ipc/handlers.ts index 255fc9c6c..93c514c5e 100644 --- a/electron/ipc/handlers.ts +++ b/electron/ipc/handlers.ts @@ -1996,9 +1996,9 @@ export function registerIpcHandlers( * for a client's take to start, and the tray shows it. A native path that called * `onRecordingStateChange` itself left the MCP recorder waiting on a take already running. */ - const setAppRecording = (recording: boolean, sourceName: string) => { + const setAppRecording = (recording: boolean, sourceName: string, notifyRecorder = true) => { appRecording = recording; - mcpAppRecorder.recordingChanged(recording); + if (notifyRecorder) mcpAppRecorder.recordingChanged(recording); onRecordingStateChange?.(recording, sourceName); }; const appSettings = new AppSettingsStore(app.getPath("userData")); @@ -4157,7 +4157,22 @@ export function registerIpcHandlers( ipcMain.handle( "set-recording-state", - async (_, recording: boolean, recordingId?: number, cursorCaptureMode?: CursorCaptureMode) => { + async ( + _, + recording: boolean, + recordingId?: number, + cursorCaptureMode?: CursorCaptureMode, + reserveOnly?: boolean, + ) => { + if (recording && isWindowsUpdatePending()) { + return { success: false, error: WINDOWS_UPDATE_PENDING_MESSAGE }; + } + const source = selectedSource || { name: "Screen" }; + if (recording) { + // Claim the take synchronously, before cursor setup yields to an updater. + setAppRecording(recording, source.name, false); + if (reserveOnly) return { success: true }; + } const normalizedCursorCaptureMode = normalizeCursorCaptureMode(cursorCaptureMode) ?? "editable-overlay"; if (recording && normalizedCursorCaptureMode === "editable-overlay") { @@ -4166,8 +4181,8 @@ export function registerIpcHandlers( await stopCursorRecording(); } - const source = selectedSource || { name: "Screen" }; setAppRecording(recording, source.name); + return { success: true }; }, ); diff --git a/electron/main.ts b/electron/main.ts index 255782c4a..506b45da0 100644 --- a/electron/main.ts +++ b/electron/main.ts @@ -685,6 +685,7 @@ function setUpdateBadge(badge: UpdateBadge | null, releasePage: string | null = function installBlock(): string | null { return blockedFromInstalling({ recording: isRecording, + editorHasUnsavedChanges, inApplicationsFolder: process.platform === "darwin" ? (app.isInApplicationsFolder?.() ?? true) : true, platform: process.platform, @@ -695,13 +696,25 @@ async function showBlockedInstall(blocked: string) { await showMessageBox({ type: "info", title: PRODUCT_NAME, - message: mainT( - "common", - blocked === "recording" ? "updates.blockedRecording" : "updates.blockedLocation", - ), + message: + blocked === "unsaved-changes" + ? mainT("dialogs", "unsavedChanges.message") + : mainT( + "common", + blocked === "recording" ? "updates.blockedRecording" : "updates.blockedLocation", + ), }); } +async function installWhenReady() { + let blocked: string | null = null; + await installSelfUpdate(() => { + blocked = installBlock(); + return blocked === null; + }); + if (blocked) await showBlockedInstall(blocked); +} + let usagePingEnabled = true; /** Hidden where the ping never runs anyway: a dev build, or DO_NOT_TRACK set. */ @@ -743,7 +756,7 @@ async function downloadAndInstall(latestVersion: string) { }); return restart.response; }, - install: installSelfUpdate, + install: installWhenReady, }); if (result.status === "failed") { await showMessageBox({ @@ -832,7 +845,7 @@ async function applyUpdate() { const blocked = installBlock(); if (blocked) await showBlockedInstall(blocked); else if (badge.state === "available") await downloadAndInstall(badge.version); - else await installSelfUpdate(); + else await installWhenReady(); } finally { applyUpdateInFlight = false; } diff --git a/electron/preload.ts b/electron/preload.ts index 4e96cdb76..021932a33 100644 --- a/electron/preload.ts +++ b/electron/preload.ts @@ -241,8 +241,15 @@ contextBridge.exposeInMainWorld("electronAPI", { recording: boolean, recordingId?: number, cursorCaptureMode?: import("../src/lib/recordingSession").CursorCaptureMode, + reserveOnly?: boolean, ) => { - return ipcRenderer.invoke("set-recording-state", recording, recordingId, cursorCaptureMode); + return ipcRenderer.invoke( + "set-recording-state", + recording, + recordingId, + cursorCaptureMode, + reserveOnly, + ); }, isNativeWindowsCaptureAvailable: () => { return ipcRenderer.invoke("is-native-windows-capture-available"); diff --git a/electron/windows-update.test.ts b/electron/windows-update.test.ts index 4a0d73c1e..3474eff29 100644 --- a/electron/windows-update.test.ts +++ b/electron/windows-update.test.ts @@ -90,6 +90,39 @@ describe("installWindowsUpdate", () => { expect(isWindowsUpdatePending()).toBe(false); }); + it("checks edits made during download before disabling windows or elevating", async () => { + let finishDownload: ((paths: string[]) => void) | undefined; + download.mockImplementation( + () => + new Promise((resolve) => { + finishDownload = resolve; + }), + ); + let unsaved = false; + const canInstall = vi.fn(() => !unsaved); + const install = installWindowsUpdate(download, canInstall); + expect(canInstall).not.toHaveBeenCalled(); + unsaved = true; + finishDownload?.(["installer.exe"]); + await install; + expect(canInstall).toHaveBeenCalledOnce(); + expect(spawn).not.toHaveBeenCalled(); + expect(mocks.window.setEnabled).not.toHaveBeenCalled(); + expect(isWindowsUpdatePending()).toBe(false); + }); + + it("handles stderr pipe errors and still uses the helper outcome", async () => { + const install = installWindowsUpdate(download); + await vi.waitFor(() => expect(spawn).toHaveBeenCalledOnce()); + expect(() => child.stderr.emit("error", new Error("pipe failed"))).not.toThrow(); + child.stderr.write("installer.exe could not be launched: 1223\r\n"); + child.emit("close", 1); + await install; + expect(mocks.quit).not.toHaveBeenCalled(); + expect(mocks.window.setEnabled.mock.calls).toEqual([[false], [true]]); + expect(isWindowsUpdatePending()).toBe(false); + }); + it("retains the app and controls after No, and permits retry", async () => { const install = installWindowsUpdate(download); await vi.waitFor(() => expect(spawn).toHaveBeenCalledOnce()); diff --git a/electron/windows-update.ts b/electron/windows-update.ts index 7c1982978..458aba03f 100644 --- a/electron/windows-update.ts +++ b/electron/windows-update.ts @@ -19,6 +19,7 @@ export function assertWindowsUpdateIdle(): void { * The prompt names the installer itself; no elevated shell or probe is launched. */ export async function installWindowsUpdate( downloadInstaller: () => Promise, + canInstall: () => boolean = () => true, ): Promise { assertWindowsUpdateIdle(); updatePending = true; @@ -27,6 +28,8 @@ export async function installWindowsUpdate( // A missing cache can require a network download: keep the windows interactive. const [installerPath] = await downloadInstaller(); if (!installerPath) throw new Error("No downloaded Windows update installer"); + // No await between this late readiness check and disabling the current windows. + if (!canInstall()) return; handedOver = await elevateInstaller(installerPath); } finally { // An accepted handoff remains pending through asynchronous before-quit cleanup. @@ -58,6 +61,9 @@ async function elevateInstaller(installerPath: string): Promise { child.stderr.on("data", (chunk: string) => { stderr += chunk; }); + child.stderr.on("error", () => { + // The helper's close result remains authoritative if its stderr pipe fails. + }); child.once("error", reject); // Wait for close so the helper's error output has drained too; do not unref it. child.once("close", (exitCode) => resolve({ exitCode, stderr })); diff --git a/src/hooks/useScreenRecorder.restartFailure.test.tsx b/src/hooks/useScreenRecorder.restartFailure.test.tsx index 63c78d916..990e2745d 100644 --- a/src/hooks/useScreenRecorder.restartFailure.test.tsx +++ b/src/hooks/useScreenRecorder.restartFailure.test.tsx @@ -13,7 +13,7 @@ vi.mock("sonner", () => ({ // The browser pipeline's MediaRecorder, cut down to what Restart touches: a stop that // fires "stop", and a take already streamed to disk. vi.mock("./recorderHandle", () => ({ - createRecorderHandle: () => { + createRecorderHandle: vi.fn(() => { const recorder = Object.assign(new EventTarget(), { state: "recording" as RecordingState, stop: () => undefined, @@ -28,10 +28,11 @@ vi.mock("./recorderHandle", () => ({ isStreaming: () => true, discard: async () => undefined, }; - }, + }), })); import { toast } from "sonner"; +import { createRecorderHandle } from "./recorderHandle"; import { useScreenRecorder } from "./useScreenRecorder"; type ElectronAPI = Window["electronAPI"]; @@ -50,7 +51,7 @@ function stubElectronAPI(platform: "win32" | "linux") { showCountdownOverlay: vi.fn(async () => true), setCountdownOverlayValue: vi.fn(async () => true), hideCountdownOverlay: vi.fn(async () => true), - setRecordingState: vi.fn(), + setRecordingState: vi.fn(async () => ({ success: true })), discardCursorTelemetry: vi.fn(), isNativeWindowsCaptureAvailable: vi.fn(async () => ({ success: true, available: true })), startNativeWindowsRecording: vi.fn(async () => ({ success: true, recordingId: 7 })), @@ -84,7 +85,7 @@ function stubBrowserCapture() { })); Object.defineProperty(navigator, "mediaDevices", { configurable: true, - value: { getUserMedia }, + value: { getUserMedia, getDisplayMedia: getUserMedia }, }); vi.stubGlobal( "MediaStream", @@ -132,6 +133,7 @@ async function restart(view: RecorderView) { beforeEach(() => { vi.useFakeTimers(); vi.mocked(toast.error).mockClear(); + vi.mocked(createRecorderHandle).mockClear(); }); afterEach(() => { @@ -143,6 +145,46 @@ afterEach(() => { // Restart discards the take before the next one is asked for. When that one does not // start, the take is gone and the user has to be told (#995). +describe("browser take start reservation", () => { + it("does not construct a recorder when the main process refuses a queued Windows take", async () => { + const capture = stubBrowserCapture(); + api.getPlatform.mockReturnValue("win32"); + api.isNativeWindowsCaptureAvailable.mockResolvedValue({ + success: true, + available: false, + reason: "missing-helper", + }); + api.setRecordingState.mockResolvedValue({ + success: false, + error: "A Windows update is pending", + }); + const view = renderHook(() => useScreenRecorder()); + await settle(); + await act(async () => { + await view.result.current.startRecordingImmediately(); + }); + expect(view.result.current.recording).toBe(false); + expect(createRecorderHandle).not.toHaveBeenCalled(); + expect(toast.error).toHaveBeenCalledWith("A Windows update is pending"); + const stream = await capture.mock.results[0].value; + expect(stream.getTracks()[0].stop).toHaveBeenCalled(); + }); + + it("releases the claim when recorder construction fails", async () => { + stubBrowserCapture(); + vi.mocked(createRecorderHandle).mockImplementationOnce(() => { + throw new Error("recorder failed"); + }); + const view = renderHook(() => useScreenRecorder()); + await settle(); + await act(async () => { + await view.result.current.startRecordingImmediately(); + }); + expect(view.result.current.recording).toBe(false); + expect(api.setRecordingState).toHaveBeenCalledWith(false); + }); +}); + describe("useScreenRecorder Restart that does not start a new take", () => { it("says the take was discarded on native Windows", async () => { stubElectronAPI("win32"); diff --git a/src/hooks/useScreenRecorder.ts b/src/hooks/useScreenRecorder.ts index 3ec0a4ac4..65b750fac 100644 --- a/src/hooks/useScreenRecorder.ts +++ b/src/hooks/useScreenRecorder.ts @@ -1813,6 +1813,8 @@ export function useScreenRecorder(): UseScreenRecorderReturn { ) => { const platform = window.electronAPI.getPlatform(); const browserCursorCaptureMode = effectiveBrowserCursorMode(platform, cursorCaptureMode); + let browserStartClaimed = false; + let browserRecorderStarted = false; try { if (!isCountdownRunActive(countdownRunToken)) { @@ -2072,6 +2074,20 @@ export function useScreenRecorder(): UseScreenRecorderReturn { return; } + // Reserve the take before constructing MediaRecorder. Start cursor sampling below + // as before, so this handshake does not shift the cursor/video time origin. + const claim = await window.electronAPI.setRecordingState( + true, + undefined, + browserCursorCaptureMode, + true, + ); + if (!claim.success) throw new Error(claim.error ?? "The recording could not start."); + browserStartClaimed = true; + if (!isCountdownRunActive(countdownRunToken)) { + teardownMedia(); + return; + } recordingId.current = Date.now(); const activeRecordingId = recordingId.current; screenRecorder.current = createRecorderHandle( @@ -2110,6 +2126,7 @@ export function useScreenRecorder(): UseScreenRecorderReturn { setRecording(true); setPaused(false); setElapsedSeconds(0); + browserRecorderStarted = true; window.electronAPI?.setRecordingState(true, recordingId.current, browserCursorCaptureMode); const activeScreenRecorder = screenRecorder.current; @@ -2147,6 +2164,12 @@ export function useScreenRecorder(): UseScreenRecorderReturn { screenRecorder.current = null; webcamRecorder.current = null; teardownMedia(); + } finally { + if (browserStartClaimed && !browserRecorderStarted) { + await window.electronAPI.setRecordingState(false).catch((error) => { + console.warn("Failed to release the recording start:", error); + }); + } } }; From 728789428d0efd35a629777db980bbe916def4bf Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Mon, 2 Nov 2026 11:15:46 +0100 Subject: [PATCH 6/8] fix(updater): complete recording startup and failure guards --- electron/auto-updater.test.ts | 12 +++--- electron/auto-updater.ts | 12 ++++-- electron/ipc/handlers.ts | 20 +++++++--- electron/ipc/recordingStateRouting.test.ts | 2 +- electron/main.ts | 14 ++++--- src/hooks/recorderHandle.test.ts | 22 +++++++++++ src/hooks/recorderHandle.ts | 2 + .../useScreenRecorder.restartFailure.test.tsx | 38 +++++++++++++++++++ src/hooks/useScreenRecorder.ts | 16 ++++++++ src/native/browserShim.test.ts | 7 ++++ src/native/browserShim.ts | 1 + 11 files changed, 127 insertions(+), 19 deletions(-) diff --git a/electron/auto-updater.test.ts b/electron/auto-updater.test.ts index 154c06291..50dcbd7af 100644 --- a/electron/auto-updater.test.ts +++ b/electron/auto-updater.test.ts @@ -259,20 +259,22 @@ describe("self-update flow", () => { expect(mocks.installWindowsUpdate).not.toHaveBeenCalled(); }); - it("does not force elevation when the machine install cannot be confirmed", async () => { + it.each(["HKLM", "HKCU"])("keeps the app open when the %s query fails", async (hive) => { const platform = Object.getOwnPropertyDescriptor(process, "platform"); Object.defineProperty(process, "platform", { value: "win32" }); vi.mocked(spawnSync) - .mockReturnValueOnce({ status: null } as ReturnType) - .mockReturnValueOnce({ status: 1 } as ReturnType) + .mockReturnValueOnce({ status: hive === "HKLM" ? null : 0 } as ReturnType) + .mockReturnValueOnce({ status: hive === "HKCU" ? null : 1 } as ReturnType) .mockReturnValue({ status: null } as ReturnType); try { - await installSelfUpdate(); + await expect(installSelfUpdate()).rejects.toThrow( + "Could not confirm the Windows installation type", + ); } finally { if (platform) Object.defineProperty(process, "platform", platform); vi.mocked(spawnSync).mockReturnValue({ status: 1 } as ReturnType); } - expect(mocks.autoUpdater.quitAndInstall).toHaveBeenCalledWith(true, true); + expect(mocks.autoUpdater.quitAndInstall).not.toHaveBeenCalled(); expect(mocks.installWindowsUpdate).not.toHaveBeenCalled(); }); }); diff --git a/electron/auto-updater.ts b/electron/auto-updater.ts index a2c108b0a..8b69b4f71 100644 --- a/electron/auto-updater.ts +++ b/electron/auto-updater.ts @@ -98,7 +98,8 @@ function nsisInstallStatus(hive: "HKLM" | "HKCU"): number | null { function hasNsisInstall(hive: "HKLM" | "HKCU"): boolean { // Exit 1 is reg's "key not found". Anything else, reg itself failing to run included, counts - // as present, so a doubt keeps the wizard rather than risking the wrong copy. + // as present, so this selector never treats an uncertain copy as absent. The installer + // coordinator below refuses failed queries rather than quitting with an unknown target. return nsisInstallStatus(hive) !== 1; } @@ -117,12 +118,17 @@ export function installsSilently( /** Hand over to the installer. Callers MUST have checked `blockedFromInstalling`. */ export async function installSelfUpdate(canInstall: () => boolean = () => true): Promise { const autoUpdater = await getUpdater(); - const silent = installsSilently(); + let silent = true; if (process.platform === "win32") { const { installWindowsUpdate, assertWindowsUpdateIdle } = await import("./windows-update"); // A concurrent check must not fall back to quitting while another UAC is pending. assertWindowsUpdateIdle(); - if (silent && nsisInstallStatus("HKLM") === 0) { + const installed = { HKLM: nsisInstallStatus("HKLM"), HKCU: nsisInstallStatus("HKCU") }; + if (Object.values(installed).some((status) => status !== 0 && status !== 1)) { + throw new Error("Could not confirm the Windows installation type. Please retry the update."); + } + silent = installsSilently("win32", (hive) => installed[hive] === 0); + if (silent && installed.HKLM === 0) { await installWindowsUpdate(() => autoUpdater.downloadUpdate(), canInstall); return; } diff --git a/electron/ipc/handlers.ts b/electron/ipc/handlers.ts index 93c514c5e..5249de925 100644 --- a/electron/ipc/handlers.ts +++ b/electron/ipc/handlers.ts @@ -1983,7 +1983,7 @@ export function registerIpcHandlers( getSourceSelectorWindow: () => BrowserWindow | null, getNotesWindow: () => BrowserWindow | null, getCountdownOverlayWindow?: () => BrowserWindow | null, - onRecordingStateChange?: (recording: boolean, sourceName: string) => void, + onRecordingStateChange?: (recording: boolean, sourceName: string, preparing?: boolean) => void, _switchToHud?: () => void, /** The editor holds changes it has not saved (main.ts): an MCP take would close it. */ editorHasUnsavedChanges?: () => boolean, @@ -1996,10 +1996,10 @@ export function registerIpcHandlers( * for a client's take to start, and the tray shows it. A native path that called * `onRecordingStateChange` itself left the MCP recorder waiting on a take already running. */ - const setAppRecording = (recording: boolean, sourceName: string, notifyRecorder = true) => { + const setAppRecording = (recording: boolean, sourceName: string, preparing = false) => { appRecording = recording; - if (notifyRecorder) mcpAppRecorder.recordingChanged(recording); - onRecordingStateChange?.(recording, sourceName); + if (!preparing) mcpAppRecorder.recordingChanged(recording); + onRecordingStateChange?.(recording, sourceName, preparing); }; const appSettings = new AppSettingsStore(app.getPath("userData")); const broadcastSelectedSource = (source: SelectedSource | null) => { @@ -2907,6 +2907,7 @@ export function registerIpcHandlers( if (isWindowsUpdatePending()) { return { success: false, error: WINDOWS_UPDATE_PENDING_MESSAGE }; } + let startReserved = false; try { if (!isWindowsGraphicsCaptureOsSupported()) { return { @@ -3035,6 +3036,13 @@ export function registerIpcHandlers( outputPath, }); + assertWindowsUpdateIdle(); + if (appRecording || nativeWindowsCaptureProcess) { + return { success: false, error: "Native Windows capture is already running." }; + } + // Reserve startup before cursor/helper awaits without reporting a started take. + startReserved = true; + setAppRecording(true, selectedSource?.name ?? "Screen", true); await fs.mkdir(RECORDINGS_DIR, { recursive: true }); nativeWindowsCaptureOutput = ""; nativeWindowsCaptureTargetPath = outputPath; @@ -3133,6 +3141,7 @@ export function registerIpcHandlers( }; } catch (error) { console.error("Failed to start native Windows recording:", error); + if (!startReserved) return { success: false, error: String(error) }; const failedProc = nativeWindowsCaptureProcess; const failedOutputPaths = [ nativeWindowsCaptureTargetPath, @@ -3151,6 +3160,7 @@ export function registerIpcHandlers( } } await stopCursorRecording(); + setAppRecording(false, selectedSource?.name ?? "Screen"); return { success: false, error: String(error) }; } }, @@ -4170,7 +4180,7 @@ export function registerIpcHandlers( const source = selectedSource || { name: "Screen" }; if (recording) { // Claim the take synchronously, before cursor setup yields to an updater. - setAppRecording(recording, source.name, false); + setAppRecording(recording, source.name, true); if (reserveOnly) return { success: true }; } const normalizedCursorCaptureMode = diff --git a/electron/ipc/recordingStateRouting.test.ts b/electron/ipc/recordingStateRouting.test.ts index 13c7193f1..fa3f85909 100644 --- a/electron/ipc/recordingStateRouting.test.ts +++ b/electron/ipc/recordingStateRouting.test.ts @@ -37,7 +37,7 @@ describe("recording state routing", () => { const body = setter.slice(0, setter.indexOf("};")); expect(body).toContain("appRecording = recording"); expect(body).toContain("mcpAppRecorder.recordingChanged(recording)"); - expect(body).toContain("onRecordingStateChange?.(recording, sourceName)"); + expect(body).toContain("onRecordingStateChange?.(recording, sourceName, preparing)"); }); it.each(["windows", "mac", "linux"])("a native %s start and stop notify it", (platform) => { diff --git a/electron/main.ts b/electron/main.ts index 506b45da0..60523105d 100644 --- a/electron/main.ts +++ b/electron/main.ts @@ -496,7 +496,9 @@ let updateCheckAbort: AbortController | null = null; * the install, i.e. after a 240 MB download has already competed with the encoder. Every * surface is rebuilt when the flag flips (see `setupApplicationMenu`/`updateTrayMenu`). */ function canOfferUpdateCheck(): boolean { - return offersUpdateCheck(getInstallChannel(), { recording: isRecording }); + return offersUpdateCheck(getInstallChannel(), { + recording: isRecording || recordingStartPending, + }); } /** What the HUD is told at mount, and only the permanent half of the veto. The recording half @@ -664,6 +666,7 @@ function runSaveDiagnostics() { * and on Windows it physically cannot, because the capture helpers spawn from inside the * install directory and NSIS cannot overwrite a running .exe. */ let isRecording = false; +let recordingStartPending = false; let backgroundUpdateTimer: ReturnType | null = null; /** The update button, shown by the HUD, the editor and the tray; null hides it. Only a background * check and the downloads it leads to set it: the manual check keeps its own dialogs. */ @@ -684,7 +687,7 @@ function setUpdateBadge(badge: UpdateBadge | null, releasePage: string | null = function installBlock(): string | null { return blockedFromInstalling({ - recording: isRecording, + recording: isRecording || recordingStartPending, editorHasUnsavedChanges, inApplicationsFolder: process.platform === "darwin" ? (app.isInApplicationsFolder?.() ?? true) : true, @@ -1510,12 +1513,13 @@ appReady?.then(async () => { () => sourceSelectorWindow, () => notesWindow, () => countdownOverlayWindow, - (recording: boolean, sourceName: string) => { + (recording: boolean, sourceName: string, preparing = false) => { selectedSourceName = sourceName; - isRecording = recording; + recordingStartPending = recording && preparing; + isRecording = recording && !preparing; setDisplaySleepBlocked(recording); if (!tray) createTray(); - updateTrayMenu(recording); + updateTrayMenu(isRecording); // `canOfferUpdateCheck()` now answers "not mid-take" too, and the app/Help menus are // built once at startup — without this they keep offering the check during a take. setupApplicationMenu(); diff --git a/src/hooks/recorderHandle.test.ts b/src/hooks/recorderHandle.test.ts index 91d7154b9..59f731785 100644 --- a/src/hooks/recorderHandle.test.ts +++ b/src/hooks/recorderHandle.test.ts @@ -245,6 +245,28 @@ describe("createRecorderHandle", () => { expect(closeRecordingStream).toHaveBeenCalledWith("rec.webm"); }); + it("discard waits for a pending open before closing its stream", async () => { + let finishOpen: ((result: { success: boolean }) => void) | undefined; + const open = new Promise<{ success: boolean }>((resolve) => { + finishOpen = resolve; + }); + const closeRecordingStream = vi.fn(async () => ({ success: true })); + stubElectronAPI({ + openRecordingStream: vi.fn(() => open), + appendRecordingChunk: vi.fn(async () => ({ success: true })), + closeRecordingStream, + }); + const handle = createRecorderHandle({} as MediaStream, { mimeType: "video/webm" }, "rec.webm"); + const discarded = handle.discard(); + await tick(); + expect(closeRecordingStream).not.toHaveBeenCalled(); + finishOpen?.({ success: true }); + await discarded; + expect(closeRecordingStream).toHaveBeenCalledWith("rec.webm"); + driver(handle).stop(); + await handle.recordedBlobPromise; + }); + it("discard is a no-op when the stream never opened", async () => { const closeRecordingStream = vi.fn(async () => ({ success: true })); stubElectronAPI({ diff --git a/src/hooks/recorderHandle.ts b/src/hooks/recorderHandle.ts index 6264b9be5..0e4b2df8f 100644 --- a/src/hooks/recorderHandle.ts +++ b/src/hooks/recorderHandle.ts @@ -133,6 +133,8 @@ export function createRecorderHandle( } async function discard(): Promise { + await openPromise.catch(() => undefined); + await writeChain; if (streamOpened && fileName && api?.closeRecordingStream) { await api.closeRecordingStream(fileName); } diff --git a/src/hooks/useScreenRecorder.restartFailure.test.tsx b/src/hooks/useScreenRecorder.restartFailure.test.tsx index 990e2745d..00293717c 100644 --- a/src/hooks/useScreenRecorder.restartFailure.test.tsx +++ b/src/hooks/useScreenRecorder.restartFailure.test.tsx @@ -182,6 +182,44 @@ describe("browser take start reservation", () => { }); expect(view.result.current.recording).toBe(false); expect(api.setRecordingState).toHaveBeenCalledWith(false); + expect(api.setRecordingState.mock.calls[0]).toEqual([ + true, + undefined, + expect.any(String), + true, + ]); + expect(api.setRecordingState.mock.invocationCallOrder[0]).toBeLessThan( + vi.mocked(createRecorderHandle).mock.invocationCallOrder[0], + ); + }); + + it("stops and discards a handle retained when later recorder setup fails", async () => { + stubBrowserCapture(); + const recorder = Object.assign(new EventTarget(), { + state: "recording" as RecordingState, + stop: vi.fn(() => { + recorder.state = "inactive"; + }), + }); + vi.spyOn(recorder, "addEventListener").mockImplementationOnce(() => { + throw new Error("later setup failed"); + }); + const failed = { + recorder: recorder as unknown as MediaRecorder, + recordedBlobPromise: Promise.resolve(new Blob()), + isStreaming: () => true, + discard: vi.fn(async () => undefined), + }; + vi.mocked(createRecorderHandle).mockReturnValueOnce(failed); + const view = renderHook(() => useScreenRecorder()); + await settle(); + await act(async () => { + await view.result.current.startRecordingImmediately(); + }); + expect(recorder.stop).toHaveBeenCalledOnce(); + expect(failed.discard).toHaveBeenCalledOnce(); + expect(api.setRecordingState).toHaveBeenCalledWith(false); + expect(view.result.current.recording).toBe(false); }); }); diff --git a/src/hooks/useScreenRecorder.ts b/src/hooks/useScreenRecorder.ts index 65b750fac..8ed63698c 100644 --- a/src/hooks/useScreenRecorder.ts +++ b/src/hooks/useScreenRecorder.ts @@ -2161,8 +2161,24 @@ export function useScreenRecorder(): UseScreenRecorderReturn { setElapsedSeconds(0); accumulatedDurationMs.current = 0; segmentStartedAt.current = null; + const failedRecorders = [screenRecorder.current, webcamRecorder.current]; screenRecorder.current = null; webcamRecorder.current = null; + allowAutoFinalize.current = false; + for (const failed of failedRecorders) { + if (!failed) continue; + try { + if (failed.recorder.state !== "inactive") failed.recorder.stop(); + } catch (stopError) { + console.warn("Failed to stop the incomplete recording:", stopError); + } + void failed.recordedBlobPromise + .catch(() => undefined) + .then(() => failed.discard()) + .catch((discardError) => + console.warn("Failed to discard the incomplete recording:", discardError), + ); + } teardownMedia(); } finally { if (browserStartClaimed && !browserRecorderStarted) { diff --git a/src/native/browserShim.test.ts b/src/native/browserShim.test.ts index 3360d0ddf..6395f2045 100644 --- a/src/native/browserShim.test.ts +++ b/src/native/browserShim.test.ts @@ -60,6 +60,13 @@ describe("browserShim addAsset (issue #350)", () => { }); describe("browserShim recording settings", () => { + it("acknowledges recording reservations and releases without a main process", async () => { + await expect( + window.electronAPI.setRecordingState(true, undefined, "editable-overlay", true), + ).resolves.toEqual({ success: true }); + await expect(window.electronAPI.setRecordingState(false)).resolves.toEqual({ success: true }); + }); + it("publishes recording settings only after localStorage succeeds", async () => { await window.electronAPI.setRecordingPrefs({ micEnabled: false }); const setItem = Storage.prototype.setItem; diff --git a/src/native/browserShim.ts b/src/native/browserShim.ts index 3b7b704d2..64b70a0d9 100644 --- a/src/native/browserShim.ts +++ b/src/native/browserShim.ts @@ -187,6 +187,7 @@ function createShimElectronAPI() { return () => shimSelectedSourceListeners.delete(callback); }, getRecordingPrefs: () => Promise.resolve(shimRecordingPrefs), + setRecordingState: () => Promise.resolve({ success: true as const }), setRecordingPrefs: (patch: Partial) => { const next = { ...shimRecordingPrefs, ...patch }; try { From 949af2fed7980fb1d47aa7ea2cf7af9314fb2e14 Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Mon, 2 Nov 2026 11:30:19 +0100 Subject: [PATCH 7/8] fix(updater): confirm missing registry keys before silent install --- electron/auto-updater.test.ts | 76 +++++++++++++++---- electron/auto-updater.ts | 15 +++- src/hooks/recorderHandle.test.ts | 15 +++- .../useScreenRecorder.restartFailure.test.tsx | 10 ++- 4 files changed, 97 insertions(+), 19 deletions(-) diff --git a/electron/auto-updater.test.ts b/electron/auto-updater.test.ts index 50dcbd7af..36c9168df 100644 --- a/electron/auto-updater.test.ts +++ b/electron/auto-updater.test.ts @@ -34,8 +34,19 @@ vi.mock("./windows-update", () => ({ installWindowsUpdate: mocks.installWindowsUpdate, assertWindowsUpdateIdle: mocks.assertWindowsUpdateIdle, })); -// `reg query` finds no install, so the handover below is the same on every platform. -vi.mock("node:child_process", () => ({ spawnSync: vi.fn(() => ({ status: 1 })) })); +vi.mock("node:child_process", () => ({ spawnSync: vi.fn() })); + +function missingInstallQuery(_command: string, args?: readonly string[]) { + const root = args?.[1]?.startsWith("HKLM") ? "HKEY_LOCAL_MACHINE" : "HKEY_CURRENT_USER"; + return { + status: args?.includes("/v") ? 1 : 0, + stdout: `\r\n${root}\\Software\\OtherApp\r\n`, + } as ReturnType; +} + +beforeEach(() => { + vi.mocked(spawnSync).mockReset().mockImplementation(missingInstallQuery); +}); function state(overrides: Partial = {}): InstallReadiness { return { recording: false, inApplicationsFolder: true, platform: "linux", ...overrides }; @@ -192,7 +203,7 @@ describe("self-update flow", () => { await installSelfUpdate(); } finally { if (platform) Object.defineProperty(process, "platform", platform); - vi.mocked(spawnSync).mockReturnValue({ status: 1 } as ReturnType); + vi.mocked(spawnSync).mockImplementation(missingInstallQuery); } expect(mocks.autoUpdater.quitAndInstall).toHaveBeenCalledWith(false, true); expect(mocks.installWindowsUpdate).not.toHaveBeenCalled(); @@ -209,10 +220,11 @@ describe("self-update flow", () => { }); }); mocks.autoUpdater.downloadUpdate.mockResolvedValue(["verified-installer.exe"]); - vi.mocked(spawnSync) - .mockReturnValueOnce({ status: 0 } as ReturnType) - .mockReturnValueOnce({ status: 1 } as ReturnType) - .mockReturnValue({ status: 0 } as ReturnType); + vi.mocked(spawnSync).mockImplementation((command, args) => + args?.[1]?.startsWith("HKLM") + ? ({ status: 0 } as ReturnType) + : missingInstallQuery(command, args), + ); Object.defineProperty(process, "platform", { value: "win32" }); try { let settled = false; @@ -228,7 +240,7 @@ describe("self-update flow", () => { } finally { finish?.(); if (platform) Object.defineProperty(process, "platform", platform); - vi.mocked(spawnSync).mockReturnValue({ status: 1 } as ReturnType); + vi.mocked(spawnSync).mockImplementation(missingInstallQuery); } }); @@ -262,17 +274,55 @@ describe("self-update flow", () => { it.each(["HKLM", "HKCU"])("keeps the app open when the %s query fails", async (hive) => { const platform = Object.getOwnPropertyDescriptor(process, "platform"); Object.defineProperty(process, "platform", { value: "win32" }); - vi.mocked(spawnSync) - .mockReturnValueOnce({ status: hive === "HKLM" ? null : 0 } as ReturnType) - .mockReturnValueOnce({ status: hive === "HKCU" ? null : 1 } as ReturnType) - .mockReturnValue({ status: null } as ReturnType); + vi.mocked(spawnSync).mockImplementation((command, args) => + args?.[1]?.startsWith(hive) + ? ({ status: null } as ReturnType) + : missingInstallQuery(command, args), + ); try { await expect(installSelfUpdate()).rejects.toThrow( "Could not confirm the Windows installation type", ); } finally { if (platform) Object.defineProperty(process, "platform", platform); - vi.mocked(spawnSync).mockReturnValue({ status: 1 } as ReturnType); + vi.mocked(spawnSync).mockImplementation(missingInstallQuery); + } + expect(mocks.autoUpdater.quitAndInstall).not.toHaveBeenCalled(); + expect(mocks.installWindowsUpdate).not.toHaveBeenCalled(); + }); + + it.each([ + "HKLM", + "HKCU", + ])("does not mistake a failed %s value query for absence", async (hive) => { + const platform = Object.getOwnPropertyDescriptor(process, "platform"); + Object.defineProperty(process, "platform", { value: "win32" }); + vi.mocked(spawnSync).mockImplementation((command, args) => { + if (!args?.[1]?.startsWith(hive)) return missingInstallQuery(command, args); + if (args.includes("/v")) return { status: 1 } as ReturnType; + const root = hive === "HKLM" ? "HKEY_LOCAL_MACHINE" : "HKEY_CURRENT_USER"; + return { + status: 0, + stdout: `${root}\\${NSIS_INSTALL_KEY}\r\n`, + } as ReturnType; + }); + try { + await expect(installSelfUpdate()).rejects.toThrow("Could not confirm"); + } finally { + if (platform) Object.defineProperty(process, "platform", platform); + } + expect(mocks.autoUpdater.quitAndInstall).not.toHaveBeenCalled(); + expect(mocks.installWindowsUpdate).not.toHaveBeenCalled(); + }); + + it("keeps the app open when parent enumeration also fails", async () => { + const platform = Object.getOwnPropertyDescriptor(process, "platform"); + Object.defineProperty(process, "platform", { value: "win32" }); + vi.mocked(spawnSync).mockReturnValue({ status: 1 } as ReturnType); + try { + await expect(installSelfUpdate()).rejects.toThrow("Could not confirm"); + } finally { + if (platform) Object.defineProperty(process, "platform", platform); } expect(mocks.autoUpdater.quitAndInstall).not.toHaveBeenCalled(); expect(mocks.installWindowsUpdate).not.toHaveBeenCalled(); diff --git a/electron/auto-updater.ts b/electron/auto-updater.ts index 8b69b4f71..684c5398d 100644 --- a/electron/auto-updater.ts +++ b/electron/auto-updater.ts @@ -93,11 +93,22 @@ export const NSIS_INSTALL_KEY = "Software\\5b412b41-8d8b-5faf-b557-de4fe681e5fc" function nsisInstallStatus(hive: "HKLM" | "HKCU"): number | null { const key = `${hive}\\${NSIS_INSTALL_KEY}`; const args = ["query", key, "/v", "InstallLocation", "/reg:64"]; - return spawnSync("reg", args, { windowsHide: true }).status; + const { status } = spawnSync("reg", args, { windowsHide: true }); + if (status !== 1) return status; + // reg's exit 1 means any failure, including access denied. Confirm absence by + // successfully enumerating the parent, without parsing localized error text. + const parent = spawnSync("reg", ["query", `${hive}\\Software`, "/reg:64"], { + windowsHide: true, + encoding: "utf8", + }); + if (parent.status !== 0) return null; + const root = hive === "HKLM" ? "HKEY_LOCAL_MACHINE" : "HKEY_CURRENT_USER"; + const keys = parent.stdout.split(/\r?\n/).map((line) => line.trim().toLowerCase()); + return keys.includes(`${root}\\${NSIS_INSTALL_KEY}`.toLowerCase()) ? null : 1; } function hasNsisInstall(hive: "HKLM" | "HKCU"): boolean { - // Exit 1 is reg's "key not found". Anything else, reg itself failing to run included, counts + // Status 1 is returned only after confirming absence. Any uncertain result counts // as present, so this selector never treats an uncertain copy as absent. The installer // coordinator below refuses failed queries rather than quitting with an unknown target. return nsisInstallStatus(hive) !== 1; diff --git a/src/hooks/recorderHandle.test.ts b/src/hooks/recorderHandle.test.ts index 59f731785..c3bf1a33a 100644 --- a/src/hooks/recorderHandle.test.ts +++ b/src/hooks/recorderHandle.test.ts @@ -245,22 +245,33 @@ describe("createRecorderHandle", () => { expect(closeRecordingStream).toHaveBeenCalledWith("rec.webm"); }); - it("discard waits for a pending open before closing its stream", async () => { + it("discard drains a write queued during a pending open before closing its stream", async () => { let finishOpen: ((result: { success: boolean }) => void) | undefined; const open = new Promise<{ success: boolean }>((resolve) => { finishOpen = resolve; }); + let finishAppend: (() => void) | undefined; + const append = new Promise((resolve) => { + finishAppend = resolve; + }); const closeRecordingStream = vi.fn(async () => ({ success: true })); stubElectronAPI({ openRecordingStream: vi.fn(() => open), - appendRecordingChunk: vi.fn(async () => ({ success: true })), + appendRecordingChunk: vi.fn(async () => { + await append; + return { success: true }; + }), closeRecordingStream, }); const handle = createRecorderHandle({} as MediaStream, { mimeType: "video/webm" }, "rec.webm"); + driver(handle).emit(new Blob(["pending chunk"])); const discarded = handle.discard(); await tick(); expect(closeRecordingStream).not.toHaveBeenCalled(); finishOpen?.({ success: true }); + await tick(); + expect(closeRecordingStream).not.toHaveBeenCalled(); + finishAppend?.(); await discarded; expect(closeRecordingStream).toHaveBeenCalledWith("rec.webm"); driver(handle).stop(); diff --git a/src/hooks/useScreenRecorder.restartFailure.test.tsx b/src/hooks/useScreenRecorder.restartFailure.test.tsx index 00293717c..564936f41 100644 --- a/src/hooks/useScreenRecorder.restartFailure.test.tsx +++ b/src/hooks/useScreenRecorder.restartFailure.test.tsx @@ -193,7 +193,7 @@ describe("browser take start reservation", () => { ); }); - it("stops and discards a handle retained when later recorder setup fails", async () => { + it("stops a retained handle and waits for its blob before discarding failed startup", async () => { stubBrowserCapture(); const recorder = Object.assign(new EventTarget(), { state: "recording" as RecordingState, @@ -204,9 +204,12 @@ describe("browser take start reservation", () => { vi.spyOn(recorder, "addEventListener").mockImplementationOnce(() => { throw new Error("later setup failed"); }); + let finishBlob: ((blob: Blob) => void) | undefined; const failed = { recorder: recorder as unknown as MediaRecorder, - recordedBlobPromise: Promise.resolve(new Blob()), + recordedBlobPromise: new Promise((resolve) => { + finishBlob = resolve; + }), isStreaming: () => true, discard: vi.fn(async () => undefined), }; @@ -217,6 +220,9 @@ describe("browser take start reservation", () => { await view.result.current.startRecordingImmediately(); }); expect(recorder.stop).toHaveBeenCalledOnce(); + expect(failed.discard).not.toHaveBeenCalled(); + finishBlob?.(new Blob()); + await settle(); expect(failed.discard).toHaveBeenCalledOnce(); expect(api.setRecordingState).toHaveBeenCalledWith(false); expect(view.result.current.recording).toBe(false); From f0bc69c5c871092fe2b012c98e42ab2010605f3d Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Mon, 2 Nov 2026 11:42:54 +0100 Subject: [PATCH 8/8] fix(updater): allow larger registry enumeration output --- electron/auto-updater.ts | 1 + src/hooks/recorderHandle.test.ts | 10 ++++++---- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/electron/auto-updater.ts b/electron/auto-updater.ts index 684c5398d..41a0f87b1 100644 --- a/electron/auto-updater.ts +++ b/electron/auto-updater.ts @@ -100,6 +100,7 @@ function nsisInstallStatus(hive: "HKLM" | "HKCU"): number | null { const parent = spawnSync("reg", ["query", `${hive}\\Software`, "/reg:64"], { windowsHide: true, encoding: "utf8", + maxBuffer: 16 * 1024 * 1024, }); if (parent.status !== 0) return null; const root = hive === "HKLM" ? "HKEY_LOCAL_MACHINE" : "HKEY_CURRENT_USER"; diff --git a/src/hooks/recorderHandle.test.ts b/src/hooks/recorderHandle.test.ts index c3bf1a33a..8fb7b8105 100644 --- a/src/hooks/recorderHandle.test.ts +++ b/src/hooks/recorderHandle.test.ts @@ -255,12 +255,13 @@ describe("createRecorderHandle", () => { finishAppend = resolve; }); const closeRecordingStream = vi.fn(async () => ({ success: true })); + const appendRecordingChunk = vi.fn(async () => { + await append; + return { success: true }; + }); stubElectronAPI({ openRecordingStream: vi.fn(() => open), - appendRecordingChunk: vi.fn(async () => { - await append; - return { success: true }; - }), + appendRecordingChunk, closeRecordingStream, }); const handle = createRecorderHandle({} as MediaStream, { mimeType: "video/webm" }, "rec.webm"); @@ -274,6 +275,7 @@ describe("createRecorderHandle", () => { finishAppend?.(); await discarded; expect(closeRecordingStream).toHaveBeenCalledWith("rec.webm"); + expect(appendRecordingChunk).toHaveBeenCalledWith("rec.webm", expect.anything()); driver(handle).stop(); await handle.recordedBlobPromise; });