From f7033e0e70b9679469ca3d40a61f75d59a8d7e00 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Mon, 24 Aug 2026 21:43:22 -0700 Subject: [PATCH] build(windows): drop the packaged node-pty prebuild that can silently replace the patch (#16350) * build(windows): drop the packaged node-pty prebuild that can silently replace the patch node-pty's loader tries build/Release, then build/Debug, then prebuilds/-, and swallows every failure in between. Windows packaging ships both the source build and the prebuild, and only the source build carries Orca's job-object exports (listJobProcessIds, terminateJob, assignCurrentProcessToJob). So an ABI mismatch, a truncated file, or an AV quarantine of build/Release/conpty.node degrades the shipped app to the UNPATCHED prebuild: PTY teardown silently falls back to guessing by PID ancestry, with no error anywhere. That is the failure mode that made #16059 hard to see -- an install that looks fine and quietly cannot own a PTY tree. Removing the fallback turns a silent downgrade into a loud load failure. Scoped narrowly: only win32, and only when the source build is actually present, so a build that legitimately has no build/Release keeps something loadable. macOS and Linux prebuilds are untouched -- they have no patched export to lose. Refs #16059. * fix: delete only the stale conpty fallback, not the whole prebuilds tree Review caught a P0 in the first version of this change, and it was the same defect the PR exists to prevent, pointed at a different target. Orca's own patch removes the `conpty_console_list` and winpty `pty` gyp targets, so a Windows source build emits conpty.node and nothing else. conpty_console_list.node, pty.node, winpty.dll and winpty-agent.exe therefore exist ONLY in prebuilds/. Deleting the tree removed them: - the forked console-list agent throws at require, and its caller resolves null with silent: true, so console-membership probing dies with no log anywhere -- a new silent degradation, in a PR whose thesis is "make it loud"; - node-pty still selects winpty below Windows build 18309, so PTY spawn would fail outright on Server 2019 / Win10 LTSC 2019. Now removes only prebuilds/win32-/conpty{.node,.pdb}, and only when electronArch matches the host arch -- a cross-arch package copies the host's build/Release, so its presence does not mean it matches the target, and deleting the target-arch prebuild would remove the only loadable binary. The old fixture wrote just conpty.node, so it could not see any of this. It now seeds a realistic prebuilds directory, and four tests assert each sibling survives; all four fail against the broad delete. Credit: review counsel. --- config/packaged-runtime-node-modules.cjs | 29 +++++ .../packaged-node-pty-prebuild-prune.test.mjs | 105 ++++++++++++++++++ 2 files changed, 134 insertions(+) create mode 100644 config/scripts/packaged-node-pty-prebuild-prune.test.mjs diff --git a/config/packaged-runtime-node-modules.cjs b/config/packaged-runtime-node-modules.cjs index 488b1bec409..88dccc5746d 100644 --- a/config/packaged-runtime-node-modules.cjs +++ b/config/packaged-runtime-node-modules.cjs @@ -348,6 +348,35 @@ function prunePackagedNodePty(resourcesDir, electronPlatformName, electronArch) return } + // Why delete only conpty.node: node-pty's loader tries build/Release, then + // build/Debug, then prebuilds/-, swallowing every failure in + // between. Only the source build carries Orca's job-object exports, so an ABI + // mismatch or an AV quarantine of build/Release/conpty.node would silently + // fall through to the UNPATCHED prebuild -- teardown back to guessing by PID + // ancestry, with no error anywhere. + // + // Why NOT the whole prebuilds/ tree: Orca's own patch deletes the + // `conpty_console_list` and winpty `pty` gyp targets, so a Windows source + // build emits conpty.node and nothing else. conpty_console_list.node, + // pty.node, winpty.dll and winpty-agent.exe exist ONLY here. Removing them + // silently kills console-membership probing (the forked agent throws at + // require, and its caller resolves null with silent: true), and removes the + // winpty backend that node-pty still selects below Windows build 18309. + // + // Why the arch check: a cross-arch package copies the host's build/Release, + // so its mere presence does not mean it matches electronArch -- deleting the + // target-arch prebuild would then remove the only loadable binary. + if ( + electronPlatformName === 'win32' && + electronArch === process.arch && + existsSync(join(nodePtyDir, 'build', 'Release', 'conpty.node')) + ) { + const prebuildDir = join(nodePtyDir, 'prebuilds', `win32-${electronArch}`) + for (const staleFallback of ['conpty.node', 'conpty.pdb']) { + rmSync(join(prebuildDir, staleFallback), { force: true }) + } + } + const allowedPrebuildPrefix = NODE_PTY_PREBUILD_PREFIX_BY_PLATFORM[electronPlatformName] if (allowedPrebuildPrefix) { pruneNodePtyNativeDirectories( diff --git a/config/scripts/packaged-node-pty-prebuild-prune.test.mjs b/config/scripts/packaged-node-pty-prebuild-prune.test.mjs new file mode 100644 index 00000000000..c0b24b8a8a2 --- /dev/null +++ b/config/scripts/packaged-node-pty-prebuild-prune.test.mjs @@ -0,0 +1,105 @@ +import { existsSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { createRequire } from 'node:module' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' + +const require = createRequire(import.meta.url) +const { prunePackagedNodePty } = require('../packaged-runtime-node-modules.cjs') + +/** + * node-pty's loader tries build/Release, then build/Debug, then + * prebuilds/-, swallowing failures in between. Only the source + * build carries Orca's job-object exports, so leaving the prebuilt conpty.node + * beside it means an ABI mismatch or an AV quarantine silently downgrades the + * app to a binary that cannot own a PTY tree. + * + * The siblings must survive: Orca's patch deletes the `conpty_console_list` and + * winpty `pty` gyp targets, so those binaries exist nowhere but here. + */ +describe('prunePackagedNodePty: the Windows conpty fallback', () => { + let resources + const HOST_ARCH = process.arch + + const nodePty = () => join(resources, 'node_modules', 'node-pty') + const write = (relative) => { + const target = join(nodePty(), relative) + mkdirSync(join(target, '..'), { recursive: true }) + writeFileSync(target, 'x') + } + const prebuilt = (name, arch = HOST_ARCH) => join(nodePty(), 'prebuilds', `win32-${arch}`, name) + + /** What a real packaged win32 prebuilds/ directory holds. */ + const SIBLINGS = ['conpty_console_list.node', 'pty.node', 'winpty.dll', 'winpty-agent.exe'] + + const seedWindowsTree = (arch = HOST_ARCH) => { + write(join('build', 'Release', 'conpty.node')) + write(join('third_party', 'conpty', 'v1', `win10-${arch}`, 'conpty.dll')) + write(join('third_party', 'conpty', 'v1', `win10-${arch}`, 'OpenConsole.exe')) + write(join('prebuilds', `win32-${arch}`, 'conpty.node')) + for (const sibling of SIBLINGS) { + write(join('prebuilds', `win32-${arch}`, sibling)) + } + } + + beforeEach(() => { + resources = mkdtempSync(join(tmpdir(), 'orca-prune-')) + }) + afterEach(() => { + rmSync(resources, { recursive: true, force: true }) + }) + + it('removes the stale conpty fallback when the source build is present', () => { + seedWindowsTree() + + prunePackagedNodePty(resources, 'win32', HOST_ARCH) + + expect(existsSync(join(nodePty(), 'build', 'Release', 'conpty.node'))).toBe(true) + expect(existsSync(prebuilt('conpty.node'))).toBe(false) + }) + + it.each(SIBLINGS)('keeps %s, which exists nowhere else in a packaged build', (sibling) => { + // Orca's patch removes the conpty_console_list and winpty gyp targets, so a + // Windows source build never produces these. Deleting them kills console + // membership silently and breaks PTY spawn below Windows build 18309. + seedWindowsTree() + + prunePackagedNodePty(resources, 'win32', HOST_ARCH) + + expect(existsSync(prebuilt(sibling))).toBe(true) + }) + + it('keeps the fallback when there is no source build to prefer', () => { + write(join('prebuilds', `win32-${HOST_ARCH}`, 'conpty.node')) + write(join('third_party', 'conpty', 'v1', `win10-${HOST_ARCH}`, 'conpty.dll')) + write(join('third_party', 'conpty', 'v1', `win10-${HOST_ARCH}`, 'OpenConsole.exe')) + + prunePackagedNodePty(resources, 'win32', HOST_ARCH) + + expect(existsSync(prebuilt('conpty.node'))).toBe(true) + }) + + it('keeps the fallback on a cross-arch package, where build/Release is the host arch', () => { + // electron-builder --win --arm64 on an x64 host copies an x64 + // build/Release; deleting the arm64 prebuild would remove the only binary + // the shipped app could load. + const target = HOST_ARCH === 'arm64' ? 'x64' : 'arm64' + seedWindowsTree(target) + + prunePackagedNodePty(resources, 'win32', target) + + expect(existsSync(prebuilt('conpty.node', target))).toBe(true) + }) + + it.each([ + ['darwin', 'arm64'], + ['linux', 'x64'] + ])('leaves %s prebuilds alone', (platform, arch) => { + write(join('build', 'Release', 'conpty.node')) + write(join('prebuilds', `${platform}-${arch}`, 'pty.node')) + + prunePackagedNodePty(resources, platform, arch) + + expect(existsSync(join(nodePty(), 'prebuilds', `${platform}-${arch}`, 'pty.node'))).toBe(true) + }) +})