[PR #3596] [CLOSED] fix(javascript): follow Electron through esbuild's __require shim #5845

New issue

Closed

opened 2026-10-31 01:09:52 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:09:52 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/3596
Author: @DeryFerd
Created: 10/14/2026
Status: ❌ Closed

Base: main ← Head: fix/esbuild-require-shim-electron-identity

📝 Commits (1)

9de7b20 fix(javascript): follow Electron through esbuild's __require shim

📊 Changes

4 files changed (+414 additions, -25 deletions)

View changed files

➕ src/domain/javascript/javascriptRequireShimOrigin.ts (+212 -0)
📝 src/domain/javascript/javascriptSemanticAnalysis.ts (+3 -1)
📝 src/domain/javascript/javascriptSemanticRequireOrigin.ts (+71 -24)
➕ src/domain/javascript/requireShimIdentity.test.ts (+128 -0)

📄 Description

Summary

Fixes #3535. analyze-javascript-application reported zero Electron browser windows and zero IPC handlers for an Electron main process that was bundled with esbuild, even though the same program produced the expected window and handler when its require("electron") calls were left unbundled. The Electron boundary summary is a primary output of JavaScript application analysis, and an empty one reads as "this app has no Electron surface", so a silent zero here is worse than a stated unknown.

Problem and expected behavior

esbuild rewrites CommonJS require into a small IIFE shim for Node and Electron targets, and minification then renames that shim to a single letter. A bundled require("electron") arrives as Q("electron") at every call site, so the direct-require resolver never sees the specifier and no Electron identity is recovered. The smallest trigger is the shim esbuild emits, followed by an ordinary Electron usage:

var Q = (e => typeof require < "u" ? require : typeof Proxy < "u"
  ? new Proxy(e, { get: (t, n) => (typeof require < "u" ? require : t)[n] })
  : e)(function (e) {
    if (typeof require < "u") return require.apply(this, arguments);
    throw Error('Dynamic require of "' + e + '" is not supported');
  });

var cy = Q("electron");
new cy.BrowserWindow({ webPreferences: { preload: "./preload.js" } });
cy.ipcMain.handle("get-version", () => "1.0.0");

On the affected base revision this reported browser_windows: [] and ipc: []. The violated invariant is that a literal require reached through a proven loader alias must carry the same module identity as the direct call, which is already the rule for webpack's bootstrap externals. The expected behavior is one browser window with preload_path: "./preload.js" and one get-version main handler, exactly as the unbundled form yields.

Change and scope

A new semanticRequireShimOrigin resolver recovers a literal require reached through the shim, and electronRequireOrigin consults it after the direct and webpack-bootstrap resolvers. The shim is proven from syntax, not from a binding name. The resolver accepts a binding only when all of the following hold: the binding has a single non-assignment definition, the binding is not resolution-blocked, its stable initializer is a call expression, the callee is a plain arrow or function expression (not a generator, not async, not a method, and with exactly one identifier parameter), the callee body is a conditional expression rather than a block, and following the first branch of each nested conditional reaches the unshadowed global require. When the proof holds, Q(specifier) in that scope carries the same literal require origin, including its member path.

Only Electron identity consumes this resolver, matching how semanticBundledRequireOrigin is scoped to rootExport and bindingOrigin. Intentionally not included: the esbuild __toESM interop helper, non-Electron consumers of the same shim, intermediate alias bindings such as const R = Q; R("electron"), and any change to the direct-require or webpack-bootstrap resolvers. Those are separable follow-ups and are not required to fix the reported bundle shape, where the shim binding is called directly at each site.

Contract and boundary impact

Semantic owner and earliest changed stage: Electron identity in indexJavaScriptElectronIdentitySteps; the earliest changed stage is JavaScript static analysis, before any Electron finding is emitted.
Contribution scope fit and linked maintainer decision: not applicable. This is a routine static-analysis fix inside existing boundaries that adds no provider, no execution, no injection, no mutation, and no protection recovery.
CLI and MCP/tool-catalog contract: none. The same CLI commands and MCP tools are affected, and no advertised schema, field, or catalog entry changes.
Provider, bridge, target-format, or platform compatibility: none. The change is host-agnostic and format-agnostic, and it touches no provider or bridge.
Evidence, artifact, provenance, or reconstruction contract: calls resolved through a proven shim become direct observations, the same way the webpack bootstrap already treats loader calls. A shim call that cannot be proven stays unresolved and keeps its existing unknown, so the change cannot introduce a false Electron association.
Target selection, authority, upstream behavior, and cleanup details (if applicable): not applicable. The analysis is inert and reads no new input.
Generated metadata, package, or installation impact: none. No ignored catalogs, no skills/, and no package metadata change.

Expected effects (check known effects; use documentation-only on its own). Unchecked items do not establish absence; leave uncertainties visible.

Read existing files or captured data.
Launch or attach to a process.
Inject instrumentation or run scripts/code.
Change process memory, registers, files, or configuration.
Contact network services.
Recover protected code or work around protections.
Documentation or usability only; no target interaction.
Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

Tests added or updated: src/domain/javascript/requireShimIdentity.test.ts. It has two positive cases (a bracketed shim alias with a window and an IPC handler, and a member read taken directly from the shim call) and seven negative cases that must stay unresolved: a shim binding reassigned before the call, a shim whose guard is evaluated under a shadowed require, a shim built on a locally defined require, a shim whose guard selects something other than require, a non-IIFE binding that merely looks like a loader, a dynamic specifier through the shim, and a shim call whose argument is not a static string.
Base reproduction or other evidence: on the affected base revision the two positive fixtures produced empty browser_windows and ipc, while an otherwise identical fixture using require("electron") produced one window and one handler. The new tests fail on the base revision for the positive cases and pass after the change.
User-visible CLI/MCP output (if applicable): not captured in this PR. The change only affects the contents of the Electron section that analyze-javascript-application and the analyze_javascript_application MCP tool already return; no new field or command is added.
Remaining proof gaps: no real-world bundled Electron application was packaged and analyzed end to end here, so the fix is established against the exact shim shape esbuild emits and the resolver's own proof conditions rather than against a third-party binary. The __toESM helper and intermediate alias bindings remain uncovered by design.

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 vitest run src/domain/javascript/requireShimIdentity.test.ts --project domain — 9 passed.
npx vitest run src/domain/javascript --project domain — 69 files, 1639 tests passed, no regressions.
npx vitest run src/domain/javascript/electronBundledRequireIdentity.test.ts --project domain — 35 passed, confirming the webpack bootstrap path is unchanged.
npx tsc --noEmit — exit 0.
npx oxlint src/domain/javascript/javascriptRequireShimOrigin.ts src/domain/javascript/requireShimIdentity.test.ts — exit 0, no warnings.
npx oxfmt --check on the changed files — clean.
node scripts/verify-module-boundaries.mjs — exit 0, 1890 source files checked.

Compatibility, safety, and release

Breaking changes or migration steps: none.
Real Hopper/Ghidra, browser, or OS coverage: none. This is pure static analysis of JavaScript source and requires no provider, browser, or host engine.
Package or release metadata impact: none.
Security, privacy, process, or containment review: none. The change adds no execution, network, or filesystem behavior, and it can only add an Electron association that a proven literal require already supports.

Author readiness

Human owner: @DeryFerd

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

Before merge

This PR addresses a concrete problem or an agreed enhancement.
Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
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.

_{🔄 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/3596 **Author:** [@DeryFerd](https://github.com/DeryFerd) **Created:** 10/14/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/esbuild-require-shim-electron-identity` --- ### 📝 Commits (1) - [`9de7b20`](https://github.com/morluto/rea/commit/9de7b2017d28a6119e20ad0bb6394cb027441f9a) fix(javascript): follow Electron through esbuild's __require shim ### 📊 Changes **4 files changed** (+414 additions, -25 deletions) <details> <summary>View changed files</summary> ➕ `src/domain/javascript/javascriptRequireShimOrigin.ts` (+212 -0) 📝 `src/domain/javascript/javascriptSemanticAnalysis.ts` (+3 -1) 📝 `src/domain/javascript/javascriptSemanticRequireOrigin.ts` (+71 -24) ➕ `src/domain/javascript/requireShimIdentity.test.ts` (+128 -0) </details> ### 📄 Description ## Summary Fixes #3535. `analyze-javascript-application` reported zero Electron browser windows and zero IPC handlers for an Electron main process that was bundled with esbuild, even though the same program produced the expected window and handler when its `require("electron")` calls were left unbundled. The Electron boundary summary is a primary output of JavaScript application analysis, and an empty one reads as "this app has no Electron surface", so a silent zero here is worse than a stated unknown. ## Problem and expected behavior esbuild rewrites CommonJS `require` into a small IIFE shim for Node and Electron targets, and minification then renames that shim to a single letter. A bundled `require("electron")` arrives as `Q("electron")` at every call site, so the direct-require resolver never sees the specifier and no Electron identity is recovered. The smallest trigger is the shim esbuild emits, followed by an ordinary Electron usage: ```js var Q = (e => typeof require < "u" ? require : typeof Proxy < "u" ? new Proxy(e, { get: (t, n) => (typeof require < "u" ? require : t)[n] }) : e)(function (e) { if (typeof require < "u") return require.apply(this, arguments); throw Error('Dynamic require of "' + e + '" is not supported'); }); var cy = Q("electron"); new cy.BrowserWindow({ webPreferences: { preload: "./preload.js" } }); cy.ipcMain.handle("get-version", () => "1.0.0"); ``` On the affected base revision this reported `browser_windows: []` and `ipc: []`. The violated invariant is that a literal require reached through a proven loader alias must carry the same module identity as the direct call, which is already the rule for webpack's bootstrap externals. The expected behavior is one browser window with `preload_path: "./preload.js"` and one `get-version` main handler, exactly as the unbundled form yields. ## Change and scope A new `semanticRequireShimOrigin` resolver recovers a literal require reached through the shim, and `electronRequireOrigin` consults it after the direct and webpack-bootstrap resolvers. The shim is proven from syntax, not from a binding name. The resolver accepts a binding only when all of the following hold: the binding has a single non-assignment definition, the binding is not resolution-blocked, its stable initializer is a call expression, the callee is a plain arrow or function expression (not a generator, not async, not a method, and with exactly one identifier parameter), the callee body is a conditional expression rather than a block, and following the first branch of each nested conditional reaches the unshadowed global `require`. When the proof holds, `Q(specifier)` in that scope carries the same literal require origin, including its member path. Only Electron identity consumes this resolver, matching how `semanticBundledRequireOrigin` is scoped to `rootExport` and `bindingOrigin`. Intentionally not included: the esbuild `__toESM` interop helper, non-Electron consumers of the same shim, intermediate alias bindings such as `const R = Q; R("electron")`, and any change to the direct-require or webpack-bootstrap resolvers. Those are separable follow-ups and are not required to fix the reported bundle shape, where the shim binding is called directly at each site. ## Contract and boundary impact - Semantic owner and earliest changed stage: Electron identity in `indexJavaScriptElectronIdentitySteps`; the earliest changed stage is JavaScript static analysis, before any Electron finding is emitted. - Contribution scope fit and linked maintainer decision: not applicable. This is a routine static-analysis fix inside existing boundaries that adds no provider, no execution, no injection, no mutation, and no protection recovery. - CLI and MCP/tool-catalog contract: none. The same CLI commands and MCP tools are affected, and no advertised schema, field, or catalog entry changes. - Provider, bridge, target-format, or platform compatibility: none. The change is host-agnostic and format-agnostic, and it touches no provider or bridge. - Evidence, artifact, provenance, or reconstruction contract: calls resolved through a proven shim become direct observations, the same way the webpack bootstrap already treats loader calls. A shim call that cannot be proven stays unresolved and keeps its existing unknown, so the change cannot introduce a false Electron association. - Target selection, authority, upstream behavior, and cleanup details (if applicable): not applicable. The analysis is inert and reads no new input. - Generated metadata, package, or installation impact: none. No ignored catalogs, no `skills/`, and no package metadata change. Expected effects (check known effects; use documentation-only on its own). Unchecked items do not establish absence; leave uncertainties visible. - [x] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage - Tests added or updated: `src/domain/javascript/requireShimIdentity.test.ts`. It has two positive cases (a bracketed shim alias with a window and an IPC handler, and a member read taken directly from the shim call) and seven negative cases that must stay unresolved: a shim binding reassigned before the call, a shim whose guard is evaluated under a shadowed `require`, a shim built on a locally defined `require`, a shim whose guard selects something other than `require`, a non-IIFE binding that merely looks like a loader, a dynamic specifier through the shim, and a shim call whose argument is not a static string. - Base reproduction or other evidence: on the affected base revision the two positive fixtures produced empty `browser_windows` and `ipc`, while an otherwise identical fixture using `require("electron")` produced one window and one handler. The new tests fail on the base revision for the positive cases and pass after the change. - User-visible CLI/MCP output (if applicable): not captured in this PR. The change only affects the contents of the Electron section that `analyze-javascript-application` and the `analyze_javascript_application` MCP tool already return; no new field or command is added. - Remaining proof gaps: no real-world bundled Electron application was packaged and analyzed end to end here, so the fix is established against the exact shim shape esbuild emits and the resolver's own proof conditions rather than against a third-party binary. The `__toESM` helper and intermediate alias bindings remain uncovered by design. 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 vitest run src/domain/javascript/requireShimIdentity.test.ts --project domain` — 9 passed. - `npx vitest run src/domain/javascript --project domain` — 69 files, 1639 tests passed, no regressions. - `npx vitest run src/domain/javascript/electronBundledRequireIdentity.test.ts --project domain` — 35 passed, confirming the webpack bootstrap path is unchanged. - `npx tsc --noEmit` — exit 0. - `npx oxlint src/domain/javascript/javascriptRequireShimOrigin.ts src/domain/javascript/requireShimIdentity.test.ts` — exit 0, no warnings. - `npx oxfmt --check` on the changed files — clean. - `node scripts/verify-module-boundaries.mjs` — exit 0, 1890 source files checked. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: none. This is pure static analysis of JavaScript source and requires no provider, browser, or host engine. - Package or release metadata impact: none. - Security, privacy, process, or containment review: none. The change adds no execution, network, or filesystem behavior, and it can only add an Electron association that a proven literal require already supports. ## Author readiness Human owner: @DeryFerd - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. ## Before merge - [ ] This PR addresses a concrete problem or an agreed enhancement. - [ ] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [ ] 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. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:09:52 +00:00