From 4e2a566d6c7e053ecbbee1b684311ed31ea163bf Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Sat, 24 Oct 2026 02:34:27 +0800 Subject: [PATCH] fix: retain cancellation during Electron handle finalization --- .../PlaywrightElectronActiveActions.test.ts | 37 +++++++++++++++++++ .../PlaywrightElectronActiveActions.ts | 23 +++++++++++- 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/src/browser/PlaywrightElectronActiveActions.test.ts b/src/browser/PlaywrightElectronActiveActions.test.ts index 58d7fc4f9d..16618377db 100644 --- a/src/browser/PlaywrightElectronActiveActions.test.ts +++ b/src/browser/PlaywrightElectronActiveActions.test.ts @@ -748,3 +748,40 @@ it.each(["acquisition", "identity"] as const)( expect(dispose).toHaveBeenCalledOnce(); }, ); + +it("retains the caller reason when cancellation precedes failed handle disposal", async () => { + const controller = new AbortController(); + const reason = { stop: "during handle disposal" }; + const dispose = vi + .fn() + .mockImplementationOnce(async () => { + controller.abort(reason); + throw new Error("dispose rejected"); + }) + .mockResolvedValue(undefined); + const page = { locator: () => ({ click: async () => {} }) }; + const resources = new ArtifactResourceScope(); + const outcome = await runElectronActions( + { + windows: () => [page], + browserWindow: async () => ({ evaluate: async () => 99, dispose }), + } 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: "click", selector: "#selected" }], + }), + { signal: controller.signal }, + resources, + ); + expect(outcome.actions[0]).toMatchObject({ + status: "completed", + target: "window:99", + }); + expect(outcome.cleanupError?.cleanupPrimary?.cause).toBe(reason); + expect(outcome.cleanupError?.cleanupIncomplete).toBe(true); + await resources.close(); + expect(outcome.cleanupError?.cleanupIncomplete).toBe(false); + expect(outcome.cleanupError?.cleanupPrimary?.cause).toBe(reason); +}); diff --git a/src/browser/PlaywrightElectronActiveActions.ts b/src/browser/PlaywrightElectronActiveActions.ts index d87804ab58..0376935610 100644 --- a/src/browser/PlaywrightElectronActiveActions.ts +++ b/src/browser/PlaywrightElectronActiveActions.ts @@ -9,6 +9,8 @@ import type { } from "../artifacts/ArtifactResourceScope.js"; import { ProviderCleanupError } from "../domain/providerCleanupError.js"; import type { AnalysisError } from "../domain/analysisErrorBase.js"; +import { AnalysisCancelledError } from "../domain/analysisErrorCore.js"; +import { analysisErrorWithCleanupFailure } from "../domain/analysisErrorCleanup.js"; import { BrowserObservationError } from "../domain/browserObservationError.js"; import { electronActiveTimelineEventSchema, @@ -159,7 +161,12 @@ export const runElectronActions = async ( error: actionError, }); if (handleOwner !== undefined) { - cleanupError = await releaseActionWindow(resources, handleOwner); + cleanupError = await releaseActionWindow( + resources, + handleOwner, + stoppedBy ?? "completed", + options.signal, + ); if (cleanupError !== undefined) stoppedBy = "failed"; } } @@ -206,6 +213,8 @@ const ownActionWindow = ( const releaseActionWindow = async ( resources: ArtifactResourceScope, owner: ArtifactResourceOwner, + actionStatus: ElectronActions[number]["status"], + signal: AbortSignal | undefined, ): Promise => { const released = await resources.release(owner); if (released.kind === "released") return undefined; @@ -219,7 +228,17 @@ const releaseActionWindow = async ( ); error.bindCleanupResourceOwner(released.lease); released.lease.bindCleanupError(error); - return error; + if (!signal?.aborted) return error; + const cancellation = new AnalysisCancelledError(OPERATION, { + cause: signal.reason, + }); + return actionStatus === "failed" + ? error.retainSecondaryFailure(cancellation) + : analysisErrorWithCleanupFailure( + cancellation, + error, + OPERATION, + ).bindCleanupDependencies([cancellation, error]); }; type RunActionContext = {