diff --git a/src/main/ssh/ssh-relay-deploy.ts b/src/main/ssh/ssh-relay-deploy.ts index a68b3207478..5be20774de6 100644 --- a/src/main/ssh/ssh-relay-deploy.ts +++ b/src/main/ssh/ssh-relay-deploy.ts @@ -491,8 +491,10 @@ type RelayNativeDepName = keyof typeof RELAY_NATIVE_DEPS const RELAY_NATIVE_DEP_NAMES = Object.keys(RELAY_NATIVE_DEPS) as RelayNativeDepName[] const NATIVE_DEPS_MISSING_PREFIX = 'ORCA-NATIVE-DEPS-MISSING:' -// Why: npm 12 blocks dependency lifecycle scripts unless each exact package -// version is approved, even when ignore-scripts is explicitly disabled. +// Why (#8720/#8758): npm 12 blocks dependency lifecycle scripts unless each +// exact package@version is approved — even when --ignore-scripts=false is set. +// Linux has no node-pty prebuild, so a missing allowlist yields exit 0 + no +// pty.node and "node-pty is not available on this remote host" at spawn. const RELAY_NATIVE_DEP_SCRIPT_ALLOWLIST = Object.fromEntries( Object.entries(RELAY_NATIVE_DEPS).map(([name, version]) => [`${name}@${version}`, true]) ) diff --git a/src/main/ssh/ssh-relay-npm12-allow-scripts.test.ts b/src/main/ssh/ssh-relay-npm12-allow-scripts.test.ts new file mode 100644 index 00000000000..18563fc9dea --- /dev/null +++ b/src/main/ssh/ssh-relay-npm12-allow-scripts.test.ts @@ -0,0 +1,65 @@ +/** + * #8720 / #8758 — npm 12 allowScripts for relay native deps. + * + * npm 12 blocks lifecycle scripts not listed in package.json `allowScripts` + * (warn + exit 0 when strict-allow-scripts is false). Linux has no node-pty + * prebuild, so a missing allowlist leaves pty.node unbuilt while deploy still + * "succeeds". `--ignore-scripts=false` alone does NOT override allowScripts. + * + * Regression: + * pnpm exec vitest run --config config/vitest.config.ts \ + * src/main/ssh/ssh-relay-npm12-allow-scripts.test.ts + */ +import { readFileSync } from 'node:fs' +import { join } from 'node:path' +import { describe, expect, it } from 'vitest' + +const deploySource = readFileSync(join(__dirname, 'ssh-relay-deploy.ts'), 'utf8') +const installHarness = readFileSync( + join(__dirname, 'ssh-relay-native-deps-install.test.ts'), + 'utf8' +) +const ptyHandlerSource = readFileSync(join(__dirname, '../../relay/pty-handler.ts'), 'utf8') + +describe('#8720/#8758 npm 12 allowScripts for relay native deps', () => { + it('writes versioned allowScripts for both native addons in package.json', () => { + // Why: npm 12 requires each exact package@version in allowScripts; name-only + // keys are accepted on some builds, but the deploy pins exact versions so the + // allowlist must match RELAY_NATIVE_DEPS. + expect(deploySource).toMatch(/'node-pty':\s*'1\.1\.0'/) + expect(deploySource).toMatch(/'@parcel\/watcher':\s*'2\.5\.6'/) + expect(deploySource).toMatch(/allowScripts:\s*RELAY_NATIVE_DEP_SCRIPT_ALLOWLIST/) + expect(deploySource).toMatch( + /Object\.entries\(RELAY_NATIVE_DEPS\)\.map\(\(\[name,\s*version\]\)\s*=>\s*\[`\$\{name\}@\$\{version\}`,\s*true\]\)/ + ) + }) + + it('forces lifecycle scripts on install and rebuild (ignore-scripts hosts)', () => { + // Complementary to allowScripts: NPM_CONFIG_IGNORE_SCRIPTS / npmrc ignore-scripts + // still need the CLI override. allowScripts alone does not re-enable a host that + // sets ignore-scripts=true, and ignore-scripts=false alone does not satisfy npm 12. + expect(deploySource).toMatch( + /npm install --ignore-scripts=false --omit=dev --no-audit --no-fund/ + ) + expect(deploySource).toMatch(/npm rebuild --ignore-scripts=false/) + }) + + it('probes with real require() so a present-but-unbuilt binding is not healthy', () => { + // require.resolve only checks the JS entrypoint and passed when scripts were + // blocked; the deploy probe must load the native binding. + expect(deploySource).toMatch(/require\("node-pty"\)/) + expect(deploySource).toMatch(/require\("@parcel\/watcher"\)/) + expect(deploySource).not.toMatch(/require\.resolve\("node-pty"\)/) + }) + + it('install harness pins the allowScripts contract', () => { + expect(installHarness).toContain("'node-pty@1.1.0': true") + expect(installHarness).toContain("'@parcel/watcher@2.5.6': true") + expect(installHarness).toContain('--ignore-scripts=false') + }) + + it('keeps the user-facing remote PTY error string for already-broken remotes', () => { + // #8758 surface: if repair cannot build, spawn still explains the failure. + expect(ptyHandlerSource).toContain('node-pty is not available on this remote host') + }) +}) diff --git a/src/relay/pty-handler.ts b/src/relay/pty-handler.ts index 1e0e3d65ba1..a52e9804c12 100644 --- a/src/relay/pty-handler.ts +++ b/src/relay/pty-handler.ts @@ -289,6 +289,10 @@ export class PtyHandler { private worktreeRemovalCoordinator: RelayPtyWorktreeRemovalCoordinator | null = null private disposePromise: Promise | null = null private ptyModule: typeof NodePty | null = null + // Why: single-flight the load so concurrent pty.spawn calls cannot race one + // successful dynamic import against a parallel require() of a different + // node-pty copy (vitest mock vs monorepo install, or pre-repair vs repaired). + private ptyModuleLoad: Promise | null = null private reloadPtyModuleFromDisk = false // Why: external observers need to drop per-pane state when a PTY exits. // Today the relay composes multiple consumers (hook-server cache eviction @@ -314,6 +318,18 @@ export class PtyHandler { if (this.ptyModule) { return this.ptyModule } + if (!this.ptyModuleLoad) { + this.ptyModuleLoad = this.loadPtyUncached().finally(() => { + this.ptyModuleLoad = null + }) + } + return this.ptyModuleLoad + } + + private async loadPtyUncached(): Promise { + // Why: after a failed import or a late native-binding failure, ESM may keep + // the rejected/broken module in cache. Force a cwd-relative require so an + // in-place npm repair is visible without restarting the relay (#8720). if (!this.reloadPtyModuleFromDisk) { try { this.ptyModule = await import('node-pty') @@ -322,20 +338,38 @@ export class PtyHandler { this.reloadPtyModuleFromDisk = true } } - const moduleEntry = join(process.cwd(), 'node_modules', 'node-pty', 'lib', 'index.js') - if (!existsSync(moduleEntry)) { - return null - } - try { - this.ptyModule = require(moduleEntry) as typeof NodePty - return this.ptyModule - } catch { - return null + return this.loadPtyFromDisk() + } + + private loadPtyFromDisk(): typeof NodePty | null { + // Prefer the published entry, then a bare package root (repair tests / odd packs). + const candidates = [ + join(process.cwd(), 'node_modules', 'node-pty', 'lib', 'index.js'), + join(process.cwd(), 'node_modules', 'node-pty', 'index.js') + ] + for (const moduleEntry of candidates) { + if (!existsSync(moduleEntry)) { + continue + } + try { + // Drop any prior resolution so a just-repaired tree is not sticky-null. + try { + delete require.cache[require.resolve(moduleEntry)] + } catch { + delete require.cache[moduleEntry] + } + this.ptyModule = require(moduleEntry) as typeof NodePty + return this.ptyModule + } catch { + // try next candidate + } } + return null } private invalidatePtyModuleAfterBindingFailure(): void { this.ptyModule = null + this.ptyModuleLoad = null this.reloadPtyModuleFromDisk = true const moduleRoot = join(process.cwd(), 'node_modules', 'node-pty') for (const cachedPath of Object.keys(require.cache)) { diff --git a/src/relay/subprocess-test-utils.ts b/src/relay/subprocess-test-utils.ts index 69a6e607a06..a5d38e5f36e 100644 --- a/src/relay/subprocess-test-utils.ts +++ b/src/relay/subprocess-test-utils.ts @@ -25,7 +25,7 @@ export type RelayProcess = { export function spawnRelay( entryPath: string, args: string[] = [], - options: Pick = {} + options: Pick = {} ): RelayProcess { const proc = spawn('node', [entryPath, ...args], { stdio: ['pipe', 'pipe', 'pipe'], diff --git a/src/relay/subprocess.test.ts b/src/relay/subprocess.test.ts index ea017bb8316..3be867d9d11 100644 --- a/src/relay/subprocess.test.ts +++ b/src/relay/subprocess.test.ts @@ -116,17 +116,20 @@ describe('Subprocess: Relay entry point', () => { const repairedRelayEntry = path.join(tmpDir, 'relay.js') copyFileSync(relayEntry, repairedRelayEntry) - relay = spawnRelay(repairedRelayEntry) + // Why: cwd must be the relay dir so loadPty's disk fallback cannot resolve + // the monorepo's node-pty, and so an in-place install under this tree is + // visible without restarting the process (#8720 ESM/cache repair path). + relay = spawnRelay(repairedRelayEntry, [], { cwd: tmpDir }) await relay.sentinelReceived const failedId = relay.send('pty.spawn', { cols: 80, rows: 24 }) const failed = await relay.waitForResponse(failedId) expect(failed.error?.message).toContain('node-pty is not available') - const nodePtyDir = path.join(tmpDir, 'node_modules', 'node-pty') - mkdirSync(nodePtyDir, { recursive: true }) + const nodePtyLibDir = path.join(tmpDir, 'node_modules', 'node-pty', 'lib') + mkdirSync(nodePtyLibDir, { recursive: true }) writeFileSync( - path.join(nodePtyDir, 'index.js'), + path.join(nodePtyLibDir, 'index.js'), `module.exports = { spawn() { return { pid: process.pid, process: 'mock-shell',