From f7c0d56c2a780c1bdfd0eaf3ad7a623b4ebb10e7 Mon Sep 17 00:00:00 2001 From: Kiet Ho Date: Tue, 21 Apr 2026 11:39:54 -0700 Subject: [PATCH 1/4] fix(desktop): keep v2 host-service alive across app updates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The host-service child was spawned without `detached: true`, so it shared the Electron main's process group. When Squirrel relaunched the app on auto-update, the old app's process group got killed — taking the host-service and all its PTYs with it. The whole manifest-adoption design in HOST_SERVICE_LIFECYCLE.md assumed the child survives, but the spawn options contradicted that. Mirror the v1 terminal-host daemon pattern (client.ts:1160-1221): `detached: true` in prod, stdio pointed at a per-org rotating log file at `~/.superset/host/{orgId}/host.log` (piped stdio would EPIPE once the parent exits). Dev mode keeps pipes for live console logs since enableDevReload restarts instances on bundle rebuild anyway. --- .../src/main/lib/host-service-coordinator.ts | 89 ++++++++++++++++--- 1 file changed, 76 insertions(+), 13 deletions(-) diff --git a/apps/desktop/src/main/lib/host-service-coordinator.ts b/apps/desktop/src/main/lib/host-service-coordinator.ts index a39c1898dde..c5afaec42d6 100644 --- a/apps/desktop/src/main/lib/host-service-coordinator.ts +++ b/apps/desktop/src/main/lib/host-service-coordinator.ts @@ -25,6 +25,9 @@ import { HOOK_PROTOCOL_VERSION } from "./terminal/env"; /** Minimum host-service version this app can work with. */ const MIN_HOST_SERVICE_VERSION = "0.1.0"; +/** Rotate per-org host.log once it exceeds this size. */ +const MAX_HOST_LOG_BYTES = 5 * 1024 * 1024; + export type HostServiceStatus = "starting" | "running" | "stopped"; export interface Connection { @@ -55,6 +58,30 @@ const HEALTH_POLL_INTERVAL = 200; const HEALTH_POLL_TIMEOUT = 10_000; const ADOPTED_LIVENESS_INTERVAL = 5_000; +/** + * Open an append-mode log fd, truncating first if it exceeds maxBytes. + * Returns -1 on failure so callers can fall back to ignoring child stdio. + */ +function openRotatingLogFd(logPath: string, maxBytes: number): number { + try { + fs.mkdirSync(path.dirname(logPath), { recursive: true, mode: 0o700 }); + if (fs.existsSync(logPath)) { + try { + const { size } = fs.statSync(logPath); + if (size > maxBytes) { + fs.writeFileSync(logPath, "", { mode: 0o600 }); + } + } catch { + // Best-effort rotate + } + } + return fs.openSync(logPath, "a", 0o600); + } catch (error) { + console.warn(`[host-service] Failed to open log file ${logPath}: ${error}`); + return -1; + } +} + async function findFreePort(): Promise { return new Promise((resolve, reject) => { const server = createServer(); @@ -401,11 +428,43 @@ export class HostServiceCoordinator extends EventEmitter { this.instances.set(organizationId, instance); this.emitStatus(organizationId, "starting", null); - const env = await this.buildEnv(organizationId, port, secret, config); - const child = childProcess.spawn(process.execPath, [this.scriptPath], { - stdio: ["ignore", "pipe", "pipe"], - env, - }); + const childEnv = await this.buildEnv(organizationId, port, secret, config); + const isDev = process.env.NODE_ENV === "development"; + + // In prod, detach so the child survives app relaunch: auto-updater's + // quitAndInstall would otherwise take the host-service (and its PTYs) + // down with the old app's process group. Stdio must point at real + // fds — piped stdio would EPIPE once the parent exits. In dev we + // keep pipes so logs flow to the Electron console; dev restarts via + // enableDevReload anyway, so survival isn't needed. + const logFd = isDev + ? -1 + : openRotatingLogFd( + path.join(manifestDir(organizationId), "host.log"), + MAX_HOST_LOG_BYTES, + ); + const stdio: childProcess.StdioOptions = isDev + ? ["ignore", "pipe", "pipe"] + : logFd >= 0 + ? ["ignore", logFd, logFd] + : ["ignore", "ignore", "ignore"]; + + let child: ReturnType; + try { + child = childProcess.spawn(process.execPath, [this.scriptPath], { + detached: !isDev, + stdio, + env: childEnv, + }); + } finally { + if (logFd >= 0) { + try { + fs.closeSync(logFd); + } catch { + // Best-effort — child has its own dup of the fd. + } + } + } const childPid = child.pid; if (!childPid) { @@ -415,14 +474,18 @@ export class HostServiceCoordinator extends EventEmitter { instance.pid = childPid; - child.stdout?.on("data", (data: Buffer) => { - console.log(`[host-service:${organizationId}] ${data.toString().trim()}`); - }); - child.stderr?.on("data", (data: Buffer) => { - console.error( - `[host-service:${organizationId}] ${data.toString().trim()}`, - ); - }); + if (isDev) { + child.stdout?.on("data", (data: Buffer) => { + console.log( + `[host-service:${organizationId}] ${data.toString().trim()}`, + ); + }); + child.stderr?.on("data", (data: Buffer) => { + console.error( + `[host-service:${organizationId}] ${data.toString().trim()}`, + ); + }); + } child.on("exit", (code) => { console.log(`[host-service:${organizationId}] exited with code ${code}`); const current = this.instances.get(organizationId); From 0979a0528cda0b53c75ef7ab6c883cd86cbb0faf Mon Sep 17 00:00:00 2001 From: Kiet Ho Date: Tue, 21 Apr 2026 11:55:40 -0700 Subject: [PATCH 2/4] address review: normalize log perms + windowsHide - chmodSync log file after open: openSync's mode arg only applies on create, so a pre-existing file rotated out-of-band keeps old perms. - windowsHide: true on the spawn so the detached Node child doesn't flash a CMD window on Windows. --- .../desktop/src/main/lib/host-service-coordinator.ts | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/apps/desktop/src/main/lib/host-service-coordinator.ts b/apps/desktop/src/main/lib/host-service-coordinator.ts index c5afaec42d6..4f37ece2c57 100644 --- a/apps/desktop/src/main/lib/host-service-coordinator.ts +++ b/apps/desktop/src/main/lib/host-service-coordinator.ts @@ -75,7 +75,15 @@ function openRotatingLogFd(logPath: string, maxBytes: number): number { // Best-effort rotate } } - return fs.openSync(logPath, "a", 0o600); + const fd = fs.openSync(logPath, "a", 0o600); + // openSync's mode arg only applies on create — normalize an existing + // file's perms in case it was rotated out-of-band with laxer bits. + try { + fs.chmodSync(logPath, 0o600); + } catch { + // Best-effort + } + return fd; } catch (error) { console.warn(`[host-service] Failed to open log file ${logPath}: ${error}`); return -1; @@ -455,6 +463,8 @@ export class HostServiceCoordinator extends EventEmitter { detached: !isDev, stdio, env: childEnv, + // Avoid a flashing CMD window on Windows for the detached child. + windowsHide: true, }); } finally { if (logFd >= 0) { From f79f57ded25566c7002ea610c5e3a391a76ed56d Mon Sep 17 00:00:00 2001 From: Kiet Ho Date: Tue, 21 Apr 2026 13:14:36 -0700 Subject: [PATCH 3/4] refactor(desktop): extract pure host-service helpers to host-service-utils --- .../src/main/lib/host-service-coordinator.ts | 87 ++----------------- .../src/main/lib/host-service-utils.ts | 82 +++++++++++++++++ 2 files changed, 90 insertions(+), 79 deletions(-) create mode 100644 apps/desktop/src/main/lib/host-service-utils.ts diff --git a/apps/desktop/src/main/lib/host-service-coordinator.ts b/apps/desktop/src/main/lib/host-service-coordinator.ts index 4f37ece2c57..dfd194b6ebb 100644 --- a/apps/desktop/src/main/lib/host-service-coordinator.ts +++ b/apps/desktop/src/main/lib/host-service-coordinator.ts @@ -2,7 +2,6 @@ import * as childProcess from "node:child_process"; import { randomBytes } from "node:crypto"; import { EventEmitter } from "node:events"; import * as fs from "node:fs"; -import { createServer } from "node:net"; import path from "node:path"; import { settings } from "@superset/local-db"; import { getDeviceName, getHashedDeviceId } from "@superset/shared/device-info"; @@ -19,15 +18,19 @@ import { readManifest, removeManifest, } from "./host-service-manifest"; +import { + findFreePort, + HEALTH_POLL_TIMEOUT_MS, + MAX_HOST_LOG_BYTES, + openRotatingLogFd, + pollHealthCheck, +} from "./host-service-utils"; import { localDb } from "./local-db"; import { HOOK_PROTOCOL_VERSION } from "./terminal/env"; /** Minimum host-service version this app can work with. */ const MIN_HOST_SERVICE_VERSION = "0.1.0"; -/** Rotate per-org host.log once it exceeds this size. */ -const MAX_HOST_LOG_BYTES = 5 * 1024 * 1024; - export type HostServiceStatus = "starting" | "running" | "stopped"; export interface Connection { @@ -54,82 +57,8 @@ interface HostServiceProcess { status: HostServiceStatus; } -const HEALTH_POLL_INTERVAL = 200; -const HEALTH_POLL_TIMEOUT = 10_000; const ADOPTED_LIVENESS_INTERVAL = 5_000; -/** - * Open an append-mode log fd, truncating first if it exceeds maxBytes. - * Returns -1 on failure so callers can fall back to ignoring child stdio. - */ -function openRotatingLogFd(logPath: string, maxBytes: number): number { - try { - fs.mkdirSync(path.dirname(logPath), { recursive: true, mode: 0o700 }); - if (fs.existsSync(logPath)) { - try { - const { size } = fs.statSync(logPath); - if (size > maxBytes) { - fs.writeFileSync(logPath, "", { mode: 0o600 }); - } - } catch { - // Best-effort rotate - } - } - const fd = fs.openSync(logPath, "a", 0o600); - // openSync's mode arg only applies on create — normalize an existing - // file's perms in case it was rotated out-of-band with laxer bits. - try { - fs.chmodSync(logPath, 0o600); - } catch { - // Best-effort - } - return fd; - } catch (error) { - console.warn(`[host-service] Failed to open log file ${logPath}: ${error}`); - return -1; - } -} - -async function findFreePort(): Promise { - return new Promise((resolve, reject) => { - const server = createServer(); - server.listen(0, "127.0.0.1", () => { - const addr = server.address(); - if (addr && typeof addr === "object") { - const { port } = addr; - server.close(() => resolve(port)); - } else { - server.close(() => reject(new Error("Could not get port"))); - } - }); - server.on("error", reject); - }); -} - -async function pollHealthCheck( - endpoint: string, - secret: string, - timeoutMs = HEALTH_POLL_TIMEOUT, -): Promise { - const deadline = Date.now() + timeoutMs; - while (Date.now() < deadline) { - try { - const controller = new AbortController(); - const timeout = setTimeout(() => controller.abort(), 2_000); - const res = await fetch(`${endpoint}/trpc/health.check`, { - signal: controller.signal, - headers: { Authorization: `Bearer ${secret}` }, - }); - clearTimeout(timeout); - if (res.ok) return true; - } catch { - // Not ready yet - } - await new Promise((r) => setTimeout(r, HEALTH_POLL_INTERVAL)); - } - return false; -} - export class HostServiceCoordinator extends EventEmitter { private instances = new Map(); private pendingStarts = new Map>(); @@ -514,7 +443,7 @@ export class HostServiceCoordinator extends EventEmitter { child.kill("SIGTERM"); this.instances.delete(organizationId); throw new Error( - `Host service failed to start within ${HEALTH_POLL_TIMEOUT}ms`, + `Host service failed to start within ${HEALTH_POLL_TIMEOUT_MS}ms`, ); } diff --git a/apps/desktop/src/main/lib/host-service-utils.ts b/apps/desktop/src/main/lib/host-service-utils.ts new file mode 100644 index 00000000000..9edaf19e9a8 --- /dev/null +++ b/apps/desktop/src/main/lib/host-service-utils.ts @@ -0,0 +1,82 @@ +import * as fs from "node:fs"; +import { createServer } from "node:net"; +import path from "node:path"; + +/** Rotate per-org host.log once it exceeds this size. */ +export const MAX_HOST_LOG_BYTES = 5 * 1024 * 1024; + +export const HEALTH_POLL_TIMEOUT_MS = 10_000; + +const HEALTH_POLL_INTERVAL_MS = 200; + +/** + * Open an append-mode log fd, truncating first if it exceeds maxBytes. + * Returns -1 on failure so callers can fall back to ignoring child stdio. + */ +export function openRotatingLogFd(logPath: string, maxBytes: number): number { + try { + fs.mkdirSync(path.dirname(logPath), { recursive: true, mode: 0o700 }); + if (fs.existsSync(logPath)) { + try { + const { size } = fs.statSync(logPath); + if (size > maxBytes) { + fs.writeFileSync(logPath, "", { mode: 0o600 }); + } + } catch { + // Best-effort rotate + } + } + const fd = fs.openSync(logPath, "a", 0o600); + // openSync's mode arg only applies on create — normalize an existing + // file's perms in case it was rotated out-of-band with laxer bits. + try { + fs.chmodSync(logPath, 0o600); + } catch { + // Best-effort + } + return fd; + } catch (error) { + console.warn(`[host-service] Failed to open log file ${logPath}: ${error}`); + return -1; + } +} + +export async function findFreePort(): Promise { + return new Promise((resolve, reject) => { + const server = createServer(); + server.listen(0, "127.0.0.1", () => { + const addr = server.address(); + if (addr && typeof addr === "object") { + const { port } = addr; + server.close(() => resolve(port)); + } else { + server.close(() => reject(new Error("Could not get port"))); + } + }); + server.on("error", reject); + }); +} + +export async function pollHealthCheck( + endpoint: string, + secret: string, + timeoutMs = HEALTH_POLL_TIMEOUT_MS, +): Promise { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + try { + const controller = new AbortController(); + const timeout = setTimeout(() => controller.abort(), 2_000); + const res = await fetch(`${endpoint}/trpc/health.check`, { + signal: controller.signal, + headers: { Authorization: `Bearer ${secret}` }, + }); + clearTimeout(timeout); + if (res.ok) return true; + } catch { + // Not ready yet + } + await new Promise((r) => setTimeout(r, HEALTH_POLL_INTERVAL_MS)); + } + return false; +} From affabbcca4a2617cc60b60583efbcbbb13aee594 Mon Sep 17 00:00:00 2001 From: Kiet Ho Date: Tue, 21 Apr 2026 14:01:19 -0700 Subject: [PATCH 4/4] address review feedback - Gate detach on app.isPackaged instead of NODE_ENV. NODE_ENV is ambient and could silently flip detach off in a packaged app (e.g. shell env), reintroducing the Squirrel kill-chain this PR exists to fix. - Log a warning on chmodSync failure so permission issues are observable instead of being swallowed silently. - Move clearTimeout into finally in pollHealthCheck so the 2s abort timer is also cleared when fetch rejects, not just on success. --- .../src/main/lib/host-service-coordinator.ts | 35 +++++++++++-------- .../src/main/lib/host-service-utils.ts | 13 ++++--- 2 files changed, 28 insertions(+), 20 deletions(-) diff --git a/apps/desktop/src/main/lib/host-service-coordinator.ts b/apps/desktop/src/main/lib/host-service-coordinator.ts index b52d555bc2f..efec5c71b99 100644 --- a/apps/desktop/src/main/lib/host-service-coordinator.ts +++ b/apps/desktop/src/main/lib/host-service-coordinator.ts @@ -366,21 +366,26 @@ export class HostServiceCoordinator extends EventEmitter { this.emitStatus(organizationId, "starting", null); const childEnv = await this.buildEnv(organizationId, port, secret, config); - const isDev = process.env.NODE_ENV === "development"; - - // In prod, detach so the child survives app relaunch: auto-updater's - // quitAndInstall would otherwise take the host-service (and its PTYs) - // down with the old app's process group. Stdio must point at real - // fds — piped stdio would EPIPE once the parent exits. In dev we - // keep pipes so logs flow to the Electron console; dev restarts via - // enableDevReload anyway, so survival isn't needed. - const logFd = isDev - ? -1 - : openRotatingLogFd( + // Gate on app.isPackaged — the authoritative "running from an installed + // bundle" signal. NODE_ENV is ambient (shell, wrappers, debug launches) + // and could silently flip detach off in a packaged app, which would + // re-introduce the exact Squirrel kill-chain this file exists to fix. + const isPackaged = app.isPackaged; + + // In packaged builds, detach so the child survives app relaunch: + // auto-updater's quitAndInstall would otherwise take the host-service + // (and its PTYs) down with the old app's process group. Stdio must + // point at real fds — piped stdio would EPIPE once the parent exits. + // Unpackaged (dev) keeps pipes so logs flow to the Electron console; + // enableDevReload restarts instances on rebuild, so survival isn't + // needed. + const logFd = isPackaged + ? openRotatingLogFd( path.join(manifestDir(organizationId), "host-service.log"), MAX_HOST_LOG_BYTES, - ); - const stdio: childProcess.StdioOptions = isDev + ) + : -1; + const stdio: childProcess.StdioOptions = !isPackaged ? ["ignore", "pipe", "pipe"] : logFd >= 0 ? ["ignore", logFd, logFd] @@ -389,7 +394,7 @@ export class HostServiceCoordinator extends EventEmitter { let child: ReturnType; try { child = childProcess.spawn(process.execPath, [this.scriptPath], { - detached: !isDev, + detached: isPackaged, stdio, env: childEnv, // Avoid a flashing CMD window on Windows for the detached child. @@ -413,7 +418,7 @@ export class HostServiceCoordinator extends EventEmitter { instance.pid = childPid; - if (isDev) { + if (!isPackaged) { child.stdout?.on("data", (data: Buffer) => { console.log( `[host-service:${organizationId}] ${data.toString().trim()}`, diff --git a/apps/desktop/src/main/lib/host-service-utils.ts b/apps/desktop/src/main/lib/host-service-utils.ts index 02c59496ee0..e638c47a4d1 100644 --- a/apps/desktop/src/main/lib/host-service-utils.ts +++ b/apps/desktop/src/main/lib/host-service-utils.ts @@ -31,8 +31,10 @@ export function openRotatingLogFd(logPath: string, maxBytes: number): number { // file's perms in case it was rotated out-of-band with laxer bits. try { fs.chmodSync(logPath, 0o600); - } catch { - // Best-effort + } catch (error) { + console.warn( + `[host-service] Failed to chmod log file ${logPath}: ${error}`, + ); } return fd; } catch (error) { @@ -64,17 +66,18 @@ export async function pollHealthCheck( ): Promise { const deadline = Date.now() + timeoutMs; while (Date.now() < deadline) { + const controller = new AbortController(); + const timeout = setTimeout(() => controller.abort(), 2_000); try { - const controller = new AbortController(); - const timeout = setTimeout(() => controller.abort(), 2_000); const res = await fetch(`${endpoint}/trpc/health.check`, { signal: controller.signal, headers: { Authorization: `Bearer ${secret}` }, }); - clearTimeout(timeout); if (res.ok) return true; } catch { // Not ready yet + } finally { + clearTimeout(timeout); } await new Promise((r) => setTimeout(r, HEALTH_POLL_INTERVAL_MS)); }