From 1a6ccec636a2ce811245fab86a6bfa7f8597ba85 Mon Sep 17 00:00:00 2001
From: Adolanium <94890352+Adolanium@users.noreply.github.com>
Date: Sun, 11 Oct 2026 01:22:54 +0300
Subject: [PATCH] fix(browser): skip Windows Electron signals without verified
 lineage (#1588)

capture_electron_scenario called taskkill on the recorded leader PID before lineage could be verified. On Windows the process table is empty, so that proof never arrives and the signal still ran. Refuse the cleanup without signaling. A verified lineage still terminates the leader and its descendants.

Co-authored-by: morluto <76467478+morluto@users.noreply.github.com>
---
 .../PlaywrightElectronActiveProvider.test.ts  | 73 ++++++++++++++++++-
 .../PlaywrightElectronActiveProvider.ts       | 17 ++++-
 2 files changed, 84 insertions(+), 6 deletions(-)

diff --git a/src/browser/PlaywrightElectronActiveProvider.test.ts b/src/browser/PlaywrightElectronActiveProvider.test.ts
index 2ddece3a6c..4c7b738b6c 100644
--- a/src/browser/PlaywrightElectronActiveProvider.test.ts
+++ b/src/browser/PlaywrightElectronActiveProvider.test.ts
@@ -1,9 +1,13 @@
 import { afterEach, expect, it, vi } from "vitest";
 
 import * as processOwnership from "../process/ProcessOwnership.js";
-import { electronActiveObservationInputSchema } from "../domain/javascript/electronActiveObservation.js";
 import * as electronActions from "./PlaywrightElectronActiveActions.js";
-import { PlaywrightElectronActiveProvider } from "./PlaywrightElectronActiveProvider.js";
+import { electronActiveObservationInputSchema } from "../domain/javascript/electronActiveObservation.js";
+import type { OwnedProcessGroup } from "../process/ProcessOwnership.js";
+import {
+  cleanupElectronProcesses,
+  PlaywrightElectronActiveProvider,
+} from "./PlaywrightElectronActiveProvider.js";
 
 afterEach(() => {
   vi.restoreAllMocks();
@@ -39,6 +43,71 @@ it("passes the selected environment and ownership token to Electron launch", asy
   });
 });
 
+const ownership = {
+  runId: "run",
+  leaderPid: 4242,
+  processGroupId: 4242,
+  expectedParentPid: 1,
+} satisfies OwnedProcessGroup;
+
+it("does not signal a Windows Electron PID when lineage is unavailable", async () => {
+  const terminateTree = vi.fn(async () => ({
+    cleaned: true as const,
+    signaled: true,
+  }));
+  const result = await cleanupElectronProcesses(
+    ownership,
+    {
+      status: "unavailable",
+      observedAt: "2026-10-10T00:00:00.000Z",
+      runId: ownership.runId,
+      launcherPid: ownership.leaderPid,
+      processGroupId: ownership.processGroupId,
+      reason: "owned launcher is not live",
+    },
+    { platform: "win32", terminateTree },
+  );
+
+  expect(terminateTree).not.toHaveBeenCalled();
+  expect(result).toEqual({
+    cleaned: false,
+    reason:
+      "owned Electron lineage was unavailable; helper cleanup was not proven",
+  });
+});
+
+it("signals a Windows Electron tree only after lineage is verified", async () => {
+  const terminateTree = vi.fn(async () => ({
+    cleaned: true as const,
+    signaled: true,
+  }));
+  const result = await cleanupElectronProcesses(
+    ownership,
+    {
+      status: "verified",
+      observedAt: "2026-10-10T00:00:00.000Z",
+      lineage: {
+        runId: ownership.runId,
+        launcherPid: ownership.leaderPid,
+        launcherParentPid: 1,
+        processGroupId: ownership.processGroupId,
+        descendants: [
+          {
+            pid: 4343,
+            parentPid: ownership.leaderPid,
+            processGroupId: ownership.processGroupId,
+          },
+        ],
+      },
+    },
+    { platform: "win32", terminateTree },
+  );
+
+  expect(terminateTree).toHaveBeenNthCalledWith(1, 4242);
+  expect(terminateTree).toHaveBeenNthCalledWith(2, 4343);
+  expect(result).toEqual({ cleaned: true, signaled: true });
+});
+
 it("retains the partial capture without taskkill when Windows lineage is unavailable", async () => {
   // Exercise the Windows decision branch; this is not native Windows coverage.
   const originalPlatform = Object.getOwnPropertyDescriptor(process, "platform");
diff --git a/src/browser/PlaywrightElectronActiveProvider.ts b/src/browser/PlaywrightElectronActiveProvider.ts
index 8faf1249fd..5c730e2fdf 100644
--- a/src/browser/PlaywrightElectronActiveProvider.ts
+++ b/src/browser/PlaywrightElectronActiveProvider.ts
@@ -259,21 +259,30 @@ export class PlaywrightElectronActiveProvider implements ElectronActiveObservati
   }
 }
 
-const cleanupElectronProcesses = async (
+/** Clean up an Electron launch. Windows signals a process only after lineage is verified. */
+export const cleanupElectronProcesses = async (
   ownership: OwnedProcessGroup,
   lineage: ProcessLineageObservation | undefined,
+  host: {
+    readonly platform: NodeJS.Platform;
+    readonly terminateTree: typeof cleanupWindowsProcessTree;
+  } = {
+    platform: process.platform,
+    terminateTree: cleanupWindowsProcessTree,
+  },
 ): Promise<ProcessCleanupResult> => {
-  if (process.platform === "win32") {
+  if (host.platform === "win32") {
+    // An empty Windows process table is not ownership proof for this PID.
     if (lineage?.status !== "verified")
       return {
         cleaned: false,
         reason:
           "owned Electron lineage was unavailable; helper cleanup was not proven",
       };
-    const root = await cleanupWindowsProcessTree(ownership.leaderPid);
+    const root = await host.terminateTree(ownership.leaderPid);
     if (!root.cleaned) return root;
     for (const descendant of lineage.lineage.descendants) {
-      const result = await cleanupWindowsProcessTree(descendant.pid);
+      const result = await host.terminateTree(descendant.pid);
       if (!result.cleaned) return result;
     }
     return root;