From 2158d26bbbf53d546b4cda8241d2481208d0e7f0 Mon Sep 17 00:00:00 2001 From: "dayuan.jiang" Date: Fri, 3 Apr 2026 17:59:57 +0900 Subject: [PATCH] fix: address PR review feedback for auto-update - Register manual check listeners before checkForUpdates() to avoid race - Strip prerelease/build metadata in version comparison - Add res.setEncoding("utf8") for GitHub API response - Stream file hashing in CI script instead of reading into memory - Add directory existence checks in fix-latest-yml.mjs - Define UpdateStatus type locally in preload to avoid tsconfig scope issues - Show error dialog for manual check failures (parse/network errors) --- electron/main/auto-updater.ts | 19 +++++++++++-------- electron/preload/index.ts | 12 ++++++++++-- scripts/fix-latest-yml.mjs | 35 ++++++++++++++++++++++++++++++++--- 3 files changed, 53 insertions(+), 13 deletions(-) diff --git a/electron/main/auto-updater.ts b/electron/main/auto-updater.ts index 862e6b9..054c9d5 100644 --- a/electron/main/auto-updater.ts +++ b/electron/main/auto-updater.ts @@ -28,10 +28,12 @@ function supportsAutoUpdate(): boolean { /** * Compare two semver-like version strings numerically. * Returns true if remote > local. + * Strips prerelease/build metadata (e.g., "1.2.3-beta.1" → "1.2.3"). */ function isNewerVersion(remote: string, local: string): boolean { - const r = remote.replace(/^v/, "").split(".").map(Number) - const l = local.replace(/^v/, "").split(".").map(Number) + const strip = (v: string) => v.replace(/^v/, "").split("-")[0].split("+")[0] + const r = strip(remote).split(".").map(Number) + const l = strip(local).split(".").map(Number) const len = Math.max(r.length, l.length) for (let i = 0; i < len; i++) { const rv = r[i] || 0 @@ -77,6 +79,7 @@ function checkGitHubRelease(manual: boolean) { return } + res.setEncoding("utf8") let body = "" res.on("data", (chunk: string) => { body += chunk @@ -211,11 +214,8 @@ function doCheck(manual: boolean) { } if (supportsAutoUpdate()) { - autoUpdater.checkForUpdates().catch((err) => { - console.error("checkForUpdates failed:", err) - isChecking = false - }) - // For manual check, show "up to date" if no update found + // For manual check, register listeners BEFORE triggering check + // to avoid race where event fires before listeners are attached if (manual) { const onNotAvailable = () => { dialog.showMessageBox({ @@ -226,7 +226,6 @@ function doCheck(manual: boolean) { }) cleanup() } - // Clean up listeners when either event fires, preventing stale listeners const onAvailable = () => cleanup() const onError = () => cleanup() const cleanup = () => { @@ -238,6 +237,10 @@ function doCheck(manual: boolean) { autoUpdater.once("update-available", onAvailable) autoUpdater.once("error", onError) } + autoUpdater.checkForUpdates().catch((err) => { + console.error("checkForUpdates failed:", err) + isChecking = false + }) } else { checkGitHubRelease(manual) } diff --git a/electron/preload/index.ts b/electron/preload/index.ts index cdb2010..1363c9c 100644 --- a/electron/preload/index.ts +++ b/electron/preload/index.ts @@ -1,5 +1,13 @@ import { contextBridge, type IpcRendererEvent, ipcRenderer } from "electron" +// Locally defined to avoid dependency on electron.d.ts compilation scope +type UpdateStatus = + | { status: "available"; version: string } + | { status: "available-manual"; version: string; url: string } + | { status: "downloading"; percent: number } + | { status: "downloaded" } + | { status: "error"; message: string } + /** * Expose safe APIs to the renderer process */ @@ -33,8 +41,8 @@ contextBridge.exposeInMainWorld("electronAPI", { ipcRenderer.invoke("set-user-locale", locale), // Auto-update - onUpdateStatus: (callback: (data: UpdateStatusData) => void) => { - const handler = (_event: IpcRendererEvent, data: UpdateStatusData) => + onUpdateStatus: (callback: (data: UpdateStatus) => void) => { + const handler = (_event: IpcRendererEvent, data: UpdateStatus) => callback(data) ipcRenderer.on("update-status", handler) return () => ipcRenderer.removeListener("update-status", handler) diff --git a/scripts/fix-latest-yml.mjs b/scripts/fix-latest-yml.mjs index d39d250..3cb6d83 100644 --- a/scripts/fix-latest-yml.mjs +++ b/scripts/fix-latest-yml.mjs @@ -10,12 +10,29 @@ */ import { createHash } from "node:crypto" -import { readdirSync, readFileSync, statSync, writeFileSync } from "node:fs" +import { + createReadStream, + existsSync, + readdirSync, + readFileSync, + statSync, + writeFileSync, +} from "node:fs" import { join } from "node:path" const RELEASE_DIR = "release" const SIGNED_DIR = "release-signed" +// Verify directories exist +if (!existsSync(RELEASE_DIR)) { + console.error(`Error: ${RELEASE_DIR}/ directory does not exist`) + process.exit(1) +} +if (!existsSync(SIGNED_DIR)) { + console.error(`Error: ${SIGNED_DIR}/ directory does not exist`) + process.exit(1) +} + // Find all latest*.yml files const ymlFiles = readdirSync(RELEASE_DIR).filter( (f) => f.startsWith("latest") && f.endsWith(".yml"), @@ -26,13 +43,25 @@ if (ymlFiles.length === 0) { process.exit(0) } +/** + * Compute SHA-512 hash of a file using streaming (avoids loading large exe into memory) + */ +function hashFile(filePath) { + return new Promise((resolve, reject) => { + const hash = createHash("sha512") + const stream = createReadStream(filePath) + stream.on("data", (chunk) => hash.update(chunk)) + stream.on("end", () => resolve(hash.digest("base64"))) + stream.on("error", reject) + }) +} + // Build a map of signed exe filenames to their hash and size const signedFiles = new Map() for (const f of readdirSync(SIGNED_DIR)) { if (!f.endsWith(".exe")) continue const filePath = join(SIGNED_DIR, f) - const buffer = readFileSync(filePath) - const sha512 = createHash("sha512").update(buffer).digest("base64") + const sha512 = await hashFile(filePath) const size = statSync(filePath).size signedFiles.set(f, { sha512, size }) }