From 8e3bf56e11bcb56d0e8754fdfa32a7a9f004ca13 Mon Sep 17 00:00:00 2001 From: Kaoru0822 Date: Wed, 21 Oct 2026 08:06:25 -0700 Subject: [PATCH] fix(electron): record actions skipped after a failed action (#6023) An Electron scenario stopped at the first failed or cancelled action and returned no record for the remaining declared actions, so normalized_result.actions could not account for the whole scenario. Record every remaining action as cancelled with a not-executed error, the representation browser scenarios already use. Both runners now share one helper for that error, which also distinguishes a stop caused by request cancellation from one caused by an action failure. Fixes #5943 Co-authored-by: Claude Opus 5.5 --- docs/browser-scenario-contract.md | 4 +- docs/electron-observation.md | 4 +- .../PlaywrightBrowserScenarioProvider.test.ts | 15 +++- .../PlaywrightBrowserScenarioProvider.ts | 26 ++++--- .../PlaywrightElectronActiveActions.test.ts | 40 ++++++++++ .../PlaywrightElectronActiveActions.ts | 27 ++++++- .../PlaywrightElectronActiveProvider.test.ts | 74 +++++++++++++++++++ src/browser/ScenarioActionSequence.ts | 14 ++++ 8 files changed, 186 insertions(+), 18 deletions(-) create mode 100644 src/browser/ScenarioActionSequence.ts diff --git a/docs/browser-scenario-contract.md b/docs/browser-scenario-contract.md index 787f91bcfa..0421bb4218 100644 --- a/docs/browser-scenario-contract.md +++ b/docs/browser-scenario-contract.md @@ -212,7 +212,9 @@ statements remain under the attached debugger client's control. The result is Evidence with an initial state followed by one record per declared action. Each step reports action status, elapsed time, sanitized URLs, event bounds, and independently typed capture states for screenshot, DOM, -accessibility, URL, history, and storage. Console, page-error, network, +accessibility, URL, history, and storage. Actions after a failed or cancelled +action are not run and are recorded as `cancelled` with a not-executed error. +Console, page-error, network, WebSocket, frame, worker, popup, and cancelled-download events are attributed to steps. Retained events and requested artifacts are returned inline; omitted events are counted by kind and identify affected network transactions. diff --git a/docs/electron-observation.md b/docs/electron-observation.md index 8248237d12..612dd98f89 100644 --- a/docs/electron-observation.md +++ b/docs/electron-observation.md @@ -116,7 +116,9 @@ The CLI and MCP surfaces accept the same schema. For a JSON request file: rea capture-electron-scenario scenario.json --json ``` -The result records action status and targets, correlated app/window/WebContents, +The result records one status and target per declared action; actions after a +failed or cancelled action are not run and are recorded as `cancelled` with a +not-executed error. It also records correlated app/window/WebContents, preload, session, navigation, shell, permission, popup, download, protocol, native-addon, process, and IPC timeline events. IPC channels and argument-shape metadata retain their observed lengths; payload values are never retained. diff --git a/src/browser/PlaywrightBrowserScenarioProvider.test.ts b/src/browser/PlaywrightBrowserScenarioProvider.test.ts index 88fe1f97d8..842f894b66 100644 --- a/src/browser/PlaywrightBrowserScenarioProvider.test.ts +++ b/src/browser/PlaywrightBrowserScenarioProvider.test.ts @@ -362,6 +362,9 @@ describe("PlaywrightBrowserScenarioProvider", () => { expect(result.value.steps[1]?.error).toBe( "fixture [REDACTED] action failure", ); + expect(result.value.steps[2]?.error).toBe( + "not executed after an earlier action failure", + ); expect(session.performCalls).toBe(1); expect(session.closeCalls).toBe(1); expect(result.value.completeness.equality_eligible).toBe(false); @@ -713,7 +716,7 @@ describe("PlaywrightBrowserScenarioProvider request cancellation", () => { const provider = new PlaywrightBrowserScenarioProvider(() => Promise.resolve(session), ); - const result = await provider.captureScenario(scenario(), { + const result = await provider.captureScenario(scenario({ actions: 2 }), { signal: controller.signal, }); expect(result).toMatchObject({ @@ -724,12 +727,20 @@ describe("PlaywrightBrowserScenarioProvider request cancellation", () => { kind: "browser-scenario-observation", capture: { browser: { cleanup: "terminated-owned-process" }, - steps: [{ status: "completed" }, { status: "cancelled" }], + steps: [ + { status: "completed" }, + { status: "cancelled" }, + { + status: "cancelled", + error: "not executed after scenario request cancellation", + }, + ], }, }, }, }); if (result.ok) throw new Error("Expected active scenario cancellation"); + expect(session.performCalls).toBe(1); expect(result.error.cause).toBe(cancellationReason); expect(session.closeCalls).toBe(1); }); diff --git a/src/browser/PlaywrightBrowserScenarioProvider.ts b/src/browser/PlaywrightBrowserScenarioProvider.ts index 65e8fb67cc..018adbebaf 100644 --- a/src/browser/PlaywrightBrowserScenarioProvider.ts +++ b/src/browser/PlaywrightBrowserScenarioProvider.ts @@ -39,6 +39,10 @@ import type { BrowserScenarioSessionOpener, BrowserScenarioSessionPort, } from "./BrowserScenarioSessionPort.js"; +import { + notExecutedActionError, + type ScenarioActionStop, +} from "./ScenarioActionSequence.js"; import { browserScenarioCleanupObservation, browserScenarioOperationFailure, @@ -161,14 +165,15 @@ const executeStep = async (input: { readonly scenario: BrowserScenario; readonly action: BrowserScenarioAction; readonly stepIndex: number; - readonly priorFailure: boolean; + readonly stoppedBy: ScenarioActionStop | undefined; readonly signal: AbortSignal | undefined; }): Promise => { - const { session, scenario, action, stepIndex, priorFailure, signal } = input; + const { session, scenario, action, stepIndex, stoppedBy, signal } = input; const beforeUrl = session.sanitizeUrl(session.currentUrl()); const eventStart = session.nextEventSequence(); const requested = requestedForStep(scenario, stepIndex); - if (priorFailure) + if (stoppedBy !== undefined) { + const error = notExecutedActionError(stoppedBy); return createStep({ stepIndex, stepId: action.step_id, @@ -177,14 +182,12 @@ const executeStep = async (input: { elapsedMs: 0, beforeUrl, afterUrl: beforeUrl, - error: "not executed after an earlier action failure", + error, eventStart, eventEnd: session.lastEventSequence(), - artifacts: unavailableArtifacts( - requested, - "action was not executed after an earlier failure", - ), + artifacts: unavailableArtifacts(requested, error), }); + } const actionStartedAt = performance.now(); session.setStep(stepIndex); @@ -270,18 +273,19 @@ const runScenario = async ( signal: options.signal, }), ); - let failed = false; + let stoppedBy: ScenarioActionStop | undefined; for (const [offset, action] of scenario.actions.entries()) { const step = await executeStep({ session, scenario, action, stepIndex: offset + 1, - priorFailure: failed, + stoppedBy, signal: options.signal, }); steps.push(step); - failed ||= step.status !== "completed"; + if (stoppedBy === undefined && step.status !== "completed") + stoppedBy = step.status; } } catch (cause: unknown) { operationOutcome = { ok: false, cause }; diff --git a/src/browser/PlaywrightElectronActiveActions.test.ts b/src/browser/PlaywrightElectronActiveActions.test.ts index d0ba53574b..d07db0c9ff 100644 --- a/src/browser/PlaywrightElectronActiveActions.test.ts +++ b/src/browser/PlaywrightElectronActiveActions.test.ts @@ -410,6 +410,46 @@ it("cancels a wait without window_index while the app has no window", async () = ]); }); +it("records actions after a cancelled action as not executed", async () => { + const controller = new AbortController(); + const reload = vi.fn(async () => undefined); + const page = { + locator: () => ({ + click: () => { + queueMicrotask(() => controller.abort()); + return new Promise(() => undefined); + }, + }), + reload, + }; + + const actions = await runElectronActions( + { windows: () => [page] } as unknown as ElectronApplication, + electronActiveObservationInputSchema.parse({ + executable_path: "/opt/electron", + application_path: "/opt/app/main.js", + application_root: "/opt/app", + actions: [ + { step_id: "open", kind: "click", selector: "#open" }, + { step_id: "reload", kind: "renderer-reload" }, + ], + }), + { signal: controller.signal }, + ); + + expect(reload).not.toHaveBeenCalled(); + expect(actions.map(({ step_id, status }) => [step_id, status])).toEqual([ + ["open", "cancelled"], + ["reload", "cancelled"], + ]); + expect(actions[1]).toMatchObject({ + window_index: 0, + target: null, + elapsed_ms: 0, + error: "not executed after scenario request cancellation", + }); +}); + it("bounds a final state read that is still pending after cancellation", async () => { vi.useFakeTimers(); const controller = new AbortController(); diff --git a/src/browser/PlaywrightElectronActiveActions.ts b/src/browser/PlaywrightElectronActiveActions.ts index 5f6e97b894..5661e5d75c 100644 --- a/src/browser/PlaywrightElectronActiveActions.ts +++ b/src/browser/PlaywrightElectronActiveActions.ts @@ -8,6 +8,10 @@ import { type ElectronActiveObservationInput, type ElectronActiveObservationResult, } from "../domain/javascript/electronActiveObservation.js"; +import { + notExecutedActionError, + type ScenarioActionStop, +} from "./ScenarioActionSequence.js"; const OPERATION = "capture_electron_scenario" as const; const CANCELLED_STATE_READ_TIMEOUT_MS = 5_000; @@ -69,16 +73,33 @@ type ElectronApplicationState = Awaited< ReturnType >; -/** Run explicit actions against provider-owned Electron windows. */ +/** + * Run explicit actions against provider-owned Electron windows. Every declared + * action has one result; actions after a failed or cancelled action are + * recorded as not executed. + */ export const runElectronActions = async ( application: ElectronApplication, input: ElectronActiveObservationInput, options: ExecutionOptions, ): Promise => { const actions: ElectronActions = []; + let stoppedBy: ScenarioActionStop | undefined; for (const action of input.actions) { const actionStartedAt = performance.now(); const windowIndex = actionWindowIndex(action); + if (stoppedBy !== undefined) { + actions.push({ + step_id: action.step_id, + kind: action.kind, + window_index: windowIndex, + target: null, + status: "cancelled", + elapsed_ms: 0, + error: notExecutedActionError(stoppedBy), + }); + continue; + } let selectedWindow: Page | undefined; try { if (options.signal?.aborted === true) @@ -119,20 +140,20 @@ export const runElectronActions = async ( error: null, }); } catch (cause: unknown) { + stoppedBy = options.signal?.aborted ? "cancelled" : "failed"; actions.push({ step_id: action.step_id, kind: action.kind, window_index: windowIndex, target: selectedWindow === undefined ? null : `window:${String(windowIndex)}`, - status: options.signal?.aborted ? "cancelled" : "failed", + status: stoppedBy, elapsed_ms: Math.max( 0, Math.round(performance.now() - actionStartedAt), ), error: safeActionErrorMessage(cause), }); - break; } } return actions; diff --git a/src/browser/PlaywrightElectronActiveProvider.test.ts b/src/browser/PlaywrightElectronActiveProvider.test.ts index 146c4a2528..82ee0dad9c 100644 --- a/src/browser/PlaywrightElectronActiveProvider.test.ts +++ b/src/browser/PlaywrightElectronActiveProvider.test.ts @@ -490,6 +490,80 @@ it("cancels a windowless wait action and cleans up the owned application", async expect(result.error.cause).toBe(reason); }); +it("records actions after a failed action as not executed and keeps the capture", async () => { + stubVerifiedElectronCleanup(); + const state = { + windows: [], + metrics: [], + electronVersion: "44.0.0", + hookSnapshot: emptyHookSnapshot(), + }; + vi.spyOn(electronActions, "readApplicationState").mockResolvedValue(state); + vi.spyOn(electronActions, "readFinalApplicationState").mockResolvedValue( + state, + ); + const click = vi.fn(async () => { + throw new Error("strict mode violation"); + }); + const waitForTimeout = vi.fn(async () => undefined); + const page = { locator: () => ({ click }), waitForTimeout }; + const application = { + process: () => ({ pid: 424242 }), + close: vi.fn(async () => undefined), + windows: () => [page], + firstWindow: async () => page, + }; + + const result = await captureWithController( + new AbortController(), + application, + [ + { step_id: "fail", kind: "click", selector: "#duplicate" }, + { step_id: "settle", kind: "wait", duration_ms: 0, window_index: 0 }, + { + step_id: "link", + kind: "deep-link", + delivery: "open-url", + url: "rea-test://open", + }, + ], + ); + + if (!result.ok) throw result.error; + expect(click).toHaveBeenCalledOnce(); + expect(waitForTimeout).not.toHaveBeenCalled(); + expect(application.close).toHaveBeenCalledOnce(); + expect(result.value.actions).toEqual([ + { + step_id: "fail", + kind: "click", + window_index: 0, + target: "window:0", + status: "failed", + elapsed_ms: expect.any(Number), + error: "strict mode violation", + }, + { + step_id: "settle", + kind: "wait", + window_index: 0, + target: null, + status: "cancelled", + elapsed_ms: 0, + error: "not executed after an earlier action failure", + }, + { + step_id: "link", + kind: "deep-link", + window_index: null, + target: null, + status: "cancelled", + elapsed_ms: 0, + error: "not executed after an earlier action failure", + }, + ]); +}); + it("cancels a pending initial state read and cleans up the owned application", async () => { const cleanupSpies = stubVerifiedElectronCleanup(); const controller = new AbortController(); diff --git a/src/browser/ScenarioActionSequence.ts b/src/browser/ScenarioActionSequence.ts new file mode 100644 index 0000000000..7b9d66775d --- /dev/null +++ b/src/browser/ScenarioActionSequence.ts @@ -0,0 +1,14 @@ +/** Status of the earlier action that stopped a declared scenario sequence. */ +export type ScenarioActionStop = "failed" | "cancelled"; + +/** + * Explain a declared action recorded as `cancelled` without running because an + * earlier action stopped the sequence. Scenario producers record every + * remaining action so results account for the whole declared sequence. + */ +export const notExecutedActionError = ( + stoppedBy: ScenarioActionStop, +): string => + stoppedBy === "failed" + ? "not executed after an earlier action failure" + : "not executed after scenario request cancellation";