From 0047f87fabe0aa66a4f0b96de584443a81de3a8f Mon Sep 17 00:00:00 2001 From: LukeParkerDev <10430890+Hona@users.noreply.github.com> Date: Wed, 6 May 2026 14:07:48 +1000 Subject: [PATCH] fix(desktop): harden sidecar lifecycle --- packages/desktop/src/main/index.ts | 21 ++--- packages/desktop/src/main/ipc.ts | 2 +- packages/desktop/src/main/server.ts | 118 +++++++++++++++++++++++---- packages/desktop/src/main/sidecar.ts | 33 ++++++++ 4 files changed, 148 insertions(+), 26 deletions(-) diff --git a/packages/desktop/src/main/index.ts b/packages/desktop/src/main/index.ts index 0f23e50b42..e41324df6a 100644 --- a/packages/desktop/src/main/index.ts +++ b/packages/desktop/src/main/index.ts @@ -113,17 +113,16 @@ function setupApp() { }) app.on("before-quit", () => { - killSidecar() + void killSidecar() }) app.on("will-quit", () => { - killSidecar() + void killSidecar() }) for (const signal of ["SIGINT", "SIGTERM"] as const) { process.on(signal, () => { - killSidecar() - app.exit(0) + void killSidecar().finally(() => app.exit(0)) }) } @@ -260,9 +259,10 @@ function wireMenu() { }, reload: () => mainWindow?.reload(), relaunch: () => { - killSidecar() - app.relaunch() - app.exit(0) + void killSidecar().finally(() => { + app.relaunch() + app.exit(0) + }) }, }) } @@ -301,10 +301,11 @@ registerIpcHandlers({ setBackgroundColor: (color) => setBackgroundColor(color), }) -function killSidecar() { +async function killSidecar() { if (!server) return - server.stop() + const current = server server = null + await current.stop() } function ensureLoopbackNoProxy() { @@ -425,7 +426,7 @@ async function installUpdate() { logger.log("installing downloaded update", { version: downloadedUpdateVersion, }) - killSidecar() + await killSidecar() autoUpdater.quitAndInstall() } diff --git a/packages/desktop/src/main/ipc.ts b/packages/desktop/src/main/ipc.ts index 1c4af0eb60..dbcd4239dc 100644 --- a/packages/desktop/src/main/ipc.ts +++ b/packages/desktop/src/main/ipc.ts @@ -19,7 +19,7 @@ const pickerFilters = (ext?: string[]) => { } type Deps = { - killSidecar: () => void + killSidecar: () => Promise | void awaitInitialization: (sendStep: (step: InitStep) => void) => Promise getWindowConfig: () => Promise | WindowConfig consumeInitialDeepLinks: () => Promise | string[] diff --git a/packages/desktop/src/main/server.ts b/packages/desktop/src/main/server.ts index e80c0bddbb..4738693231 100644 --- a/packages/desktop/src/main/server.ts +++ b/packages/desktop/src/main/server.ts @@ -1,6 +1,7 @@ import { dirname, join } from "node:path" import { fileURLToPath } from "node:url" -import { utilityProcess } from "electron" +import { app, utilityProcess } from "electron" +import type { Details } from "electron" import { DEFAULT_SERVER_URL_KEY, WSL_ENABLED_KEY } from "./constants" import { getStore } from "./store" import type { SqliteMigrationProgress } from "../preload/types" @@ -15,7 +16,11 @@ type SidecarMessage = | { type: "stopped" } | { type: "error"; error: { message: string; stack?: string } } -export type SidecarListener = { stop: () => void } +export type SidecarListener = { stop: () => Promise } + +const SIDECAR_SERVICE_NAME = "opencode server" +const SIDECAR_START_STALL_TIMEOUT = 60_000 +const SIDECAR_STOP_TIMEOUT = 6_000 type SpawnLocalServerOptions = { needsMigration: boolean @@ -57,43 +62,80 @@ export async function spawnLocalServer( options: SpawnLocalServerOptions, ) { configureEnv?.() - const child = utilityProcess.fork(join(dirname(fileURLToPath(import.meta.url)), "sidecar.js"), [], { + const sidecar = join(dirname(fileURLToPath(import.meta.url)), "sidecar.js") + const child = utilityProcess.fork(sidecar, [], { cwd: process.cwd(), - env: process.env, - serviceName: "opencode server", + env: createSidecarEnv(), + serviceName: SIDECAR_SERVICE_NAME, stdio: "pipe", }) + let exited = false + const exit = defer() + + const onProcessGone = (_event: unknown, details: Details) => { + if (details.type !== "Utility" || details.name !== SIDECAR_SERVICE_NAME) return + options.onStderr?.(`utility process gone reason=${details.reason} exitCode=${details.exitCode}`) + } + + app.on("child-process-gone", onProcessGone) + child.once("exit", (code) => { + exited = true + app.off("child-process-gone", onProcessGone) + options.onExit?.(code) + exit.resolve(code) + }) + child.on("error", (error) => options.onStderr?.(`utility process error: ${serializeError(error).message}`)) child.stdout?.on("data", (chunk: Buffer) => options.onStdout?.(chunk.toString("utf8").trimEnd())) child.stderr?.on("data", (chunk: Buffer) => options.onStderr?.(chunk.toString("utf8").trimEnd())) await new Promise((resolve, reject) => { + let done = false + let timeout: NodeJS.Timeout + + const fail = (error: Error) => { + if (done) return + done = true + cleanup() + reject(error) + } + + const refreshTimeout = () => { + clearTimeout(timeout) + timeout = setTimeout(() => { + fail(new Error(`Sidecar did not become ready within ${SIDECAR_START_STALL_TIMEOUT}ms: ${sidecar}`)) + }, SIDECAR_START_STALL_TIMEOUT) + } + const onMessage = (message: SidecarMessage) => { if (message.type === "sqlite") { + refreshTimeout() options.onSqliteProgress?.(message.progress) return } if (message.type === "ready") { + if (done) return + done = true cleanup() resolve() return } if (message.type === "error") { - cleanup() - reject(Object.assign(new Error(message.error.message), { stack: message.error.stack })) + fail(Object.assign(new Error(message.error.message), { stack: message.error.stack })) } } const onExit = (code: number) => { - cleanup() - reject(new Error(`Sidecar exited before ready with code ${code}`)) + fail(new Error(`Sidecar exited before ready with code ${code}`)) } const cleanup = () => { + clearTimeout(timeout) child.off("message", onMessage) child.off("exit", onExit) } child.on("message", onMessage) child.on("exit", onExit) + refreshTimeout() child.postMessage({ type: "start", hostname, @@ -102,29 +144,47 @@ export async function spawnLocalServer( userDataPath: options.userDataPath, needsMigration: options.needsMigration, }) + }).catch((error) => { + if (!exited) child.kill() + throw error }) - child.on("exit", (code: number) => options.onExit?.(code)) const wait = (async () => { const url = `http://${hostname}:${port}` + let healthy = false + const gone = exit.promise.then((code) => { + if (healthy) return + throw new Error(`Sidecar exited before health check passed with code ${code}`) + }) const ready = async () => { while (true) { await new Promise((resolve) => setTimeout(resolve, 100)) - if (await checkHealth(url, password)) return + if (await checkHealth(url, password)) { + healthy = true + return + } } } - await ready() + await Promise.race([ready(), gone]) })() + let stopping: Promise | undefined + return { listener: { stop: () => { + if (stopping) return stopping + if (exited) return Promise.resolve() child.postMessage({ type: "stop" }) - setTimeout(() => { - if (child.pid) child.kill() - }, 2_000).unref() + stopping = Promise.race([ + exit.promise.then(() => undefined), + delay(SIDECAR_STOP_TIMEOUT).then(() => { + if (!exited) child.kill() + }), + ]) + return stopping }, }, health: { wait }, @@ -156,3 +216,31 @@ export async function checkHealth(url: string, password?: string | null): Promis return false } } + +function createSidecarEnv(): Record { + const env = Object.fromEntries( + Object.entries(process.env).flatMap(([key, value]) => (value === undefined ? [] : [[key, String(value)]])), + ) + delete env.DEBUG + if (process.platform === "linux") delete env.LD_PRELOAD + return env +} + +function delay(ms: number) { + return new Promise((resolve) => setTimeout(resolve, ms)) +} + +function serializeError(error: unknown) { + if (error instanceof Error) return { message: error.message, stack: error.stack } + return { message: String(error) } +} + +function defer() { + let resolve!: (value: T) => void + let reject!: (error: Error) => void + const promise = new Promise((res, rej) => { + resolve = res + reject = rej + }) + return { promise, resolve, reject } +} diff --git a/packages/desktop/src/main/sidecar.ts b/packages/desktop/src/main/sidecar.ts index 2a4ddcd819..c592884c99 100644 --- a/packages/desktop/src/main/sidecar.ts +++ b/packages/desktop/src/main/sidecar.ts @@ -1,7 +1,18 @@ import { drizzle } from "drizzle-orm/node-sqlite/driver" +import * as http from "node:http" +import * as tls from "node:tls" import { getUserShell, loadShellEnv } from "./shell-env" +type NodeHttpWithEnvProxy = typeof http & { + setGlobalProxyFromEnv: () => void +} + +type NodeTlsWithSystemCertificates = typeof tls & { + getCACertificates: (type: "default" | "system") => string[] + setDefaultCACertificates: (certificates: string[]) => void +} + type StartCommand = { type: "start" hostname: string @@ -45,6 +56,8 @@ parentPort.on("message", (event) => { async function start(command: StartCommand) { try { prepareServerEnv(command.password, command.userDataPath) + useSystemCertificates() + useEnvProxy() const { Database, JsonMigration, Log, Server } = await import("virtual:opencode-server") await Log.init({ level: "WARN" }) @@ -73,6 +86,7 @@ async function start(command: StartCommand) { parentPort.postMessage({ type: "ready" }) } catch (error) { parentPort.postMessage({ type: "error", error: serializeError(error) }) + setImmediate(() => process.exit(1)) } } @@ -101,6 +115,25 @@ function prepareServerEnv(password: string, userDataPath: string) { }) } +function useSystemCertificates() { + try { + const nodeTls = tls as NodeTlsWithSystemCertificates + nodeTls.setDefaultCACertificates([ + ...new Set([...nodeTls.getCACertificates("default"), ...nodeTls.getCACertificates("system")]), + ]) + } catch (error) { + console.warn("failed to load system certificates", error) + } +} + +function useEnvProxy() { + try { + ;(http as NodeHttpWithEnvProxy).setGlobalProxyFromEnv() + } catch (error) { + console.warn("failed to load proxy environment", error) + } +} + function parseCommand(value: unknown): SidecarCommand | undefined { if (!value || typeof value !== "object") return const command = value as Partial