From 82980953c524190d67dbc8e61c96eaf70ac76f59 Mon Sep 17 00:00:00 2001 From: Nick Gomez <122398915+nick-inkeep@users.noreply.github.com> Date: Mon, 29 Jun 2026 17:07:09 -0700 Subject: [PATCH] fix(open-knowledge): node-pty spawn-helper +x in dev/CI build (#2257) * fix: make node-pty spawn-helper executable in dev/CI build+install node-pty ships its prebuilt spawn-helper non-executable (mode 0644, node-pty#850) and bun install preserves that mode. The packaged .app re-enforces +x in scripts/afterPack.mjs, but the dev/CI build path (bun install + bun run build:desktop, electron-vite, no afterPack) had no equivalent step, so node-pty.spawn() threw "posix_spawnp failed" and the in-app terminal dock showed "exited" in the lume macOS VM and the CI desktop-smoke job. Enforce the executable bit at the install producer. Refactor ensure-node-pty-exec.mjs to share one chmod primitive between the packaged layout (afterPack, unchanged signature and error contract) and the dev layout (node_modules/node-pty/prebuilds), then call the dev wrapper first and unconditionally at the top of the desktop postinstall, before the ELECTRON_SKIP_REBUILD and CI early-exits, since CI desktop-smoke is one of the affected environments and the executable bit is independent of the native-rebuild those guards gate. A failure is softened to a warning so a pathological node-pty layout cannot gate the monorepo install. Adds a RED test: tests/unit/ensure-node-pty-exec.test.ts gains a dev-path suite (node_modules layout 0644 to 0755, multi-arch, absent shipped-arch throws) mirroring the existing afterPack coverage. Verified in a worktree off origin/main: bug test 3-fail to 6-pass, full desktop suite 2064 pass 0 fail (post-build), tsc exit 0, biome and oxlint exit 0, plus a live postinstall smoke that flips a forced 0644 helper back to 0755 ahead of the skip. Does not modify CI workflows or un-quarantine the node-pty terminal-dock smoke suite. * refactor(open-knowledge): tidy postinstall spawn-helper fixup per review - Strip D33/D34 decision-marker citations from postinstall.mjs comments (OK comment-discipline prohibits D-num markers in source; substance kept). - Extract the postinstall soft-fail into a pure, tested ensureNodePtySpawnHelperExecutableInNodeModulesSafe wrapper returning { ok, ... } instead of throwing, so a pathological node-pty layout can never gate the monorepo bun install. Pin the non-throw contract with two unit tests. No behavior change: spawn-helper is still chmod'd +x on install; failures still soften to a warning. GitOrigin-RevId: 238e487c4e8323f35f6038a9899372c5e29fa558 --- .../desktop/scripts/ensure-node-pty-exec.mjs | 51 ++++++++++--- packages/desktop/scripts/postinstall.mjs | 12 +++ .../tests/unit/ensure-node-pty-exec.test.ts | 75 ++++++++++++++++++- 3 files changed, 128 insertions(+), 10 deletions(-) diff --git a/packages/desktop/scripts/ensure-node-pty-exec.mjs b/packages/desktop/scripts/ensure-node-pty-exec.mjs index a09270092..0b2e32285 100644 --- a/packages/desktop/scripts/ensure-node-pty-exec.mjs +++ b/packages/desktop/scripts/ensure-node-pty-exec.mjs @@ -1,21 +1,16 @@ #!/usr/bin/env node import { chmodSync, existsSync, readdirSync, statSync } from 'node:fs'; -import { join } from 'node:path'; +import { createRequire } from 'node:module'; +import { dirname, join } from 'node:path'; const SHIPPED_ARCH = 'darwin-arm64'; -const prebuildsDirFor = (resourcesDir) => - join(resourcesDir, 'app.asar.unpacked', 'node_modules', 'node-pty', 'prebuilds'); - -export function ensureNodePtySpawnHelperExecutable(resourcesDir) { - const prebuildsDir = prebuildsDirFor(resourcesDir); +function chmodSpawnHelpersUnderPrebuilds(prebuildsDir, remediation) { const requiredHelper = join(prebuildsDir, SHIPPED_ARCH, 'spawn-helper'); if (!existsSync(requiredHelper)) { throw new Error( `[ensure-node-pty-exec] node-pty ${SHIPPED_ARCH} spawn-helper missing at ${requiredHelper}. ` + - `Confirm node-pty is a desktop dependency and the '**/node-pty/prebuilds/**' asarUnpack ` + - `rule in electron-builder.yml unpacked it — without an executable spawn-helper on the real ` + - `filesystem, pty.fork() fails at runtime with "posix_spawnp failed".`, + remediation, ); } @@ -29,3 +24,41 @@ export function ensureNodePtySpawnHelperExecutable(resourcesDir) { } return chmodded; } + +function resolveNodePtyDir() { + const require = createRequire(import.meta.url); + return dirname(require.resolve('node-pty/package.json')); +} + +export function ensureNodePtySpawnHelperExecutable(resourcesDir) { + const prebuildsDir = join( + resourcesDir, + 'app.asar.unpacked', + 'node_modules', + 'node-pty', + 'prebuilds', + ); + return chmodSpawnHelpersUnderPrebuilds( + prebuildsDir, + `Confirm node-pty is a desktop dependency and the '**/node-pty/prebuilds/**' asarUnpack ` + + `rule in electron-builder.yml unpacked it — without an executable spawn-helper on the real ` + + `filesystem, pty.fork() fails at runtime with "posix_spawnp failed".`, + ); +} + +export function ensureNodePtySpawnHelperExecutableInNodeModules(nodePtyDir = resolveNodePtyDir()) { + return chmodSpawnHelpersUnderPrebuilds( + join(nodePtyDir, 'prebuilds'), + `Confirm node-pty is installed (it is a desktop dependency) and 'bun install' did not run ` + + `with --ignore-scripts — without an executable spawn-helper, pty.fork() fails at runtime ` + + `with "posix_spawnp failed" and the in-app terminal cannot spawn a shell.`, + ); +} + +export function ensureNodePtySpawnHelperExecutableInNodeModulesSafe(nodePtyDir) { + try { + return { ok: true, chmodded: ensureNodePtySpawnHelperExecutableInNodeModules(nodePtyDir) }; + } catch (err) { + return { ok: false, error: err instanceof Error ? err : new Error(String(err)) }; + } +} diff --git a/packages/desktop/scripts/postinstall.mjs b/packages/desktop/scripts/postinstall.mjs index c4815004a..1daaba9e4 100644 --- a/packages/desktop/scripts/postinstall.mjs +++ b/packages/desktop/scripts/postinstall.mjs @@ -1,5 +1,17 @@ #!/usr/bin/env node import { spawn } from 'node:child_process'; +import { ensureNodePtySpawnHelperExecutableInNodeModulesSafe } from './ensure-node-pty-exec.mjs'; + +const spawnHelper = ensureNodePtySpawnHelperExecutableInNodeModulesSafe(); +if (spawnHelper.ok) { + console.log( + `[desktop postinstall] node-pty spawn-helper marked executable (${spawnHelper.chmodded.length} file(s))`, + ); +} else { + console.warn( + `[desktop postinstall] could not make node-pty spawn-helper executable: ${spawnHelper.error.message}`, + ); +} if (process.env.ELECTRON_SKIP_REBUILD === '1') { console.log( diff --git a/packages/desktop/tests/unit/ensure-node-pty-exec.test.ts b/packages/desktop/tests/unit/ensure-node-pty-exec.test.ts index 2ffad50cc..36573c9a5 100644 --- a/packages/desktop/tests/unit/ensure-node-pty-exec.test.ts +++ b/packages/desktop/tests/unit/ensure-node-pty-exec.test.ts @@ -2,7 +2,11 @@ import { afterEach, describe, expect, test } from 'bun:test'; import { chmodSync, mkdirSync, mkdtempSync, rmSync, statSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; -import { ensureNodePtySpawnHelperExecutable } from '../../scripts/ensure-node-pty-exec.mjs'; +import { + ensureNodePtySpawnHelperExecutable, + ensureNodePtySpawnHelperExecutableInNodeModules, + ensureNodePtySpawnHelperExecutableInNodeModulesSafe, +} from '../../scripts/ensure-node-pty-exec.mjs'; const tmpRoots: string[] = []; @@ -67,3 +71,72 @@ describe('ensureNodePtySpawnHelperExecutable', () => { ); }); }); + +function nodeModulesHelperPath(nodePtyDir: string, arch: string): string { + return join(nodePtyDir, 'prebuilds', arch, 'spawn-helper'); +} + +function makeNodeModulesFixture(archDirs: string[]): string { + const root = mkdtempSync(join(tmpdir(), 'ok-nodepty-dev-exec-')); + tmpRoots.push(root); + const nodePtyDir = join(root, 'node_modules', 'node-pty'); + for (const arch of archDirs) { + const helper = nodeModulesHelperPath(nodePtyDir, arch); + mkdirSync(join(helper, '..'), { recursive: true }); + writeFileSync(helper, 'fake-mach-o'); + chmodSync(helper, 0o644); + } + return nodePtyDir; +} + +describe('ensureNodePtySpawnHelperExecutableInNodeModules', () => { + test('promotes the shipped darwin-arm64 spawn-helper from 0644 to executable 0755', () => { + const nodePtyDir = makeNodeModulesFixture(['darwin-arm64']); + const helper = nodeModulesHelperPath(nodePtyDir, 'darwin-arm64'); + expect(statSync(helper).mode & 0o111).toBe(0); // no execute bits to start + + const chmodded = ensureNodePtySpawnHelperExecutableInNodeModules(nodePtyDir); + + expect(chmodded).toContain(helper); + expect(statSync(helper).mode & 0o777).toBe(0o755); + }); + + test('chmods every prebuild arch present in node_modules, not just the shipped one', () => { + const nodePtyDir = makeNodeModulesFixture(['darwin-arm64', 'darwin-x64']); + + const chmodded = ensureNodePtySpawnHelperExecutableInNodeModules(nodePtyDir); + + expect(chmodded.length).toBe(2); + for (const arch of ['darwin-arm64', 'darwin-x64']) { + expect(statSync(nodeModulesHelperPath(nodePtyDir, arch)).mode & 0o777).toBe(0o755); + } + }); + + test('throws when the shipped darwin-arm64 spawn-helper is absent (broken install is a hard error)', () => { + const nodePtyDir = makeNodeModulesFixture(['darwin-x64']); // arm64 helper missing + expect(() => ensureNodePtySpawnHelperExecutableInNodeModules(nodePtyDir)).toThrow( + /darwin-arm64 spawn-helper missing/, + ); + }); +}); + +describe('ensureNodePtySpawnHelperExecutableInNodeModulesSafe', () => { + test('returns ok + chmodded on a healthy install and actually flips the bit', () => { + const nodePtyDir = makeNodeModulesFixture(['darwin-arm64']); + const result = ensureNodePtySpawnHelperExecutableInNodeModulesSafe(nodePtyDir); + expect(result.ok).toBe(true); + if (result.ok) { + expect(result.chmodded).toContain(nodeModulesHelperPath(nodePtyDir, 'darwin-arm64')); + } + expect(statSync(nodeModulesHelperPath(nodePtyDir, 'darwin-arm64')).mode & 0o777).toBe(0o755); + }); + + test('does NOT throw when the shipped helper is absent — returns ok:false so postinstall stays exit-0', () => { + const nodePtyDir = makeNodeModulesFixture(['darwin-x64']); + const result = ensureNodePtySpawnHelperExecutableInNodeModulesSafe(nodePtyDir); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.error.message).toMatch(/darwin-arm64 spawn-helper missing/); + } + }); +});