diff --git a/docs/electron-observation.md b/docs/electron-observation.md index 945661ac28..de5ac0327b 100644 --- a/docs/electron-observation.md +++ b/docs/electron-observation.md @@ -103,6 +103,8 @@ window-targeted renderer reload/crash actions, and synthetic `open-url` or WebContents identities, process metrics, and IPC channel and value-shape metadata. Payload values are not retained. +`window_index` selects from Playwright's current Page list at the start of each action. REA resolves that Page to its corresponding BrowserWindow once; the action's `target` uses the BrowserWindow's capture-scoped `window_id`, matching the window inventory. Reordering either enumeration after selection does not retarget the action. Handle cleanup failures retain the completed action records and remain owned for provider cleanup retry. + Run the real fixture verifier with an operator-provided Electron runtime: ```bash diff --git a/scripts/verify-real-electron.mjs b/scripts/verify-real-electron.mjs index 8f647e6495..45306de604 100644 --- a/scripts/verify-real-electron.mjs +++ b/scripts/verify-real-electron.mjs @@ -62,6 +62,12 @@ if (!result.ok) throw result.error; const capture = electronActiveObservationResultSchema.parse( result.value.normalized_result, ); +const crashAction = capture.actions.find( + ({ kind }) => kind === "renderer-crash", +); +const crashedWindow = capture.windows.find( + ({ window_id }) => window_id === crashAction?.target, +); const output = { verifier_run: await completeVerifierRun(verifierRun), @@ -83,6 +89,17 @@ const output = { kind === "renderer-crash" && window_index === 1, ) && capture.windows.length >= 2 && + capture.actions + .filter(({ target }) => target !== null) + .every(({ target }) => + capture.windows.some(({ window_id }) => window_id === target), + ) && + crashedWindow !== undefined && + capture.timeline.events.some( + ({ event, target }) => + event === "render-process-gone" && + target === crashedWindow.web_contents_id, + ) && capture.ipc.events.some( ({ kind, channel }) => kind === "main-handler-invocation" && channel === "readiness:echo", diff --git a/src/browser/PlaywrightElectronActiveActions.test.ts b/src/browser/PlaywrightElectronActiveActions.test.ts index d07db0c9ff..c1265edf4c 100644 --- a/src/browser/PlaywrightElectronActiveActions.test.ts +++ b/src/browser/PlaywrightElectronActiveActions.test.ts @@ -2,6 +2,7 @@ // `npm run verify:electron`. import { afterEach, expect, it, vi } from "vitest"; import type { ElectronApplication } from "playwright-core"; +import { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js"; import { electronActiveObservationInputSchema, @@ -14,6 +15,12 @@ import { runWithExecutionLimits, } from "./PlaywrightElectronActiveActions.js"; +const fixtureBrowserWindow = async () => ({ + evaluate: async (read: (window: { id: number }) => unknown) => + read({ id: 42 }), + dispose: async () => {}, +}); + afterEach(() => { vi.useRealTimers(); }); @@ -166,9 +173,15 @@ it("preserves local Playwright action diagnostics", async () => { }; const application = { windows: () => [page], + browserWindow: fixtureBrowserWindow, }; - const result = await runElectronActions(application as never, input, {}); + const { actions: result } = await runElectronActions( + application as never, + input, + {}, + new ArtifactResourceScope(), + ); expect(result[0]).toMatchObject({ status: "failed", @@ -191,10 +204,11 @@ it("retains complete action errors and Electron window text", async () => { click: async () => Promise.reject(new Error(longError)), }), }; - const actions = await runElectronActions( - { windows: () => [page] } as never, + const { actions } = await runElectronActions( + { windows: () => [page], browserWindow: fixtureBrowserWindow } as never, input, {}, + new ArtifactResourceScope(), ); const result = createElectronActiveObservationFixtureResult("/opt/app"); const longUrl = `file://${"/segment".repeat(10_000)}`; @@ -322,7 +336,12 @@ it("runs an untargeted deep-link without requiring a BrowserWindow", async () => evaluate: async () => true, }; - const result = await runElectronActions(application as never, input, {}); + const { actions: result } = await runElectronActions( + application as never, + input, + {}, + new ArtifactResourceScope(), + ); expect(result).toMatchObject([ { @@ -348,10 +367,16 @@ it("honors caller-selected Electron waits beyond the former action timeout", asy }; const application = { windows: () => [page], + browserWindow: fixtureBrowserWindow, firstWindow: async () => page, }; - const result = await runElectronActions(application as never, input, {}); + const { actions: result } = await runElectronActions( + application as never, + input, + {}, + new ArtifactResourceScope(), + ); expect(result[0]).toMatchObject({ status: "completed" }); expect(duration).toBe(60_000); @@ -386,7 +411,7 @@ it("cancels a wait without window_index while the app has no window", async () = }), } as unknown as ElectronApplication; - const actions = await runElectronActions( + const { actions } = await runElectronActions( application, electronActiveObservationInputSchema.parse({ executable_path: "/opt/electron", @@ -395,6 +420,7 @@ it("cancels a wait without window_index while the app has no window", async () = actions: [{ step_id: "settle", kind: "wait", duration_ms: 10 }], }), { signal: controller.signal }, + new ArtifactResourceScope(), ); expect(actions).toEqual([ @@ -423,8 +449,11 @@ it("records actions after a cancelled action as not executed", async () => { reload, }; - const actions = await runElectronActions( - { windows: () => [page] } as unknown as ElectronApplication, + const { actions } = await runElectronActions( + { + windows: () => [page], + browserWindow: fixtureBrowserWindow, + } as unknown as ElectronApplication, electronActiveObservationInputSchema.parse({ executable_path: "/opt/electron", application_path: "/opt/app/main.js", @@ -435,6 +464,7 @@ it("records actions after a cancelled action as not executed", async () => { ], }), { signal: controller.signal }, + new ArtifactResourceScope(), ); expect(reload).not.toHaveBeenCalled(); @@ -593,3 +623,68 @@ it("observes an operation abandoned by an already-cancelled signal", async () => process.off("unhandledRejection", recordUnhandled); } }); + +it.each(["click", "renderer-crash"] as const)( + "keeps the selected Page's native identity and completed %s through handle cleanup failure", + async (kind) => { + const selectedEffect = vi.fn(); + const page = { + locator: () => ({ + click: async () => { + selectedEffect(); + }, + }), + }; + const other = {}; + const pages = [other, page]; + const dispose = vi + .fn() + .mockRejectedValueOnce(new Error("handle disposal failed")) + .mockResolvedValue(undefined); + const nativeWindow = { + id: 99, + isDestroyed: () => false, + webContents: { forcefullyCrashRenderer: selectedEffect }, + }; + const browserWindow = vi.fn(async (selected: unknown) => { + expect(selected).toBe(page); + pages.reverse(); + return { + evaluate: async (read: (window: typeof nativeWindow) => unknown) => + read(nativeWindow), + dispose, + }; + }); + const resources = new ArtifactResourceScope(); + const outcome = await runElectronActions( + { windows: () => pages, browserWindow } as unknown as ElectronApplication, + electronActiveObservationInputSchema.parse({ + executable_path: "/opt/electron", + application_path: "/opt/app/main.js", + application_root: "/opt/app", + actions: [ + { + step_id: "selected", + kind, + window_index: 1, + ...(kind === "click" ? { selector: "#selected" } : {}), + }, + { step_id: "later", kind: "renderer-reload", window_index: 0 }, + ], + }), + {}, + resources, + ); + expect(selectedEffect).toHaveBeenCalledOnce(); + expect(browserWindow).toHaveBeenCalledOnce(); + expect(outcome.actions).toMatchObject([ + { target: "window:99", status: "completed" }, + { target: null, status: "cancelled" }, + ]); + expect(outcome.cleanupError?.cleanupIncomplete).toBe(true); + expect(resources.hasPending).toBe(true); + await resources.close(); + expect(dispose).toHaveBeenCalledTimes(2); + expect(outcome.cleanupError?.cleanupIncomplete).toBe(false); + }, +); diff --git a/src/browser/PlaywrightElectronActiveActions.ts b/src/browser/PlaywrightElectronActiveActions.ts index 32c2af5fb9..ef89959320 100644 --- a/src/browser/PlaywrightElectronActiveActions.ts +++ b/src/browser/PlaywrightElectronActiveActions.ts @@ -1,7 +1,13 @@ -import type { ElectronApplication, Page } from "playwright-core"; +import type { ElectronApplication, JSHandle, Page } from "playwright-core"; import { z } from "zod"; import type { ExecutionOptions } from "../application/AnalysisProvider.js"; +import type { + ArtifactResourceScope, + ArtifactResourceOwner, +} from "../artifacts/ArtifactResourceScope.js"; +import { ProviderCleanupError } from "../domain/providerCleanupError.js"; +import type { AnalysisError } from "../domain/analysisErrorBase.js"; import { BrowserObservationError } from "../domain/browserObservationError.js"; import { electronActiveTimelineEventSchema, @@ -41,6 +47,11 @@ const hookSnapshotSchema = z.strictObject({ type ElectronAction = ElectronActiveObservationInput["actions"][number]; type ElectronActions = ElectronActiveObservationResult["actions"]; +type SelectedElectronWindow = JSHandle<{ + readonly id: number; + isDestroyed(): boolean; + readonly webContents: { forcefullyCrashRenderer(): void }; +}>; export type ElectronHookSnapshot = z.infer; export type ElectronHookEvent = z.infer< @@ -63,8 +74,13 @@ export const runElectronActions = async ( application: ElectronApplication, input: ElectronActiveObservationInput, options: ExecutionOptions, -): Promise => { + resources: ArtifactResourceScope, +): Promise<{ + readonly actions: ElectronActions; + readonly cleanupError?: AnalysisError; +}> => { const actions: ElectronActions = []; + let cleanupError: AnalysisError | undefined; let stoppedBy: ScenarioActionStop | undefined; for (const action of input.actions) { const actionStartedAt = performance.now(); @@ -82,6 +98,9 @@ export const runElectronActions = async ( continue; } let selectedWindow: Page | undefined; + let selectedHandle: SelectedElectronWindow | undefined; + let handleOwner: ArtifactResourceOwner | undefined; + let target: string | null = null; try { if (options.signal?.aborted === true) throw new BrowserObservationError(OPERATION, "cancelled"); @@ -93,26 +112,35 @@ export const runElectronActions = async ( windowIndex, signal: options.signal, }); - if ( - (action.kind === "click" || - action.kind === "renderer-reload" || - action.kind === "renderer-crash") && - selectedWindow === undefined - ) - throw new BrowserObservationError(OPERATION, "window_not_found"); + if (selectedWindow !== undefined) { + const handle = await application.browserWindow(selectedWindow); + selectedHandle = handle; + handleOwner = { + kind: "lease", + resource: "selected Electron BrowserWindow handle", + lease: { release: () => handle.dispose() }, + }; + resources.retain(handleOwner); + const id = z + .int() + .safe() + .nonnegative() + .parse(await selectedHandle.evaluate((window) => window.id)); + target = `window:${String(id)}`; + } await runAction({ application, action, selectedWindow, + selectedHandle, options, }); actions.push({ step_id: action.step_id, kind: action.kind, window_index: windowIndex, - target: - selectedWindow === undefined ? null : `window:${String(windowIndex)}`, + target, status: "completed", elapsed_ms: Math.max( 0, @@ -126,8 +154,7 @@ export const runElectronActions = async ( step_id: action.step_id, kind: action.kind, window_index: windowIndex, - target: - selectedWindow === undefined ? null : `window:${String(windowIndex)}`, + target, status: stoppedBy, elapsed_ms: Math.max( 0, @@ -136,14 +163,38 @@ export const runElectronActions = async ( error: safeActionErrorMessage(cause), }); } + if (handleOwner !== undefined) { + cleanupError = await releaseActionWindow(resources, handleOwner); + if (cleanupError !== undefined) stoppedBy = "failed"; + } } - return actions; + return { actions, ...(cleanupError === undefined ? {} : { cleanupError }) }; +}; + +const releaseActionWindow = async ( + resources: ArtifactResourceScope, + owner: ArtifactResourceOwner, +): Promise => { + const released = await resources.release(owner); + if (released.kind === "released") return undefined; + const error = new ProviderCleanupError( + "playwright-electron-active", + released.lease.cleanupResources ?? [owner.resource], + { + reason: safeActionErrorMessage(released.cause), + }, + { cause: released.cause, operation: OPERATION }, + ); + error.bindCleanupResourceOwner(released.lease); + released.lease.bindCleanupError(error); + return error; }; type RunActionContext = { readonly application: ElectronApplication; readonly action: ElectronAction; readonly selectedWindow: Page | undefined; + readonly selectedHandle: SelectedElectronWindow | undefined; readonly options: ExecutionOptions; }; @@ -151,6 +202,7 @@ const runAction = async ({ application, action, selectedWindow, + selectedHandle, options, }: RunActionContext): Promise => { switch (action.kind) { @@ -190,13 +242,14 @@ const runAction = async ({ ); return; case "renderer-crash": { + if (selectedHandle === undefined) + throw new BrowserObservationError(OPERATION, "window_not_found"); const crashed = await runWithExecutionLimits( - application.evaluate(({ BrowserWindow }, index) => { - const candidate = BrowserWindow.getAllWindows()[index]; - if (candidate === undefined || candidate.isDestroyed()) return false; - candidate.webContents.forcefullyCrashRenderer(); + selectedHandle.evaluate((window) => { + if (window.isDestroyed()) return false; + window.webContents.forcefullyCrashRenderer(); return true; - }, action.window_index), + }), options.signal, ); if (!crashed) diff --git a/src/browser/PlaywrightElectronActiveProvider.test.ts b/src/browser/PlaywrightElectronActiveProvider.test.ts index 82ee0dad9c..8fa445f387 100644 --- a/src/browser/PlaywrightElectronActiveProvider.test.ts +++ b/src/browser/PlaywrightElectronActiveProvider.test.ts @@ -511,6 +511,11 @@ it("records actions after a failed action as not executed and keeps the capture" process: () => ({ pid: 424242 }), close: vi.fn(async () => undefined), windows: () => [page], + browserWindow: async () => ({ + evaluate: async (read: (window: { id: number }) => unknown) => + read({ id: 42 }), + dispose: async () => {}, + }), firstWindow: async () => page, }; @@ -538,7 +543,7 @@ it("records actions after a failed action as not executed and keeps the capture" step_id: "fail", kind: "click", window_index: 0, - target: "window:0", + target: "window:42", status: "failed", elapsed_ms: expect.any(Number), error: "strict mode violation", @@ -859,7 +864,7 @@ it.each(["process", "transport"] as const)( vi.spyOn(electronActions, "runElectronActions").mockImplementation( async () => { controller.abort(reason); - return []; + return { actions: [] }; }, ); if (failure === "process") { diff --git a/src/browser/PlaywrightElectronActiveProvider.ts b/src/browser/PlaywrightElectronActiveProvider.ts index 70c8b6f63f..79bb49a3a9 100644 --- a/src/browser/PlaywrightElectronActiveProvider.ts +++ b/src/browser/PlaywrightElectronActiveProvider.ts @@ -192,7 +192,15 @@ export class PlaywrightElectronActiveProvider implements ElectronActiveObservati options.signal, startupDeadline, ); - actions = await runElectronActions(application, input, options); + const actionOutcome = await runElectronActions( + application, + input, + options, + this.#resources, + ); + actions = actionOutcome.actions; + if (actionOutcome.cleanupError !== undefined) + throw actionOutcome.cleanupError; const state = await readFinalApplicationState( application, options.signal, @@ -218,9 +226,14 @@ export class PlaywrightElectronActiveProvider implements ElectronActiveObservati ], }; } - if (cause instanceof AnalysisError && cause.cleanupIncomplete) + if (cause instanceof AnalysisError && cause.cleanupIncomplete) { + if (capture !== undefined) + cause.retainPartialObservation({ + kind: "electron-active-observation", + capture, + }); outcome = err(cause); - else if (options.signal?.aborted === true) + } else if (options.signal?.aborted === true) outcome = err( new BrowserObservationError(OPERATION, "cancelled", { ...cancellationCause(cause, options.signal),