From f4faafa9874c188be461d3138c9040b6332b44c1 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 5 Jul 2026 19:52:06 -0700 Subject: [PATCH] fix(daemon): relocate daemon host image out of the install-dir kill zone (#7473) Co-authored-by: Neil Co-authored-by: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> --- .github/workflows/release-cut.yml | 8 + config/daemon-host-node-runtime.cjs | 41 +++ config/electron-builder.config.cjs | 5 + .../scripts/electron-builder-config.test.mjs | 62 +++++ ...package-electron-runtime-contract.test.mjs | 3 + .../daemon/daemon-host-relocation.test.ts | 100 +++++++ src/main/daemon/daemon-host-relocation.ts | 83 ++++++ src/main/daemon/daemon-init.ts | 53 +++- src/main/index.ts | 3 + .../install-dir-runtime-relocation.test.ts | 251 ++++++++++++++++++ .../pty/install-dir-runtime-relocation.ts | 145 ++++++++++ .../pty/node-pty-runtime-relocation.test.ts | 239 +---------------- src/main/pty/node-pty-runtime-relocation.ts | 134 +--------- 13 files changed, 756 insertions(+), 371 deletions(-) create mode 100644 config/daemon-host-node-runtime.cjs create mode 100644 src/main/daemon/daemon-host-relocation.test.ts create mode 100644 src/main/daemon/daemon-host-relocation.ts create mode 100644 src/main/pty/install-dir-runtime-relocation.test.ts create mode 100644 src/main/pty/install-dir-runtime-relocation.ts diff --git a/.github/workflows/release-cut.yml b/.github/workflows/release-cut.yml index 96af9bd0c..a107536f3 100644 --- a/.github/workflows/release-cut.yml +++ b/.github/workflows/release-cut.yml @@ -878,6 +878,14 @@ jobs: } Get-Item -LiteralPath $file } + # The standalone node.exe that hosts the terminal daemon outside the + # install-dir kill zone the NSIS updater sweeps. See + # src/main/daemon/daemon-host-relocation.ts. + $daemonHostNode = 'dist/win-unpacked/resources/daemon-host/node.exe' + if (-not (Test-Path -LiteralPath $daemonHostNode -PathType Leaf)) { + throw "Missing Windows daemon-host runtime file: $daemonHostNode" + } + Get-Item -LiteralPath $daemonHostNode - name: Install SignPath PowerShell module if: matrix.platform == 'win' diff --git a/config/daemon-host-node-runtime.cjs b/config/daemon-host-node-runtime.cjs new file mode 100644 index 000000000..944d9a94d --- /dev/null +++ b/config/daemon-host-node-runtime.cjs @@ -0,0 +1,41 @@ +const { copyFileSync, existsSync, mkdirSync } = require('node:fs') +const { join } = require('node:path') + +// Where the standalone daemon-host node.exe is staged under packaged resources. +// Mirrored at runtime by src/main/daemon/daemon-host-relocation.ts, which copies +// it into userData/daemon-host// and forks the terminal daemon from it. +const DAEMON_HOST_DIR = 'daemon-host' +const DAEMON_HOST_NODE_EXE = 'node.exe' + +/** + * Stages a standalone Windows node.exe into resources/daemon-host so the + * detached terminal daemon can be forked from a userData copy that lives + * outside the install directory the NSIS updater force-closes mid-update. + * + * Source is the build host's own node.exe (process.execPath). It is version- + * correct by construction: engines.node pins the build to Node 24.x, whose NAPI + * 10 runtime loads node-pty's Electron-42-built conpty.node. Being an official + * Node binary, it also carries the OpenJS Foundation Authenticode signature, so + * it ships validly signed regardless of whether SignPath re-signs nested PEs. + * + * Windows x64 only — the sole Windows build target. Other targets ship no + * node.exe and the runtime falls open to forking the install-dir Electron host. + */ +function ensurePackagedDaemonHostNode(resourcesDir, electronPlatformName) { + if (electronPlatformName !== 'win32') { + return + } + const nodeExePath = process.execPath + // The build must run under standalone Node (not Electron) so execPath is a + // real node.exe with a NAPI runtime; anything else would strand the daemon. + if (!/node\.exe$/i.test(nodeExePath) || !existsSync(nodeExePath)) { + throw new Error( + `[daemon-host] cannot stage node.exe: build host execPath is not a node.exe (${nodeExePath})` + ) + } + const destDir = join(resourcesDir, DAEMON_HOST_DIR) + mkdirSync(destDir, { recursive: true }) + copyFileSync(nodeExePath, join(destDir, DAEMON_HOST_NODE_EXE)) +} + +module.exports = { ensurePackagedDaemonHostNode, DAEMON_HOST_DIR, DAEMON_HOST_NODE_EXE } diff --git a/config/electron-builder.config.cjs b/config/electron-builder.config.cjs index 9e482825a..9880f0d9d 100644 --- a/config/electron-builder.config.cjs +++ b/config/electron-builder.config.cjs @@ -7,6 +7,7 @@ const { prunePackagedRuntimeNodeModules, verifyPackagedMainRuntimeDeps } = require('./packaged-runtime-node-modules.cjs') +const { ensurePackagedDaemonHostNode } = require('./daemon-host-node-runtime.cjs') const isMacRelease = process.env.ORCA_MAC_RELEASE === '1' const isLinuxArm64Release = process.env.ORCA_LINUX_ARM64_RELEASE === '1' @@ -128,6 +129,10 @@ module.exports = { return } prunePackagedRuntimeNodeModules(resourcesDir, context.electronPlatformName, context.arch) + // Why: stage the standalone node.exe that hosts the terminal daemon outside + // the install-dir kill zone the NSIS updater sweeps (win32 only; no-ops + // elsewhere). See src/main/daemon/daemon-host-relocation.ts. + ensurePackagedDaemonHostNode(resourcesDir, context.electronPlatformName) verifyPackagedMainRuntimeDeps(resourcesDir) chmodUnixCliLaunchers(resourcesDir, context.electronPlatformName) chmodMacServeSimHelpers(resourcesDir, context.electronPlatformName) diff --git a/config/scripts/electron-builder-config.test.mjs b/config/scripts/electron-builder-config.test.mjs index 3f318219e..ce03bd6b4 100644 --- a/config/scripts/electron-builder-config.test.mjs +++ b/config/scripts/electron-builder-config.test.mjs @@ -17,6 +17,22 @@ const { prunePackagedZodSources, verifyPackagedMainRuntimeDeps } = require('../packaged-runtime-node-modules.cjs') +const { ensurePackagedDaemonHostNode } = require('../daemon-host-node-runtime.cjs') + +// process.execPath is read-only in the types; point it at a fixture node.exe. +function withStubbedExecPath(value, fn) { + const original = process.execPath + Object.defineProperty(process, 'execPath', { value, configurable: true, writable: true }) + try { + return fn() + } finally { + Object.defineProperty(process, 'execPath', { + value: original, + configurable: true, + writable: true + }) + } +} describe('electron-builder config', () => { it('excludes repo-only source trees from app.asar', () => { @@ -207,6 +223,52 @@ describe('electron-builder config', () => { } }) + it('stages the build host node.exe into resources/daemon-host for Windows', async () => { + const root = await mkdtemp(join(tmpdir(), 'orca-daemon-host-node-')) + try { + const fakeHostDir = join(root, 'host') + await mkdir(fakeHostDir, { recursive: true }) + const fakeNodeExe = join(fakeHostDir, 'node.exe') + await writeFile(fakeNodeExe, 'fake-node-binary', 'utf8') + const resourcesDir = join(root, 'resources') + await mkdir(resourcesDir, { recursive: true }) + + withStubbedExecPath(fakeNodeExe, () => ensurePackagedDaemonHostNode(resourcesDir, 'win32')) + + await expect(readFile(join(resourcesDir, 'daemon-host', 'node.exe'), 'utf8')).resolves.toBe( + 'fake-node-binary' + ) + } finally { + await rm(root, { recursive: true, force: true }) + } + }) + + it('does not stage a daemon-host node.exe on non-Windows platforms', async () => { + const resourcesDir = await mkdtemp(join(tmpdir(), 'orca-daemon-host-node-skip-')) + try { + ensurePackagedDaemonHostNode(resourcesDir, 'linux') + await expect(readdir(resourcesDir)).resolves.toEqual([]) + } finally { + await rm(resourcesDir, { recursive: true, force: true }) + } + }) + + it('fails the Windows build when the host binary is not a node.exe', async () => { + const root = await mkdtemp(join(tmpdir(), 'orca-daemon-host-node-bad-')) + try { + const notNode = join(root, 'electron.exe') + await writeFile(notNode, 'not-node', 'utf8') + const resourcesDir = join(root, 'resources') + await mkdir(resourcesDir, { recursive: true }) + + expect(() => + withStubbedExecPath(notNode, () => ensurePackagedDaemonHostNode(resourcesDir, 'win32')) + ).toThrow(/not a node\.exe/) + } finally { + await rm(root, { recursive: true, force: true }) + } + }) + it('includes @parcel/watcher in the packaged runtime closure', () => { // Why: the main process imports '@parcel/watcher' for filesystem change // events; if it is absent from the packaged closure the serve host silently diff --git a/config/scripts/package-electron-runtime-contract.test.mjs b/config/scripts/package-electron-runtime-contract.test.mjs index c106b166f..43a898dbc 100644 --- a/config/scripts/package-electron-runtime-contract.test.mjs +++ b/config/scripts/package-electron-runtime-contract.test.mjs @@ -178,6 +178,9 @@ describe('Electron runtime package contract', () => { 'dist/win-unpacked/resources/node_modules/node-pty/build/Release' ) expect(steps[verifyNodePtyIndex].run).toContain('conpty/conpty.dll') + // The daemon-host node.exe must be staged alongside the node-pty runtime so + // the terminal daemon can be forked from outside the install-dir kill zone. + expect(steps[verifyNodePtyIndex].run).toContain('resources/daemon-host/node.exe') const uploadThroughDownloadScript = steps .slice(uploadIndex, downloadIndex + 1) diff --git a/src/main/daemon/daemon-host-relocation.test.ts b/src/main/daemon/daemon-host-relocation.test.ts new file mode 100644 index 000000000..965616409 --- /dev/null +++ b/src/main/daemon/daemon-host-relocation.test.ts @@ -0,0 +1,100 @@ +import { mkdtempSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import os from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +// Mutable Electron app stub, hoisted so the vi.mock factory can close over it. +const { electronApp } = vi.hoisted(() => ({ + electronApp: { + isPackaged: false, + userDataPath: '', + version: '0.0.0-test', + getPath: (): string => electronApp.userDataPath, + getVersion: (): string => electronApp.version + } +})) + +vi.mock('electron', () => ({ app: electronApp })) + +import { resolveDaemonHostSourceDir } from './daemon-host-relocation' + +let tempDir: string + +beforeEach(() => { + tempDir = mkdtempSync(join(os.tmpdir(), 'daemon-host-relocation-')) +}) + +afterEach(() => { + vi.unstubAllEnvs() + try { + rmSync(tempDir, { recursive: true, force: true }) + } catch {} +}) + +describe('resolveDaemonHostSourceDir', () => { + it('returns the daemon-host dir when it holds a node.exe', () => { + const resources = join(tempDir, 'resources') + mkdirSync(join(resources, 'daemon-host'), { recursive: true }) + writeFileSync(join(resources, 'daemon-host', 'node.exe'), 'host') + expect(resolveDaemonHostSourceDir(resources)).toBe(join(resources, 'daemon-host')) + }) + + it('returns null when the daemon-host dir has no node.exe', () => { + const resources = join(tempDir, 'resources') + mkdirSync(join(resources, 'daemon-host'), { recursive: true }) + expect(resolveDaemonHostSourceDir(resources)).toBeNull() + }) + + it('returns null when the daemon-host dir is absent', () => { + expect(resolveDaemonHostSourceDir(join(tempDir, 'resources'))).toBeNull() + }) +}) + +// process.resourcesPath is typed read-only; point it at a fixture for the test. +function stubResourcesPath(value: string | undefined): void { + Object.defineProperty(process, 'resourcesPath', { value, configurable: true, writable: true }) +} + +// Copies a real node.exe path shape; the win32 guard in the module means this +// only exercises the relocation path on Windows. +describe.runIf(process.platform === 'win32')('installRelocatedDaemonHost', () => { + const originalResourcesPath = process.resourcesPath + + afterEach(() => { + stubResourcesPath(originalResourcesPath) + vi.resetModules() + }) + + it('relocates a bundled node.exe and exposes its userData path', async () => { + const resources = join(tempDir, 'resources') + mkdirSync(join(resources, 'daemon-host'), { recursive: true }) + writeFileSync(join(resources, 'daemon-host', 'node.exe'), 'fake-node-host') + stubResourcesPath(resources) + electronApp.userDataPath = join(tempDir, 'userData') + electronApp.version = '9.9.9' + vi.stubEnv('ORCA_FORCE_DAEMON_HOST_RELOCATION', '1') + + // Fresh module so the one-shot install/singleton state is not carried over. + vi.resetModules() + const mod = await import('./daemon-host-relocation') + mod.installRelocatedDaemonHost() + + const execPath = mod.getRelocatedDaemonHostExecPath() + expect(execPath).toBe(join(electronApp.userDataPath, 'daemon-host', '9.9.9', 'node.exe')) + expect(readFileSync(execPath!, 'utf8')).toBe('fake-node-host') + }) + + it('fails open to null when no bundled node.exe is present', async () => { + const resources = join(tempDir, 'resources') + mkdirSync(resources, { recursive: true }) + stubResourcesPath(resources) + electronApp.userDataPath = join(tempDir, 'userData') + vi.stubEnv('ORCA_FORCE_DAEMON_HOST_RELOCATION', '1') + + vi.resetModules() + const mod = await import('./daemon-host-relocation') + mod.installRelocatedDaemonHost() + + expect(mod.getRelocatedDaemonHostExecPath()).toBeNull() + }) +}) diff --git a/src/main/daemon/daemon-host-relocation.ts b/src/main/daemon/daemon-host-relocation.ts new file mode 100644 index 000000000..21cdcffe3 --- /dev/null +++ b/src/main/daemon/daemon-host-relocation.ts @@ -0,0 +1,83 @@ +import { existsSync } from 'node:fs' +import { join } from 'node:path' +import { app } from 'electron' +import { ensureRelocatedRuntime } from '../pty/install-dir-runtime-relocation' + +/** + * Relocates the Node host that runs the detached terminal daemon out of the + * app install directory into userData. + * + * Why: the daemon is `fork()`ed as plain Node via ELECTRON_RUN_AS_NODE, so its + * process image is the install-dir Orca.exe. The NSIS update installer + * force-closes every process whose image path is under the install directory + * before replacing files, which kills the daemon — and every live terminal it + * owns — mid-update. Running the daemon from a version-keyed node.exe staged in + * userData takes its image out of the installer's kill zone so it survives. + * + * Fail-open: if no bundled node.exe is present (dev, or a build that predates + * shipping it), relocation no-ops and the caller keeps forking the install-dir + * Orca.exe host — the pre-relocation behavior, with zero regression. + */ +const RELOCATED_DAEMON_HOST_EXE = 'node.exe' + +let relocatedDaemonHostExecPath: string | null = null +let installed = false + +/** + * The bundled daemon-host dir under app resources, or null when no node.exe is + * shipped there. Kept separate from install so the resources path is testable. + */ +export function resolveDaemonHostSourceDir(resourcesPath: string): string | null { + const dir = join(resourcesPath, 'daemon-host') + return existsSync(join(dir, RELOCATED_DAEMON_HOST_EXE)) ? dir : null +} + +/** + * Copies the bundled daemon-host node.exe into a version-keyed userData dir + * (once) and records its path for the daemon fork. Safe to call more than once; + * only the first call does work. + */ +export function installRelocatedDaemonHost(): void { + if (installed) { + return + } + installed = true + if (process.platform !== 'win32') { + return + } + // The install-dir kill zone only exists for packaged installs; a dev override + // lets the relocation path be exercised without a full NSIS build. + if (!app.isPackaged && process.env.ORCA_FORCE_DAEMON_HOST_RELOCATION !== '1') { + return + } + // Why: fail-open contract — this must never throw. resourcesPath is unset + // outside a real Electron process (tests, node-hosted tooling). + if (typeof process.resourcesPath !== 'string') { + return + } + const sourceDir = resolveDaemonHostSourceDir(process.resourcesPath) + if (!sourceDir) { + return + } + const userData = app.getPath('userData') + const destDir = ensureRelocatedRuntime({ + sourceDir, + destRoot: join(userData, 'daemon-host'), + version: app.getVersion(), + // Same runtimeDir daemon-init writes daemon-v.pid into, so a surviving + // daemon's host dir is pinned by its recorded appVersion and never reclaimed + // while it runs. + daemonRuntimeDir: join(userData, 'daemon') + }) + if (destDir) { + relocatedDaemonHostExecPath = join(destDir, RELOCATED_DAEMON_HOST_EXE) + } +} + +/** + * The relocated node.exe to fork the daemon from, or null to fall back to the + * install-dir Electron host (ELECTRON_RUN_AS_NODE). + */ +export function getRelocatedDaemonHostExecPath(): string | null { + return relocatedDaemonHostExecPath +} diff --git a/src/main/daemon/daemon-init.ts b/src/main/daemon/daemon-init.ts index ff18ecc33..68c492a24 100644 --- a/src/main/daemon/daemon-init.ts +++ b/src/main/daemon/daemon-init.ts @@ -36,6 +36,11 @@ import { isDaemonStaleForCurrentBundle, killStaleDaemon } from './daemon-health' +import { + getRelocatedDaemonHostExecPath, + installRelocatedDaemonHost +} from './daemon-host-relocation' +import { startSpan } from '../observability/tracer' import { DegradedDaemonPtyProvider } from './degraded-daemon-pty-provider' import { getLocalPtyProvider, @@ -153,6 +158,16 @@ function createPreservedDaemonHandle( protocolVersion = PROTOCOL_VERSION, mode?: 'degraded-new-pty-fallback' ): DaemonProcessHandle { + // Why: the trace file is the only artifact available in field + // investigations of update-survival incidents; adopt-vs-fresh-fork is the + // first question every one of them asks. + const adoptSpan = startSpan('daemon.adopt', { + attributes: { + 'daemon.protocol_version': protocolVersion, + ...(mode ? { 'daemon.mode': mode } : {}) + } + }) + adoptSpan.end() const handle: DaemonProcessHandle = { shutdown: async () => { await cleanupDaemonForProtocol(runtimeDir, protocolVersion) @@ -260,6 +275,25 @@ function createOutOfProcessLauncher(runtimeDir: string): DaemonLauncher { await killStaleDaemon(runtimeDir, socketPath, tokenPath) const userDataPath = app.getPath('userData') + // Why: staged here instead of app startup so the ~70MB node.exe copy stays + // off the first-paint path, and is skipped entirely on launches that adopt + // a live daemon (no fork). Latched — repeat calls are free. + installRelocatedDaemonHost() + // Why: on Windows a relocated node.exe in userData hosts the daemon so its + // process image escapes the install-dir the NSIS updater force-closes; when + // absent (dev, non-win32, pre-relocation builds) fall back to the install-dir + // Electron host run as plain Node via ELECTRON_RUN_AS_NODE. + const relocatedHostExecPath = getRelocatedDaemonHostExecPath() + // Why: without this span a silent fallback to the install-dir host (back + // inside the updater kill zone) is indistinguishable from the fixed path + // in field trace files. + const forkSpan = startSpan('daemon.fork', { + attributes: { + 'daemon.host': relocatedHostExecPath ? 'relocated-node' : 'install-dir-electron', + ...(relocatedHostExecPath ? { 'daemon.host_exec_path': relocatedHostExecPath } : {}), + 'app.version': app.getVersion() + } + }) const child = fork(entryPath, ['--socket', socketPath, '--token', tokenPath], { // Why: detached daemons can outlive dev worktrees. Starting from // userData keeps process.cwd() valid after a repo/worktree is deleted. @@ -269,13 +303,17 @@ function createOutOfProcessLauncher(runtimeDir: string): DaemonLauncher { // open, which would prevent Electron from exiting cleanly. detached: true, stdio: ['ignore', 'ignore', 'ignore', 'ipc'], - // Why: ELECTRON_RUN_AS_NODE makes the forked process run as a plain - // Node.js process instead of an Electron renderer/main process. Without - // it, Electron's GPU/display initialization can interfere with native - // module operations like node-pty's posix_spawn of the spawn-helper. + // Why: a standalone node.exe is already plain Node; reset execArgv so the + // Electron main process's node flags don't leak into it. + ...(relocatedHostExecPath ? { execPath: relocatedHostExecPath, execArgv: [] } : {}), env: { ...process.env, - ELECTRON_RUN_AS_NODE: '1', + // Why: ELECTRON_RUN_AS_NODE makes the forked Electron binary run as a + // plain Node.js process instead of an Electron main process. Without it, + // Electron's GPU/display initialization can interfere with native module + // operations like node-pty's posix_spawn of the spawn-helper. A relocated + // node.exe is already plain Node, so the var is omitted there. + ...(relocatedHostExecPath ? {} : { ELECTRON_RUN_AS_NODE: '1' }), // Why: the detached daemon is plain Node and cannot call Electron's // app.getPath(), but shell-ready rcfiles must live outside swept tmp. ORCA_USER_DATA_PATH: userDataPath @@ -307,6 +345,7 @@ function createOutOfProcessLauncher(runtimeDir: string): DaemonLauncher { // Already dead } } + forkSpan.fail(error) reject(error) } function onReadyMessage(msg: unknown): void { @@ -318,6 +357,10 @@ function createOutOfProcessLauncher(runtimeDir: string): DaemonLauncher { // Why: the daemon process is detached after readiness; leaving // startup listeners attached retains this launch promise closure. cleanupStartupListeners() + if (child.pid !== undefined) { + forkSpan.setAttribute('daemon.pid', child.pid) + } + forkSpan.end() if (child.pid) { // Why: JSON pid file carries pid + process start time so later // killStaleDaemon() can verify the pid still belongs to the daemon diff --git a/src/main/index.ts b/src/main/index.ts index a361df3e3..7eccdffc7 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -578,6 +578,9 @@ if (hasSingleInstanceLock) { initOpenCodeUsagePath() // Why: must run before anything loads node-pty (first PTY spawn) so main // and the daemon it forks both pick up ORCA_NODE_PTY_NATIVE_DIR. + // (The daemon-host node.exe relocation deliberately does NOT run here: its + // ~70MB copy would sit on the first-paint path. daemon-init stages it at + // fork time instead.) installRelocatedNodePtyNativeRuntime() crashReports = CrashReportStore.fromUserData() recordCrashBreadcrumb('app_started', { diff --git a/src/main/pty/install-dir-runtime-relocation.test.ts b/src/main/pty/install-dir-runtime-relocation.test.ts new file mode 100644 index 000000000..0844b5a83 --- /dev/null +++ b/src/main/pty/install-dir-runtime-relocation.test.ts @@ -0,0 +1,251 @@ +import { mkdtempSync, mkdirSync, readFileSync, rmSync, writeFileSync, existsSync } from 'node:fs' +import os from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' + +import { getDaemonPidPath, serializeDaemonPidFile } from '../daemon/daemon-spawner' +import type { DaemonPidFile } from '../daemon/daemon-spawner' +import { + collectInUseRuntimeVersions, + ensureRelocatedRuntime +} from './install-dir-runtime-relocation' + +let tempDir: string + +beforeEach(() => { + tempDir = mkdtempSync(join(os.tmpdir(), 'install-dir-relocation-')) +}) + +afterEach(() => { + try { + rmSync(tempDir, { recursive: true, force: true }) + } catch {} +}) + +// A generic runtime tree with a nested dir and a .pdb symbol file, to exercise +// recursive copy and the symbol-exclusion rule independent of any one runtime. +function seedSourceDir(dir: string): void { + mkdirSync(join(dir, 'nested'), { recursive: true }) + writeFileSync(join(dir, 'runtime.exe'), 'host') + writeFileSync(join(dir, 'runtime.dll'), 'lib') + writeFileSync(join(dir, 'runtime.pdb'), 'symbols') + writeFileSync(join(dir, 'nested', 'inner.bin'), 'inner') +} + +// Writes a daemon pid file exactly as the daemon does (userData/daemon/daemon-v.pid). +function writeDaemonPidFile( + daemonRuntimeDir: string, + protocolVersion: number, + pidFile: DaemonPidFile +): void { + mkdirSync(daemonRuntimeDir, { recursive: true }) + writeFileSync( + getDaemonPidPath(daemonRuntimeDir, protocolVersion), + serializeDaemonPidFile(pidFile) + ) +} + +// Deterministic liveness that treats the given pids as running, so tests never +// depend on the host's real process table. +function aliveFor(...alivePids: number[]): (pid: number) => boolean { + const set = new Set(alivePids) + return (pid) => set.has(pid) +} + +describe('collectInUseRuntimeVersions', () => { + it('returns an empty set when the daemon dir does not exist', () => { + expect(collectInUseRuntimeVersions(join(tempDir, 'no-daemon-dir')).size).toBe(0) + }) + + it('collects the app version a live daemon pins', () => { + const daemonDir = join(tempDir, 'daemon') + writeDaemonPidFile(daemonDir, 18, { pid: 4321, startedAtMs: null, appVersion: '1.4.124-rc.1' }) + + const inUse = collectInUseRuntimeVersions(daemonDir, aliveFor(4321)) + + expect([...inUse]).toEqual(['1.4.124-rc.1']) + }) + + it('omits the version of a daemon whose pid is dead', () => { + const daemonDir = join(tempDir, 'daemon') + writeDaemonPidFile(daemonDir, 18, { pid: 4321, startedAtMs: null, appVersion: '1.4.124-rc.1' }) + + const inUse = collectInUseRuntimeVersions(daemonDir, aliveFor(/* nobody */)) + + expect(inUse.size).toBe(0) + }) + + it('collects one version per live daemon across protocol versions', () => { + const daemonDir = join(tempDir, 'daemon') + writeDaemonPidFile(daemonDir, 18, { pid: 100, startedAtMs: null, appVersion: '1.4.124-rc.1' }) + writeDaemonPidFile(daemonDir, 19, { pid: 200, startedAtMs: null, appVersion: '1.4.124-rc.2' }) + + const inUse = collectInUseRuntimeVersions(daemonDir, aliveFor(100, 200)) + + expect([...inUse].sort()).toEqual(['1.4.124-rc.1', '1.4.124-rc.2']) + }) + + it('ignores a daemon whose pid file records no app version', () => { + const daemonDir = join(tempDir, 'daemon') + // Pre-relocation daemon: JSON pid file without appVersion. + writeDaemonPidFile(daemonDir, 18, { pid: 4321, startedAtMs: null }) + // Legacy daemon: bare-integer pid file (appVersion parses to null). + mkdirSync(daemonDir, { recursive: true }) + writeFileSync(getDaemonPidPath(daemonDir, 17), '9999') + + const inUse = collectInUseRuntimeVersions(daemonDir, aliveFor(4321, 9999)) + + expect(inUse.size).toBe(0) + }) + + it('ignores non-pid files in the daemon dir', () => { + const daemonDir = join(tempDir, 'daemon') + mkdirSync(daemonDir, { recursive: true }) + writeFileSync(join(daemonDir, 'daemon-v18.token'), 'secret') + writeFileSync(join(daemonDir, 'daemon-v18.sock'), 'x') + writeFileSync(join(daemonDir, 'notes.txt'), 'x') + + expect(collectInUseRuntimeVersions(daemonDir, aliveFor(1, 2, 3)).size).toBe(0) + }) + + it('skips a malformed pid file without throwing', () => { + const daemonDir = join(tempDir, 'daemon') + mkdirSync(daemonDir, { recursive: true }) + writeFileSync(getDaemonPidPath(daemonDir, 18), 'not-a-pid-at-all') + writeDaemonPidFile(daemonDir, 19, { pid: 200, startedAtMs: null, appVersion: '2.0.0' }) + + const inUse = collectInUseRuntimeVersions(daemonDir, aliveFor(200)) + + expect([...inUse]).toEqual(['2.0.0']) + }) + + it('treats the current process as alive under the default liveness probe', () => { + const daemonDir = join(tempDir, 'daemon') + // startedAtMs null so startTimeMatches short-circuits true on every platform. + writeDaemonPidFile(daemonDir, 18, { + pid: process.pid, + startedAtMs: null, + appVersion: '3.0.0' + }) + + expect([...collectInUseRuntimeVersions(daemonDir)]).toEqual(['3.0.0']) + }) +}) + +describe('ensureRelocatedRuntime', () => { + it('copies the runtime tree (without symbols) and returns the version dir', () => { + const sourceDir = join(tempDir, 'source') + seedSourceDir(sourceDir) + const destRoot = join(tempDir, 'dest') + + const destDir = ensureRelocatedRuntime({ + sourceDir, + destRoot, + version: '1.2.3', + daemonRuntimeDir: join(tempDir, 'daemon') + }) + + expect(destDir).toBe(join(destRoot, '1.2.3')) + expect(readFileSync(join(destDir!, 'runtime.exe'), 'utf8')).toBe('host') + expect(readFileSync(join(destDir!, 'runtime.dll'), 'utf8')).toBe('lib') + expect(readFileSync(join(destDir!, 'nested', 'inner.bin'), 'utf8')).toBe('inner') + expect(existsSync(join(destDir!, 'runtime.pdb'))).toBe(false) + }) + + it('skips recopying once the completion marker exists', () => { + const sourceDir = join(tempDir, 'source') + seedSourceDir(sourceDir) + const destRoot = join(tempDir, 'dest') + ensureRelocatedRuntime({ + sourceDir, + destRoot, + version: '1.2.3', + daemonRuntimeDir: join(tempDir, 'daemon') + }) + + writeFileSync(join(sourceDir, 'runtime.exe'), 'changed-after-first-copy') + const destDir = ensureRelocatedRuntime({ + sourceDir, + destRoot, + version: '1.2.3', + daemonRuntimeDir: join(tempDir, 'daemon') + }) + + expect(readFileSync(join(destDir!, 'runtime.exe'), 'utf8')).toBe('host') + }) + + it('redoes an interrupted copy that has no completion marker', () => { + const sourceDir = join(tempDir, 'source') + seedSourceDir(sourceDir) + const destRoot = join(tempDir, 'dest') + mkdirSync(join(destRoot, '1.2.3'), { recursive: true }) + writeFileSync(join(destRoot, '1.2.3', 'runtime.exe'), 'torn partial copy') + + const destDir = ensureRelocatedRuntime({ + sourceDir, + destRoot, + version: '1.2.3', + daemonRuntimeDir: join(tempDir, 'daemon') + }) + + expect(readFileSync(join(destDir!, 'runtime.exe'), 'utf8')).toBe('host') + }) + + it('reclaims a stale version dir once no live daemon pins it', () => { + const sourceDir = join(tempDir, 'source') + seedSourceDir(sourceDir) + const destRoot = join(tempDir, 'dest') + const daemonRuntimeDir = join(tempDir, 'daemon') + ensureRelocatedRuntime({ sourceDir, destRoot, version: '1.0.0', daemonRuntimeDir }) + + // The 1.0.0 daemon exited, so its pid is dead — nothing pins 1.0.0. + writeDaemonPidFile(daemonRuntimeDir, 18, { pid: 4321, startedAtMs: null, appVersion: '1.0.0' }) + + const destDir = ensureRelocatedRuntime({ + sourceDir, + destRoot, + version: '2.0.0', + daemonRuntimeDir, + isDaemonPidAlive: aliveFor(/* 4321 is dead */) + }) + + expect(destDir).toBe(join(destRoot, '2.0.0')) + expect(existsSync(join(destRoot, '1.0.0'))).toBe(false) + expect(existsSync(join(destRoot, '2.0.0', 'runtime.exe'))).toBe(true) + }) + + it('preserves a stale version dir a surviving daemon still pins', () => { + const sourceDir = join(tempDir, 'source') + seedSourceDir(sourceDir) + const destRoot = join(tempDir, 'dest') + const daemonRuntimeDir = join(tempDir, 'daemon') + ensureRelocatedRuntime({ sourceDir, destRoot, version: '1.0.0', daemonRuntimeDir }) + + // The 1.0.0 daemon survived the update and still loads its 1.0.0 runtime dir + // on demand — deleting it would strand the running daemon. + writeDaemonPidFile(daemonRuntimeDir, 18, { pid: 4321, startedAtMs: null, appVersion: '1.0.0' }) + + const destDir = ensureRelocatedRuntime({ + sourceDir, + destRoot, + version: '2.0.0', + daemonRuntimeDir, + isDaemonPidAlive: aliveFor(4321) + }) + + expect(destDir).toBe(join(destRoot, '2.0.0')) + expect(existsSync(join(destRoot, '1.0.0', 'runtime.exe'))).toBe(true) + expect(existsSync(join(destRoot, '2.0.0', 'runtime.exe'))).toBe(true) + }) + + it('fails open when the source dir is missing', () => { + expect( + ensureRelocatedRuntime({ + sourceDir: join(tempDir, 'does-not-exist'), + destRoot: join(tempDir, 'dest'), + version: '1.2.3', + daemonRuntimeDir: join(tempDir, 'daemon') + }) + ).toBeNull() + }) +}) diff --git a/src/main/pty/install-dir-runtime-relocation.ts b/src/main/pty/install-dir-runtime-relocation.ts new file mode 100644 index 000000000..21ce82fd0 --- /dev/null +++ b/src/main/pty/install-dir-runtime-relocation.ts @@ -0,0 +1,145 @@ +import { + copyFileSync, + existsSync, + mkdirSync, + readdirSync, + readFileSync, + rmSync, + writeFileSync +} from 'node:fs' +import { join } from 'node:path' +import { parseDaemonPidFile, startTimeMatches } from '../daemon/daemon-health' + +/** + * Relocates a per-version runtime directory out of the app install directory + * into userData, and reclaims old version dirs no live daemon still pins. + * + * Why: the Windows NSIS update installer force-closes every process (and, for + * the terminal runtime, every native binary loaded from a process) whose image + * lives under the install directory before replacing files. Copying the runtime + * to a version-keyed userData dir takes it out of that kill zone so the detached + * daemon and its PTYs survive updates. Both node-pty's native runtime and the + * daemon's own Node host share this machinery. + */ +export const RELOCATION_COMPLETE_MARKER = '.relocation-complete' + +function copyRuntimeTree(sourceDir: string, destDir: string): void { + mkdirSync(destDir, { recursive: true }) + for (const entry of readdirSync(sourceDir, { withFileTypes: true })) { + const sourcePath = join(sourceDir, entry.name) + const destPath = join(destDir, entry.name) + if (entry.isDirectory()) { + copyRuntimeTree(sourcePath, destPath) + } else if (entry.isFile() && !/\.pdb$/i.test(entry.name)) { + copyFileSync(sourcePath, destPath) + } + } +} + +export type IsDaemonPidAlive = (pid: number, startedAtMs: number | null) => boolean + +function isDaemonPidAliveDefault(pid: number, startedAtMs: number | null): boolean { + try { + process.kill(pid, 0) + } catch { + return false + } + return startTimeMatches(pid, startedAtMs) +} + +/** + * Runtime version dirs still claimed by a live daemon, read from the daemon + * pid files under `daemonRuntimeDir` (`daemon-v.pid`). + * + * Why: a daemon deliberately survives app updates, and it loads its version's + * relocated runtime (node-pty binaries, and — once relocated — its own Node + * host) on demand. Each pid file records that `appVersion`, so a live daemon's + * version dir must never be reclaimed while the process is still running. + */ +export function collectInUseRuntimeVersions( + daemonRuntimeDir: string, + isPidAlive: IsDaemonPidAlive = isDaemonPidAliveDefault +): Set { + const inUse = new Set() + let entries + try { + entries = readdirSync(daemonRuntimeDir, { withFileTypes: true }) + } catch { + return inUse + } + for (const entry of entries) { + if (!entry.isFile() || !/^daemon-v\d+\.pid$/.test(entry.name)) { + continue + } + let parsed + try { + parsed = parseDaemonPidFile(readFileSync(join(daemonRuntimeDir, entry.name), 'utf8')) + } catch { + continue + } + // appVersion null => a pre-relocation daemon that loads from the install + // dir and thus pins no version dir here. + if (parsed && parsed.appVersion !== null && isPidAlive(parsed.pid, parsed.startedAtMs)) { + inUse.add(parsed.appVersion) + } + } + return inUse +} + +function removeStaleRuntimeVersions( + destRoot: string, + keepVersion: string, + inUseVersions: ReadonlySet +): void { + let entries + try { + entries = readdirSync(destRoot, { withFileTypes: true }) + } catch { + return + } + for (const entry of entries) { + if (!entry.isDirectory() || entry.name === keepVersion || inUseVersions.has(entry.name)) { + continue + } + // Why: Windows maps the daemon's binaries with FILE_SHARE_DELETE, so a + // rename/delete succeeds even while a surviving daemon still needs them — + // a rename heuristic would delete in-use dirs and strand the daemon. Only + // dirs no live daemon claims via its pid file are safe to remove. + try { + rmSync(join(destRoot, entry.name), { recursive: true, force: true }) + } catch { + // Still locked or already gone — retry on a future launch. + } + } +} + +/** + * Ensures `sourceDir` is copied to `destRoot/version` (once, guarded by a + * completion marker) and reclaims sibling version dirs no live daemon pins. + * Returns the version dir, or null on any failure (callers fail open to + * loading from the install dir — the pre-relocation behavior). + */ +export function ensureRelocatedRuntime(options: { + sourceDir: string + destRoot: string + version: string + daemonRuntimeDir: string + isDaemonPidAlive?: IsDaemonPidAlive +}): string | null { + const { sourceDir, destRoot, version, daemonRuntimeDir, isDaemonPidAlive } = options + const destDir = join(destRoot, version) + try { + // Why: the marker is written only after a full copy, so a crash mid-copy + // leaves no marker and the next launch redoes the copy from scratch. + if (!existsSync(join(destDir, RELOCATION_COMPLETE_MARKER))) { + rmSync(destDir, { recursive: true, force: true }) + copyRuntimeTree(sourceDir, destDir) + writeFileSync(join(destDir, RELOCATION_COMPLETE_MARKER), '') + } + const inUseVersions = collectInUseRuntimeVersions(daemonRuntimeDir, isDaemonPidAlive) + removeStaleRuntimeVersions(destRoot, version, inUseVersions) + return destDir + } catch { + return null + } +} diff --git a/src/main/pty/node-pty-runtime-relocation.test.ts b/src/main/pty/node-pty-runtime-relocation.test.ts index 5b4ce4886..e5f9fbd61 100644 --- a/src/main/pty/node-pty-runtime-relocation.test.ts +++ b/src/main/pty/node-pty-runtime-relocation.test.ts @@ -1,4 +1,4 @@ -import { mkdtempSync, mkdirSync, readFileSync, rmSync, writeFileSync, existsSync } from 'node:fs' +import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from 'node:fs' import { createRequire } from 'node:module' import os from 'node:os' import { dirname, join } from 'node:path' @@ -12,13 +12,8 @@ vi.mock('electron', () => ({ } })) -import { getDaemonPidPath, serializeDaemonPidFile } from '../daemon/daemon-spawner' -import type { DaemonPidFile } from '../daemon/daemon-spawner' -import { - collectInUseRuntimeVersions, - ensureRelocatedNodePtyNativeRuntime, - resolveNodePtyNativeSourceDir -} from './node-pty-runtime-relocation' +import { ensureRelocatedRuntime } from './install-dir-runtime-relocation' +import { resolveNodePtyNativeSourceDir } from './node-pty-runtime-relocation' let tempDir: string @@ -34,34 +29,6 @@ afterEach(() => { } catch {} }) -function seedSourceDir(dir: string): void { - mkdirSync(join(dir, 'conpty'), { recursive: true }) - writeFileSync(join(dir, 'conpty.node'), 'binding') - writeFileSync(join(dir, 'conpty_console_list.node'), 'listing') - writeFileSync(join(dir, 'pty.node'), 'winpty-binding') - writeFileSync(join(dir, 'winpty-agent.exe'), 'agent') - writeFileSync(join(dir, 'conpty.pdb'), 'symbols') - writeFileSync(join(dir, 'conpty', 'conpty.dll'), 'dll') - writeFileSync(join(dir, 'conpty', 'OpenConsole.exe'), 'console-host') -} - -// Writes a daemon pid file exactly as the daemon does (userData/daemon/daemon-v.pid). -function writeDaemonPidFile( - daemonRuntimeDir: string, - protocolVersion: number, - pidFile: DaemonPidFile -): void { - mkdirSync(daemonRuntimeDir, { recursive: true }) - writeFileSync(getDaemonPidPath(daemonRuntimeDir, protocolVersion), serializeDaemonPidFile(pidFile)) -} - -// Deterministic liveness that treats the given pids as running, so tests never -// depend on the host's real process table. -function aliveFor(...alivePids: number[]): (pid: number) => boolean { - const set = new Set(alivePids) - return (pid) => set.has(pid) -} - describe('resolveNodePtyNativeSourceDir', () => { it('prefers the rebuilt build/Release binding over prebuilds', () => { const pkg = join(tempDir, 'node-pty') @@ -86,204 +53,6 @@ describe('resolveNodePtyNativeSourceDir', () => { }) }) -describe('collectInUseRuntimeVersions', () => { - it('returns an empty set when the daemon dir does not exist', () => { - expect(collectInUseRuntimeVersions(join(tempDir, 'no-daemon-dir')).size).toBe(0) - }) - - it('collects the app version a live daemon pins', () => { - const daemonDir = join(tempDir, 'daemon') - writeDaemonPidFile(daemonDir, 18, { pid: 4321, startedAtMs: null, appVersion: '1.4.124-rc.1' }) - - const inUse = collectInUseRuntimeVersions(daemonDir, aliveFor(4321)) - - expect([...inUse]).toEqual(['1.4.124-rc.1']) - }) - - it('omits the version of a daemon whose pid is dead', () => { - const daemonDir = join(tempDir, 'daemon') - writeDaemonPidFile(daemonDir, 18, { pid: 4321, startedAtMs: null, appVersion: '1.4.124-rc.1' }) - - const inUse = collectInUseRuntimeVersions(daemonDir, aliveFor(/* nobody */)) - - expect(inUse.size).toBe(0) - }) - - it('collects one version per live daemon across protocol versions', () => { - const daemonDir = join(tempDir, 'daemon') - writeDaemonPidFile(daemonDir, 18, { pid: 100, startedAtMs: null, appVersion: '1.4.124-rc.1' }) - writeDaemonPidFile(daemonDir, 19, { pid: 200, startedAtMs: null, appVersion: '1.4.124-rc.2' }) - - const inUse = collectInUseRuntimeVersions(daemonDir, aliveFor(100, 200)) - - expect([...inUse].sort()).toEqual(['1.4.124-rc.1', '1.4.124-rc.2']) - }) - - it('ignores a daemon whose pid file records no app version', () => { - const daemonDir = join(tempDir, 'daemon') - // Pre-relocation daemon: JSON pid file without appVersion. - writeDaemonPidFile(daemonDir, 18, { pid: 4321, startedAtMs: null }) - // Legacy daemon: bare-integer pid file (appVersion parses to null). - mkdirSync(daemonDir, { recursive: true }) - writeFileSync(getDaemonPidPath(daemonDir, 17), '9999') - - const inUse = collectInUseRuntimeVersions(daemonDir, aliveFor(4321, 9999)) - - expect(inUse.size).toBe(0) - }) - - it('ignores non-pid files in the daemon dir', () => { - const daemonDir = join(tempDir, 'daemon') - mkdirSync(daemonDir, { recursive: true }) - writeFileSync(join(daemonDir, 'daemon-v18.token'), 'secret') - writeFileSync(join(daemonDir, 'daemon-v18.sock'), 'x') - writeFileSync(join(daemonDir, 'notes.txt'), 'x') - - expect(collectInUseRuntimeVersions(daemonDir, aliveFor(1, 2, 3)).size).toBe(0) - }) - - it('skips a malformed pid file without throwing', () => { - const daemonDir = join(tempDir, 'daemon') - mkdirSync(daemonDir, { recursive: true }) - writeFileSync(getDaemonPidPath(daemonDir, 18), 'not-a-pid-at-all') - writeDaemonPidFile(daemonDir, 19, { pid: 200, startedAtMs: null, appVersion: '2.0.0' }) - - const inUse = collectInUseRuntimeVersions(daemonDir, aliveFor(200)) - - expect([...inUse]).toEqual(['2.0.0']) - }) - - it('treats the current process as alive under the default liveness probe', () => { - const daemonDir = join(tempDir, 'daemon') - // startedAtMs null so startTimeMatches short-circuits true on every platform. - writeDaemonPidFile(daemonDir, 18, { - pid: process.pid, - startedAtMs: null, - appVersion: '3.0.0' - }) - - expect([...collectInUseRuntimeVersions(daemonDir)]).toEqual(['3.0.0']) - }) -}) - -describe('ensureRelocatedNodePtyNativeRuntime', () => { - it('copies the runtime tree (without symbols) and returns the version dir', () => { - const sourceDir = join(tempDir, 'source') - seedSourceDir(sourceDir) - const destRoot = join(tempDir, 'dest') - - const destDir = ensureRelocatedNodePtyNativeRuntime({ - sourceDir, - destRoot, - version: '1.2.3', - daemonRuntimeDir: join(tempDir, 'daemon') - }) - - expect(destDir).toBe(join(destRoot, '1.2.3')) - expect(readFileSync(join(destDir!, 'conpty.node'), 'utf8')).toBe('binding') - expect(readFileSync(join(destDir!, 'conpty', 'OpenConsole.exe'), 'utf8')).toBe('console-host') - expect(readFileSync(join(destDir!, 'conpty', 'conpty.dll'), 'utf8')).toBe('dll') - expect(existsSync(join(destDir!, 'conpty.pdb'))).toBe(false) - }) - - it('skips recopying once the completion marker exists', () => { - const sourceDir = join(tempDir, 'source') - seedSourceDir(sourceDir) - const destRoot = join(tempDir, 'dest') - ensureRelocatedNodePtyNativeRuntime({ - sourceDir, - destRoot, - version: '1.2.3', - daemonRuntimeDir: join(tempDir, 'daemon') - }) - - writeFileSync(join(sourceDir, 'conpty.node'), 'changed-after-first-copy') - const destDir = ensureRelocatedNodePtyNativeRuntime({ - sourceDir, - destRoot, - version: '1.2.3', - daemonRuntimeDir: join(tempDir, 'daemon') - }) - - expect(readFileSync(join(destDir!, 'conpty.node'), 'utf8')).toBe('binding') - }) - - it('redoes an interrupted copy that has no completion marker', () => { - const sourceDir = join(tempDir, 'source') - seedSourceDir(sourceDir) - const destRoot = join(tempDir, 'dest') - mkdirSync(join(destRoot, '1.2.3'), { recursive: true }) - writeFileSync(join(destRoot, '1.2.3', 'conpty.node'), 'torn partial copy') - - const destDir = ensureRelocatedNodePtyNativeRuntime({ - sourceDir, - destRoot, - version: '1.2.3', - daemonRuntimeDir: join(tempDir, 'daemon') - }) - - expect(readFileSync(join(destDir!, 'conpty.node'), 'utf8')).toBe('binding') - }) - - it('reclaims a stale version dir once no live daemon pins it', () => { - const sourceDir = join(tempDir, 'source') - seedSourceDir(sourceDir) - const destRoot = join(tempDir, 'dest') - const daemonRuntimeDir = join(tempDir, 'daemon') - ensureRelocatedNodePtyNativeRuntime({ sourceDir, destRoot, version: '1.0.0', daemonRuntimeDir }) - - // The 1.0.0 daemon exited, so its pid is dead — nothing pins 1.0.0. - writeDaemonPidFile(daemonRuntimeDir, 18, { pid: 4321, startedAtMs: null, appVersion: '1.0.0' }) - - const destDir = ensureRelocatedNodePtyNativeRuntime({ - sourceDir, - destRoot, - version: '2.0.0', - daemonRuntimeDir, - isDaemonPidAlive: aliveFor(/* 4321 is dead */) - }) - - expect(destDir).toBe(join(destRoot, '2.0.0')) - expect(existsSync(join(destRoot, '1.0.0'))).toBe(false) - expect(existsSync(join(destRoot, '2.0.0', 'conpty.node'))).toBe(true) - }) - - it('preserves a stale version dir a surviving daemon still pins', () => { - const sourceDir = join(tempDir, 'source') - seedSourceDir(sourceDir) - const destRoot = join(tempDir, 'dest') - const daemonRuntimeDir = join(tempDir, 'daemon') - ensureRelocatedNodePtyNativeRuntime({ sourceDir, destRoot, version: '1.0.0', daemonRuntimeDir }) - - // The 1.0.0 daemon survived the update and still loads conpty.dll from its - // 1.0.0 runtime dir on every new spawn — deleting it would strand it. - writeDaemonPidFile(daemonRuntimeDir, 18, { pid: 4321, startedAtMs: null, appVersion: '1.0.0' }) - - const destDir = ensureRelocatedNodePtyNativeRuntime({ - sourceDir, - destRoot, - version: '2.0.0', - daemonRuntimeDir, - isDaemonPidAlive: aliveFor(4321) - }) - - expect(destDir).toBe(join(destRoot, '2.0.0')) - expect(existsSync(join(destRoot, '1.0.0', 'conpty.node'))).toBe(true) - expect(existsSync(join(destRoot, '2.0.0', 'conpty.node'))).toBe(true) - }) - - it('fails open when the source dir is missing', () => { - expect( - ensureRelocatedNodePtyNativeRuntime({ - sourceDir: join(tempDir, 'does-not-exist'), - destRoot: join(tempDir, 'dest'), - version: '1.2.3', - daemonRuntimeDir: join(tempDir, 'daemon') - }) - ).toBeNull() - }) -}) - // Loads the real win32 conpty binding, so it cannot run on other platforms. describe.runIf(process.platform === 'win32')('patched node-pty loader override', () => { it('loads the conpty binding from ORCA_NODE_PTY_NATIVE_DIR', () => { @@ -292,7 +61,7 @@ describe.runIf(process.platform === 'win32')('patched node-pty loader override', const sourceDir = resolveNodePtyNativeSourceDir(nodePtyPackageDir) expect(sourceDir).not.toBeNull() - const destDir = ensureRelocatedNodePtyNativeRuntime({ + const destDir = ensureRelocatedRuntime({ sourceDir: sourceDir!, destRoot: join(tempDir, 'relocated-runtime'), version: 'loader-test', diff --git a/src/main/pty/node-pty-runtime-relocation.ts b/src/main/pty/node-pty-runtime-relocation.ts index 0e8600662..7c1631d78 100644 --- a/src/main/pty/node-pty-runtime-relocation.ts +++ b/src/main/pty/node-pty-runtime-relocation.ts @@ -1,16 +1,8 @@ -import { - copyFileSync, - existsSync, - mkdirSync, - readdirSync, - readFileSync, - rmSync, - writeFileSync -} from 'node:fs' +import { existsSync } from 'node:fs' import { createRequire } from 'node:module' import { dirname, join } from 'node:path' import { app } from 'electron' -import { parseDaemonPidFile, startTimeMatches } from '../daemon/daemon-health' +import { ensureRelocatedRuntime } from './install-dir-runtime-relocation' /** * Relocates node-pty's Windows native runtime (conpty.node, conpty.dll, @@ -26,8 +18,6 @@ import { parseDaemonPidFile, startTimeMatches } from '../daemon/daemon-health' */ export const NODE_PTY_NATIVE_DIR_ENV_VAR = 'ORCA_NODE_PTY_NATIVE_DIR' -const RELOCATION_COMPLETE_MARKER = '.relocation-complete' - export function resolveNodePtyNativeSourceDir(nodePtyPackageDir: string): string | null { // Packaged builds carry the rebuilt binding in build/Release; dev installs // (and the forced-relocation test path) load from prebuilds. @@ -43,124 +33,6 @@ export function resolveNodePtyNativeSourceDir(nodePtyPackageDir: string): string return null } -function copyRuntimeTree(sourceDir: string, destDir: string): void { - mkdirSync(destDir, { recursive: true }) - for (const entry of readdirSync(sourceDir, { withFileTypes: true })) { - const sourcePath = join(sourceDir, entry.name) - const destPath = join(destDir, entry.name) - if (entry.isDirectory()) { - copyRuntimeTree(sourcePath, destPath) - } else if (entry.isFile() && !/\.pdb$/i.test(entry.name)) { - copyFileSync(sourcePath, destPath) - } - } -} - -type IsDaemonPidAlive = (pid: number, startedAtMs: number | null) => boolean - -function isDaemonPidAliveDefault(pid: number, startedAtMs: number | null): boolean { - try { - process.kill(pid, 0) - } catch { - return false - } - return startTimeMatches(pid, startedAtMs) -} - -/** - * Runtime version dirs still claimed by a live daemon, read from the daemon - * pid files under `daemonRuntimeDir` (`daemon-v.pid`). - * - * Why: a daemon deliberately survives app updates, and it was forked with - * ORCA_NODE_PTY_NATIVE_DIR pinned to its own app-version runtime dir — which - * it reloads conpty.dll from on every new spawn. Each pid file records that - * `appVersion`, so a live daemon's version dir must never be reclaimed. - */ -export function collectInUseRuntimeVersions( - daemonRuntimeDir: string, - isPidAlive: IsDaemonPidAlive = isDaemonPidAliveDefault -): Set { - const inUse = new Set() - let entries - try { - entries = readdirSync(daemonRuntimeDir, { withFileTypes: true }) - } catch { - return inUse - } - for (const entry of entries) { - if (!entry.isFile() || !/^daemon-v\d+\.pid$/.test(entry.name)) { - continue - } - let parsed - try { - parsed = parseDaemonPidFile(readFileSync(join(daemonRuntimeDir, entry.name), 'utf8')) - } catch { - continue - } - // appVersion null => a pre-relocation daemon that loads from the install - // dir and thus pins no version dir here. - if (parsed && parsed.appVersion !== null && isPidAlive(parsed.pid, parsed.startedAtMs)) { - inUse.add(parsed.appVersion) - } - } - return inUse -} - -function removeStaleRuntimeVersions( - destRoot: string, - keepVersion: string, - inUseVersions: ReadonlySet -): void { - let entries - try { - entries = readdirSync(destRoot, { withFileTypes: true }) - } catch { - return - } - for (const entry of entries) { - if (!entry.isDirectory() || entry.name === keepVersion || inUseVersions.has(entry.name)) { - continue - } - // Why: Windows maps the daemon's conpty.dll/OpenConsole.exe with - // FILE_SHARE_DELETE, so a rename/delete succeeds even while a surviving - // daemon still needs them — the old rename heuristic deleted in-use dirs - // and stranded the daemon (new spawn -> conpty.dll ERROR_PATH_NOT_FOUND). - // Only dirs no live daemon claims via its pid file are safe to remove. - try { - rmSync(join(destRoot, entry.name), { recursive: true, force: true }) - } catch { - // Still locked or already gone — retry on a future launch. - } - } -} - -export function ensureRelocatedNodePtyNativeRuntime(options: { - sourceDir: string - destRoot: string - version: string - daemonRuntimeDir: string - isDaemonPidAlive?: IsDaemonPidAlive -}): string | null { - const { sourceDir, destRoot, version, daemonRuntimeDir, isDaemonPidAlive } = options - const destDir = join(destRoot, version) - try { - // Why: the marker is written only after a full copy, so a crash mid-copy - // leaves no marker and the next launch redoes the copy from scratch. - if (!existsSync(join(destDir, RELOCATION_COMPLETE_MARKER))) { - rmSync(destDir, { recursive: true, force: true }) - copyRuntimeTree(sourceDir, destDir) - writeFileSync(join(destDir, RELOCATION_COMPLETE_MARKER), '') - } - const inUseVersions = collectInUseRuntimeVersions(daemonRuntimeDir, isDaemonPidAlive) - removeStaleRuntimeVersions(destRoot, version, inUseVersions) - return destDir - } catch { - // Fail open: node-pty keeps loading from the install dir, which is the - // pre-relocation behavior (sessions then don't survive updates). - return null - } -} - export function installRelocatedNodePtyNativeRuntime(): void { if (process.platform !== 'win32') { return @@ -184,7 +56,7 @@ export function installRelocatedNodePtyNativeRuntime(): void { return } const userData = app.getPath('userData') - const destDir = ensureRelocatedNodePtyNativeRuntime({ + const destDir = ensureRelocatedRuntime({ sourceDir, destRoot: join(userData, 'node-pty-runtime'), version: app.getVersion(),