From c8e2edc0c004df311242deebe1f7fc973b4748df Mon Sep 17 00:00:00 2001 From: Pandipipas Date: Sun, 24 May 2026 22:31:18 +0200 Subject: [PATCH] feat: Implement update management refactor with new dialog and settings handling --- docs/refactor/PHASE_3_SUMMARY.md | 107 +++++++++++++++++++++ src/main/updates/update-dialogs.ts | 46 ++++++++++ src/main/updates/update-download.ts | 34 +++++++ src/main/updates/update-manager.ts | 138 ++++------------------------ src/main/updates/update-settings.ts | 71 ++++++++++++++ src/tests/update-settings.test.ts | 93 +++++++++++++++++++ 6 files changed, 368 insertions(+), 121 deletions(-) create mode 100644 docs/refactor/PHASE_3_SUMMARY.md create mode 100644 src/main/updates/update-dialogs.ts create mode 100644 src/main/updates/update-download.ts create mode 100644 src/main/updates/update-settings.ts create mode 100644 src/tests/update-settings.test.ts diff --git a/docs/refactor/PHASE_3_SUMMARY.md b/docs/refactor/PHASE_3_SUMMARY.md new file mode 100644 index 0000000..fc51f3d --- /dev/null +++ b/docs/refactor/PHASE_3_SUMMARY.md @@ -0,0 +1,107 @@ +# Phase 3 Summary + +## Scope + +Executed the UI and settings cleanup phase only for the Electron package. + +Documentation used as source of truth: + +- `docs/refactor/ARCHITECTURE_AUDIT.md` +- `docs/refactor/ARCHITECTURE_RULES.md` +- `docs/refactor/TARGET_ARCHITECTURE.md` +- `docs/refactor/MIGRATION_PLAN.md` +- `docs/refactor/SESSION_HANDOFF.md` + +## Changes Made + +- Split update dialog UI out of `src/main/updates/update-manager.ts` into `src/main/updates/update-dialogs.ts`. +- Split update settings loading and file-config normalization into `src/main/updates/update-settings.ts`. +- Split installer download behavior into `src/main/updates/update-download.ts`. +- Kept `src/main/updates/update-manager.ts` focused on orchestration: + - load settings + - fetch latest release + - ask the user what to do + - download installer + - run install handoff +- Added defensive update config parsing from `unknown` JSON without introducing `any`. +- Added settings tests covering: + - runtime config disabling updates + - runtime config overriding file settings + - malformed update config normalization + - invalid JSON fallback and logging +- Fixed the existing Spanish mojibake in update dialogs touched by this phase. + +## Intentionally Not Changed + +- No UX flow changes. +- No new features. +- No custom renderer was added. +- No preload was added. +- No IPC was added. +- No parent bundle source was modified. +- No generated `dist` or `lib` source was edited manually. +- No forms or controls were changed in the NodeCG dashboard. + +## Verification + +Commands run successfully: + +```text +npm run typecheck +npm test +npm run lint +``` + +Current test result: + +```text +59 tests passing +``` + +Sanity searches: + +```text +rg -n "\bany\b" src/main src/tests +rg -n "ActualizaciÃ|estÃ|versiÃ|cerrarÃ" src/main src/tests +rg -n "ipcMain|ipcRenderer|contextBridge|preload|nodeIntegration:\s*true|webSecurity:\s*false" src/main src/tests +``` + +Result: + +- No `any` was introduced. +- No touched Spanish update-dialog text remains mojibaked. +- No production IPC or preload surface exists. +- No unsafe Electron window settings were introduced. +- Remaining IPC/preload matches are limited to the regression test that guards the zero-surface policy. + +## UI Verification + +The Electron launch path prepared a temporary managed runtime, but the NodeCG child did not expose port `9090` within the verification window. To verify the served UI without touching the user's real runtime data, NodeCG was launched from a temporary Electron `userData` directory: + +```text +SCOREKO_APP_USER_DATA_DIRECTORY=scoreko-codex-ui-check +SCOREKO_UPDATES_ENABLED=false +ELECTRON_LOAD_DELAY_MS=0 +``` + +The temporary NodeCG runtime served: + +```text +http://127.0.0.1:9090 -> 200 OK +``` + +Browser verification loaded: + +```text +http://localhost:9090/bundles/scoreko-dev/dashboard/scoreko-dev/main.html?standalone=true#/ +``` + +Observed UI signals: + +- Page title: `Dashboard` +- Scoreko sidebar rendered. +- Main navigation rendered. +- `Settings` navigation entry rendered. +- Dashboard form controls rendered. + +The temporary NodeCG process was stopped after verification. diff --git a/src/main/updates/update-dialogs.ts b/src/main/updates/update-dialogs.ts new file mode 100644 index 0000000..75d4549 --- /dev/null +++ b/src/main/updates/update-dialogs.ts @@ -0,0 +1,46 @@ +import { BrowserWindow, dialog } from "electron"; +import type { MessageBoxOptions } from "electron"; + +import { ReleaseUpdate } from "./update-utils"; + +export type DownloadUpdateChoice = "download" | "open-release" | "dismiss"; + +export async function askToDownloadUpdate( + update: ReleaseUpdate, + releasePageUrl: string | undefined, + parentWindow: BrowserWindow | null, +): Promise { + const result = await showMessageBox(parentWindow, { + type: "info", + title: "Actualización disponible", + message: `Scoreko ${update.version} está disponible.`, + detail: "Puedes descargarla ahora o seguir usando esta versión.", + buttons: releasePageUrl ? ["Descargar", "Ver release", "Ahora no"] : ["Descargar", "Ahora no"], + defaultId: 0, + cancelId: releasePageUrl ? 2 : 1, + }); + + if (releasePageUrl && result.response === 1) { + return "open-release"; + } + + return result.response === 0 ? "download" : "dismiss"; +} + +export async function askToInstallUpdate(update: ReleaseUpdate, parentWindow: BrowserWindow | null): Promise { + const result = await showMessageBox(parentWindow, { + type: "question", + title: "Actualización descargada", + message: `Scoreko ${update.version} se ha descargado.`, + detail: "Para instalarla se cerrará Scoreko y se abrirá el instalador.", + buttons: ["Instalar y cerrar", "Luego"], + defaultId: 0, + cancelId: 1, + }); + + return result.response === 0; +} + +function showMessageBox(parentWindow: BrowserWindow | null, options: MessageBoxOptions) { + return parentWindow ? dialog.showMessageBox(parentWindow, options) : dialog.showMessageBox(options); +} diff --git a/src/main/updates/update-download.ts b/src/main/updates/update-download.ts new file mode 100644 index 0000000..54d3511 --- /dev/null +++ b/src/main/updates/update-download.ts @@ -0,0 +1,34 @@ +import fs from "node:fs"; +import path from "node:path"; +import { Readable } from "node:stream"; + +import { ReleaseUpdate, sanitizeFileName } from "./update-utils"; + +type UpdateDownloadConfig = { + tempDirectory: string; +}; + +export async function downloadInstaller(update: ReleaseUpdate, config: UpdateDownloadConfig): Promise { + const safeFileName = sanitizeFileName(update.installer.name); + const downloadDirectory = path.join(config.tempDirectory, "scoreko-updates"); + const targetPath = path.join(downloadDirectory, safeFileName); + + fs.mkdirSync(downloadDirectory, { recursive: true }); + + const response = await fetch(update.installer.downloadUrl); + if (!response.ok || !response.body) { + throw new Error(`Could not download update installer. HTTP ${response.status}.`); + } + + await new Promise((resolve, reject) => { + const fileStream = fs.createWriteStream(targetPath); + const responseStream = Readable.fromWeb(response.body as Parameters[0]); + + responseStream.on("error", reject); + fileStream.on("error", reject); + fileStream.on("finish", resolve); + responseStream.pipe(fileStream); + }); + + return targetPath; +} diff --git a/src/main/updates/update-manager.ts b/src/main/updates/update-manager.ts index f10cce1..9aafaf7 100644 --- a/src/main/updates/update-manager.ts +++ b/src/main/updates/update-manager.ts @@ -1,11 +1,10 @@ -import { app, BrowserWindow, dialog, shell } from "electron"; -import type { MessageBoxOptions } from "electron"; -import fs from "node:fs"; -import path from "node:path"; -import { Readable } from "node:stream"; +import { app, BrowserWindow, shell } from "electron"; import { AppRuntimeConfig } from "../config/runtime-config"; -import { buildReleaseUpdate, GiteaRelease, ReleaseUpdate, sanitizeFileName, UpdateFileConfig } from "./update-utils"; +import { askToDownloadUpdate, askToInstallUpdate } from "./update-dialogs"; +import { downloadInstaller } from "./update-download"; +import { loadUpdateSettings, UpdateSettings } from "./update-settings"; +import { buildReleaseUpdate, GiteaRelease } from "./update-utils"; type UpdateManagerConfig = { appConfig: AppRuntimeConfig; @@ -15,13 +14,6 @@ type UpdateManagerConfig = { log: (...args: unknown[]) => void; }; -type UpdateSettings = { - enabled: boolean; - apiUrl?: string; - releasePageUrl?: string; - assetPattern: string; -}; - export function scheduleUpdateCheck({ appConfig, rootPath, @@ -65,16 +57,19 @@ async function checkForUpdates({ return; } - const shouldDownload = await askToDownloadUpdate( - update, - settings.releasePageUrl ?? update.pageUrl, - getParentWindow(), - ); - if (!shouldDownload) { + const releasePageUrl = settings.releasePageUrl ?? update.pageUrl; + const downloadChoice = await askToDownloadUpdate(update, releasePageUrl, getParentWindow()); + + if (downloadChoice === "open-release") { + await openReleasePage(releasePageUrl); return; } - const installerPath = await downloadInstaller(update); + if (downloadChoice !== "download") { + return; + } + + const installerPath = await downloadInstaller(update, { tempDirectory: app.getPath("temp") }); const shouldInstall = await askToInstallUpdate(update, getParentWindow()); if (!shouldInstall) { await shell.showItemInFolder(installerPath); @@ -93,41 +88,6 @@ async function checkForUpdates({ } } -function loadUpdateSettings( - appConfig: AppRuntimeConfig, - rootPath: string, - log: (...args: unknown[]) => void, -): UpdateSettings { - const fileConfig = readUpdateFileConfig(appConfig, rootPath, log); - - return { - enabled: appConfig.updatesEnabled && (Boolean(fileConfig.enabled) || Boolean(appConfig.updateApiUrl)), - apiUrl: appConfig.updateApiUrl ?? readOptionalString(fileConfig.apiUrl), - releasePageUrl: appConfig.updateReleasePageUrl ?? readOptionalString(fileConfig.releasePageUrl), - assetPattern: - appConfig.updateAssetPattern || readOptionalString(fileConfig.assetPattern) || "Scoreko-setup-.*\\.exe$", - }; -} - -function readUpdateFileConfig( - appConfig: AppRuntimeConfig, - rootPath: string, - log: (...args: unknown[]) => void, -): UpdateFileConfig { - const configPath = appConfig.updateConfigPathOverride ?? path.join(rootPath, "static", "updates.json"); - - if (!fs.existsSync(configPath)) { - return {}; - } - - try { - return JSON.parse(fs.readFileSync(configPath, "utf8")) as UpdateFileConfig; - } catch (error) { - log(`Could not read update config at ${configPath}.`, error); - return {}; - } -} - async function fetchLatestRelease(apiUrl: string): Promise { const response = await fetch(apiUrl, { headers: { @@ -142,72 +102,8 @@ async function fetchLatestRelease(apiUrl: string): Promise { return (await response.json()) as GiteaRelease; } -async function askToDownloadUpdate( - update: ReleaseUpdate, - releasePageUrl: string | undefined, - parentWindow: BrowserWindow | null, -): Promise { - const result = await showMessageBox(parentWindow, { - type: "info", - title: "Actualización disponible", - message: `Scoreko ${update.version} está disponible.`, - detail: "Puedes descargarla ahora o seguir usando esta versión.", - buttons: releasePageUrl ? ["Descargar", "Ver release", "Ahora no"] : ["Descargar", "Ahora no"], - defaultId: 0, - cancelId: releasePageUrl ? 2 : 1, - }); - - if (releasePageUrl && result.response === 1) { +async function openReleasePage(releasePageUrl: string | undefined): Promise { + if (releasePageUrl) { await shell.openExternal(releasePageUrl); - return false; } - - return result.response === 0; -} - -async function askToInstallUpdate(update: ReleaseUpdate, parentWindow: BrowserWindow | null): Promise { - const result = await showMessageBox(parentWindow, { - type: "question", - title: "Actualización descargada", - message: `Scoreko ${update.version} se ha descargado.`, - detail: "Para instalarla se cerrará Scoreko y se abrirá el instalador.", - buttons: ["Instalar y cerrar", "Luego"], - defaultId: 0, - cancelId: 1, - }); - - return result.response === 0; -} - -async function downloadInstaller(update: ReleaseUpdate): Promise { - const safeFileName = sanitizeFileName(update.installer.name); - const downloadDirectory = path.join(app.getPath("temp"), "scoreko-updates"); - const targetPath = path.join(downloadDirectory, safeFileName); - - fs.mkdirSync(downloadDirectory, { recursive: true }); - - const response = await fetch(update.installer.downloadUrl); - if (!response.ok || !response.body) { - throw new Error(`Could not download update installer. HTTP ${response.status}.`); - } - - await new Promise((resolve, reject) => { - const fileStream = fs.createWriteStream(targetPath); - const responseStream = Readable.fromWeb(response.body as Parameters[0]); - - responseStream.on("error", reject); - fileStream.on("error", reject); - fileStream.on("finish", resolve); - responseStream.pipe(fileStream); - }); - - return targetPath; -} - -function readOptionalString(value: unknown): string | undefined { - return typeof value === "string" && value.trim().length > 0 ? value.trim() : undefined; -} - -function showMessageBox(parentWindow: BrowserWindow | null, options: MessageBoxOptions) { - return parentWindow ? dialog.showMessageBox(parentWindow, options) : dialog.showMessageBox(options); } diff --git a/src/main/updates/update-settings.ts b/src/main/updates/update-settings.ts new file mode 100644 index 0000000..acbd2ac --- /dev/null +++ b/src/main/updates/update-settings.ts @@ -0,0 +1,71 @@ +import fs from "node:fs"; +import path from "node:path"; + +import { AppRuntimeConfig } from "../config/runtime-config"; +import { UpdateFileConfig } from "./update-utils"; + +const DEFAULT_UPDATE_ASSET_PATTERN = "Scoreko-setup-.*\\.exe$"; + +export type UpdateSettings = { + enabled: boolean; + apiUrl?: string; + releasePageUrl?: string; + assetPattern: string; +}; + +export function loadUpdateSettings( + appConfig: AppRuntimeConfig, + rootPath: string, + log: (...args: unknown[]) => void, +): UpdateSettings { + const fileConfig = readUpdateFileConfig(appConfig, rootPath, log); + + return { + enabled: appConfig.updatesEnabled && (Boolean(fileConfig.enabled) || Boolean(appConfig.updateApiUrl)), + apiUrl: appConfig.updateApiUrl ?? readOptionalString(fileConfig.apiUrl), + releasePageUrl: appConfig.updateReleasePageUrl ?? readOptionalString(fileConfig.releasePageUrl), + assetPattern: + appConfig.updateAssetPattern || readOptionalString(fileConfig.assetPattern) || DEFAULT_UPDATE_ASSET_PATTERN, + }; +} + +export function readUpdateFileConfig( + appConfig: AppRuntimeConfig, + rootPath: string, + log: (...args: unknown[]) => void, +): UpdateFileConfig { + const configPath = appConfig.updateConfigPathOverride ?? path.join(rootPath, "static", "updates.json"); + + if (!fs.existsSync(configPath)) { + return {}; + } + + try { + const parsedConfig: unknown = JSON.parse(fs.readFileSync(configPath, "utf8")); + return normalizeUpdateFileConfig(parsedConfig); + } catch (error) { + log(`Could not read update config at ${configPath}.`, error); + return {}; + } +} + +function normalizeUpdateFileConfig(value: unknown): UpdateFileConfig { + if (!isRecord(value)) { + return {}; + } + + return { + enabled: value.enabled, + apiUrl: value.apiUrl, + releasePageUrl: value.releasePageUrl, + assetPattern: value.assetPattern, + }; +} + +function readOptionalString(value: unknown): string | undefined { + return typeof value === "string" && value.trim().length > 0 ? value.trim() : undefined; +} + +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} diff --git a/src/tests/update-settings.test.ts b/src/tests/update-settings.test.ts new file mode 100644 index 0000000..49f9eec --- /dev/null +++ b/src/tests/update-settings.test.ts @@ -0,0 +1,93 @@ +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import test from "node:test"; + +import { AppRuntimeConfig } from "../main/config/runtime-config"; +import { loadUpdateSettings, readUpdateFileConfig } from "../main/updates/update-settings"; + +const baseConfig: AppRuntimeConfig = { + title: "Scoreko", + userModelId: "com.scoreko.desktop", + userDataDirectoryName: "scoreko", + nodecgPort: "9090", + bundleName: "scoreko-dev", + mainDashboardRoute: "dashboard/scoreko-dev/main.html?standalone=true", + loadingDashboardRoute: "dashboard/loading/main.html?standalone=true", + loadDelayMs: 0, + startupTimeoutMs: 30000, + nodecgKillTimeoutMs: 2500, + updatesEnabled: true, + updateCheckDelayMs: 5000, +}; + +test("loadUpdateSettings keeps updates disabled when the runtime config disables them", () => { + const rootPath = makeTempRoot({ + enabled: true, + apiUrl: "https://gitea.local/releases/latest", + }); + + const settings = loadUpdateSettings({ ...baseConfig, updatesEnabled: false }, rootPath, () => undefined); + + assert.equal(settings.enabled, false); + assert.equal(settings.apiUrl, "https://gitea.local/releases/latest"); +}); + +test("loadUpdateSettings lets runtime config override file settings", () => { + const rootPath = makeTempRoot({ + enabled: true, + apiUrl: "https://file.local/releases/latest", + releasePageUrl: "https://file.local/releases", + assetPattern: "File-.*\\.exe$", + }); + + const settings = loadUpdateSettings( + { + ...baseConfig, + updateApiUrl: "https://env.local/releases/latest", + updateReleasePageUrl: "https://env.local/releases", + updateAssetPattern: "Env-.*\\.exe$", + }, + rootPath, + () => undefined, + ); + + assert.deepEqual(settings, { + enabled: true, + apiUrl: "https://env.local/releases/latest", + releasePageUrl: "https://env.local/releases", + assetPattern: "Env-.*\\.exe$", + }); +}); + +test("readUpdateFileConfig normalizes malformed config into an empty file config", () => { + const rootPath = makeTempRoot(["not", "an", "object"]); + + assert.deepEqual(readUpdateFileConfig(baseConfig, rootPath, () => undefined), {}); +}); + +test("readUpdateFileConfig logs invalid JSON and returns an empty file config", () => { + const rootPath = fs.mkdtempSync(path.join(os.tmpdir(), "scoreko-update-settings-")); + const staticPath = path.join(rootPath, "static"); + fs.mkdirSync(staticPath, { recursive: true }); + fs.writeFileSync(path.join(staticPath, "updates.json"), "{ invalid", "utf8"); + const messages: unknown[][] = []; + + const settings = readUpdateFileConfig(baseConfig, rootPath, (...args: unknown[]) => { + messages.push(args); + }); + + assert.deepEqual(settings, {}); + assert.equal(messages.length, 1); +}); + +function makeTempRoot(config: unknown): string { + const rootPath = fs.mkdtempSync(path.join(os.tmpdir(), "scoreko-update-settings-")); + const staticPath = path.join(rootPath, "static"); + + fs.mkdirSync(staticPath, { recursive: true }); + fs.writeFileSync(path.join(staticPath, "updates.json"), JSON.stringify(config), "utf8"); + + return rootPath; +}