morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #1391] [MERGED] fix(javascript): recognize Electron APIs through renamed bindings #2848

New issue

Closed

opened 2026-10-17 21:10:59 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-17 21:10:59 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/1391
Author: @marnixpluim
Created: 10/10/2026
Status: ✅ Merged
Merged: 10/10/2026
Merged by: @morluto

Base: main ← Head: fix/electron-import-aliases

📝 Commits (2)

e4a8e0b fix(javascript): recognize Electron APIs through renamed bindings
0d2082d fix(javascript): recognize unchanged var and let Electron aliases

📊 Changes

7 files changed (+250 additions, -20 deletions)

View changed files

📝 src/domain/javascript/electronStaticAnalysisBrowser.ts (+4 -4)
📝 src/domain/javascript/electronStaticAnalysisIpc.ts (+2 -1)
📝 src/domain/javascript/electronStaticAnalysisValues.ts (+20 -1)
📝 src/domain/javascript/javascriptSemanticAnalysis.ts (+87 -0)
📝 src/domain/javascript/javascriptStaticAnalysis.ts (+13 -14)
📝 src/domain/javascript/javascriptStaticAnalysisState.ts (+2 -0)
📝 tests/composition/javascript-applications/electronApplicationAnalysis.test.ts (+122 -0)

📄 Description

Requested by Marnix · project thread

Summary

REA recognizes Electron APIs by the callee's spelling, such as ipcMain.handle or new BrowserWindow. Bundlers rename imports when names collide, so a bundled main process calls ipcMain2.handle(...) and new BrowserWindow2(...), and REA reports none of them. This change resolves each call or construction root to its Electron import or require origin and matches the callee under the export's name.

No issue is linked. This was found while analyzing the packaged Hermes Desktop v0.21.6 (NousResearch/hermes-agent, MIT), whose main process is one esbuild ESM bundle.

Problem and expected behavior

Smallest trigger, a directory with package.json ({ "name": "app", "main": "main.mjs" }) and:

// main.mjs
import { ipcMain as ipcMain2 } from "electron";
ipcMain2.handle("app:read", async () => 1);

// preload.js
const { contextBridge, ipcRenderer } = require("electron");
contextBridge.exposeInMainWorld("api", { read: () => ipcRenderer.invoke("app:read") });

Expected: one main-process handler for app:read, paired with the preload's invoke. On 7aa4d768, electron_summary.ipc reports main_handlers: 0 and unpaired_literal_renderer_transmissions: 1; with this change, main_handlers: 1 and paired_renderer_transmissions: 1. Renamed BrowserWindow, contextBridge and utilityProcess bindings are missed the same way.

Invariant: whether a call is an Electron API depends on where its binding comes from, not on the name a bundler gave it.

Change and scope

classifyParsedJavaScriptElectronBindings maps each call or new root bound to electron, electron/main, electron/renderer, electron/common or electron/utility under another name to its export path. It reads import and require origins directly, including destructured require results, rather than running general value evaluation, which is costly on large bundles.
electronCalleeName respells a renamed root as its export before the existing matchers run; roots already spelled as their export are untouched.
Shadowed locals keep their own meaning: a function parameter that happens to be named ipcMain2 is not Electron's.

Not included: Electron objects passed in as function arguments or re-exported from another module, and channels built from template literals. These need more than local binding origins.

Contract and boundary impact

Semantic owner and earliest changed stage: domain JavaScript static analysis (src/domain/javascript), when static findings are collected.
CLI and MCP/tool-catalog contract: none; a renamed binding now produces the findings its unrenamed spelling already produced.
Provider, bridge, target-format, or platform compatibility: none.
Evidence, artifact, provenance, or reconstruction contract: none; findings keep their evidence and source ranges.
Process execution, authorization, cleanup, or containment impact: none; analysis stays static.
Generated metadata (docs/public/product-catalog.json), package, or installation impact: none.

Evidence and regression coverage

Tests added or updated: tests/composition/javascript-applications/electronApplicationAnalysis.test.ts, "maps Electron boundaries through bundler-renamed bindings": an ESM main with renamed BrowserWindow, ipcMain (also via electron/main) and utilityProcess imports and a shadowing ipcMain2 parameter, plus a CommonJS preload destructuring renamed contextBridge and ipcRenderer. It expects one window, one bridge with two members, two paired handlers, one utility process, and no handler for the shadowed channel.
Base reproduction or other evidence: on 7aa4d768 the new test finds no window, bridge, handler or utility process. On the Hermes Desktop v0.21.6 Linux app.asar, with the member-mutation fix in both runs, since without it the app's React vendor chunk alone has no result after 300 s:

electron_summary before after

browser_windows 0 11

ipc.main_handlers 1 204

ipc.paired_renderer_transmissions 1 201

