diff --git a/src/application/javascript/ElectronActiveObservationService.ts b/src/application/javascript/ElectronActiveObservationService.ts index 2d3036228..faf68db80 100644 --- a/src/application/javascript/ElectronActiveObservationService.ts +++ b/src/application/javascript/ElectronActiveObservationService.ts @@ -1,5 +1,6 @@ import { dirname } from "node:path"; -import { realpath } from "node:fs/promises"; +import { constants } from "node:fs"; +import { access, realpath, stat } from "node:fs/promises"; import type { ExecutionOptions } from "../AnalysisProvider.js"; import type { @@ -43,11 +44,35 @@ const resolveInputPath = async ( } }; +/** Resolve the executable and require a launchable regular file before any spawn. */ +const resolveExecutablePath = async ( + selected: string, +): ReturnType => { + const resolved = await resolveInputPath("executable_path", selected); + if (!resolved.ok) return resolved; + try { + if (!(await stat(resolved.value)).isFile()) + throw new Error("not a regular file"); + if (process.platform !== "win32") + await access(resolved.value, constants.X_OK); + return resolved; + } catch (cause: unknown) { + return err({ + issue: { + path: ["executable_path"], + reason: "invalid_value", + message: `executable_path ${selected} is not an executable file: ${cause instanceof Error ? cause.message : String(cause)}`, + }, + cause, + }); + } +}; + const canonicalizeInput = async ( input: ElectronActiveObservationInput, ): Promise> => { const [executable, application, root] = await Promise.all([ - resolveInputPath("executable_path", input.executable_path), + resolveExecutablePath(input.executable_path), resolveInputPath("application_path", input.application_path), input.application_root === undefined ? Promise.resolve(ok(undefined)) diff --git a/src/browser/PlaywrightElectronActiveProvider.ts b/src/browser/PlaywrightElectronActiveProvider.ts index 23379524d..14a98aab4 100644 --- a/src/browser/PlaywrightElectronActiveProvider.ts +++ b/src/browser/PlaywrightElectronActiveProvider.ts @@ -96,13 +96,16 @@ export class PlaywrightElectronActiveProvider implements ElectronActiveObservati await systemProcessOwnershipHost.captureBaseline?.(options.signal); if (options.signal?.aborted) throw new BrowserObservationError(OPERATION, "cancelled"); - application = await this.launch({ + const launchOptions: Parameters[0] = { executablePath: paths.executable, cwd: paths.root, env: safeElectronEnvironment(this.environment, runId), args: ["-r", hookPath, paths.application, ...input.args], timeout: Math.max(1, startupDeadline - performance.now()), - }); + }; + application = await containLaunchRejections(() => + this.launch(launchOptions), + ); const leaderPid = application.process().pid; if ( typeof leaderPid !== "number" || @@ -377,6 +380,88 @@ const safeElectronEnvironment = ( ["REA_PROCESS_RUN_ID", runId] as const, ]); +const PLAYWRIGHT_INTERNAL_FRAME = /playwright-core[\\/]lib[\\/]/; +interface LaunchRejectionScope { + failure?: unknown; +} +const activeLaunches = new Set(); +const heldLaunchRejections = new Map< + Promise, + { + readonly reason: Error; + readonly candidates: Set; + readonly hadExternalListener: boolean; + } +>(); + +const holdLaunchRejection = ( + reason: unknown, + promise: Promise, +): void => { + if ( + !(reason instanceof Error) || + !PLAYWRIGHT_INTERNAL_FRAME.test(reason.stack ?? "") + ) + throw reason; + if (!heldLaunchRejections.has(promise)) + heldLaunchRejections.set(promise, { + reason, + candidates: new Set(activeLaunches), + hadExternalListener: process + .listeners("unhandledRejection") + .some((listener) => listener !== holdLaunchRejection), + }); +}; + +const forwardLaunchRejection = ( + reason: Error, + hadExternalListener: boolean, +): void => { + // Listeners present at the original event already observed it. When the + // holding listener was the only listener, restore Node's fatal behavior. + if (!hadExternalListener) + process.nextTick(() => { + throw reason; + }); +}; + +/** Playwright leaves rejected launch promises unhandled; absorb only those that repeat this launch's failure. */ +const containLaunchRejections = async ( + launch: () => Promise, +): Promise => { + const scope: LaunchRejectionScope = {}; + if (activeLaunches.size === 0) + process.on("unhandledRejection", holdLaunchRejection); + activeLaunches.add(scope); + try { + return await launch(); + } catch (cause: unknown) { + scope.failure = cause; + throw cause; + } finally { + await new Promise((resolve) => setImmediate(resolve)); + activeLaunches.delete(scope); + if (activeLaunches.size === 0) + process.off("unhandledRejection", holdLaunchRejection); + for (const [ + promise, + { reason, candidates, hadExternalListener }, + ] of heldLaunchRejections) { + if (!candidates.delete(scope)) continue; + if ( + scope.failure instanceof Error && + reason.message !== "" && + scope.failure.message.includes(reason.message) + ) + heldLaunchRejections.delete(promise); + else if (candidates.size === 0) { + heldLaunchRejections.delete(promise); + forwardLaunchRejection(reason, hadExternalListener); + } + } + } +}; + const providerError = (cause: unknown): AnalysisError => cause instanceof AnalysisError ? cause diff --git a/tests/acceptance/applications/electronInvalidPaths.test.ts b/tests/acceptance/applications/electronInvalidPaths.test.ts index e7d5935e2..d1b2d5487 100644 --- a/tests/acceptance/applications/electronInvalidPaths.test.ts +++ b/tests/acceptance/applications/electronInvalidPaths.test.ts @@ -1,4 +1,4 @@ -import { writeFile } from "node:fs/promises"; +import { chmod, mkdir, writeFile } from "node:fs/promises"; import { join } from "node:path"; import { expect } from "vitest"; @@ -68,6 +68,130 @@ cliTest( }, ); +const isWindows = process.platform === "win32"; + +const errorBody = (result: { + readonly content: readonly { readonly type: string }[]; +}) => { + const content = result.content.find((item) => item.type === "text"); + if (content?.type !== "text" || !("text" in content)) + throw new Error("Missing error response"); + return JSON.parse(String(content.text)); +}; + +const createElectronApplication = async (root: string) => { + const application = join(root, "app"); + await mkdir(application); + await writeFile( + join(application, "package.json"), + JSON.stringify({ name: "fixture", main: "main.js" }), + ); + await writeFile(join(application, "main.js"), ""); + return application; +}; + +cliTest.skipIf(isWindows)( + "rejects a non-executable Electron executable_path before launch through CLI and MCP", + async ({ cli }) => { + const root = await createTestTempDirectory("rea-electron-not-executable-"); + const application = await createElectronApplication(root); + const executable = join(root, "not-an-executable.txt"); + await writeFile(executable, "plain text"); + await chmod(executable, 0o644); + const request = { + executable_path: executable, + application_path: application, + actions: [], + }; + const requestPath = join(root, "input.json"); + await writeFile(requestPath, JSON.stringify(request)); + + const response = await cli.run({ + arguments: ["capture-electron-scenario", requestPath, "--format", "json"], + environment: { REA_LOG_LEVEL: "silent" }, + }); + expect(response.exitCode).toBe(1); + expect(response.json).toMatchObject({ + code: "invalid_request", + details: { + issues: [{ path: ["executable_path"], reason: "invalid_value" }], + }, + }); + + const { call } = await connectLocalToolsMcp(); + const result = await call("capture_electron_scenario", request); + expect(result.isError).toBe(true); + expect(errorBody(result)).toMatchObject({ + error: { + code: "invalid_request", + details: { + issues: [{ path: ["executable_path"], reason: "invalid_value" }], + }, + }, + }); + const session = await call("binary_session", {}); + expect(session.isError, JSON.stringify(session)).not.toBe(true); + }, +); + +cliTest.skipIf(isWindows)( + "returns a typed failure when an executable exits before Electron starts and keeps the MCP server and its Evidence", + async ({ cli }) => { + const root = await createTestTempDirectory("rea-electron-early-exit-"); + const application = await createElectronApplication(root); + const executable = join(root, "exits-immediately"); + await writeFile(executable, "#!/bin/sh\nexit 0\n"); + await chmod(executable, 0o755); + const request = { + executable_path: executable, + application_path: application, + actions: [], + }; + const requestPath = join(root, "input.json"); + await writeFile(requestPath, JSON.stringify(request)); + + const response = await cli.run({ + arguments: ["capture-electron-scenario", requestPath, "--format", "json"], + environment: { REA_LOG_LEVEL: "silent" }, + }); + expect(response.exitCode).toBe(1); + expect(response.json).toMatchObject({ + code: "execution_failure", + details: { operation: "capture_electron_scenario" }, + }); + expect(response.stdout).toContain("Process failed to launch"); + + const { call } = await connectLocalToolsMcp(); + const analysis = await call("analyze_javascript_application", { + input_path: application, + }); + expect(analysis.isError, JSON.stringify(analysis)).not.toBe(true); + const retained = async () => { + const bundle = await call("get_evidence_bundle", {}); + expect(bundle.isError, JSON.stringify(bundle)).not.toBe(true); + const structured = bundle.structuredContent as { + result: { records: { evidence_id: string }[] }; + }; + return structured.result.records.map((record) => record.evidence_id); + }; + const before = await retained(); + expect(before).toHaveLength(1); + + const result = await call("capture_electron_scenario", request); + expect(result.isError).toBe(true); + expect(errorBody(result)).toMatchObject({ + error: { + code: "execution_failure", + details: { operation: "capture_electron_scenario" }, + }, + }); + const session = await call("binary_session", {}); + expect(session.isError, JSON.stringify(session)).not.toBe(true); + expect(await retained()).toEqual(before); + }, + 180_000, +); + cliTest( "rejects a file URL path field without resolving an invented local path", async ({ cli }) => { diff --git a/tests/boundary/process/electronLaunchRejections.test.ts b/tests/boundary/process/electronLaunchRejections.test.ts new file mode 100644 index 000000000..c2f32e5c8 --- /dev/null +++ b/tests/boundary/process/electronLaunchRejections.test.ts @@ -0,0 +1,323 @@ +import { chmod, mkdir, writeFile } from "node:fs/promises"; +import { join } from "node:path"; + +import { expect, vi } from "vitest"; + +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; +import { + processTest, + type TestProcesses, +} from "../../support/process/processFixture.js"; + +const script = ` +import { PlaywrightElectronActiveProvider } from "./dist/browser/PlaywrightElectronActiveProvider.js"; +import { electronActiveObservationInputSchema } from "./dist/domain/javascript/electronActiveObservation.js"; + +const [scenario, executable, root] = JSON.parse(process.argv[1]); +let observedRejections = 0; +if (scenario === "oneFailureWithManySuccesses" || scenario === "externalListenerSeesUnrelatedOnce") + process.on("unhandledRejection", () => { observedRejections += 1; }); +const orphanStack = (message, file) => + message + "\\n at waitForLine (/app/node_modules/playwright-core/lib/server/" + file + ":247:21)"; +const playwrightError = (message, file = "electron/electron.js") => { + const error = new Error(message); + error.stack = "Error: " + orphanStack(message, file); + return error; +}; +const rejectLater = (error, ms) => setTimeout(() => void Promise.reject(error), ms); +const fakeApplication = { process: () => ({ pid: undefined }), close: async () => undefined }; +const launchFailure = (message) => + new Error("electron.launch: " + message + "\\nCall log:\\n - "); +const after = (ms, action) => new Promise((resolve, reject) => setTimeout(() => { + try { resolve(action()); } catch (error) { reject(error); } +}, ms)); +const orphans = (message) => { + for (let index = 0; index < 3; index += 1) void Promise.reject(playwrightError(message)); +}; + +let during = -1; +const launches = { + realLaunch: undefined, + realWithUnrelatedPlaywrightRejection: async (options) => { + void Promise.reject(playwrightError("unrelated playwright failure", "page.js")); + return (await import("playwright-core"))._electron.launch(options); + }, + failureWithOrphans: async () => { + during = process.listenerCount("unhandledRejection"); + orphans("Process failed to launch!"); + await after(10, () => { throw launchFailure("Process failed to launch!"); }); + }, + failureInOneTick: async () => { + orphans("Process failed to launch!"); + throw launchFailure("Process failed to launch!"); + }, + failureWithUnrelatedPlaywrightRejection: async () => { + orphans("Process failed to launch!"); + void Promise.reject(playwrightError("Target page, context or browser has been closed", "page.js")); + await after(10, () => { throw launchFailure("Process failed to launch!"); }); + }, + externalListenerSeesUnrelatedOnce: async () => { + void Promise.reject(playwrightError("unrelated playwright failure", "page.js")); + await after(10, () => { throw launchFailure("Process failed to launch!"); }); + }, + pendingWithUnrelatedRejection: () => { + setInterval(() => {}, 1000); + rejectLater(new Error("unrelated application failure"), 20); + return new Promise(() => {}); + }, + success: async () => { + during = process.listenerCount("unhandledRejection"); + return fakeApplication; + }, + successWithPlaywrightRejection: async () => { + void Promise.reject(playwrightError("stray playwright failure", "page.js")); + await after(10, () => fakeApplication); + return fakeApplication; + }, +}; + +const input = (executablePath) => ({ + ...electronActiveObservationInputSchema.parse({ + executable_path: executablePath, + application_path: root, + }), + application_root: root, +}); +const capture = (launch, executablePath = executable) => + new PlaywrightElectronActiveProvider({}, launch).capture(input(executablePath)); + +const before = process.listenerCount("unhandledRejection"); +let results; +if (scenario === "concurrentFailures" || scenario === "failureThenSuccess" || scenario === "successThenFailure" || scenario === "oneFailureWithManySuccesses") { + const failing = (message, orphanAt, settleAt) => async () => { + during = Math.max(during, process.listenerCount("unhandledRejection")); + await after(orphanAt, () => orphans(message)); + await after(settleAt - orphanAt, () => { throw launchFailure(message); }); + }; + const succeeding = (settleAt) => () => { + during = Math.max(during, process.listenerCount("unhandledRejection")); + return after(settleAt, () => fakeApplication); + }; + const pair = { + concurrentFailures: [failing("alpha failed", 10, 30), failing("beta failed", 20, 50)], + failureThenSuccess: [failing("alpha failed", 10, 20), succeeding(40)], + successThenFailure: [failing("alpha failed", 20, 40), succeeding(30)], + oneFailureWithManySuccesses: [failing("alpha failed", 10, 80), ...[20, 30, 40, 50].map(succeeding)], + }[scenario]; + results = await Promise.all(pair.map((launch) => capture(launch))); +} else { + results = [await capture(launches[scenario] ?? undefined)]; +} +await new Promise((resolve) => setTimeout(resolve, 50)); +console.log(JSON.stringify({ + before, + during, + after: process.listenerCount("unhandledRejection"), + observedRejections, + reasons: results.map((result) => result.ok ? null : result.error.reason ?? null), + ok: results.map((result) => result.ok), + causeReasons: results.map((result) => result.ok ? null : result.error.cause?.reason ?? null), + cleanupResources: results.map((result) => result.ok ? null : result.error.cleanupResources ?? null), + causes: results.map((result) => result.ok ? null : String(result.error.cause?.message).split("\\n")[0]), +})); +`; + +// A cold macOS Swift toolchain may spend up to 120 seconds building the +// ownership reader before any child launch reaches its assertion. +vi.setConfig({ testTimeout: 180_000 }); + +type Outcome = { + readonly before: number; + readonly during: number; + readonly after: number; + readonly ok: readonly boolean[]; + readonly causes: readonly (string | null)[]; + readonly observedRejections: number; + readonly reasons: readonly (string | null)[]; + readonly causeReasons: readonly (string | null)[]; + readonly cleanupResources: readonly (readonly string[] | null)[]; +}; + +const run = async ( + processes: TestProcesses, + scenario: string, + executable = "/not/launched", + root = "/", +) => + processes.run( + process.execPath, + [ + "--input-type=module", + "--eval", + script, + JSON.stringify([scenario, executable, root]), + ], + { timeoutMs: 150_000 }, + ); + +const outcome = (result: { readonly stdout: string }): Outcome => + JSON.parse(result.stdout.trim().split("\n").at(-1) ?? "") as Outcome; + +const earlyExit = async () => { + const root = await createTestTempDirectory("rea-launch-rejections-"); + const executable = join(root, "exits-immediately"); + await mkdir(join(root, "app")); + await writeFile(executable, "#!/bin/sh\nexit 0\n"); + await chmod(executable, 0o755); + return { executable, root: join(root, "app") }; +}; + +processTest( + "absorbs the orphaned rejections of a failed launch and removes its listener", + async ({ processes }) => { + const result = await run(processes, "failureWithOrphans"); + expect(result.exitCode, result.stderr).toBe(0); + expect(outcome(result)).toMatchObject({ + during: outcome(result).before + 1, + after: outcome(result).before, + ok: [false], + causes: ["electron.launch: Process failed to launch!"], + }); + }, +); + +processTest( + "absorbs orphans raised in the same tick as the launch failure", + async ({ processes }) => { + const result = await run(processes, "failureInOneTick"); + expect(result.exitCode, result.stderr).toBe(0); + expect(outcome(result)).toMatchObject({ ok: [false] }); + }, +); + +processTest.skipIf(process.platform === "win32")( + "absorbs the orphans of a real Playwright launch that exits at once", + async ({ processes }) => { + const { executable, root } = await earlyExit(); + const result = await run(processes, "realLaunch", executable, root); + expect(result.exitCode, result.stderr).toBe(0); + expect(outcome(result)).toMatchObject({ + ok: [false], + causes: ["electron.launch: Process failed to launch!"], + }); + expect(outcome(result).after).toBe(outcome(result).before); + }, +); + +processTest.skipIf(process.platform === "win32")( + "still fails on an unrelated Playwright rejection during a real failing launch", + async ({ processes }) => { + const { executable, root } = await earlyExit(); + const result = await run( + processes, + "realWithUnrelatedPlaywrightRejection", + executable, + root, + ); + expect(result.exitCode, result.stderr).toBe(1); + expect(result.stderr).toContain("unrelated playwright failure"); + expect(result.stdout).toBe(""); + }, +); + +processTest( + "does not replay an unrelated orphan to a listener that already received it", + async ({ processes }) => { + const result = await run(processes, "externalListenerSeesUnrelatedOnce"); + expect(result.exitCode, result.stderr).toBe(0); + expect(outcome(result)).toMatchObject({ + observedRejections: 1, + ok: [false], + causes: ["electron.launch: Process failed to launch!"], + }); + }, +); + +processTest( + "still fails on an unrelated Playwright rejection beside a failing launch's own orphans", + async ({ processes }) => { + const result = await run( + processes, + "failureWithUnrelatedPlaywrightRejection", + ); + expect(result.exitCode, result.stderr).toBe(1); + expect(result.stderr).toContain( + "Target page, context or browser has been closed", + ); + expect(result.stdout).toBe(""); + }, +); + +processTest( + "still fails at once on an unrelated rejection while a launch is pending", + async ({ processes }) => { + const result = await run(processes, "pendingWithUnrelatedRejection"); + expect(result.exitCode, result.stderr).toBe(1); + expect(result.stderr).toContain("unrelated application failure"); + expect(result.stdout).toBe(""); + }, +); + +processTest( + "removes its listener after a successful launch", + async ({ processes }) => { + const result = await run(processes, "success"); + expect(result.exitCode, result.stderr).toBe(0); + expect(outcome(result)).toMatchObject({ + during: outcome(result).before + 1, + after: outcome(result).before, + }); + }, +); + +processTest( + "re-raises a Playwright rejection when the launch succeeds", + async ({ processes }) => { + const result = await run(processes, "successWithPlaywrightRejection"); + expect(result.exitCode, result.stderr).toBe(1); + expect(result.stderr).toContain("stray playwright failure"); + expect(result.stdout).toBe(""); + }, +); + +processTest.for([ + "concurrentFailures", + "failureThenSuccess", + "successThenFailure", +])( + "keeps overlapping launches from re-raising each other's orphans (%s)", + async (scenario, { processes }) => { + const result = await run(processes, scenario); + expect(result.exitCode, result.stderr).toBe(0); + expect(outcome(result).after).toBe(outcome(result).before); + }, +); + +processTest( + "holds each orphan once across many overlapping successful launches", + async ({ processes }) => { + const result = await run(processes, "oneFailureWithManySuccesses"); + expect(result.exitCode, result.stderr).toBe(0); + const observed = outcome(result); + expect(observed.during).toBe(observed.before + 1); + expect(observed.after).toBe(observed.before); + expect(observed.observedRejections).toBe(3); + expect(observed.ok).toEqual([false, false, false, false, false]); + expect(observed.causes[0]).toBe("electron.launch: alpha failed"); + expect(observed.reasons).toEqual([ + null, + "cleanup_failed", + "cleanup_failed", + "cleanup_failed", + "cleanup_failed", + ]); + expect(observed.causeReasons).toEqual([ + null, + ...Array.from({ length: 4 }, () => "process_ownership_unavailable"), + ]); + expect(observed.cleanupResources).toEqual([ + [], + ...Array.from({ length: 4 }, () => ["owned_process_group"]), + ]); + }, +);