mirror of
https://github.com/stablyai/orca.git
synced 2026-10-18 08:03:22 +00:00
fix(ssh): allow npm 12 lifecycle scripts for relay native deps
npm 12 blocks node-pty/@parcel/watcher install scripts unless package.json lists them in allowScripts (exit 0 + warn). Linux has no node-pty prebuild, so terminals failed with "node-pty is not available on this remote host". Rebase #8686 onto main and harden: - emit versioned allowScripts + --ignore-scripts=false install/rebuild - real require() health probe + best-effort repair - single-flight loadPty so concurrent spawns cannot race mock vs disk module - repair test uses relay cwd and lib/index.js entry Closes #8720 Closes #8758
This commit is contained in:
@@ -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])
|
||||
)
|
||||
|
||||
@@ -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')
|
||||
})
|
||||
})
|
||||
@@ -289,6 +289,10 @@ export class PtyHandler {
|
||||
private worktreeRemovalCoordinator: RelayPtyWorktreeRemovalCoordinator | null = null
|
||||
private disposePromise: Promise<void> | 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<typeof NodePty | null> | 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<typeof NodePty | null> {
|
||||
// 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)) {
|
||||
|
||||
@@ -25,7 +25,7 @@ export type RelayProcess = {
|
||||
export function spawnRelay(
|
||||
entryPath: string,
|
||||
args: string[] = [],
|
||||
options: Pick<SpawnOptions, 'env'> = {}
|
||||
options: Pick<SpawnOptions, 'env' | 'cwd'> = {}
|
||||
): RelayProcess {
|
||||
const proc = spawn('node', [entryPath, ...args], {
|
||||
stdio: ['pipe', 'pipe', 'pipe'],
|
||||
|
||||
@@ -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',
|
||||
|
||||
Reference in New Issue
Block a user