build(windows): refuse unpatched node-pty prebuilds

Merged after clean CI, Windows packaging verification, and readiness review.

.github/workflows/pr.yml

@@ -642,6 +642,7 @@ jobs:
       - name: Test Windows-specific boundaries
         run: >-
           pnpm exec vitest run --config config/vitest.config.ts
+          config/scripts/rebuild-native-deps.test.mjs
           src/main/providers/windows-conpty-wide-char-duplication.node-pty.test.ts
           src/main/providers/pty-repaint-wide-char-buffer.node-pty.test.ts
           src/shared/child-process/windows-command-line.win32.test.ts

config/electron-builder.config.cjs

@@ -14,6 +14,9 @@ const {
 const { verifyLinuxGlibcFloor } = require('./scripts/verify-linux-glibc-floor.cjs')
 const { writeMacBuildCompatibility } = require('./scripts/mac-build-compatibility.cjs')
 const { verifyPackagedPluginResources } = require('./scripts/verify-packaged-plugin-resources.cjs')
+const {
+  verifyPackagedNodePtyJobOwnership
+} = require('./scripts/verify-packaged-node-pty-job-ownership.cjs')
 const { verifySkillsCliRuntime } = require('./scripts/verify-skills-cli-runtime.cjs')
 
 // Why: dev-channel builds must carry the *release* identity — same bundle id,
@@ -266,6 +269,13 @@ module.exports = {
     const archEnumByNodeArch = { ia32: 0, x64: 1, armv7l: 2, arm64: 3 }
     const hostArchEnum = archEnumByNodeArch[process.arch]
     const canExecuteTargetArch = context.arch === hostArchEnum || context.arch === 4
+    if (context.electronPlatformName === 'win32') {
+      if (process.platform === 'win32' && canExecuteTargetArch) {
+        verifyPackagedNodePtyJobOwnership(resourcesDir)
+      } else {
+        console.log('[verify-packaged-node-pty] skipped cross-platform or cross-arch package')
+      }
+    }
     verifySkillsCliRuntime(join(resourcesDir, 'app.asar.unpacked', 'out'), resourcesDir, {
       executeCommands: canExecuteTargetArch
     })

config/patches/node-pty@1.1.0.patch

@@ -2,7 +2,12 @@ diff --git a/binding.gyp b/binding.gyp
 index 5f63978b07ab50aaf7523219a2170ec737a6b5db..bbd9e06136e8922f40b5779e35d4fc835f1479ab 100644
 --- a/binding.gyp
 +++ b/binding.gyp
-@@ -5,9 +5,6 @@
+@@ -1,13 +1,10 @@
+ {
+   'target_defaults': {
+     'dependencies': [
+-      "<!(node -p \"require('node-addon-api').targets\"):node_addon_api_except",
++      "<!(node -p \"require.resolve('node-addon-api/node_addon_api.gyp')\"):node_addon_api_except",
      ],
      'conditions': [
        ['OS=="win"', {
@@ -12,6 +17,39 @@ index 5f63978b07ab50aaf7523219a2170ec737a6b5db..bbd9e06136e8922f40b5779e35d4fc83
          'msvs_settings': {
              'VCCLCompilerTool': {
                'AdditionalOptions': [
+@@ -42,32 +39,6 @@
+             '-lshlwapi'
+           ],
+         },
+-        {
+-          'target_name': 'conpty_console_list',
+-          'sources' : [
+-            'src/win/conpty_console_list.cc'
+-          ],
+-        },
+-        {
+-          'target_name': 'pty',
+-          'include_dirs' : [
+-            '<!(node -p "require(\'node-addon-api\').include_dir")',
+-            'deps/winpty/src/include',
+-          ],
+-          # Disabled due to winpty
+-          'msvs_disabled_warnings': [ 4506, 4530 ],
+-          'dependencies' : [
+-            'deps/winpty/src/winpty.gyp:winpty-agent',
+-            'deps/winpty/src/winpty.gyp:winpty',
+-          ],
+-          'sources' : [
+-            'src/win/winpty.cc',
+-            'src/win/path_util.cc'
+-          ],
+-          'libraries': [
+-            '-lshlwapi'
+-          ],
+-        }
+       ]
+     }, { # OS!="win"
+       'targets': [
 @@ -88,6 +85,16 @@
                'libraries!': [
                  '-lutil'

config/scripts/ensure-native-runtime-job-ownership.test.mjs

@@ -0,0 +1,67 @@
+import { readFileSync } from 'node:fs'
+import { createRequire } from 'node:module'
+import { describe, expect, it } from 'vitest'
+
+const require = createRequire(import.meta.url)
+const { assertNodePtyJobOwnership } = require('./node-pty-job-ownership.cjs')
+const NODE_PTY_PATCH = readFileSync(
+  new URL('../patches/node-pty@1.1.0.patch', import.meta.url),
+  'utf8'
+)
+
+const PATCHED = {
+  dir: 'build/Release/',
+  module: {
+    listJobProcessIds: () => [],
+    terminateJob: () => true,
+    assignCurrentProcessToJob: () => true
+  }
+}
+const PREBUILD = {
+  dir: 'prebuilds/win32-x64/',
+  module: {
+    startProcess: () => {},
+    connect: () => {},
+    resize: () => {},
+    clear: () => {},
+    kill: () => {}
+  }
+}
+
+describe('assertNodePtyJobOwnership', () => {
+  it('keeps node-addon-api project paths absolute during Windows source builds', () => {
+    expect(NODE_PTY_PATCH).toContain(
+      `+      "<!(node -p \\"require.resolve('node-addon-api/node_addon_api.gyp')\\"):node_addon_api_except"`
+    )
+  })
+
+  it('leaves the unchanged Windows helper and fallback on their upstream prebuilds', () => {
+    expect(NODE_PTY_PATCH).toContain("-          'target_name': 'conpty_console_list'")
+    expect(NODE_PTY_PATCH).toContain("-          'target_name': 'pty'")
+  })
+
+  it('rejects the prebuild that shipped without the job exports', () => {
+    expect(() =>
+      assertNodePtyJobOwnership({ platform: 'win32', nativeName: 'conpty', native: PREBUILD })
+    ).toThrow(/listJobProcessIds, terminateJob, assignCurrentProcessToJob/)
+  })
+
+  it('names where the bad native came from, so the fix is obvious', () => {
+    expect(() =>
+      assertNodePtyJobOwnership({ platform: 'win32', nativeName: 'conpty', native: PREBUILD })
+    ).toThrow(/prebuilds\/win32-x64/)
+  })
+
+  it('accepts a source build carrying the patch', () => {
+    expect(() =>
+      assertNodePtyJobOwnership({ platform: 'win32', nativeName: 'conpty', native: PATCHED })
+    ).not.toThrow()
+  })
+
+  it.each([
+    ['non-Windows hosts', { platform: 'darwin', nativeName: 'pty' }],
+    ['the pre-ConPTY winpty backend', { platform: 'win32', nativeName: 'pty' }]
+  ])('stays out of the way on %s', (_case, spec) => {
+    expect(() => assertNodePtyJobOwnership({ ...spec, native: PREBUILD })).not.toThrow()
+  })
+})

config/scripts/ensure-native-runtime.mjs

@@ -7,6 +7,7 @@ import { release } from 'node:os'
 import { basename, resolve } from 'node:path'
 
 const require = createRequire(import.meta.url)
+const { assertNodePtyJobOwnership } = require('./node-pty-job-ownership.cjs')
 const scriptPath = import.meta.filename
 const projectDir = resolve(import.meta.dirname, '../..')
 const runtime = readRuntimeArg()
@@ -277,6 +278,7 @@ function loadNodePtyNativeModule() {
   // terminal is created, so require('node-pty') alone can miss ABI mismatches.
   const native = loadNativeModule(nativeName)
   assertNodePtyWindowsConptyRuntime(native?.dir)
+  assertNodePtyJobOwnership({ nativeName, native })
   if (requiresPatchedNodePtySourceBuild() && !isNodePtyReleaseBuildDir(native?.dir)) {
     throw new Error(
       `node-pty resolved to ${native.dir}; expected build/Release so Orca's node-pty patch is active`

config/scripts/ensure-native-runtime.test.mjs

@@ -14,6 +14,9 @@ import { fileURLToPath } from 'node:url'
 import { describe, expect, it } from 'vitest'
 
 const sourceScriptPath = fileURLToPath(new URL('./ensure-native-runtime.mjs', import.meta.url))
+const sourceNodePtyJobOwnershipPath = fileURLToPath(
+  new URL('./node-pty-job-ownership.cjs', import.meta.url)
+)
 
 describe('ensure-native-runtime', () => {
   it('rechecks Node native modules in fresh child processes after rebuilding', () => {
@@ -156,6 +159,10 @@ describe('ensure-native-runtime', () => {
 function mkTempProject() {
   const projectDir = mkdtempSync(join(tmpdir(), 'orca-native-runtime-'))
   mkdirSync(join(projectDir, 'config', 'scripts'), { recursive: true })
+  copyFileSync(
+    sourceNodePtyJobOwnershipPath,
+    join(projectDir, 'config', 'scripts', 'node-pty-job-ownership.cjs')
+  )
   return projectDir
 }
 

config/scripts/node-pty-job-ownership.cjs

@@ -0,0 +1,29 @@
+'use strict'
+
+const NODE_PTY_JOB_EXPORTS = [
+  'listJobProcessIds',
+  'terminateJob',
+  'assignCurrentProcessToJob'
+]
+
+function assertNodePtyJobOwnership({ nativeName, native, platform = process.platform }) {
+  if (platform !== 'win32' || nativeName !== 'conpty') {
+    return
+  }
+  const exported = native?.module ?? native
+  const missing = NODE_PTY_JOB_EXPORTS.filter((name) => typeof exported?.[name] !== 'function')
+  if (missing.length === 0) {
+    return
+  }
+  throw new Error(
+    [
+      `node-pty's conpty native is missing ${missing.join(', ')}.`,
+      `Resolved from: ${native?.dir ?? 'unknown'}`,
+      'That build cannot own a PTY tree, so terminatePtyJob degrades to "unavailable"',
+      'and pane teardown falls back to guessing by PID ancestry.',
+      'Rebuild node-pty from source so config/patches/node-pty@1.1.0.patch applies.'
+    ].join(' ')
+  )
+}
+
+module.exports = { assertNodePtyJobOwnership }

config/scripts/rebuild-native-deps.mjs

@@ -475,9 +475,14 @@ function loadNativeModule(moduleName) {
   }
   if (moduleName === 'node-pty') {
     projectRequire('node-pty')
+    const { assertNodePtyJobOwnership } = projectRequire(
+      './config/scripts/node-pty-job-ownership.cjs'
+    )
     const { loadNativeModule } = projectRequire('node-pty/lib/utils')
-    const native = loadNativeModule(getNodePtyNativeModuleName())
+    const nativeName = getNodePtyNativeModuleName()
+    const native = loadNativeModule(nativeName)
     assertNodePtyWindowsConptyRuntime(native.dir)
+    assertNodePtyJobOwnership({ nativeName, native })
     if (requirePatchedNodePtySourceBuild && !isNodePtyReleaseBuildDir(native.dir)) {
       throw new Error(
         'node-pty resolved to ' +

config/scripts/rebuild-native-deps.test.mjs

@@ -18,6 +18,9 @@ const sourceScriptPath = fileURLToPath(new URL('./rebuild-native-deps.mjs', impo
 const sourceInstallScriptPath = fileURLToPath(
   new URL('./install-electron-package-binary.mjs', import.meta.url)
 )
+const sourceNodePtyJobOwnershipPath = fileURLToPath(
+  new URL('./node-pty-job-ownership.cjs', import.meta.url)
+)
 
 describe('rebuild-native-deps Electron install fallback', () => {
   it('continues non-strict postinstall when Electron retry download fails', () => {
@@ -135,6 +138,7 @@ describe('rebuild-native-deps patched node-pty rebuild', () => {
         writeFakeElectronRebuild(projectDir, { logPathEnv: 'ORCA_REBUILD_TEST_LOG' })
         writeFakeLoadableNodePty(projectDir, { nativeDir: '../build/Release/' })
         writeFakeWindowsRegistry(projectDir)
+        writeFakeWindowsProcessTree(projectDir)
         writeFakeNodePtyConptyPayload(projectDir, process.arch)
 
         const result = runRebuildScript(projectDir, {
@@ -189,6 +193,7 @@ describe('rebuild-native-deps patched node-pty rebuild', () => {
         writeFakeUsableElectronPackage(projectDir, { platform: 'win32' })
         writeFakeElectronRebuild(projectDir, { logPathEnv: 'ORCA_REBUILD_TEST_LOG' })
         writeFakeLoadableNodePty(projectDir)
+        writeFakeWindowsProcessTree(projectDir)
         writeFakeNodePtyConptyPayload(projectDir, process.arch)
 
         const result = runRebuildScript(projectDir, {
@@ -207,6 +212,36 @@ describe('rebuild-native-deps patched node-pty rebuild', () => {
     }
   )
 
+  it.skipIf(process.platform !== 'win32')(
+    'rebuilds a loadable ConPTY native that lacks Orca job ownership',
+    () => {
+      const projectDir = mkTempProject()
+
+      try {
+        const rebuildLogPath = join(projectDir, 'electron-rebuild.log')
+        writeFakeUsableElectronPackage(projectDir, { platform: 'win32' })
+        writeFakeElectronRebuild(projectDir, { logPathEnv: 'ORCA_REBUILD_TEST_LOG' })
+        writeFakeLoadableNodePty(projectDir, { ownsPtyJob: false })
+        writeFakeWindowsRegistry(projectDir)
+        writeFakeWindowsProcessTree(projectDir)
+
+        const result = runRebuildScript(projectDir, {
+          ORCA_REBUILD_TEST_LOG: rebuildLogPath,
+          npm_config_platform: 'win32',
+          npm_config_arch: process.arch
+        })
+
+        expect(result.status, result.stderr).toBe(0)
+        expect(result.stdout).toContain('Rebuilding failed native modules: node-pty')
+        expect(result.stdout).toContain('missing listJobProcessIds')
+        const rebuildCall = JSON.parse(readFileSync(rebuildLogPath, 'utf8').trim())
+        expect(rebuildCall.onlyModules).toEqual(['node-pty'])
+      } finally {
+        rmSync(projectDir, { recursive: true, force: true })
+      }
+    }
+  )
+
   it.skipIf(process.platform === 'win32')(
     'rebuilds when Electron can load node-pty but patched build artifacts are missing',
     () => {
@@ -305,6 +340,10 @@ function mkTempProject() {
     sourceInstallScriptPath,
     join(projectDir, 'config', 'scripts', 'install-electron-package-binary.mjs')
   )
+  copyFileSync(
+    sourceNodePtyJobOwnershipPath,
+    join(projectDir, 'config', 'scripts', 'node-pty-job-ownership.cjs')
+  )
   return projectDir
 }
 
@@ -495,7 +534,10 @@ function writeFakeNodePtyConptyPayload(projectDir, arch) {
   writeFileSync(join(sourceDir, 'OpenConsole.exe'), `OpenConsole.exe ${arch}`)
 }
 
-function writeFakeLoadableNodePty(projectDir, { nativeDir = 'prebuilds/pty' } = {}) {
+function writeFakeLoadableNodePty(
+  projectDir,
+  { nativeDir = 'prebuilds/pty', ownsPtyJob = true } = {}
+) {
   const nodePtyDir = join(projectDir, 'node_modules', 'node-pty')
   mkdirSync(join(nodePtyDir, 'lib'), { recursive: true })
   writeFileSync(join(nodePtyDir, 'index.js'), 'module.exports = {}\n')
@@ -503,7 +545,19 @@ function writeFakeLoadableNodePty(projectDir, { nativeDir = 'prebuilds/pty' } =
     join(nodePtyDir, 'lib', 'utils.js'),
     `
 exports.loadNativeModule = function loadNativeModule(nativeName) {
-  return { dir: ${JSON.stringify(nativeDir)}, module: { nativeName } }
+  return {
+    dir: ${JSON.stringify(nativeDir)},
+    module: {
+      nativeName,
+      ...(${JSON.stringify(ownsPtyJob)}
+        ? {
+            listJobProcessIds() {},
+            terminateJob() {},
+            assignCurrentProcessToJob() {}
+          }
+        : {})
+    }
+  }
 }
 `
   )
@@ -518,6 +572,12 @@ function writeFakeWindowsRegistry(projectDir) {
   )
 }
 
+function writeFakeWindowsProcessTree(projectDir) {
+  const processTreeDir = join(projectDir, 'node_modules', '@vscode', 'windows-process-tree')
+  mkdirSync(processTreeDir, { recursive: true })
+  writeFileSync(join(processTreeDir, 'index.js'), 'module.exports = {}\n')
+}
+
 function writeNodePtyPatchFile(projectDir) {
   mkdirSync(join(projectDir, 'config', 'patches'), { recursive: true })
   writeFileSync(join(projectDir, 'config', 'patches', 'node-pty@1.1.0.patch'), 'patch marker\n')

config/scripts/verify-packaged-node-pty-job-ownership.cjs

@@ -0,0 +1,25 @@
+const { createRequire } = require('node:module')
+const { join } = require('node:path')
+const { assertNodePtyJobOwnership } = require('./node-pty-job-ownership.cjs')
+
+function loadPackagedConpty(resourcesDir) {
+  const packagedRequire = createRequire(join(resourcesDir, 'package.json'))
+  const { loadNativeModule } = packagedRequire('./node_modules/node-pty/lib/utils')
+  return loadNativeModule('conpty')
+}
+
+function verifyPackagedNodePtyJobOwnership(resourcesDir, options = {}) {
+  const platform = options.platform ?? process.platform
+  if (platform !== 'win32') {
+    return
+  }
+
+  const native = (options.loadNative ?? loadPackagedConpty)(resourcesDir)
+  assertNodePtyJobOwnership({ platform, nativeName: 'conpty', native })
+  if (!native.dir.replace(/\\/g, '/').includes('build/Release/')) {
+    throw new Error(`Packaged node-pty resolved to ${native.dir}; expected patched build/Release`)
+  }
+  console.log('[verify-packaged-node-pty] OK — packaged ConPTY owns process trees')
+}
+
+module.exports = { verifyPackagedNodePtyJobOwnership }

config/scripts/verify-packaged-node-pty-job-ownership.test.mjs

@@ -0,0 +1,51 @@
+import { createRequire } from 'node:module'
+import { describe, expect, it, vi } from 'vitest'
+
+const require = createRequire(import.meta.url)
+const {
+  verifyPackagedNodePtyJobOwnership
+} = require('./verify-packaged-node-pty-job-ownership.cjs')
+
+const PATCHED = {
+  dir: '../build/Release/',
+  module: {
+    listJobProcessIds: () => [],
+    terminateJob: () => true,
+    assignCurrentProcessToJob: () => true
+  }
+}
+
+describe('verifyPackagedNodePtyJobOwnership', () => {
+  it('accepts the packaged patched ConPTY binding', () => {
+    expect(() =>
+      verifyPackagedNodePtyJobOwnership('resources', {
+        platform: 'win32',
+        loadNative: () => PATCHED
+      })
+    ).not.toThrow()
+  })
+
+  it('rejects a packaged upstream prebuild', () => {
+    expect(() =>
+      verifyPackagedNodePtyJobOwnership('resources', {
+        platform: 'win32',
+        loadNative: () => ({ dir: '../prebuilds/win32-x64/', module: {} })
+      })
+    ).toThrow(/missing listJobProcessIds, terminateJob, assignCurrentProcessToJob/)
+  })
+
+  it('requires the patched source-build directory', () => {
+    expect(() =>
+      verifyPackagedNodePtyJobOwnership('resources', {
+        platform: 'win32',
+        loadNative: () => ({ ...PATCHED, dir: '../prebuilds/win32-x64/' })
+      })
+    ).toThrow(/expected patched build\/Release/)
+  })
+
+  it('does not load Windows natives for other targets', () => {
+    const loadNative = vi.fn()
+    verifyPackagedNodePtyJobOwnership('resources', { platform: 'linux', loadNative })
+    expect(loadNative).not.toHaveBeenCalled()
+  })
+})

pnpm-lock.yaml

@@ -27,7 +27,7 @@ patchedDependencies:
     hash: 7333b3837f80a7fbd045964db6d76ba4fc118e49134bdbabb00585b6b7b60673
     path: config/patches/lint-staged@16.4.0.patch
   node-pty@1.1.0:
-    hash: 710d5c74d1b738c41316e997e7cb860b61147604f98380ccf4f5bb20b6ed0ac5
+    hash: 9c665d9e41571341cb8986607d531b953a7f32c0dd60e1e74e633c098d80f2c1
     path: config/patches/node-pty@1.1.0.patch
 
 importers:
@@ -69,7 +69,7 @@ importers:
         version: 3.3.1
       node-pty:
         specifier: ^1.1.0
-        version: 1.1.0(patch_hash=710d5c74d1b738c41316e997e7cb860b61147604f98380ccf4f5bb20b6ed0ac5)
+        version: 1.1.0(patch_hash=9c665d9e41571341cb8986607d531b953a7f32c0dd60e1e74e633c098d80f2c1)
       posthog-node:
         specifier: ^5.33.3
         version: 5.33.3
@@ -12046,7 +12046,7 @@ snapshots:
 
   node-int64@0.4.0: {}
 
-  node-pty@1.1.0(patch_hash=710d5c74d1b738c41316e997e7cb860b61147604f98380ccf4f5bb20b6ed0ac5):
+  node-pty@1.1.0(patch_hash=9c665d9e41571341cb8986607d531b953a7f32c0dd60e1e74e633c098d80f2c1):
     dependencies:
       node-addon-api: 7.1.1