Cleanup final

docs/refactor/FINAL_SUMMARY.md

@@ -0,0 +1,97 @@
+# Final Cleanup Summary
+
+## Scope
+
+Executed the final global cleanup pass using `docs/refactor` as the source of truth.
+
+Source documents reviewed before code changes:
+
+- `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`
+- `docs/refactor/PHASE_1_SUMMARY.md`
+- `docs/refactor/PHASE_1_FIX_SUMMARY.md`
+- `docs/refactor/PHASE_2_SUMMARY.md`
+- `docs/refactor/PHASE_3_SUMMARY.md`
+- `docs/refactor/PHASE_4_SUMMARY.md`
+
+## Cleanup Completed
+
+- Removed the unused `parseEnvInt` helper and its tests.
+- Consolidated duplicated `unknown` parsing helpers into `src/main/utils/unknown-values.ts`.
+- Narrowed `ApplicationController` path typing to reuse `ApplicationPaths`.
+- Narrowed update config module boundaries so updater settings only accept the runtime config fields they need.
+- Narrowed the NodeCG process manager child-process contract to the actual process surface it uses.
+- Removed `as unknown as ChildProcess` casts from process-manager tests.
+- Fixed update dialog Spanish text encoding.
+- Added an explicit return type to the update dialog message-box helper.
+- Renamed legacy updater test files:
+  - `update-settings.test.ts` -> `update-config.test.ts`
+  - `update-utils.test.ts` -> `update-schema.test.ts`
+
+## Architecture Preserved
+
+- No UX changes.
+- No new features.
+- No renderer, preload, or IPC layer.
+- No NodeCG runtime model changes.
+- No Electron permission expansion.
+- No broad framework or new lifecycle layer.
+- BrowserWindow security posture remains explicit.
+- Update validation and download boundaries remain separated.
+- Managed runtime preservation of `cfg`, `db`, and `logs` remains unchanged.
+
+## Verification
+
+Passed:
+
+```text
+npm.cmd run typecheck
+npm.cmd exec -- tsc --noEmit --noUnusedLocals --noUnusedParameters
+npm.cmd test
+npm.cmd run lint
+npm.cmd run build
+npm.cmd run doctor
+```
+
+Test result:
+
+```text
+63 tests passing
+```
+
+Sanity searches passed for production/test source:
+
+```text
+rg -n "parseEnvInt\(|ActualizaciÃ|estÃ|versiÃ|cerrarÃ|update-utils|update-settings|\bany\b|unknown as|as unknown|@ts-ignore|@ts-expect-error|eslint-disable|nodeIntegration:\s*true|webSecurity:\s*false" src scripts
+```
+
+Result:
+
+- No `any` in `src` or `scripts`.
+- No `as unknown` casts remain.
+- No legacy updater module names remain in `src`.
+- No Spanish mojibake remains in update dialog source.
+- No unsafe Electron settings were introduced.
+
+IPC/preload sanity:
+
+```text
+rg -n "ipcMain|ipcRenderer|contextBridge|preload" src scripts
+```
+
+Result:
+
+- Matches are limited to the regression test that guards the no-IPC/no-preload policy.
+
+## Build Notes
+
+The first non-escalated `npm.cmd run build` attempt was blocked by sandbox permissions while creating generated parent-repo output at:
+
+```text
+C:\Users\pcantos\Documents\scoreko-dev\shared\dist
+```
+
+The escalated rerun passed. The build emitted existing dependency/deprecation warnings during runtime dependency installation, but completed successfully and `npm.cmd run doctor` validated the prepared runtime.

src/main/app/application-controller.ts

@@ -2,6 +2,7 @@ import { AppRuntimeConfig } from "../config/runtime-config";
 import { NodecgProcessManager } from "../nodecg/process-manager";
 import { PreparedNodecgRuntime } from "../nodecg/runtime-provisioner";
 import { getRemainingDelayMs } from "../utils/timing";