ipc.unpaired_literal_renderer_transmissions 222 22

The 11 windows and 204 handlers match the bundle's Electron-bound new BrowserWindowN(...) and ipcMainN.handle/on(...) calls one for one. The one window and 13 handlers still missed receive BrowserWindow or ipcMain as a function parameter.
User-visible CLI/MCP output (if applicable): the summary changes as in the table; no field is added or removed.
Remaining proof gaps: the cases under Not included.

`electron_summary`	before	after
`browser_windows`	0	11
`ipc.main_handlers`	1	204
`ipc.paired_renderer_transmissions`	1	201
`ipc.unpaired_literal_renderer_transmissions`	222	22

For evidence-bearing changes:

Observed, derived, and inferred claims remain distinguishable.
Artifact identity, source provenance, and failed attempts remain preserved.
Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

npx oxfmt --check on the changed files: all formatted
npx oxlint on the changed files: no new warnings; javascriptSemanticAnalysis.ts was already over the line limit and grows from 907 to 974 lines
node scripts/verify-module-boundaries.mjs: passed, 1,313 source files
npx tsc --noEmit: exit 0
npx vitest run tests/composition/javascript-applications/electronApplicationAnalysis.test.ts src/domain/javascript: 38 files, 689 tests passed
Full suite on the four branches from this Hermes Desktop run, merged (npm run test:prepare, then npx vitest run; root, Linux x64, Node 22.22): 6,246 tests passed, 114 skipped, 1 failed. The failure is analysisSelectorCli.test.ts "rejects missing and conflicting selectors before opening a target": it starts the CLI 12 times, hit its 30 s timeout under the parallel run, and passes alone in 27.6 s. It covers CLI argument parsing, which these changes don't touch.

Compatibility, safety, and release

Breaking changes or migration steps: none.
Real Hopper/Ghidra, browser, or OS coverage: not applicable; static JavaScript analysis only. Run on Linux x64 with Node 22.
Package or release metadata impact: none.
Security, privacy, process, or containment review: no new process execution, file access or network access.

Review checklist

This PR addresses a concrete problem or an agreed enhancement.
The PR has one focused outcome and the title follows type(scope): outcome.
Related issue is linked, or the reason for not linking one is stated above.
Tests cover changed observable behavior and meaningful failure paths.
Owning docs, contracts, and generated metadata are updated where needed.
User-visible CLI/MCP changes include representative output.
I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

🤖 Generated with Claude Code

https://claude.ai/code/session_01XPY2GmufMNGQKoEgRUx1pd

