[GH-ISSUE #3936] [Bug] Electron action targets use the selection index instead of window identity #1545

Closed
opened 2026-10-16 21:06:55 +00:00 by bsakaguchi · 4 comments
Owner

Originally created by @coygeek on GitHub (Oct 15, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/3936

Before submitting

  • I searched the open and closed issues for a related report.
  • I removed credentials, tokens, private paths, and sensitive target data.

Area

Evidence, artifacts, or reconstruction

Observed behavior

capture_electron_scenario returns action targets that do not identify the acted-on window in its window inventory. In an executed MCP reproduction with simulated windows whose IDs are 1 and 3, clicks on those windows return window:0 and window:1. The first target matches no window, and the second names the other window. This also affects a failed action after a window has been selected.

Reproduction

Use the baseline under Runtime or environment with its locked dependencies installed. This reproduction uses the registered MCP tool and real provider action, snapshot and projection logic. It simulates the Electron launch boundary and process supervision; it launches no Electron process and sends no process signals.

  1. Save the following temporary test as src/browser/electronActionIdentityRepro.test.ts.
import { expect, it, vi } from "vitest";
import { Client, InMemoryTransport } from "@modelcontextprotocol/client";
import { PlaywrightElectronActiveProvider } from "./PlaywrightElectronActiveProvider.js";
import { createServer } from "../server/createServer.js";
import { createTestBinarySession } from "../../tests/fixtures/binarySession.js";
import { observed } from "../../tests/fixtures/analysisExecution.js";
import { ok } from "../domain/result.js";

vi.mock("../process/ProcessOwnershipObservation.js", () => ({
  prepareProcessOwnershipInspection: async () => undefined,
  systemProcessOwnershipHost: {},
  observeOwnedProcessLineage: async (ownership: any) => ({
    status: "verified",
    lineage: { runId: ownership.runId, launcherPid: ownership.leaderPid,
      launcherParentPid: process.pid, processGroupId: ownership.leaderPid, descendants: [] },
  }),
}));
vi.mock("../process/ProcessOwnership.js", async (original) => ({
  ...await original<any>(),
  cleanupOwnedProcessGroup: async () => ({ cleaned: true, signaled: false }),
  verifyNoTokenOwnedProcesses: async () => ({ cleaned: true, signaled: false }),
}));

it("exposes action targets through the registered MCP tool", async () => {
  const clicked: number[] = [];
  const ids = [1, 3];
  const pages = ids.map((id) => ({
    locator: (selector: string) => ({ click: async () => {
      if (selector === "#fail") throw new Error("simulated click failure");
      clicked.push(id);
    } }),
    waitForTimeout: async () => undefined,
  }));
  const windows = ids.map((id) => ({
    id, webContents: { id: id + 10, getURL: () => "about:blank", getTitle: () => `Window ${id}` },
    isDestroyed: () => false, isVisible: () => true,
  }));
  let evaluation = 0;
  const application = {
    process: () => ({ pid: 424242 }), close: async () => undefined,
    windows: () => pages, firstWindow: async () => pages[0],
    evaluate: async (fn: any) => {
      const kind = evaluation++ % 4;
      if (kind === 0) return fn({ BrowserWindow: { getAllWindows: () => windows } });
      if (kind === 1) return fn({ app: { getAppMetrics: () => [] } });
      if (kind === 2) return "simulated-electron";
      return { events: [], retention_budget_bytes: 0, estimated_retained_bytes: 0,
        event_serialized_byte_upper_bound: 0, retained: 0, dropped: 0,
        dropped_ipc: 0, dropped_runtime: 0, dropped_event_families: [],
        dropped_event_roles: [], observed: 0, observed_ipc: 0,
        observed_runtime: 0, hook_error: false };
    },
  };
  const provider = new PlaywrightElectronActiveProvider({}, async () => application as any);
  const session = createTestBinarySession(() => ({
    execute: async () => observed(null), close: async () => ok(null),
  }));
  const server = createServer({ kind: "session", session }, { electronActiveObservation: provider });
  const client = new Client({ name: "electron-action-reproduction", version: "1" });
  const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair();
  try {
    await server.connect(serverTransport);
    await client.connect(clientTransport);
    const reply = await client.callTool({ name: "capture_electron_scenario", arguments: {
      executable_path: process.execPath, application_path: process.execPath,
      actions: [
        { step_id: "first", kind: "click", window_index: 0, selector: "#ok" },
        { step_id: "second", kind: "click", window_index: 1, selector: "#ok" },
        { step_id: "untargeted", kind: "wait", duration_ms: 0 },
        { step_id: "failed", kind: "click", window_index: 1, selector: "#fail" },
      ],
    } });
    expect(reply.isError, JSON.stringify(reply)).not.toBe(true);
    const result = (reply.structuredContent as any).normalized_result;
    console.log(JSON.stringify({ clicked, actions: result.actions, windows: result.windows }));
    expect(clicked).toEqual([1, 3]);
    expect(result.windows.map((window: any) => window.window_id)).toEqual(["window:1", "window:3"]);
    expect(result.actions.map((action: any) => action.target)).toEqual(["window:0", "window:1", null, "window:1"]);
    expect(result.actions.map((action: any) => action.status)).toEqual(["completed", "completed", "completed", "failed"]);
  } finally {
    await client.close(); await server.close(); await session.close();
  }
});

  1. Run the focused reproduction from the repository root with the pinned toolchain. Set HOME to a disposable directory to isolate configuration.
repro_home=$(mktemp -d)
HOME="$repro_home" npx vitest run src/browser/electronActionIdentityRepro.test.ts --maxWorkers=1
rm -rf -- "$repro_home"
  1. Inspect the logged MCP result and the recorded clicked IDs. The test asserts the faulty baseline output, so its passing result demonstrates the mismatch. Delete the temporary test afterward.

Expected behavior

Each action that selects a window should identify that actual window using its capture-scoped window identity. A click on the window whose inventory identity is window:3 should report that identity rather than the selection index. This applies to action records that complete or fail after selecting a window. The documented correlated window evidence and action targets, together with the repository fixture pairing index 0 with target and window identity window:1, establish the existing contract.

See src/contracts/javascript/electronToolContracts.ts:144, docs/electron-observation.md:75,95,107, and the index-0/action-target/window-identity pairing in src/domain/javascript/electronActiveObservation.fixture.ts:44,52.

Impact and scope

P2. Actions that select windows can return an unmatched target or a target naming another window, corrupting action attribution. Click selection itself succeeded in the reproduction, and the window inventory remained available. Real Electron behavior was not tested. A reliable general caller workaround is unverified because consistent ordering across the two enumerations is not established.

REA version

rea 6.5.0 at source commit 4692bb3330

Runtime and platform

Node 24.18.0; macOS arm64

Target and artifact context

Two simulated Electron windows with IDs 1 and 3. Production MCP registration, action runner, snapshot callback, projection, and Evidence delivery executed; launch, process observation, and cleanup were simulated. No real Electron process or process signals were used.

Interface

MCP

Earliest failure stage

Capture or observation

Relevant output or evidence

Observed in one executed MCP reproduction: the reply was successful and its window inventory contained window:1 and window:3. Recorded click effects were [1,3], but the successful action targets were window:0 and window:1. A subsequent failed click selected the second window and also reported window:1. The untargeted wait correctly reported null. Relevant output fields were:

{
  "clicked": [1, 3],
  "actions": [
    {"step_id":"first","window_index":0,"target":"window:0","status":"completed"},
    {"step_id":"second","window_index":1,"target":"window:1","status":"completed"},
    {"step_id":"untargeted","window_index":null,"target":null,"status":"completed"},
    {"step_id":"failed","window_index":1,"target":"window:1","status":"failed"}
  ],
  "windows": [{"window_id":"window:1"},{"window_id":"window:3"}]
}

Source-derived conclusion: the action record formats the selected array index as the target, whereas the window snapshot formats the Electron window ID. Both the successful and failed action paths use this index-based target, and result projection preserves it. No real Electron capture was run; the observation establishes behavior through the registered MCP tool with a simulated application at the production launch boundary.

Typed outcome or recovery expected

With the same two-window input, verify that the first completed click reports window:1, the second reports window:3, and the failed click on the second window also reports window:3. Confirm that the click effects still identify windows 1 and 3 so a target-only change cannot mask acting on the wrong page. Preserve null for the untargeted wait and preserve the separate action-selection indices. Real Electron verification should also confirm that returned targets correlate with the actual selected windows.

Analysis or suspected cause

Source-derived: src/browser/PlaywrightElectronActiveActions.ts:107,121 formats the selected array index as the action target; line 232 uses that index for page selection, while line 288 formats the Electron window ID for inventory identity. src/browser/PlaywrightElectronActiveProjection.ts:155 and src/application/javascript/ElectronActiveEvidence.ts:19 preserve the mismatch.

Additional context

The cited failing code is unchanged on current origin/main at 22ec8bdb2f7d4626587f23b1eed4a207df600386. Related closed #2649 and #2650 concern dropped preload-configuration coverage and limitation wording, rather than action/window identity.

Originally created by @coygeek on GitHub (Oct 15, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/3936 ### Before submitting - [x] I searched the open and closed issues for a related report. - [x] I removed credentials, tokens, private paths, and sensitive target data. ### Area Evidence, artifacts, or reconstruction ### Observed behavior `capture_electron_scenario` returns action targets that do not identify the acted-on window in its window inventory. In an executed MCP reproduction with simulated windows whose IDs are 1 and 3, clicks on those windows return `window:0` and `window:1`. The first target matches no window, and the second names the other window. This also affects a failed action after a window has been selected. ### Reproduction Use the baseline under Runtime or environment with its locked dependencies installed. This reproduction uses the registered MCP tool and real provider action, snapshot and projection logic. It simulates the Electron launch boundary and process supervision; it launches no Electron process and sends no process signals. 1. Save the following temporary test as `src/browser/electronActionIdentityRepro.test.ts`. ```typescript import { expect, it, vi } from "vitest"; import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; import { PlaywrightElectronActiveProvider } from "./PlaywrightElectronActiveProvider.js"; import { createServer } from "../server/createServer.js"; import { createTestBinarySession } from "../../tests/fixtures/binarySession.js"; import { observed } from "../../tests/fixtures/analysisExecution.js"; import { ok } from "../domain/result.js"; vi.mock("../process/ProcessOwnershipObservation.js", () => ({ prepareProcessOwnershipInspection: async () => undefined, systemProcessOwnershipHost: {}, observeOwnedProcessLineage: async (ownership: any) => ({ status: "verified", lineage: { runId: ownership.runId, launcherPid: ownership.leaderPid, launcherParentPid: process.pid, processGroupId: ownership.leaderPid, descendants: [] }, }), })); vi.mock("../process/ProcessOwnership.js", async (original) => ({ ...await original<any>(), cleanupOwnedProcessGroup: async () => ({ cleaned: true, signaled: false }), verifyNoTokenOwnedProcesses: async () => ({ cleaned: true, signaled: false }), })); it("exposes action targets through the registered MCP tool", async () => { const clicked: number[] = []; const ids = [1, 3]; const pages = ids.map((id) => ({ locator: (selector: string) => ({ click: async () => { if (selector === "#fail") throw new Error("simulated click failure"); clicked.push(id); } }), waitForTimeout: async () => undefined, })); const windows = ids.map((id) => ({ id, webContents: { id: id + 10, getURL: () => "about:blank", getTitle: () => `Window ${id}` }, isDestroyed: () => false, isVisible: () => true, })); let evaluation = 0; const application = { process: () => ({ pid: 424242 }), close: async () => undefined, windows: () => pages, firstWindow: async () => pages[0], evaluate: async (fn: any) => { const kind = evaluation++ % 4; if (kind === 0) return fn({ BrowserWindow: { getAllWindows: () => windows } }); if (kind === 1) return fn({ app: { getAppMetrics: () => [] } }); if (kind === 2) return "simulated-electron"; return { events: [], retention_budget_bytes: 0, estimated_retained_bytes: 0, event_serialized_byte_upper_bound: 0, retained: 0, dropped: 0, dropped_ipc: 0, dropped_runtime: 0, dropped_event_families: [], dropped_event_roles: [], observed: 0, observed_ipc: 0, observed_runtime: 0, hook_error: false }; }, }; const provider = new PlaywrightElectronActiveProvider({}, async () => application as any); const session = createTestBinarySession(() => ({ execute: async () => observed(null), close: async () => ok(null), })); const server = createServer({ kind: "session", session }, { electronActiveObservation: provider }); const client = new Client({ name: "electron-action-reproduction", version: "1" }); const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); try { await server.connect(serverTransport); await client.connect(clientTransport); const reply = await client.callTool({ name: "capture_electron_scenario", arguments: { executable_path: process.execPath, application_path: process.execPath, actions: [ { step_id: "first", kind: "click", window_index: 0, selector: "#ok" }, { step_id: "second", kind: "click", window_index: 1, selector: "#ok" }, { step_id: "untargeted", kind: "wait", duration_ms: 0 }, { step_id: "failed", kind: "click", window_index: 1, selector: "#fail" }, ], } }); expect(reply.isError, JSON.stringify(reply)).not.toBe(true); const result = (reply.structuredContent as any).normalized_result; console.log(JSON.stringify({ clicked, actions: result.actions, windows: result.windows })); expect(clicked).toEqual([1, 3]); expect(result.windows.map((window: any) => window.window_id)).toEqual(["window:1", "window:3"]); expect(result.actions.map((action: any) => action.target)).toEqual(["window:0", "window:1", null, "window:1"]); expect(result.actions.map((action: any) => action.status)).toEqual(["completed", "completed", "completed", "failed"]); } finally { await client.close(); await server.close(); await session.close(); } }); ``` 2. Run the focused reproduction from the repository root with the pinned toolchain. Set HOME to a disposable directory to isolate configuration. ```sh repro_home=$(mktemp -d) HOME="$repro_home" npx vitest run src/browser/electronActionIdentityRepro.test.ts --maxWorkers=1 rm -rf -- "$repro_home" ``` 3. Inspect the logged MCP result and the recorded clicked IDs. The test asserts the faulty baseline output, so its passing result demonstrates the mismatch. Delete the temporary test afterward. ### Expected behavior Each action that selects a window should identify that actual window using its capture-scoped window identity. A click on the window whose inventory identity is `window:3` should report that identity rather than the selection index. This applies to action records that complete or fail after selecting a window. The documented correlated window evidence and action targets, together with the repository fixture pairing index 0 with target and window identity `window:1`, establish the existing contract. See `src/contracts/javascript/electronToolContracts.ts:144`, `docs/electron-observation.md:75,95,107`, and the index-0/action-target/window-identity pairing in `src/domain/javascript/electronActiveObservation.fixture.ts:44,52`. ### Impact and scope P2. Actions that select windows can return an unmatched target or a target naming another window, corrupting action attribution. Click selection itself succeeded in the reproduction, and the window inventory remained available. Real Electron behavior was not tested. A reliable general caller workaround is unverified because consistent ordering across the two enumerations is not established. ### REA version rea 6.5.0 at source commit 4692bb33300c7cb77b095b6cc0c33da861c1b860 ### Runtime and platform Node 24.18.0; macOS arm64 ### Target and artifact context Two simulated Electron windows with IDs 1 and 3. Production MCP registration, action runner, snapshot callback, projection, and Evidence delivery executed; launch, process observation, and cleanup were simulated. No real Electron process or process signals were used. ### Interface MCP ### Earliest failure stage Capture or observation ### Relevant output or evidence Observed in one executed MCP reproduction: the reply was successful and its window inventory contained `window:1` and `window:3`. Recorded click effects were `[1,3]`, but the successful action targets were `window:0` and `window:1`. A subsequent failed click selected the second window and also reported `window:1`. The untargeted wait correctly reported null. Relevant output fields were: ```json { "clicked": [1, 3], "actions": [ {"step_id":"first","window_index":0,"target":"window:0","status":"completed"}, {"step_id":"second","window_index":1,"target":"window:1","status":"completed"}, {"step_id":"untargeted","window_index":null,"target":null,"status":"completed"}, {"step_id":"failed","window_index":1,"target":"window:1","status":"failed"} ], "windows": [{"window_id":"window:1"},{"window_id":"window:3"}] } ``` Source-derived conclusion: the action record formats the selected array index as the target, whereas the window snapshot formats the Electron window ID. Both the successful and failed action paths use this index-based target, and result projection preserves it. No real Electron capture was run; the observation establishes behavior through the registered MCP tool with a simulated application at the production launch boundary. ### Typed outcome or recovery expected With the same two-window input, verify that the first completed click reports `window:1`, the second reports `window:3`, and the failed click on the second window also reports `window:3`. Confirm that the click effects still identify windows 1 and 3 so a target-only change cannot mask acting on the wrong page. Preserve null for the untargeted wait and preserve the separate action-selection indices. Real Electron verification should also confirm that returned targets correlate with the actual selected windows. ### Analysis or suspected cause Source-derived: `src/browser/PlaywrightElectronActiveActions.ts:107,121` formats the selected array index as the action target; line 232 uses that index for page selection, while line 288 formats the Electron window ID for inventory identity. `src/browser/PlaywrightElectronActiveProjection.ts:155` and `src/application/javascript/ElectronActiveEvidence.ts:19` preserve the mismatch. ### Additional context The cited failing code is unchanged on current `origin/main` at `22ec8bdb2f7d4626587f23b1eed4a207df600386`. Related closed #2649 and #2650 concern dropped preload-configuration coverage and limitation wording, rather than action/window identity.
Author
Owner

@XinwangTao commented on GitHub (Oct 15, 2026):

Confirmed: returning selection indices (0, 1) as window targets when the inventory holds IDs 1 and 3 means the first reported target matches no window at all, and the second aliases the wrong one. Action targets should carry the window identity from the inventory, keeping the separate action-selection index out of the projected result — otherwise target-only change detection can mask acting on the wrong page.

<!-- gh-comment-id:6186176049 --> @XinwangTao commented on GitHub (Oct 15, 2026): Confirmed: returning selection indices (0, 1) as window targets when the inventory holds IDs 1 and 3 means the first reported target matches no window at all, and the second aliases the wrong one. Action targets should carry the window identity from the inventory, keeping the separate action-selection index out of the projected result — otherwise target-only change detection can mask acting on the wrong page.
Author
Owner

@N0zoM1z0 commented on GitHub (Oct 16, 2026):

Source follow-up at main 42fc6c53ae188978deb3bedfe88287b6b316ba22 (no real Electron reproduction yet): the same owner also independently reselects the execution target for renderer-crash.

runElectronActions selects a Page from application.windows()[window_index], but runAction then chooses BrowserWindow.getAllWindows()[action.window_index] again. No common ordering or stable membership between these two reads is established. Thus changing only the reported target string would leave a potential execution/identity mismatch for crash actions.

The installed Playwright 1.58.2 declaration provides ElectronApplication.browserWindow(page) for resolving the BrowserWindow corresponding to the selected Page. A prospective fix should use one selected-object owner for identity and execution, preserve failed-action identity, and dispose any acquired handle with awaited cleanup. Preserve null for untargeted waits/deep links.

Verification should assert actual click/crash effects with nonmatching enumeration orders and lifecycle transitions, alongside a real public-app control. These are proposed checks, not completed runtime verification. This is read-ahead only while #4265 awaits an unrelated upstream CI correction; no competing implementation or PR has been started.

<!-- gh-comment-id:6206742963 --> @N0zoM1z0 commented on GitHub (Oct 16, 2026): Source follow-up at main `42fc6c53ae188978deb3bedfe88287b6b316ba22` (no real Electron reproduction yet): the same owner also independently reselects the execution target for `renderer-crash`. `runElectronActions` selects a Page from `application.windows()[window_index]`, but `runAction` then chooses `BrowserWindow.getAllWindows()[action.window_index]` again. No common ordering or stable membership between these two reads is established. Thus changing only the reported target string would leave a potential execution/identity mismatch for crash actions. The installed Playwright 1.58.2 declaration provides `ElectronApplication.browserWindow(page)` for resolving the BrowserWindow corresponding to the selected Page. A prospective fix should use one selected-object owner for identity and execution, preserve failed-action identity, and dispose any acquired handle with awaited cleanup. Preserve null for untargeted waits/deep links. Verification should assert actual click/crash effects with nonmatching enumeration orders and lifecycle transitions, alongside a real public-app control. These are proposed checks, not completed runtime verification. This is read-ahead only while #4265 awaits an unrelated upstream CI correction; no competing implementation or PR has been started.
Author
Owner

@XinwangTao commented on GitHub (Oct 16, 2026):

Good catch on the double reselection — window_index being read once from application.windows() and again from BrowserWindow.getAllWindows() with no shared ordering means fixing only the reported target string would leave the crash path acting on a different window than the one reported. Using ElectronApplication.browserWindow(page) as the single owner for both identity and execution, plus the awaited handle disposal, closes that gap. Agree the verification should exercise mismatched enumeration orders, not just a happy-path two-window run.

<!-- gh-comment-id:6207574406 --> @XinwangTao commented on GitHub (Oct 16, 2026): Good catch on the double reselection — window_index being read once from application.windows() and again from BrowserWindow.getAllWindows() with no shared ordering means fixing only the reported target string would leave the crash path acting on a different window than the one reported. Using ElectronApplication.browserWindow(page) as the single owner for both identity and execution, plus the awaited handle disposal, closes that gap. Agree the verification should exercise mismatched enumeration orders, not just a happy-path two-window run.
Author
Owner

@coygeek commented on GitHub (Oct 24, 2026):

Checked the reported fix on main at d87e73f2db.

runElectronActions selects a Page once, resolves its native BrowserWindow with application.browserWindow(page), records window: before executing, and keeps that target through the catch path. renderer-crash acts through the same handle, not a second BrowserWindow.getAllWindows index read. Handle disposal is awaited and cleanup ownership retained on failure.

Implementation: PlaywrightElectronActiveActions.ts, PlaywrightElectronActiveActions.ts.

The relevant regression checks in PlaywrightElectronActiveActions.test.ts, PlaywrightElectronActiveProvider.test.ts passed in the focused test run on this revision. This closure is based on the current source and those checks; provider fixtures do not establish a real-engine run. Closing as completed on main.

<!-- gh-comment-id:6380709659 --> @coygeek commented on GitHub (Oct 24, 2026): Checked the reported fix on `main` at d87e73f2db50be72d33a4b3c7199af35518b441c. runElectronActions selects a Page once, resolves its native BrowserWindow with application.browserWindow(page), records window:<native id> before executing, and keeps that target through the catch path. renderer-crash acts through the same handle, not a second BrowserWindow.getAllWindows index read. Handle disposal is awaited and cleanup ownership retained on failure. Implementation: [PlaywrightElectronActiveActions.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/browser/PlaywrightElectronActiveActions.ts#L111), [PlaywrightElectronActiveActions.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/browser/PlaywrightElectronActiveActions.ts#L179). The relevant regression checks in [PlaywrightElectronActiveActions.test.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/browser/PlaywrightElectronActiveActions.test.ts), [PlaywrightElectronActiveProvider.test.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/browser/PlaywrightElectronActiveProvider.test.ts) passed in the focused test run on this revision. This closure is based on the current source and those checks; provider fixtures do not establish a real-engine run. Closing as completed on main.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
morluto/rea#1545
No description provided.