From 94b55fe855a301b2114968917fd07ef635fdb3be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Kuty=C5=82a?= Date: Thu, 18 Jun 2026 13:12:12 +0200 Subject: [PATCH] fix(shell): look for spawn-helper in prebuilds dir, not just build/Release MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Correction to the prior commit. node-pty 1.1.0 ships prebuilt binaries, and loadNativeModule() loads pty.node from prebuilds/-/ — so spawn-helper lives THERE, next to it, not in build/Release. The first guard only checked build/Release, so on a normal (prebuild) install it found nothing and fixed nothing, while falsely reporting the helper missing. ensureSpawnHelperExecutable now mirrors node-pty's own search order (build/Release, build/Debug, prebuilds/-) and repairs every spawn-helper it finds: chmod +x when the execute bit is missing (the real cause — pnpm store extraction drops it) and strip com.apple.quarantine. Only when none exist anywhere does it log the rebuild-from-source hint. spawnHelperCandidateDirs extracted as a pure, unit-tested helper. --- .../shell/src/runtime/pty-manager.test.ts | 20 ++++++ src/apps/shell/src/runtime/pty-manager.ts | 65 ++++++++++++------- 2 files changed, 63 insertions(+), 22 deletions(-) create mode 100644 src/apps/shell/src/runtime/pty-manager.test.ts diff --git a/src/apps/shell/src/runtime/pty-manager.test.ts b/src/apps/shell/src/runtime/pty-manager.test.ts new file mode 100644 index 0000000..7da51e7 --- /dev/null +++ b/src/apps/shell/src/runtime/pty-manager.test.ts @@ -0,0 +1,20 @@ +import { describe, expect, it } from 'vitest'; +import { spawnHelperCandidateDirs } from './pty-manager.js'; + +describe('spawnHelperCandidateDirs', () => { + it('searches build/Release, build/Debug, then prebuilds/-', () => { + const dirs = spawnHelperCandidateDirs('/np', 'darwin', 'arm64'); + // Regression: the helper ships under prebuilds/, not just build/Release — the dir must + // be included so a prebuild install gets its spawn-helper repaired. + expect(dirs).toEqual([ + '/np/build/Release', + '/np/build/Debug', + '/np/prebuilds/darwin-arm64', + ].map((p) => p.replace(/\//g, require('path').sep))); + }); + + it('encodes platform-arch into the prebuilds dir', () => { + const dirs = spawnHelperCandidateDirs('/np', 'darwin', 'x64'); + expect(dirs.some((d) => d.endsWith(`prebuilds${require('path').sep}darwin-x64`))).toBe(true); + }); +}); diff --git a/src/apps/shell/src/runtime/pty-manager.ts b/src/apps/shell/src/runtime/pty-manager.ts index 45d5309..967da3a 100644 --- a/src/apps/shell/src/runtime/pty-manager.ts +++ b/src/apps/shell/src/runtime/pty-manager.ts @@ -15,17 +15,33 @@ import { noteCursorReportRequests } from './cursor-report.js'; let spawnHelperChecked = false; +/** + * Candidate directories that may contain node-pty's `spawn-helper`, in the same order + * node-pty's loadNativeModule() searches for pty.node — the helper sits next to whichever + * binary actually loads. node-pty 1.1.0 ships prebuilds, so on macOS the helper is normally + * under prebuilds/-/, NOT build/Release. + */ +export function spawnHelperCandidateDirs(nodePtyRoot: string, platform: string, arch: string): string[] { + return [ + path.join(nodePtyRoot, 'build', 'Release'), + path.join(nodePtyRoot, 'build', 'Debug'), + path.join(nodePtyRoot, 'prebuilds', `${platform}-${arch}`), + ]; +} + /** * Ensure node-pty's `spawn-helper` is runnable on macOS. * * On macOS, node-pty's native code launches the shell via a small `spawn-helper` * binary using posix_spawn(), which requires that file to exist AND be executable. - * The error surfaces (misleadingly) as "posix_spawnp failed." pnpm's content-addressable - * store materialization — and downloaded prebuilds — can drop the execute bit or leave a + * The error surfaces (misleadingly) as "posix_spawnp failed." The prebuilt helper that + * ships with node-pty can lose its execute bit during pnpm store extraction or pick up a * macOS quarantine xattr, so a freshly installed tree throws on every pane/agent launch. * node-pty's own post-install does not chmod the helper, so we repair it here. * - * Idempotent, darwin-only, best-effort — never throws (spawning must still proceed). + * Fixes every spawn-helper found across node-pty's candidate dirs (the one node-pty loads + * from may be in prebuilds/, not build/Release). Idempotent, darwin-only, best-effort — + * never throws (spawning must still proceed). */ export function ensureSpawnHelperExecutable(): void { if (spawnHelperChecked) return; @@ -34,28 +50,33 @@ export function ensureSpawnHelperExecutable(): void { try { const require_ = createRequire(import.meta.url); - const pkgJson = require_.resolve('node-pty/package.json'); - const helper = path.join(path.dirname(pkgJson), 'build', 'Release', 'spawn-helper'); + const root = path.dirname(require_.resolve('node-pty/package.json')); + const dirs = spawnHelperCandidateDirs(root, process.platform, process.arch); + + let found = false; + for (const dir of dirs) { + const helper = path.join(dir, 'spawn-helper'); + if (!fs.existsSync(helper)) continue; + found = true; + + if (lacksExecuteBit(fs.statSync(helper).mode)) { + fs.chmodSync(helper, 0o755); + console.error(`[shell] restored execute permission on node-pty spawn-helper: ${helper}`); + } + // Strip the macOS quarantine attribute if present; harmless when absent. + try { + execFileSync('xattr', ['-d', 'com.apple.quarantine', helper], { stdio: 'ignore' }); + } catch { + // not quarantined, or xattr unavailable — nothing to do + } + } - if (!fs.existsSync(helper)) { + if (!found) { console.error( - `[shell] node-pty spawn-helper is missing at ${helper}. ` + - `This usually means node-pty was not built for this platform/arch — ` + - `run "pnpm rebuild node-pty" (or reinstall) on this machine.`, + `[shell] node-pty spawn-helper not found under ${root} ` + + `(checked build/Release, build/Debug, prebuilds/${process.platform}-${process.arch}). ` + + `Rebuild node-pty from source: npm_config_build_from_source=true pnpm rebuild node-pty`, ); - return; - } - - if (lacksExecuteBit(fs.statSync(helper).mode)) { - fs.chmodSync(helper, 0o755); - console.error(`[shell] restored execute permission on node-pty spawn-helper: ${helper}`); - } - - // Strip the macOS quarantine attribute if present; harmless when absent. - try { - execFileSync('xattr', ['-d', 'com.apple.quarantine', helper], { stdio: 'ignore' }); - } catch { - // not quarantined, or xattr unavailable — nothing to do } } catch (err) { console.error('[shell] spawn-helper check failed:', (err as Error).message);