_{🔄 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/1391 **Author:** [@marnixpluim](https://github.com/marnixpluim) **Created:** 10/10/2026 **Status:** ✅ Merged **Merged:** 10/10/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/electron-import-aliases` --- ### 📝 Commits (2) - [`e4a8e0b`](https://github.com/morluto/rea/commit/e4a8e0b728154fe0bf181684ccd0d73df45cf4bf) fix(javascript): recognize Electron APIs through renamed bindings - [`0d2082d`](https://github.com/morluto/rea/commit/0d2082d36b8bed3cda5daa22adee2163652fc511) fix(javascript): recognize unchanged var and let Electron aliases ### 📊 Changes **7 files changed** (+250 additions, -20 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/javascript/electronStaticAnalysisBrowser.ts` (+4 -4) 📝 `src/domain/javascript/electronStaticAnalysisIpc.ts` (+2 -1) 📝 `src/domain/javascript/electronStaticAnalysisValues.ts` (+20 -1) 📝 `src/domain/javascript/javascriptSemanticAnalysis.ts` (+87 -0) 📝 `src/domain/javascript/javascriptStaticAnalysis.ts` (+13 -14) 📝 `src/domain/javascript/javascriptStaticAnalysisState.ts` (+2 -0) 📝 `tests/composition/javascript-applications/electronApplicationAnalysis.test.ts` (+122 -0) </details> ### 📄 Description  _Requested by **Marnix** · [project thread](https://claude.ai/code/project/chan_015mc63Gczn836s2xXzmeLLa?thread=cmsg_015mc63Gczn836s2xXzmeLLa375aS21D2UMwQouWTw67kh)_ ## Summary REA recognizes Electron APIs by the callee's spelling, such as `ipcMain.handle` or `new BrowserWindow`. Bundlers rename imports when names collide, so a bundled main process calls `ipcMain2.handle(...)` and `new BrowserWindow2(...)`, and REA reports none of them. This change resolves each call or construction root to its Electron import or `require` origin and matches the callee under the export's name. No issue is linked. This was found while analyzing the packaged Hermes Desktop v0.21.6 ([NousResearch/hermes-agent](https://github.com/NousResearch/hermes-agent), MIT), whose main process is one esbuild ESM bundle. ## Problem and expected behavior Smallest trigger, a directory with `package.json` (`{ "name": "app", "main": "main.mjs" }`) and: ```js // main.mjs import { ipcMain as ipcMain2 } from "electron"; ipcMain2.handle("app:read", async () => 1); // preload.js const { contextBridge, ipcRenderer } = require("electron"); contextBridge.exposeInMainWorld("api", { read: () => ipcRenderer.invoke("app:read") }); ``` Expected: one main-process handler for `app:read`, paired with the preload's `invoke`. On `7aa4d768`, `electron_summary.ipc` reports `main_handlers: 0` and `unpaired_literal_renderer_transmissions: 1`; with this change, `main_handlers: 1` and `paired_renderer_transmissions: 1`. Renamed `BrowserWindow`, `contextBridge` and `utilityProcess` bindings are missed the same way. Invariant: whether a call is an Electron API depends on where its binding comes from, not on the name a bundler gave it. ## Change and scope - `classifyParsedJavaScriptElectronBindings` maps each call or `new` root bound to `electron`, `electron/main`, `electron/renderer`, `electron/common` or `electron/utility` under another name to its export path. It reads import and `require` origins directly, including destructured `require` results, rather than running general value evaluation, which is costly on large bundles. - `electronCalleeName` respells a renamed root as its export before the existing matchers run; roots already spelled as their export are untouched. - Shadowed locals keep their own meaning: a function parameter that happens to be named `ipcMain2` is not Electron's. Not included: Electron objects passed in as function arguments or re-exported from another module, and channels built from template literals. These need more than local binding origins. ## Contract and boundary impact - Semantic owner and earliest changed stage: domain JavaScript static analysis (`src/domain/javascript`), when static findings are collected. - CLI and MCP/tool-catalog contract: none; a renamed binding now produces the findings its unrenamed spelling already produced. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: none; findings keep their evidence and source ranges. - Process execution, authorization, cleanup, or containment impact: none; analysis stays static. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact: none. ## Evidence and regression coverage - Tests added or updated: `tests/composition/javascript-applications/electronApplicationAnalysis.test.ts`, "maps Electron boundaries through bundler-renamed bindings": an ESM main with renamed `BrowserWindow`, `ipcMain` (also via `electron/main`) and `utilityProcess` imports and a shadowing `ipcMain2` parameter, plus a CommonJS preload destructuring renamed `contextBridge` and `ipcRenderer`. It expects one window, one bridge with two members, two paired handlers, one utility process, and no handler for the shadowed channel. - Base reproduction or other evidence: on `7aa4d768` the new test finds no window, bridge, handler or utility process. On the Hermes Desktop v0.21.6 Linux `app.asar`, with the member-mutation fix in both runs, since without it the app's React vendor chunk alone has no result after 300 s: | `electron_summary` | before | after | | --- | ---: | ---: | | `browser_windows` | 0 | 11 | | `ipc.main_handlers` | 1 | 204 | | `ipc.paired_renderer_transmissions` | 1 | 201 | | `ipc.unpaired_literal_renderer_transmissions` | 222 | 22 | The 11 windows and 204 handlers match the bundle's Electron-bound `new BrowserWindowN(...)` and `ipcMainN.handle/on(...)` calls one for one. The one window and 13 handlers still missed receive `BrowserWindow` or `ipcMain` as a function parameter. - User-visible CLI/MCP output (if applicable): the summary changes as in the table; no field is added or removed. - Remaining proof gaps: the cases under Not included. For evidence-bearing changes: - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed - `npx oxfmt --check` on the changed files: all formatted - `npx oxlint` on the changed files: no new warnings; `javascriptSemanticAnalysis.ts` was already over the line limit and grows from 907 to 974 lines - `node scripts/verify-module-boundaries.mjs`: passed, 1,313 source files - `npx tsc --noEmit`: exit 0 - `npx vitest run tests/composition/javascript-applications/electronApplicationAnalysis.test.ts src/domain/javascript`: 38 files, 689 tests passed - Full suite on the four branches from this Hermes Desktop run, merged (`npm run test:prepare`, then `npx vitest run`; root, Linux x64, Node 22.22): 6,246 tests passed, 114 skipped, 1 failed. The failure is `analysisSelectorCli.test.ts` "rejects missing and conflicting selectors before opening a target": it starts the CLI 12 times, hit its 30 s timeout under the parallel run, and passes alone in 27.6 s. It covers CLI argument parsing, which these changes don't touch. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: not applicable; static JavaScript analysis only. Run on Linux x64 with Node 22. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no new process execution, file access or network access. ## Review checklist - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01XPY2GmufMNGQKoEgRUx1pd --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-17 21:10:59 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-10-31 01:06:36 +00:00

[PR #2848] [MERGED] fix(mcp): count the error flag in error result budgets #5435

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#2848

No description provided.

Rows
Columns