morluto/rea

Fork 0

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

[PR #7229] fix(electron): resolve module constant window options read inside functions #7731

New issue

Open

opened 2026-11-03 13:39:57 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-03 13:39:57 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/7229
Author: @jiwonsudo
Created: 10/26/2026
Status: 🔄 Open

Base: main ← Head: fix/electron-nested-const-window-options

📝 Commits (9)

88f45ce fix(electron): resolve module constant window options read inside functions
14d71a0 fix(electron): keep nested reads unknown in modules with ES exports
ccd5548 fix(electron): ignore erased type-only exports for nested option reads
7de864a fix(electron): treat exported namespaces and import aliases as exposure
2321eab fix(electron): count only instantiated namespaces as module exposure
cebd893 fix(electron): count enums whose members may hold objects
8f2278a Merge remote-tracking branch 'origin/main' into fix/electron-nested-const-window-options
591dfa7 Merge remote-tracking branch 'origin/main' into fix/electron-nested-const-window-options
480d73f Merge remote-tracking branch 'origin/main' into fix/electron-nested-const-window-options

📊 Changes

3 files changed (+355 additions, -12 deletions)

View changed files

📝 docs/javascript-artifact-reconstruction.md (+14 -4)
📝 src/domain/javascript/electronStaticAnalysisOptions.test.ts (+194 -0)
📝 src/domain/javascript/electronStaticOptionObjects.ts (+147 -8)

📄 Description

Summary

Fixes #7227. Constant BrowserWindow options and webPreferences were only resolved when the window was created in the same activation as the const. In the common createWindow() pattern, a module-level webPreferences object read inside a function or callback was always reported as dynamic, so nodeIntegration: true and similar settings were dropped.

Problem and expected behavior

const webPreferences = { nodeIntegration: true, contextIsolation: false };
function createWindow() {
  return new BrowserWindow({ width: 800, height: 600, webPreferences });
}

Before: web_preferences_status: "dynamic", no preferences. Expected: the two literal preferences, as already reported when the same code runs at module top level. Writes, escapes, and unclear orders must still stay unknown.

Change and scope

In electronStaticOptionObjects.ts, a const read that is not in the same activation as its declaration is now accepted when:

the read is inside a function, and the declaration ends before that function starts (a completed read of a const must see its initializer; hoisted or interleaved orders stay unknown)
the module does not export a runtime local value with ES module syntax. Importers can reach the object through an exported alias, container, function, instantiated namespace, enum member or export import alias, and these exports are not escapes in the shared mutation index. Erased type-only exports (export interface, export type, export type { X }, export declare ...), namespaces that hold only types, and enums whose members are only literal numbers or strings do not count. CommonJS exports are already escapes in the index, so they keep the precise check.
effectsForReferenceAt reports no write or escape for the selected path other than the constructor argument itself. Other activations and repeated calls can run in any order, so effects in other functions, later effects in the same function, and a second constructor that receives the same object all keep it unknown.

Same-activation reads keep the existing referenceMayHaveChangedAt check unchanged. I also updated the constant-options paragraph in docs/javascript-artifact-reconstruction.md.

Not included: IPC channel constants and setWindowOpenHandler overrides (#3536).

Follow the contribution scope. This extends existing constant-options resolution inside the static analyzer, so a maintainer scope decision is not needed.

Contract and boundary impact

Semantic owner and earliest changed stage: src/domain/javascript/electronStaticOptionObjects.ts, Electron BrowserWindow option resolution
Contribution scope fit and linked maintainer decision: bug fix in existing behavior, not applicable
CLI and MCP/tool-catalog contract: none. Output shape is unchanged; more windows report object-literal with resolution_sources.
Provider, bridge, target-format, or platform compatibility: none
Evidence, artifact, provenance, or reconstruction contract: resolved windows keep resolution_sources pointing at the original object literals, as for same-scope constants
Target selection, authority, upstream behavior, and cleanup details (if applicable): not applicable
Generated metadata (docs/public/product-catalog.json), package, or installation impact: none

Expected effects:

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: new Electron constant options read from nested functions block in src/domain/javascript/electronStaticAnalysisOptions.test.ts:
- resolves: function, app.whenReady().then callback, class method, whole options const, unrelated options.icon write in another function
- stays unknown: write in another function, top-level write, Object.assign in another function, escape call, CommonJS export, write or escape after the read in the same function (repeated calls), a second constructor sharing the object, function declared before the const, let, shadowing parameter, export const, export { name }, export default name
Base reproduction or other evidence: without the source change, the 6 "resolves" tests fail and the rest pass. With it, the file passes (76 tests).

User-visible CLI/MCP output (if applicable), from the issue's reproduction:

main:   summary.explicit_web_preferences = 0, web_preferences_status = dynamic, web_preferences = []
branch: summary.explicit_web_preferences = 2, web_preferences_status = object-literal,
        web_preferences = [["contextIsolation", false], ["nodeIntegration", true]]

Remaining proof gaps: not checked against a large real application (for example Joplin) or bundled Webpack output. Full npm run check:pr was not run.

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

Node v24.22.1, Linux x64:

Merged current main (5508b2af) before the final CI run.
npx vitest run src/domain/javascript/electronStaticAnalysisOptions.test.ts: 95 passed (18 failed with main's source 5508b2af)
npx vitest run src/domain/javascript src/application/javascript src/javascript: 190 files, 4960 passed, 7 skipped
npx vitest run on the 69 acceptance files under tests/acceptance/applications/ matching electron, window or javascript (after npm run build): 651 passed
npx tsc --noEmit: passed
npx oxlint on the changed files: no issues
npx oxfmt --check on the changed files: passed
npm run verify:test-discovery: passed
npm run verify:module-boundaries: passed

Compatibility, safety, and release

Breaking changes or migration steps: none. Some windows that were dynamic now report object-literal.
Real Hopper/Ghidra, browser, or OS coverage: not applicable. Linux only.
Package or release metadata impact: none
Security, privacy, process, or containment review: no target code is executed. The change only reads source syntax.

Author readiness

Human owner: @jiwonsudo

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/7229 **Author:** [@jiwonsudo](https://github.com/jiwonsudo) **Created:** 10/26/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/electron-nested-const-window-options` --- ### 📝 Commits (9) - [`88f45ce`](https://github.com/morluto/rea/commit/88f45cefac5c892e3ad563753b19cc268806f8d1) fix(electron): resolve module constant window options read inside functions - [`14d71a0`](https://github.com/morluto/rea/commit/14d71a05bd021fdf86971793360bd6836a4b89de) fix(electron): keep nested reads unknown in modules with ES exports - [`ccd5548`](https://github.com/morluto/rea/commit/ccd5548af851c2a8514ccea90a05510fcfb05f68) fix(electron): ignore erased type-only exports for nested option reads - [`7de864a`](https://github.com/morluto/rea/commit/7de864aa82858b7a8f74555ce0d85a68a723ca58) fix(electron): treat exported namespaces and import aliases as exposure - [`2321eab`](https://github.com/morluto/rea/commit/2321eab059a10acacc6d5e7652ed366e42eb0405) fix(electron): count only instantiated namespaces as module exposure - [`cebd893`](https://github.com/morluto/rea/commit/cebd893d2c0909658042974577c6b709706f712a) fix(electron): count enums whose members may hold objects - [`8f2278a`](https://github.com/morluto/rea/commit/8f2278a55b89259fbfae83e41703871c7bae7fe7) Merge remote-tracking branch 'origin/main' into fix/electron-nested-const-window-options - [`591dfa7`](https://github.com/morluto/rea/commit/591dfa722ef560818a55e8413cb64cf219af4fe8) Merge remote-tracking branch 'origin/main' into fix/electron-nested-const-window-options - [`480d73f`](https://github.com/morluto/rea/commit/480d73f2789d660c37204896fb6950800d57329a) Merge remote-tracking branch 'origin/main' into fix/electron-nested-const-window-options ### 📊 Changes **3 files changed** (+355 additions, -12 deletions) <details> <summary>View changed files</summary> 📝 `docs/javascript-artifact-reconstruction.md` (+14 -4) 📝 `src/domain/javascript/electronStaticAnalysisOptions.test.ts` (+194 -0) 📝 `src/domain/javascript/electronStaticOptionObjects.ts` (+147 -8) </details> ### 📄 Description ## Summary Fixes #7227. Constant `BrowserWindow` options and `webPreferences` were only resolved when the window was created in the same activation as the `const`. In the common `createWindow()` pattern, a module-level `webPreferences` object read inside a function or callback was always reported as `dynamic`, so `nodeIntegration: true` and similar settings were dropped. ## Problem and expected behavior ```js const webPreferences = { nodeIntegration: true, contextIsolation: false }; function createWindow() { return new BrowserWindow({ width: 800, height: 600, webPreferences }); } ``` Before: `web_preferences_status: "dynamic"`, no preferences. Expected: the two literal preferences, as already reported when the same code runs at module top level. Writes, escapes, and unclear orders must still stay unknown. ## Change and scope In `electronStaticOptionObjects.ts`, a const read that is not in the same activation as its declaration is now accepted when: - the read is inside a function, and the declaration ends before that function starts (a completed read of a const must see its initializer; hoisted or interleaved orders stay unknown) - the module does not export a runtime local value with ES module syntax. Importers can reach the object through an exported alias, container, function, instantiated namespace, enum member or `export import` alias, and these exports are not escapes in the shared mutation index. Erased type-only exports (`export interface`, `export type`, `export type { X }`, `export declare ...`), namespaces that hold only types, and enums whose members are only literal numbers or strings do not count. CommonJS exports are already escapes in the index, so they keep the precise check. - `effectsForReferenceAt` reports no write or escape for the selected path other than the constructor argument itself. Other activations and repeated calls can run in any order, so effects in other functions, later effects in the same function, and a second constructor that receives the same object all keep it unknown. Same-activation reads keep the existing `referenceMayHaveChangedAt` check unchanged. I also updated the constant-options paragraph in `docs/javascript-artifact-reconstruction.md`. Not included: IPC channel constants and `setWindowOpenHandler` overrides (#3536). Follow the [contribution scope](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope). This extends existing constant-options resolution inside the static analyzer, so a maintainer scope decision is not needed. ## Contract and boundary impact - Semantic owner and earliest changed stage: `src/domain/javascript/electronStaticOptionObjects.ts`, Electron BrowserWindow option resolution - Contribution scope fit and linked maintainer decision: bug fix in existing behavior, not applicable - CLI and MCP/tool-catalog contract: none. Output shape is unchanged; more windows report `object-literal` with `resolution_sources`. - Provider, bridge, target-format, or platform compatibility: none - Evidence, artifact, provenance, or reconstruction contract: resolved windows keep `resolution_sources` pointing at the original object literals, as for same-scope constants - Target selection, authority, upstream behavior, and cleanup details (if applicable): not applicable - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact: none Expected effects: - [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: new `Electron constant options read from nested functions` block in `src/domain/javascript/electronStaticAnalysisOptions.test.ts`: - resolves: function, `app.whenReady().then` callback, class method, whole options const, unrelated `options.icon` write in another function - stays unknown: write in another function, top-level write, `Object.assign` in another function, escape call, CommonJS export, write or escape after the read in the same function (repeated calls), a second constructor sharing the object, function declared before the const, `let`, shadowing parameter, `export const`, `export { name }`, `export default name` - Base reproduction or other evidence: without the source change, the 6 "resolves" tests fail and the rest pass. With it, the file passes (76 tests). - User-visible CLI/MCP output (if applicable), from the issue's reproduction: ``` main: summary.explicit_web_preferences = 0, web_preferences_status = dynamic, web_preferences = [] branch: summary.explicit_web_preferences = 2, web_preferences_status = object-literal, web_preferences = [["contextIsolation", false], ["nodeIntegration", true]] ``` - Remaining proof gaps: not checked against a large real application (for example Joplin) or bundled Webpack output. Full `npm run check:pr` was not run. 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 Node v24.22.1, Linux x64: - Merged current main (`5508b2af`) before the final CI run. - `npx vitest run src/domain/javascript/electronStaticAnalysisOptions.test.ts`: 95 passed (18 failed with main's source `5508b2af`) - `npx vitest run src/domain/javascript src/application/javascript src/javascript`: 190 files, 4960 passed, 7 skipped - `npx vitest run` on the 69 acceptance files under `tests/acceptance/applications/` matching electron, window or javascript (after `npm run build`): 651 passed - `npx tsc --noEmit`: passed - `npx oxlint` on the changed files: no issues - `npx oxfmt --check` on the changed files: passed - `npm run verify:test-discovery`: passed - `npm run verify:module-boundaries`: passed ## Compatibility, safety, and release - Breaking changes or migration steps: none. Some windows that were `dynamic` now report `object-literal`. - Real Hopper/Ghidra, browser, or OS coverage: not applicable. Linux only. - Package or release metadata impact: none - Security, privacy, process, or containment review: no target code is executed. The change only reads source syntax. ## Author readiness Human owner: @jiwonsudo - [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 - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [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. --- <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:39:57 +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#7731

No description provided.

Rows
Columns