morluto/rea

Fork 0

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

[PR #6993] fix(javascript): follow Electron through aliases of Webpack externals #7556

New issue

Open

opened 2026-11-03 13:38:40 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-03 13:38:40 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6993
Author: @mohammadasim224
Created: 10/24/2026
Status: 🔄 Open

Base: main ← Head: fix/4490-destructured-webpack-electron

📝 Commits (3)

81df0b3 fix(javascript): follow Electron through aliases of Webpack externals
4fecc10 fix(javascript): stop Electron alias hops at bindings exposed to direct eval
0aa85fb fix(ci): merge main to isolate JavaScript recovery checks

📊 Changes

3 files changed (+109 additions, -2 deletions)

View changed files

📝 src/domain/javascript/electronBundledRequireIdentity.test.ts (+71 -0)
📝 src/domain/javascript/javascriptSemanticAnalysis.ts (+30 -2)
📝 tests/acceptance/applications/webpackLoaderReturnsCli.test.ts (+8 -0)

📄 Description

Summary

Refs #4490 (the BrowserWindow/IPC destructuring part). Electron identity was lost when a destructuring or alias read from a binding that held a proven Webpack external: const s = n(45840); const { BrowserWindow, ipcMain } = s. Windows (including nodeIntegration: true ones) and identity-based IPC handlers were omitted with no unknown. Direct const { BrowserWindow } = n(45840) and require("electron") aliases already worked.

Problem and expected behavior

Smallest trigger, Webpack 5 bootstrap from #2948 with this factory body:

const s=n(45840);const{BrowserWindow}=s;new BrowserWindow({webPreferences:{preload:"./preload.js"}})

On main (dd02d8ff6) rea analyze-javascript-application reports browser_windows: 0. Expected: 1, with the same identity as new s.BrowserWindow(...).

Cause: bindingOrigin resolves an initializer with semanticRequireOrigin or semanticBundledRequireOrigin, which only accept a call. When the initializer is another identifier, it falls back to general provenance (evaluateSemanticProvenance), which recognizes literal require only, so the bundled external is not followed.

Change and scope

bindingOrigin (the Electron identity owner) now follows a stable lexical alias initializer to the binding it names and appends the projection. It reuses semanticStableInitializer, stops on cycles through visited binding IDs, and is bounded by SEMANTIC_ALIAS_SELECTION_LIMIT. Like general provenance, a hop stops at a rebindable binding that direct eval may have changed, so let s=require("electron");eval(code);const{BrowserWindow}=s stays unresolved as on main. Member writes through such aliases are now attributed by the existing write coverage, so const w=s; w.BrowserWindow=class{} keeps the window unresolved.

The bundled origin is still not added to general provenance; #2948 scoped it to Electron identity, and this keeps that scope.

Pre-existing and unchanged: a direct member read of a bundled external binding (let s=n(45840);eval(code);new s.BrowserWindow()) resolves on main without the direct-eval check, because semanticBundledRequireOrigin is not routed through provenance. Not changed here.

Not included: unknowns for computed-key or rest destructuring (the issue's last request). This matches plain-require destructuring, which also returns no binding there; it needs a separate decision.

Ordinary fix within existing boundaries; maintainer scope decision: not applicable.

Related: #4493 also references #4490 but bundles six issues, has conflicts with main, and was last updated 2026-10-20.

Contract and boundary impact

Semantic owner and earliest changed stage: Electron identity in src/domain/javascript/javascriptSemanticAnalysis.ts (bindingOrigin).
Contribution scope fit and linked maintainer decision: existing static analysis; not applicable.
CLI and MCP/tool-catalog contract: unchanged schema; more browser_windows/IPC findings for this input shape.
Provider, bridge, target-format, or platform compatibility: none.
Evidence, artifact, provenance, or reconstruction contract: unchanged.
Target selection, authority, upstream behavior, and cleanup details: not applicable.
Generated metadata, package, or installation impact: none.
Read existing files or captured data.

Evidence and regression coverage

Tests added or updated: src/domain/javascript/electronBundledRequireIdentity.test.ts (shorthand, renamed, plain alias, two-hop alias, top-level and guarded destructured IPC; reassigned binding, overwritten member directly and through an alias, cyclic alias, wrong table key, and direct-eval-exposed bundled and require aliases stay unresolved). tests/acceptance/applications/webpackLoaderReturnsCli.test.ts adds a CLI row for the destructured form.
Base reproduction: with the source change stashed, the 6 positive unit cases and the CLI row fail; they pass with it.
User-visible CLI output, fixtures built from the issue (browser_windows / main_handlers):

Fixture	before	after
`const{BrowserWindow}=s; new BrowserWindow(...)`	0 / 0	1 / 0
`const{BrowserWindow:W}=s; new W(...)`	0 / 0	1 / 0
`const{BrowserWindow,ipcMain}=s; ...`	0 / 1	1 / 1

Derived, not read from CLI output (it has no resolution_basis): the main_handlers: 1 before the fix most likely came from the non-identity wrapper-substitution path. The equivalent unit test, which filters that basis out, finds no handler on the base.

Remaining proof gaps: Bitwarden 2026.10.0 (34/34 handlers in the issue) not run.
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

CI follow-up at 0aa85fb86 (2026-10-25), Node 24.22.1 / npm 11.16.0 on macOS:

The failed MCP resource test checked whether analysis recovered only after retained views had independently consumed the deliberately constrained parent heap. The subsequent call correctly reported a zero parent result budget, which the test mistook for failed worker recovery.
Merged main aced9fa9b, including #7029 (bc9efc927): recovery is checked before materializing retained views; the original partial Evidence is then verified after the new analysis, with all 1,500 source occurrences and their identities preserved. No additional Electron implementation change was needed.
npm run check:fast (typecheck/lint) and npm run format:check: passed.
Electron bundled-require identity source tests: 83 passed.
MCP JavaScript resource/recovery, Webpack loader CLI, and application workflow CLI/MCP tests: 44 passed, including the previously failing oversized-timeout recovery case.
Fresh hosted CI is pending. The separate readiness check still requires a named human owner and the three human review/readiness declarations below; they remain unchanged and unchecked, as required by CONTRIBUTING.md.

Earlier author validation:

Node 24.22.1, npm 11.16.0, macOS arm64.

npm run test:local -- src/domain/javascript/*.test.ts — 149 files, 4080 tests passed
npm run test:focused -- tests/acceptance/applications/webpackLoaderReturnsCli.test.ts tests/acceptance/applications/applicationWorkflowCli.test.ts — 2 files, 41 tests passed
npm run check:fast — typecheck and lint passed
All three above re-run on the final HEAD. Cost (measured before the direct-eval guard commit, which adds two predicate checks per alias hop): analyze-javascript-application on @babel/parser/lib/index.js (481 KB), 3 warm runs each: before 6.65–6.90 s wall, 8.49–8.77 s CPU, 588–593 MiB max RSS; after 6.88–7.39 s wall, 8.68–8.88 s CPU, 588–594 MiB. Within run-to-run noise. Max RSS is /usr/bin/time -l (largest reaped process, not a process-family sum). Agent token cost unchanged except for the added findings. Not measured on a large Electron bundle.

Compatibility, safety, and release

Breaking changes or migration steps: none.
Real Hopper/Ghidra, browser, or OS coverage: not applicable; static analysis only.
Package or release metadata impact: none.
Security, privacy, process, or containment review: no new side effects; analysis remains read-only.

Author readiness

Human owner:

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.

🤖 Generated with Claude Code

_{🔄 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/6993 **Author:** [@mohammadasim224](https://github.com/mohammadasim224) **Created:** 10/24/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/4490-destructured-webpack-electron` --- ### 📝 Commits (3) - [`81df0b3`](https://github.com/morluto/rea/commit/81df0b3edaaa205ed630a760d231298d1bf7f258) fix(javascript): follow Electron through aliases of Webpack externals - [`4fecc10`](https://github.com/morluto/rea/commit/4fecc1029efb08b28c0276e2dd148ee8ef84a179) fix(javascript): stop Electron alias hops at bindings exposed to direct eval - [`0aa85fb`](https://github.com/morluto/rea/commit/0aa85fb862ffbfeef27643ced8dc0d5b27ecd15d) fix(ci): merge main to isolate JavaScript recovery checks ### 📊 Changes **3 files changed** (+109 additions, -2 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/javascript/electronBundledRequireIdentity.test.ts` (+71 -0) 📝 `src/domain/javascript/javascriptSemanticAnalysis.ts` (+30 -2) 📝 `tests/acceptance/applications/webpackLoaderReturnsCli.test.ts` (+8 -0) </details> ### 📄 Description ## Summary Refs #4490 (the `BrowserWindow`/IPC destructuring part). Electron identity was lost when a destructuring or alias read from a binding that held a proven Webpack external: `const s = n(45840); const { BrowserWindow, ipcMain } = s`. Windows (including `nodeIntegration: true` ones) and identity-based IPC handlers were omitted with no unknown. Direct `const { BrowserWindow } = n(45840)` and `require("electron")` aliases already worked. ## Problem and expected behavior Smallest trigger, Webpack 5 bootstrap from #2948 with this factory body: ```js const s=n(45840);const{BrowserWindow}=s;new BrowserWindow({webPreferences:{preload:"./preload.js"}}) ``` On `main` (`dd02d8ff6`) `rea analyze-javascript-application` reports `browser_windows: 0`. Expected: 1, with the same identity as `new s.BrowserWindow(...)`. Cause: `bindingOrigin` resolves an initializer with `semanticRequireOrigin` or `semanticBundledRequireOrigin`, which only accept a call. When the initializer is another identifier, it falls back to general provenance (`evaluateSemanticProvenance`), which recognizes literal `require` only, so the bundled external is not followed. ## Change and scope `bindingOrigin` (the Electron identity owner) now follows a stable lexical alias initializer to the binding it names and appends the projection. It reuses `semanticStableInitializer`, stops on cycles through visited binding IDs, and is bounded by `SEMANTIC_ALIAS_SELECTION_LIMIT`. Like general provenance, a hop stops at a rebindable binding that direct `eval` may have changed, so `let s=require("electron");eval(code);const{BrowserWindow}=s` stays unresolved as on `main`. Member writes through such aliases are now attributed by the existing write coverage, so `const w=s; w.BrowserWindow=class{}` keeps the window unresolved. The bundled origin is still not added to general provenance; #2948 scoped it to Electron identity, and this keeps that scope. Pre-existing and unchanged: a direct member read of a bundled external binding (`let s=n(45840);eval(code);new s.BrowserWindow()`) resolves on `main` without the direct-eval check, because `semanticBundledRequireOrigin` is not routed through provenance. Not changed here. Not included: unknowns for computed-key or rest destructuring (the issue's last request). This matches plain-`require` destructuring, which also returns no binding there; it needs a separate decision. Ordinary fix within existing boundaries; maintainer scope decision: not applicable. Related: #4493 also references #4490 but bundles six issues, has conflicts with `main`, and was last updated 2026-10-20. ## Contract and boundary impact - Semantic owner and earliest changed stage: Electron identity in `src/domain/javascript/javascriptSemanticAnalysis.ts` (`bindingOrigin`). - Contribution scope fit and linked maintainer decision: existing static analysis; not applicable. - CLI and MCP/tool-catalog contract: unchanged schema; more `browser_windows`/IPC findings for this input shape. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: unchanged. - Target selection, authority, upstream behavior, and cleanup details: not applicable. - Generated metadata, package, or installation impact: none. - [x] Read existing files or captured data. ## Evidence and regression coverage - Tests added or updated: `src/domain/javascript/electronBundledRequireIdentity.test.ts` (shorthand, renamed, plain alias, two-hop alias, top-level and guarded destructured IPC; reassigned binding, overwritten member directly and through an alias, cyclic alias, wrong table key, and direct-eval-exposed bundled and `require` aliases stay unresolved). `tests/acceptance/applications/webpackLoaderReturnsCli.test.ts` adds a CLI row for the destructured form. - Base reproduction: with the source change stashed, the 6 positive unit cases and the CLI row fail; they pass with it. - User-visible CLI output, fixtures built from the issue (`browser_windows` / `main_handlers`): | Fixture | before | after | |---|---|---| | `const{BrowserWindow}=s; new BrowserWindow(...)` | 0 / 0 | 1 / 0 | | `const{BrowserWindow:W}=s; new W(...)` | 0 / 0 | 1 / 0 | | `const{BrowserWindow,ipcMain}=s; ...` | 0 / 1 | 1 / 1 | Derived, not read from CLI output (it has no `resolution_basis`): the `main_handlers: 1` before the fix most likely came from the non-identity `wrapper-substitution` path. The equivalent unit test, which filters that basis out, finds no handler on the base. - Remaining proof gaps: Bitwarden 2026.10.0 (34/34 handlers in the issue) not run. - [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 CI follow-up at `0aa85fb86` (2026-10-25), Node 24.22.1 / npm 11.16.0 on macOS: - The failed MCP resource test checked whether analysis recovered only after retained views had independently consumed the deliberately constrained parent heap. The subsequent call correctly reported a zero parent result budget, which the test mistook for failed worker recovery. - Merged main `aced9fa9b`, including #7029 (`bc9efc927`): recovery is checked before materializing retained views; the original partial Evidence is then verified after the new analysis, with all 1,500 source occurrences and their identities preserved. No additional Electron implementation change was needed. - `npm run check:fast` (typecheck/lint) and `npm run format:check`: passed. - Electron bundled-require identity source tests: **83 passed**. - MCP JavaScript resource/recovery, Webpack loader CLI, and application workflow CLI/MCP tests: **44 passed**, including the previously failing oversized-timeout recovery case. - Fresh hosted CI is pending. The separate readiness check still requires a named human owner and the three human review/readiness declarations below; they remain unchanged and unchecked, as required by CONTRIBUTING.md. Earlier author validation: Node 24.22.1, npm 11.16.0, macOS arm64. - `npm run test:local -- src/domain/javascript/*.test.ts` — 149 files, 4080 tests passed - `npm run test:focused -- tests/acceptance/applications/webpackLoaderReturnsCli.test.ts tests/acceptance/applications/applicationWorkflowCli.test.ts` — 2 files, 41 tests passed - `npm run check:fast` — typecheck and lint passed - All three above re-run on the final HEAD. Cost (measured before the direct-eval guard commit, which adds two predicate checks per alias hop): `analyze-javascript-application` on `@babel/parser/lib/index.js` (481 KB), 3 warm runs each: before 6.65–6.90 s wall, 8.49–8.77 s CPU, 588–593 MiB max RSS; after 6.88–7.39 s wall, 8.68–8.88 s CPU, 588–594 MiB. Within run-to-run noise. Max RSS is `/usr/bin/time -l` (largest reaped process, not a process-family sum). Agent token cost unchanged except for the added findings. Not measured on a large Electron bundle. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: not applicable; static analysis only. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no new side effects; analysis remains read-only. ## Author readiness Human owner:  - [ ] 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. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi added the

pull-request

label

2026-11-03 13:38:40 +00:00

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

No description provided.

Rows
Columns