+import { ApplicationPaths } from "./paths";
 import { createShutdownService, ShutdownService } from "./shutdown-service";
 
 export type ApplicationState = "idle" | "preparing" | "starting" | "ready" | "stopping" | "stopped" | "failed";
@@ -21,14 +22,7 @@ export type ApplicationControllerConfig = {
   appVersion: string;
   isPackaged: boolean;
   isWindows: boolean;
-  paths: {
+  paths: ApplicationPaths;
-    rootPath: string;
-    sourceNodecgRuntimePath: string;
-    userDataPath: string;
-    nodecgBaseUrl: string;
-    mainDashboardUrl: string;
-    loadingDashboardUrl: string;
-  };
   deps: {
     createLoadingWindow: () => ApplicationWindow;
     createMainWindow: () => ApplicationWindow;

src/main/config/runtime-config.ts

@@ -53,16 +53,6 @@ export function getEnv(name: string, fallback: string): string {
   return getOptionalEnv(name) ?? fallback;
 }
 
-export function parseEnvInt(name: string, fallback: number): number {
-  const rawValue = process.env[name];
-  if (!rawValue) {
-    return fallback;
-  }
-
-  const parsedValue = Number.parseInt(rawValue, 10);
-  return Number.isFinite(parsedValue) ? parsedValue : fallback;
-}
-
 export function parseEnvIntInRange(name: string, fallback: number, min: number, max: number): number {
   // We throw here instead of silently coercing to avoid hidden misconfiguration in production.
   const rawValue = process.env[name];

src/main/nodecg/platform-process-killer.ts

@@ -1,12 +1,16 @@
-import { ChildProcess, SpawnOptions } from "node:child_process";
+import { SpawnOptions } from "node:child_process";
 
 export type PlatformProcessKillerDeps = {
   platform: NodeJS.Platform;
-  spawnProcess: (command: string, args: string[], options: SpawnOptions) => ChildProcess;
+  spawnProcess: (command: string, args: string[], options: SpawnOptions) => SpawnedKillerProcess;
   killProcess: (pid: number, signal: NodeJS.Signals) => void;
   log: (...args: unknown[]) => void;
 };
 
+type SpawnedKillerProcess = {
+  on: (event: "error", listener: (error: Error) => void) => unknown;
+};
+
 export function killProcessTree(pid: number, signal: NodeJS.Signals, deps: PlatformProcessKillerDeps): boolean {
   if (!Number.isSafeInteger(pid) || pid <= 0) {
     deps.log(`Invalid pid for process tree termination: ${pid}`);

src/main/nodecg/process-manager.ts

@@ -1,4 +1,4 @@
-import { ChildProcess, spawn, SpawnOptions } from "node:child_process";
+import { spawn, SpawnOptions } from "node:child_process";
 import fs from "node:fs";
 import net from "node:net";
 import path from "node:path";
@@ -17,7 +17,7 @@ type NodecgProcessManagerConfig = {
 };
 
 type NodecgProcessManagerDeps = {
-  spawnProcess: (command: string, args: string[], options: SpawnOptions) => ChildProcess;
+  spawnProcess: (command: string, args: string[], options: SpawnOptions) => NodecgChildProcess;
   pathExists: (candidatePath: string) => boolean;
   fetchUrl: typeof fetch;
   platform: NodeJS.Platform;
@@ -31,6 +31,22 @@ type NodecgProcessManagerDeps = {
   hasReadWriteAccess: (candidatePath: string) => boolean;
 };
 
+type NodecgChildProcess = {
+  pid?: number;
+  killed: boolean;
+  exitCode: number | null;
+  signalCode: NodeJS.Signals | null;
+  stdout?: ProcessOutputStream | null;
+  stderr?: ProcessOutputStream | null;
+  on(event: "exit", listener: (code: number | null, signal: NodeJS.Signals | null) => void): unknown;
+  on(event: "error", listener: (error: Error) => void): unknown;
+  once(event: "exit", listener: () => void): unknown;
+};
+
+type ProcessOutputStream = {
+  on(event: "data", listener: (chunk: unknown) => void): unknown;
+};
+
 export type NodecgProcessManager = {
   startNodecgProcess: () => Promise<void>;
   waitForNodecgReady: (startTime: number) => Promise<void>;
@@ -50,7 +66,7 @@ export function createNodecgProcessManager({
 }: NodecgProcessManagerConfig): NodecgProcessManager {
   const resolvedDeps = resolveDeps(deps);
 
-  let nodecgProcess: ChildProcess | null = null;
+  let nodecgProcess: NodecgChildProcess | null = null;
   let nodecgState: NodecgProcessState = "idle";
   let startNodecgPromise: Promise<void> | null = null;
   let stopNodecgPromise: Promise<void> | null = null;

src/main/updates/update-config.ts

@@ -2,6 +2,7 @@ import fs from "node:fs";
 
 import { getDefaultUpdateConfigPath } from "../app/paths";
 import { AppRuntimeConfig } from "../config/runtime-config";
+import { isRecord, readNonEmptyString } from "../utils/unknown-values";
 import { UpdateFileConfig, validateHttpUrl } from "./update-schema";
 
 const DEFAULT_UPDATE_ASSET_PATTERN = "Scoreko-setup-.*\\.exe$";
@@ -17,8 +18,17 @@ type UpdateConfigOptions = {
   allowInsecureHttp: boolean;
 };
 
+type UpdateRuntimeConfig = Pick<
+  AppRuntimeConfig,
+  | "updateApiUrl"
+  | "updateAssetPattern"
+  | "updateConfigPathOverride"
+  | "updateReleasePageUrl"
+  | "updatesEnabled"
+>;
+
 export function loadUpdateSettings(
-  appConfig: AppRuntimeConfig,
+  appConfig: UpdateRuntimeConfig,
   rootPath: string,
   log: (...args: unknown[]) => void,
   options: UpdateConfigOptions = { allowInsecureHttp: true },
@@ -32,12 +42,12 @@ export function loadUpdateSettings(
     ...(apiUrl ? { apiUrl } : {}),
     ...(releasePageUrl ? { releasePageUrl } : {}),
     assetPattern:
-      appConfig.updateAssetPattern || readOptionalString(fileConfig.assetPattern) || DEFAULT_UPDATE_ASSET_PATTERN,
+      appConfig.updateAssetPattern || readNonEmptyString(fileConfig.assetPattern) || DEFAULT_UPDATE_ASSET_PATTERN,
   };
 }
 
 export function readUpdateFileConfig(
-  appConfig: AppRuntimeConfig,
+  appConfig: Pick<AppRuntimeConfig, "updateConfigPathOverride">,
   rootPath: string,
   log: (...args: unknown[]) => void,
 ): UpdateFileConfig {
@@ -70,18 +80,10 @@ function normalizeUpdateFileConfig(value: unknown): UpdateFileConfig {
 }
 
 function readOptionalHttpUrl(value: unknown, options: UpdateConfigOptions): string | undefined {
-  const rawValue = readOptionalString(value);
+  const rawValue = readNonEmptyString(value);
   if (!rawValue) {
     return undefined;
   }
 
   return validateHttpUrl(rawValue, options) ?? undefined;
 }
-
-function readOptionalString(value: unknown): string | undefined {
-  return typeof value === "string" && value.trim().length > 0 ? value.trim() : undefined;
-}
-
-function isRecord(value: unknown): value is Record<string, unknown> {
-  return typeof value === "object" && value !== null && !Array.isArray(value);
-}

src/main/updates/update-dialogs.ts

@@ -1,5 +1,5 @@
 import { BrowserWindow, dialog } from "electron";
-import type { MessageBoxOptions } from "electron";
+import type { MessageBoxOptions, MessageBoxReturnValue } from "electron";
 
 import { ReleaseUpdate } from "./update-schema";
 
@@ -41,6 +41,9 @@ export async function askToInstallUpdate(update: ReleaseUpdate, parentWindow: Br
   return result.response === 0;
 }
 
-function showMessageBox(parentWindow: BrowserWindow | null, options: MessageBoxOptions) {
+function showMessageBox(
+  parentWindow: BrowserWindow | null,
+  options: MessageBoxOptions,
+): Promise<MessageBoxReturnValue> {
   return parentWindow ? dialog.showMessageBox(parentWindow, options) : dialog.showMessageBox(options);
 }

src/main/updates/update-schema.ts

@@ -1,3 +1,5 @@
+import { isRecord, readNonEmptyString } from "../utils/unknown-values";
+
 export type GiteaReleaseAsset = {
   name: string;
   browserDownloadUrl: string;
@@ -47,11 +49,14 @@ export function parseGiteaRelease(value: unknown): GiteaRelease | null {
     return null;
   }
 
+  const title = readNonEmptyString(value.name);
+  const pageUrl = readOptionalUrlString(value.html_url);
+
   return {
     tagName,
     assets,
-    ...(readOptionalString(value.name) ? { title: readOptionalString(value.name) } : {}),
+    ...(title ? { title } : {}),
-    ...(readOptionalUrlString(value.html_url) ? { pageUrl: readOptionalUrlString(value.html_url) } : {}),
+    ...(pageUrl ? { pageUrl } : {}),
   };
 }
 
@@ -177,16 +182,12 @@ function normalizeVersion(version: string): number[] {
 }
 
 function readRequiredString(value: unknown): string | null {
-  const text = readOptionalString(value);
+  const text = readNonEmptyString(value);
   return text && text.length > 0 ? text : null;
 }
 
-function readOptionalString(value: unknown): string | undefined {
-  return typeof value === "string" && value.trim().length > 0 ? value.trim() : undefined;
-}
-
 function readOptionalUrlString(value: unknown): string | undefined {
-  const rawValue = readOptionalString(value);
+  const rawValue = readNonEmptyString(value);
   if (!rawValue) {
     return undefined;
   }
@@ -194,10 +195,6 @@ function readOptionalUrlString(value: unknown): string | undefined {
   return validateHttpUrl(rawValue, { allowInsecureHttp: true }) ?? undefined;
 }
 
-function isRecord(value: unknown): value is Record<string, unknown> {
-  return typeof value === "object" && value !== null && !Array.isArray(value);
-}
-
 function isPresent<T>(value: T | null): value is T {
   return value !== null;
 }

src/main/utils/unknown-values.ts

@@ -0,0 +1,7 @@
+export function isRecord(value: unknown): value is Record<string, unknown> {
+  return typeof value === "object" && value !== null && !Array.isArray(value);
+}
+
+export function readNonEmptyString(value: unknown): string | undefined {
+  return typeof value === "string" && value.trim().length > 0 ? value.trim() : undefined;
+}

src/tests/process-manager.test.ts

@@ -85,7 +85,7 @@ test("waitForNodeCGReady resolves when endpoint returns 404", async () => {
     deps: {
       platform: "linux",
       pathExists: () => true,
-      spawnProcess: () => child as unknown as import("node:child_process").ChildProcess,
+      spawnProcess: () => child,
       fetchUrl: async () => ({ ok: false, status: 404 }) as Response,
       setTimer: (handler: (...args: unknown[]) => void, _timeoutMs: number) => {
         handler();
@@ -118,7 +118,7 @@ test("stopNodeCG sends SIGTERM and then SIGKILL if the process does not exit", a
     deps: {
       platform: "linux",
       pathExists: () => true,
-      spawnProcess: () => child as unknown as import("node:child_process").ChildProcess,
+      spawnProcess: () => child,
       fetchUrl: async () => ({ ok: false, status: 404 }) as Response,
       killProcess: (pid, signal) => {
         killSignals.push({ pid, signal });
@@ -163,7 +163,7 @@ test("stopNodeCG reuses the same promise when invoked in parallel", async () =>
     log: () => undefined,
     deps: {
       pathExists: () => true,
-      spawnProcess: () => child as unknown as import("node:child_process").ChildProcess,
+      spawnProcess: () => child,
       fetchUrl: async () => ({ ok: false, status: 404 }) as Response,
       killProcess: () => undefined,
       setTimer: () => 0,
@@ -206,7 +206,7 @@ test("startNodeCG reuses the same promise while startup is in progress", async (
         }),
       spawnProcess: () => {
         spawnCalls += 1;
-        return child as unknown as import("node:child_process").ChildProcess;
+        return child;
       },
       stdoutWrite: () => undefined,
       stderrWrite: () => undefined,
@@ -241,7 +241,7 @@ test("stopNodeCG normalizes negative timeout to zero", async () => {
     log: () => undefined,
     deps: {
       pathExists: () => true,
-      spawnProcess: () => child as unknown as import("node:child_process").ChildProcess,
+      spawnProcess: () => child,
       fetchUrl: async () => ({ ok: false, status: 404 }) as Response,
       killProcess: () => undefined,
       setTimer: (handler, timeoutMs) => {
@@ -306,7 +306,7 @@ test("startNodeCG spawns Electron directly on Windows", async () => {
         capturedCommand = command;
         capturedArgs = args;
         capturedOptions.push(options);
-        return child as unknown as import("node:child_process").ChildProcess;
+        return child;
       },
       stdoutWrite: () => undefined,
       stderrWrite: () => undefined,
@@ -333,7 +333,7 @@ test("waitForNodeCGReady exposes diagnostics when NodeCG exits before readiness"
     deps: {
       pathExists: () => true,
       platform: "linux",
-      spawnProcess: () => child as unknown as import("node:child_process").ChildProcess,
+      spawnProcess: () => child,
       fetchUrl: async () => {
         child.emit("exit", 1, null);
         throw new Error("still starting");

src/tests/runtime-config.test.ts

@@ -5,7 +5,6 @@ import {
   getEnv,
   getOptionalEnv,
   parseEnvBool,
-  parseEnvInt,
   parseEnvIntInRange,
   parseEnvPort,
   parseOptionalHttpUrl,
@@ -56,18 +55,6 @@ test("getEnv returns the value when present", () => {
   });
 });
 
-test("parseEnvInt returns fallback for invalid values", () => {
-  withEnv("TEST_ENV_INT", "abc", () => {
-    assert.equal(parseEnvInt("TEST_ENV_INT", 100), 100);
-  });
-});
-
-test("parseEnvInt parses valid integers", () => {
-  withEnv("TEST_ENV_INT", "4500", () => {
-    assert.equal(parseEnvInt("TEST_ENV_INT", 100), 4500);
-  });
-});
-
 test("parseEnvIntInRange hard-fails for out-of-range values", () => {
   withEnv("TEST_ENV_INT_RANGE", "999", () => {
     assert.throws(() => parseEnvIntInRange("TEST_ENV_INT_RANGE", 100, 0, 100), /must be an integer/);

src/tests/update-config.test.ts

@@ -80,7 +80,7 @@ test("readUpdateFileConfig normalizes malformed config into an empty file config
 });
 
 test("readUpdateFileConfig logs invalid JSON and returns an empty file config", () => {
-  const rootPath = fs.mkdtempSync(path.join(os.tmpdir(), "scoreko-update-settings-"));
+  const rootPath = fs.mkdtempSync(path.join(os.tmpdir(), "scoreko-update-config-"));
   const staticPath = path.join(rootPath, "static");
   fs.mkdirSync(staticPath, { recursive: true });
   fs.writeFileSync(path.join(staticPath, "updates.json"), "{ invalid", "utf8");
@@ -95,7 +95,7 @@ test("readUpdateFileConfig logs invalid JSON and returns an empty file config",
 });
 
 function makeTempRoot(config: unknown): string {
-  const rootPath = fs.mkdtempSync(path.join(os.tmpdir(), "scoreko-update-settings-"));
+  const rootPath = fs.mkdtempSync(path.join(os.tmpdir(), "scoreko-update-config-"));
   const staticPath = path.join(rootPath, "static");
 
   fs.mkdirSync(staticPath, { recursive: true });