[PR #4332] [CLOSED] fix(electron): resolve action target and crash execution via Page BrowserWindow handle #6177

New issue

Closed

opened 2026-10-31 01:12:28 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:12:28 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4332
Author: @1aifanatic
Created: 10/16/2026
Status: ❌ Closed

Base: main ← Head: fix/3922-ui-trace-omitted-route-relation

📝 Commits (1)

708e55b fix(electron): resolve action target and crash execution via Page BrowserWindow handle

📊 Changes

2 files changed (+485 additions, -9 deletions)

View changed files

📝 src/browser/PlaywrightElectronActiveActions.test.ts (+375 -0)
📝 src/browser/PlaywrightElectronActiveActions.ts (+110 -9)

📄 Description

Summary

Problem:
- runElectronActions in src/browser/PlaywrightElectronActiveActions.ts formatted the 0-based window_index selection position (window:${windowIndex}) as the action target for completed and failed window-targeted actions instead of the capture-scoped window identity (window:${window.id}), and renderer-crash re-selected candidate windows from BrowserWindow.getAllWindows()[index]. Because Playwright's application.windows() and Electron's BrowserWindow.getAllWindows() do not share guaranteed ordering or stable membership, action targets and crash executions could misattribute to or act on the wrong window.
Outcome:
- Window targets and renderer-crash execution resolve the BrowserWindow handle directly from the selected Page via application.browserWindow(selectedWindow).
- Temporary handles are awaited and disposed in a finally block while preserving primary action failures, and late-resolved handles are disposed when cancelled during handle acquisition.
- Action-time targets are preserved directly through final projection without post-action inventory overwriting.
Approach:
- In runElectronActions (src/browser/PlaywrightElectronActiveActions.ts), acquire the JSHandle<BrowserWindow> directly from application.browserWindow(selectedWindow) (acquireWindowHandle), extract win.id for target identity (resolveWindowTarget), and execute forcefullyCrashRenderer() on that same handle for renderer-crash.
- Ensure temporary handle disposal is awaited in finally and suppressed to preserve primary action outcomes/errors, and attach late-handshake disposal if browserWindow(page) settles after cancellation.
- Add regression tests for completed/failed window identities, mismatched Page/BrowserWindow enumeration orders, window close/replacement across sequential actions, disposal error suppression, and cancellation cleanup.
- (Note: The previous native UI trace omitted-route-relation commit for #3922 was dropped during rebase because #3922 landed on main in 65884350f.)
Scope and authority impact: None; internal active Electron observation target attribution and crash execution fixes with no new capabilities or schema changes.
Linked issues: Closes #3936

Validation

Executed:
- npm run test:local -- src/browser/PlaywrightElectronActiveActions.test.ts src/browser/PlaywrightElectronActiveProjection.test.ts src/browser/PlaywrightElectronActiveProvider.test.ts src/native/NativeUiObservation.test.ts
- npm run check:fast
- npm run format:check
Not run / unavailable:
- Full npm run check:pr gate was not run locally.
- Real Electron binaries / GUI execution not run in local headless test environment.

Performance and Evidence impact

Not performance-sensitive: Single-handle evaluation/disposal and target string formatting per window-targeted action; Evidence schemas and token footprint are unchanged.

Author readiness

Leave these unchecked until a human owner has reviewed the diff and validation evidence and is ready to request initial maintainer review.

A human contributor reviewed the final diff and owns this pull request.
Validation entries above report only checks that actually ran, and unavailable coverage is explicit.
Scope, authority, and user-visible changes are accurately described.

Human owner:

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/4332 **Author:** [@1aifanatic](https://github.com/1aifanatic) **Created:** 10/16/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/3922-ui-trace-omitted-route-relation` --- ### 📝 Commits (1) - [`708e55b`](https://github.com/morluto/rea/commit/708e55b35bd212669f70d4a341790c5716db8061) fix(electron): resolve action target and crash execution via Page BrowserWindow handle ### 📊 Changes **2 files changed** (+485 additions, -9 deletions) <details> <summary>View changed files</summary> 📝 `src/browser/PlaywrightElectronActiveActions.test.ts` (+375 -0) 📝 `src/browser/PlaywrightElectronActiveActions.ts` (+110 -9) </details> ### 📄 Description ## Summary - Problem: - `runElectronActions` in `src/browser/PlaywrightElectronActiveActions.ts` formatted the 0-based `window_index` selection position (`window:${windowIndex}`) as the action `target` for completed and failed window-targeted actions instead of the capture-scoped window identity (`window:${window.id}`), and `renderer-crash` re-selected candidate windows from `BrowserWindow.getAllWindows()[index]`. Because Playwright's `application.windows()` and Electron's `BrowserWindow.getAllWindows()` do not share guaranteed ordering or stable membership, action targets and crash executions could misattribute to or act on the wrong window. - Outcome: - Window targets and `renderer-crash` execution resolve the `BrowserWindow` handle directly from the selected `Page` via `application.browserWindow(selectedWindow)`. - Temporary handles are awaited and disposed in a `finally` block while preserving primary action failures, and late-resolved handles are disposed when cancelled during handle acquisition. - Action-time targets are preserved directly through final projection without post-action inventory overwriting. - Approach: - In `runElectronActions` (`src/browser/PlaywrightElectronActiveActions.ts`), acquire the `JSHandle<BrowserWindow>` directly from `application.browserWindow(selectedWindow)` (`acquireWindowHandle`), extract `win.id` for target identity (`resolveWindowTarget`), and execute `forcefullyCrashRenderer()` on that same handle for `renderer-crash`. - Ensure temporary handle disposal is awaited in `finally` and suppressed to preserve primary action outcomes/errors, and attach late-handshake disposal if `browserWindow(page)` settles after cancellation. - Add regression tests for completed/failed window identities, mismatched Page/BrowserWindow enumeration orders, window close/replacement across sequential actions, disposal error suppression, and cancellation cleanup. - *(Note: The previous native UI trace omitted-route-relation commit for #3922 was dropped during rebase because #3922 landed on `main` in `65884350f`.)* - Scope and authority impact: None; internal active Electron observation target attribution and crash execution fixes with no new capabilities or schema changes. - Linked issues: Closes #3936 ## Validation - Executed: - `npm run test:local -- src/browser/PlaywrightElectronActiveActions.test.ts src/browser/PlaywrightElectronActiveProjection.test.ts src/browser/PlaywrightElectronActiveProvider.test.ts src/native/NativeUiObservation.test.ts` - `npm run check:fast` - `npm run format:check` - Not run / unavailable: - Full `npm run check:pr` gate was not run locally. - Real Electron binaries / GUI execution not run in local headless test environment. ## Performance and Evidence impact - Not performance-sensitive: Single-handle evaluation/disposal and target string formatting per window-targeted action; Evidence schemas and token footprint are unchanged. ## Author readiness Leave these unchecked until a human owner has reviewed the diff and validation evidence and is ready to request initial maintainer review. - [ ] A human contributor reviewed the final diff and owns this pull request. - [ ] Validation entries above report only checks that actually ran, and unavailable coverage is explicit. - [ ] Scope, authority, and user-visible changes are accurately described. Human owner:  --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:12:28 +00:00