diff --git a/src/main/orcad/main-preflight-order.test.ts b/src/main/orcad/main-preflight-order.test.ts index 0341b4617d2..23bf711ac38 100644 --- a/src/main/orcad/main-preflight-order.test.ts +++ b/src/main/orcad/main-preflight-order.test.ts @@ -33,6 +33,9 @@ vi.mock('./orcad-profile-preflight', () => ({ forgetBundledOrcadStartupQualification: () => { order.push('forget-qualification') }, + discardBundledOrcadStartupQualification: async () => { + order.push('discard-qualification') + }, confirmBundledOrcadStartupQualified: async () => { order.push('qualified') }, @@ -128,6 +131,16 @@ describe('orcad entry', () => { expect(order).toEqual(['profile-admission', 'preflight-qualified', 'main', 'qualified']) }) + it('discards the slot qualification when node-pty later fails to load in this process', async () => { + await import('./main') + await vi.waitFor(() => expect(order).toContain('qualified')) + const { reportNodePtyLoadFailure } = await import('../providers/node-pty-load-failure') + + reportNodePtyLoadFailure(new Error('pty.node failed to load')) + + await vi.waitFor(() => expect(order).toContain('discard-qualification')) + }) + it('drops the qualification when the node-pty probe does not pass cleanly', async () => { slot.verdict = 'degraded' await import('./main') diff --git a/src/main/orcad/main.ts b/src/main/orcad/main.ts index f1de05a7a34..c76e33d1e06 100644 --- a/src/main/orcad/main.ts +++ b/src/main/orcad/main.ts @@ -8,8 +8,10 @@ import { ORCAD_PROFILE_PREFLIGHT_FLAG, ORCAD_STARTUP_PREFLIGHT_FLAG } from '../../shared/orcad-profile-preflight' +import { onNodePtyLoadFailure } from '../providers/node-pty-load-failure' import { confirmBundledOrcadStartupQualified, + discardBundledOrcadStartupQualification, forgetBundledOrcadStartupQualification, preflightBundledOrcadStartup, runOrcadProfilePreflight @@ -97,6 +99,7 @@ function startOrcadProcess(): void { reserveServeStdoutForReadiness() void preflightBundledOrcadStartup() .then((qualified) => { + onNodePtyLoadFailure(() => void discardBundledOrcadStartupQualification()) // Probe before deferred node-pty loading can crash this process; a slot qualified on this // unchanged host passed it on its last launch. if (runOrcadNativePreflightVerdict({ qualified }) !== 'ok') { diff --git a/src/main/orcad/orcad-profile-preflight.test.ts b/src/main/orcad/orcad-profile-preflight.test.ts index 90cddc81d1d..ab6788a9d43 100644 --- a/src/main/orcad/orcad-profile-preflight.test.ts +++ b/src/main/orcad/orcad-profile-preflight.test.ts @@ -9,6 +9,7 @@ import type * as BundledRuntime from './orcad-bundled-runtime' import { resolveOrcadExitCode } from './orcad-exit-code' import { confirmBundledOrcadStartupQualified, + discardBundledOrcadStartupQualification, forgetBundledOrcadStartupQualification, preflightBundledOrcadStartup, runOrcadProfilePreflight @@ -319,13 +320,57 @@ describe('startup qualification on an unchanged host', () => { vi.spyOn(process, 'platform', 'get').mockReturnValue('win32') fixture.consumeQualification.mockResolvedValue(true) - await preflightBundledOrcadStartup() + // A matching marker never qualifies a Windows slot, so its node-pty probe runs too. + await expect(preflightBundledOrcadStartup()).resolves.toBe(false) await confirmBundledOrcadStartupQualified() expect(fixture.run).toHaveBeenCalledOnce() expect(fixture.recordQualification).not.toHaveBeenCalled() }) + it('discards the slot marker after node-pty fails to load, and records nothing for this launch', async () => { + fixture.consumeQualification.mockResolvedValue(true) + await preflightBundledOrcadStartup() + + await discardBundledOrcadStartupQualification() + await confirmBundledOrcadStartupQualified() + + expect(fixture.discardQualification).toHaveBeenCalledExactlyOnceWith('/slot') + expect(fixture.recordQualification).not.toHaveBeenCalled() + }) + + it('unlinks the marker only after an in-flight record has renamed it into place', async () => { + const order: string[] = [] + const record = Promise.withResolvers() + fixture.recordQualification.mockImplementation(async () => { + await record.promise + order.push('recorded') + }) + fixture.discardQualification.mockImplementation(async () => { + order.push('discarded') + }) + await preflightBundledOrcadStartup() + + const confirming = confirmBundledOrcadStartupQualified() + const discarding = discardBundledOrcadStartupQualification() + await Promise.resolve() + expect(order).toEqual([]) + record.resolve() + await Promise.all([confirming, discarding]) + + expect(order).toEqual(['recorded', 'discarded']) + }) + + it('touches no marker on Windows or outside a managed slot when node-pty fails', async () => { + vi.spyOn(process, 'platform', 'get').mockReturnValue('win32') + await discardBundledOrcadStartupQualification() + fixture.qualifiable.mockReturnValue(false) + vi.spyOn(process, 'platform', 'get').mockReturnValue('linux') + await discardBundledOrcadStartupQualification() + + expect(fixture.discardQualification).not.toHaveBeenCalled() + }) + it('never skips the version check, which runs before any marker is read', async () => { fixture.readVersion.mockRejectedValue(new Error('ENOENT')) fixture.consumeQualification.mockResolvedValue(true) diff --git a/src/main/orcad/orcad-profile-preflight.ts b/src/main/orcad/orcad-profile-preflight.ts index 2d41ecae067..1909b5428b2 100644 --- a/src/main/orcad/orcad-profile-preflight.ts +++ b/src/main/orcad/orcad-profile-preflight.ts @@ -31,6 +31,8 @@ import { } from './orcad-bundled-runtime' let pendingQualification: { directory: string; key: string } | null = null +// The last marker write; recording fails open, so it never rejects. +let markerRecording: Promise = Promise.resolve() /** Qualify packaged startup before touching profile indexes, data-root locks or imports. */ export async function preflightBundledOrcadStartup(): Promise { @@ -60,6 +62,17 @@ export async function preflightBundledOrcadStartup(): Promise { return false } +/** node-pty failed to load at spawn: the next launch must re-probe and report why. */ +export async function discardBundledOrcadStartupQualification(): Promise { + pendingQualification = null + // A record still renaming its marker into place would recreate it after an earlier unlink. + await markerRecording + const directory = resolveBundledOrcadSlot() + if (process.platform !== 'win32' && isQualifiableOrcadSlot(directory)) { + await discardOrcadStartupQualification(directory) + } +} + /** A launch whose node-pty probe did not pass must not let the next one skip either probe. */ export function forgetBundledOrcadStartupQualification(): void { pendingQualification = null @@ -95,7 +108,8 @@ export async function confirmBundledOrcadStartupQualified(): Promise { const qualified = pendingQualification pendingQualification = null if (qualified) { - await recordOrcadStartupQualification(qualified.directory, qualified.key) + markerRecording = recordOrcadStartupQualification(qualified.directory, qualified.key) + await markerRecording } } diff --git a/src/main/orcad/orcad-startup-qualification.test.ts b/src/main/orcad/orcad-startup-qualification.test.ts index 1e3b2b222ad..87c34244841 100644 --- a/src/main/orcad/orcad-startup-qualification.test.ts +++ b/src/main/orcad/orcad-startup-qualification.test.ts @@ -46,8 +46,9 @@ describe('orcad startup qualification', () => { const key = JSON.parse(await orcadStartupQualificationKey(slot, '0.1.0+aaaaaaaaaaaa')) expect(Object.keys(key)).toEqual( - expect.arrayContaining(['runtime', 'node', 'abi', 'kernel', 'files']) + expect.arrayContaining(['runtime', 'node', 'execPath', 'abi', 'kernel', 'files']) ) + expect(key.execPath).toBe(process.execPath) expect(key.abi).toHaveProperty('glibcVersion') }) @@ -73,6 +74,13 @@ describe('orcad startup qualification', () => { } ) + it('never matches a marker written under the older schema', async () => { + const key = await orcadStartupQualificationKey(slot, '0.1.0+aaaaaaaaaaaa') + await recordOrcadStartupQualification(slot, JSON.stringify({ ...JSON.parse(key), schema: 1 })) + + expect(await consumeOrcadStartupQualification(slot, key)).toBe(false) + }) + it('fails the key closed when an installed file is missing', async () => { rmSync(join(slot, 'daemon-entry.js')) diff --git a/src/main/orcad/orcad-startup-qualification.ts b/src/main/orcad/orcad-startup-qualification.ts index 9ad8fe4f90e..4ca7643bf34 100644 --- a/src/main/orcad/orcad-startup-qualification.ts +++ b/src/main/orcad/orcad-startup-qualification.ts @@ -62,6 +62,8 @@ export async function orcadStartupQualificationKey( artifactVersion, runtime: ORCAD_NODE_RUNTIME_IDENTITY, node: process.versions.node, + // The executable that ran: a slot re-pointed at another Node build must re-qualify. + execPath: process.execPath, abi, kernel: release(), launchEnv, diff --git a/src/main/providers/local-pty-runtime-spawn.test.ts b/src/main/providers/local-pty-runtime-spawn.test.ts new file mode 100644 index 00000000000..6976e5d2a61 --- /dev/null +++ b/src/main/providers/local-pty-runtime-spawn.test.ts @@ -0,0 +1,22 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' + +vi.mock('node-pty', () => { + throw new Error('node-pty native binding failed to load') +}) + +import { loadLocalPtyRuntimeSpawn } from './local-pty-runtime-spawn' +import { onNodePtyLoadFailure } from './node-pty-load-failure' + +let unsubscribe = (): void => {} +afterEach(() => unsubscribe()) + +describe('loadLocalPtyRuntimeSpawn', () => { + it('reports a node-pty load failure to listeners, then rethrows it', async () => { + const failures = vi.fn() + unsubscribe = onNodePtyLoadFailure(failures) + + await expect(loadLocalPtyRuntimeSpawn()).rejects.toThrow() + + expect(failures).toHaveBeenCalledOnce() + }) +}) diff --git a/src/main/providers/local-pty-runtime-spawn.ts b/src/main/providers/local-pty-runtime-spawn.ts index e53eb0d2ad0..b945e0c1c80 100644 --- a/src/main/providers/local-pty-runtime-spawn.ts +++ b/src/main/providers/local-pty-runtime-spawn.ts @@ -4,6 +4,7 @@ import { type ShellSpawnParams, type ShellSpawnResult } from './local-pty-utils' +import { reportNodePtyLoadFailure } from './node-pty-load-failure' type LocalPtySpawn = ( params: Omit & { signal?: AbortSignal } @@ -11,7 +12,10 @@ type LocalPtySpawn = ( /** Degraded daemon routing loads node-pty lazily, as the daemon does. */ export async function loadLocalPtyRuntimeSpawn(): Promise { - const pty = await import('node-pty') + const pty = await import('node-pty').catch((error: unknown) => { + reportNodePtyLoadFailure(error) + throw error + }) const ptySpawn: typeof pty.spawn = (file, args, options) => { assertWslDistroAllowed(file, args) return pty.spawn(file, args, options) diff --git a/src/main/providers/node-pty-load-failure.ts b/src/main/providers/node-pty-load-failure.ts new file mode 100644 index 00000000000..266c04176c9 --- /dev/null +++ b/src/main/providers/node-pty-load-failure.ts @@ -0,0 +1,17 @@ +/** Told when node-pty fails to load in this process; orcad drops its slot's startup qualification. */ +const listeners = new Set<(error: unknown) => void>() + +export function onNodePtyLoadFailure(listener: (error: unknown) => void): () => void { + listeners.add(listener) + return () => listeners.delete(listener) +} + +export function reportNodePtyLoadFailure(error: unknown): void { + for (const listener of listeners) { + try { + listener(error) + } catch (listenerError) { + console.warn('[pty] node-pty load-failure listener failed', listenerError) + } + } +}