-
Notifications
You must be signed in to change notification settings - Fork 530
build(pty): restore bundled ConPTY after rebuild #2752
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
janburzinski
wants to merge
4
commits into
main
Choose a base branch
from
emdash/windows-cli-fix-0hz3f
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+522
−14
Open
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
44a093e
build(pty): restore bundled ConPTY after rebuild
janburzinski 5d51003
fix(pty): use bundled ConPTY for Windows PTYs
janburzinski ab114fa
fix(pty): require full bundled ConPTY payload
janburzinski 7d6470a
fix(pty): select complete bundled ConPTY payload
janburzinski File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,93 @@ | ||
| import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; | ||
| import os from 'node:os'; | ||
| import path from 'node:path'; | ||
| import { afterEach, beforeEach, describe, expect, it } from 'vitest'; | ||
| import { copyConptyDll } from './copy-conpty-dll.ts'; | ||
|
|
||
| describe('copyConptyDll', () => { | ||
| const originalPlatform = Object.getOwnPropertyDescriptor(process, 'platform')!; | ||
| let nodePtyRoot: string; | ||
|
|
||
| beforeEach(() => { | ||
| nodePtyRoot = mkdtempSync(path.join(os.tmpdir(), 'emdash-copy-conpty-')); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| Object.defineProperty(process, 'platform', originalPlatform); | ||
| rmSync(nodePtyRoot, { recursive: true, force: true }); | ||
| }); | ||
|
|
||
| function makeDir(...segments: string[]): string { | ||
| const dir = path.join(nodePtyRoot, ...segments); | ||
| mkdirSync(dir, { recursive: true }); | ||
| return dir; | ||
| } | ||
|
|
||
| function seedThirdParty(arch: string): void { | ||
| const sourceDir = makeDir('third_party', 'conpty', '1.0.0', `win10-${arch}`); | ||
| writeFileSync(path.join(sourceDir, 'conpty.dll'), `dll-${arch}`); | ||
| writeFileSync(path.join(sourceDir, 'OpenConsole.exe'), `exe-${arch}`); | ||
| } | ||
|
|
||
| function seedRebuiltOutput(): void { | ||
| const release = makeDir('build', 'Release'); | ||
| writeFileSync(path.join(release, 'conpty.node'), ''); | ||
| } | ||
|
|
||
| it('is a no-op on non-Windows platforms', () => { | ||
| Object.defineProperty(process, 'platform', { ...originalPlatform, value: 'linux' }); | ||
| seedThirdParty('x64'); | ||
| seedRebuiltOutput(); | ||
|
|
||
| expect(copyConptyDll({ nodePtyRoot, arch: 'x64' })).toBe(false); | ||
| expect(existsSync(path.join(nodePtyRoot, 'build', 'Release', 'conpty'))).toBe(false); | ||
| }); | ||
|
|
||
| describe('on Windows', () => { | ||
| beforeEach(() => { | ||
| Object.defineProperty(process, 'platform', { ...originalPlatform, value: 'win32' }); | ||
| }); | ||
|
|
||
| it('copies the bundled dll next to the rebuilt conpty.node', () => { | ||
| seedThirdParty('x64'); | ||
| seedRebuiltOutput(); | ||
|
|
||
| expect(copyConptyDll({ nodePtyRoot, arch: 'x64' })).toBe(true); | ||
|
|
||
| const destDir = path.join(nodePtyRoot, 'build', 'Release', 'conpty'); | ||
| expect(readFileSync(path.join(destDir, 'conpty.dll'), 'utf8')).toBe('dll-x64'); | ||
| expect(readFileSync(path.join(destDir, 'OpenConsole.exe'), 'utf8')).toBe('exe-x64'); | ||
| }); | ||
|
|
||
| it('copies the requested architecture', () => { | ||
| seedThirdParty('x64'); | ||
| seedThirdParty('arm64'); | ||
| seedRebuiltOutput(); | ||
|
|
||
| expect(copyConptyDll({ nodePtyRoot, arch: 'arm64' })).toBe(true); | ||
|
|
||
| const destDir = path.join(nodePtyRoot, 'build', 'Release', 'conpty'); | ||
| expect(readFileSync(path.join(destDir, 'conpty.dll'), 'utf8')).toBe('dll-arm64'); | ||
| }); | ||
|
|
||
| it('skips when there is no rebuilt output', () => { | ||
| seedThirdParty('x64'); | ||
|
|
||
| expect(copyConptyDll({ nodePtyRoot, arch: 'x64' })).toBe(false); | ||
| expect(existsSync(path.join(nodePtyRoot, 'build'))).toBe(false); | ||
| }); | ||
|
|
||
| it('skips when node-pty ships no bundled ConPTY', () => { | ||
| seedRebuiltOutput(); | ||
|
|
||
| expect(copyConptyDll({ nodePtyRoot, arch: 'x64' })).toBe(false); | ||
| }); | ||
|
|
||
| it('skips unsupported architectures', () => { | ||
| seedThirdParty('x64'); | ||
| seedRebuiltOutput(); | ||
|
|
||
| expect(copyConptyDll({ nodePtyRoot, arch: 'ia32' })).toBe(false); | ||
| }); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| import { copyFileSync, existsSync, mkdirSync, readdirSync } from 'node:fs'; | ||
| import { createRequire } from 'node:module'; | ||
| import path from 'node:path'; | ||
| import { fileURLToPath } from 'node:url'; | ||
|
|
||
| /** | ||
| * Restore node-pty's bundled ConPTY next to the rebuilt native module. | ||
| * | ||
| * node-pty's own postinstall copies `third_party/conpty/<ver>/win10-<arch>/` | ||
| * into `build/Release/conpty/`, but every electron-rebuild (dev postinstall, | ||
| * `pnpm run rebuild`, release rebuild-native.ts, CI) runs `node-gyp rebuild`, | ||
| * which wipes build/Release — and with it the dll that `useConptyDll: true` | ||
| * (see src/main/core/pty/conpty-dll.ts) resolves relative to conpty.node. | ||
| * This must therefore run after every electron-rebuild of node-pty. | ||
| * | ||
| * Returns true when the dll files were copied, false when skipped (non-win32, | ||
| * no rebuilt output, or a node-pty version without the bundled dll). | ||
| */ | ||
| export function copyConptyDll(options: { nodePtyRoot: string; arch?: string }): boolean { | ||
| if (process.platform !== 'win32') return false; | ||
|
|
||
| const { nodePtyRoot } = options; | ||
| const arch = options.arch ?? process.arch; | ||
| if (!['x64', 'arm64'].includes(arch)) { | ||
| console.warn(`copy-conpty-dll: unsupported arch ${arch}, skipping`); | ||
| return false; | ||
| } | ||
|
|
||
| const releaseDir = path.join(nodePtyRoot, 'build', 'Release'); | ||
| if (!existsSync(path.join(releaseDir, 'conpty.node'))) { | ||
| // No rebuilt output — node-pty's prebuilds ship their own conpty/ folder. | ||
| return false; | ||
| } | ||
|
|
||
| const conptyBase = path.join(nodePtyRoot, 'third_party', 'conpty'); | ||
| const versionFolder = existsSync(conptyBase) ? readdirSync(conptyBase)[0] : undefined; | ||
| if (!versionFolder) { | ||
| console.warn(`copy-conpty-dll: no bundled ConPTY found under ${conptyBase}, skipping`); | ||
| return false; | ||
| } | ||
|
|
||
| const sourceDir = path.join(conptyBase, versionFolder, `win10-${arch}`); | ||
| const destDir = path.join(releaseDir, 'conpty'); | ||
| mkdirSync(destDir, { recursive: true }); | ||
| for (const file of ['conpty.dll', 'OpenConsole.exe']) { | ||
| copyFileSync(path.join(sourceDir, file), path.join(destDir, file)); | ||
| } | ||
|
janburzinski marked this conversation as resolved.
|
||
| console.log(`copy-conpty-dll: copied ConPTY ${versionFolder} (win10-${arch}) to ${destDir}`); | ||
| return true; | ||
| } | ||
|
|
||
| export function resolveNodePtyRoot(): string { | ||
| const require = createRequire(import.meta.url); | ||
| return path.dirname(require.resolve('node-pty/package.json')); | ||
| } | ||
|
|
||
| if (process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url)) { | ||
| const archFlagIndex = process.argv.indexOf('--arch'); | ||
| const arch = archFlagIndex === -1 ? undefined : process.argv[archFlagIndex + 1]; | ||
| copyConptyDll({ nodePtyRoot: resolveNodePtyRoot(), arch }); | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,90 @@ | ||
| import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; | ||
| import os from 'node:os'; | ||
| import path from 'node:path'; | ||
| import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; | ||
| import { | ||
| conptyDllPresentUnder, | ||
| resetConptyDllCacheForTests, | ||
| resolveUseConptyDll, | ||
| } from './conpty-dll'; | ||
|
|
||
| describe('conpty-dll', () => { | ||
| const originalPlatform = Object.getOwnPropertyDescriptor(process, 'platform')!; | ||
| let tempRoot: string; | ||
|
|
||
| function setPlatform(platform: NodeJS.Platform): void { | ||
| Object.defineProperty(process, 'platform', { | ||
| ...originalPlatform, | ||
| value: platform, | ||
| }); | ||
| } | ||
|
|
||
| beforeEach(() => { | ||
| resetConptyDllCacheForTests(); | ||
| tempRoot = mkdtempSync(path.join(os.tmpdir(), 'emdash-conpty-')); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| Object.defineProperty(process, 'platform', originalPlatform); | ||
| vi.unstubAllEnvs(); | ||
| resetConptyDllCacheForTests(); | ||
| rmSync(tempRoot, { recursive: true, force: true }); | ||
| }); | ||
|
|
||
| describe('resolveUseConptyDll', () => { | ||
| it('is false on non-Windows platforms', () => { | ||
| setPlatform('darwin'); | ||
| expect(resolveUseConptyDll()).toBe(false); | ||
| }); | ||
|
|
||
| it('is false when disabled via EMDASH_DISABLE_CONPTY_DLL', () => { | ||
| setPlatform('win32'); | ||
| vi.stubEnv('EMDASH_DISABLE_CONPTY_DLL', '1'); | ||
| expect(resolveUseConptyDll()).toBe(false); | ||
| }); | ||
| }); | ||
|
|
||
| describe('conptyDllPresentUnder', () => { | ||
| function makeDir(...segments: string[]): string { | ||
| const dir = path.join(tempRoot, ...segments); | ||
| mkdirSync(dir, { recursive: true }); | ||
| return dir; | ||
| } | ||
|
|
||
| it('is true when the dll sits next to the rebuilt conpty.node', () => { | ||
| const release = makeDir('build', 'Release'); | ||
| writeFileSync(path.join(release, 'conpty.node'), ''); | ||
| const dllDir = makeDir('build', 'Release', 'conpty'); | ||
| writeFileSync(path.join(dllDir, 'conpty.dll'), ''); | ||
| writeFileSync(path.join(dllDir, 'OpenConsole.exe'), ''); | ||
|
|
||
| expect(conptyDllPresentUnder(tempRoot)).toBe(true); | ||
| }); | ||
|
|
||
| it('is false when the rebuilt output lacks the dll (electron-rebuild wiped it)', () => { | ||
| const release = makeDir('build', 'Release'); | ||
| writeFileSync(path.join(release, 'conpty.node'), ''); | ||
| // Prebuilds carry the dll, but build/Release wins node-pty's search order. | ||
| const prebuilds = makeDir('prebuilds', `${process.platform}-${process.arch}`); | ||
| writeFileSync(path.join(prebuilds, 'conpty.node'), ''); | ||
| const prebuiltDllDir = makeDir('prebuilds', `${process.platform}-${process.arch}`, 'conpty'); | ||
| writeFileSync(path.join(prebuiltDllDir, 'conpty.dll'), ''); | ||
|
|
||
| expect(conptyDllPresentUnder(tempRoot)).toBe(false); | ||
| }); | ||
|
|
||
| it('falls back to prebuilds when no rebuilt output exists', () => { | ||
| const prebuilds = makeDir('prebuilds', `${process.platform}-${process.arch}`); | ||
| writeFileSync(path.join(prebuilds, 'conpty.node'), ''); | ||
| const dllDir = makeDir('prebuilds', `${process.platform}-${process.arch}`, 'conpty'); | ||
| writeFileSync(path.join(dllDir, 'conpty.dll'), ''); | ||
| writeFileSync(path.join(dllDir, 'OpenConsole.exe'), ''); | ||
|
|
||
| expect(conptyDllPresentUnder(tempRoot)).toBe(true); | ||
| }); | ||
|
|
||
| it('is false when no native module exists at all', () => { | ||
| expect(conptyDllPresentUnder(tempRoot)).toBe(false); | ||
| }); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| import { existsSync } from 'node:fs'; | ||
| import { createRequire } from 'node:module'; | ||
| import path from 'node:path'; | ||
|
|
||
| /** | ||
| * Whether local PTYs should ask node-pty to load its bundled, up-to-date | ||
| * ConPTY (`useConptyDll: true`) instead of the OS in-box conhost. | ||
| * | ||
| * The in-box ConPTY on Windows 10 (and older Windows 11 builds) does not pass | ||
| * mouse-mode DECSETs (1000/1002/1003/1006) or the alternate-screen switch | ||
| * through to the attached terminal, and silently drops SGR mouse reports | ||
| * written to its input pipe. Fullscreen TUIs (opencode, claude, amp) are | ||
| * unusable with it: no mouse clicks, no wheel scrolling, and xterm.js never | ||
| * even learns the app entered the alternate screen. The bundled conpty.dll | ||
| * (from the Windows Terminal project) supports full passthrough in both | ||
| * directions. | ||
| * | ||
| * node-pty resolves the dll as `<dir of conpty.node>/conpty/conpty.dll` and | ||
| * throws synchronously from spawn when it is missing, so we only opt in when | ||
| * the dll is actually present next to the native module that will load | ||
| * (electron-rebuild wipes build/Release; the copy step in | ||
| * scripts/copy-conpty-dll.ts restores it). | ||
| */ | ||
| export function resolveUseConptyDll(): boolean { | ||
| if (process.platform !== 'win32') return false; | ||
| if (process.env.EMDASH_DISABLE_CONPTY_DLL === '1') return false; | ||
| return conptyDllIsPresent(); | ||
| } | ||
|
|
||
| let cachedPresence: boolean | null = null; | ||
|
|
||
| function conptyDllIsPresent(): boolean { | ||
| if (cachedPresence !== null) return cachedPresence; | ||
| try { | ||
| const require = createRequire(import.meta.url); | ||
| const packageRoot = path.dirname(require.resolve('node-pty/package.json')); | ||
| cachedPresence = conptyDllPresentUnder(packageRoot); | ||
| } catch { | ||
| cachedPresence = false; | ||
| } | ||
| return cachedPresence; | ||
| } | ||
|
|
||
| /** Test seam: clears the memoized dll-presence check. */ | ||
| export function resetConptyDllCacheForTests(): void { | ||
| cachedPresence = null; | ||
| } | ||
|
|
||
| /** | ||
| * Stop offering the bundled dll for the rest of the process — called when a | ||
| * spawn with it failed while the in-box ConPTY worked (AV quarantine, broken | ||
| * copy), so later spawns skip the doomed attempt. | ||
| */ | ||
| export function markConptyDllUnavailable(): void { | ||
| cachedPresence = false; | ||
| } | ||
|
|
||
| /** | ||
| * Mirrors node-pty's loadNativeModule() search order (lib/utils.ts): the | ||
| * first directory containing conpty.node is the one that will load, and the | ||
| * dll must sit in a `conpty/` folder next to it. | ||
| */ | ||
| export function conptyDllPresentUnder(packageRoot: string): boolean { | ||
| const candidateDirs = [ | ||
| path.join(packageRoot, 'build', 'Release'), | ||
| path.join(packageRoot, 'build', 'Debug'), | ||
| path.join(packageRoot, 'prebuilds', `${process.platform}-${process.arch}`), | ||
| ]; | ||
| for (const dir of candidateDirs) { | ||
| if (!existsSync(path.join(dir, 'conpty.node'))) continue; | ||
| return ( | ||
| existsSync(path.join(dir, 'conpty', 'conpty.dll')) && | ||
| existsSync(path.join(dir, 'conpty', 'OpenConsole.exe')) | ||
| ); | ||
| } | ||
| return false; | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.