[PR #710] [MERGED] fix(electron): respect computed option keys and overrides #2191

New issue

Closed

opened 2026-10-16 21:15:28 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-16 21:15:28 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/710
Author: @kaoru0822-kitauji
Created: 10/6/2026
Status: ✅ Merged
Merged: 10/6/2026
Merged by: @kaoru0822-kitauji

Base: main ← Head: fix/electron-option-property-keys

📝 Commits (1)

98de338 fix(electron): respect computed option keys and overrides

📊 Changes

4 files changed (+340 additions, -29 deletions)

View changed files

📝 src/domain/electronStaticAnalysisBrowser.ts (+13 -22)
➕ src/domain/electronStaticAnalysisOptions.test.ts (+157 -0)
📝 src/domain/electronStaticAnalysisValues.ts (+20 -7)
➕ tests/composition/javascript-applications/electronOptionKeys.test.ts (+150 -0)

📄 Description

Summary

Respect computed keys and property precedence when observing Electron BrowserWindow and utility-process options.

No separate issue: this is a reproduced static-analysis false positive.

Problem and expected behavior

For const webPreferences = "title"; new BrowserWindow({ [webPreferences]: { preload: "./wrong.js" } }), the options object has a title key. Static analysis nevertheless reports explicit webPreferences and a literal preload. The resulting BrowserWindow observation is marked exact with complete coverage and supports a false preload association.

The shared lookup likewise reports a service name from const serviceName = "env"; utilityProcess.fork("./worker.js", [], { [serviceName]: "wrong-name" }).

Computed identifier names do not establish their evaluated property names. Unresolved options must remain unknown.

Change and scope

Replace the two-caller named-property lookup with missing/dynamic/explicit results:

Reuse existing literal-key semantics without evaluating bindings.
Scan from the last property, preserving JavaScript's override order.
Preserve plain, quoted and computed string-literal keys.
Treat unresolved computed keys, spreads and matching methods/accessors as dynamic when they can override the requested value.
Allow a later explicit key to override earlier uncertainty.

BrowserWindow dynamic results retain existing dynamic status, empty preferences and null preload. Unresolved utility service names remain null. Both paths contribute to partial-analysis coverage.

Contract and boundary impact

Semantic owner: pure Electron option lookup before static findings are projected into application graphs.
CLI/MCP: shared analysis results stop asserting unsupported option values; no tool/schema/catalog changes.
Provider/platform: static AST interpretation only; no Electron or provider execution.
Evidence/provenance: BrowserWindow-specific observations and associations retain unknown coverage instead of a false literal premise.
Process/authorization/cleanup: unchanged.
Generated metadata/package/installation: unchanged.

The independent generic property:preload heuristic is unchanged. Full reconstruction may still retain that heuristic's preload node or mapping; this fix removes the false BrowserWindow-specific premise and association.

Evidence and regression coverage

On unchanged main c3bc38b, 40 of 76 new domain cases and 7 of 11 reconstruction cases fail. The remaining cases pass as controls.

Regressions cover both callers, computed identifiers/expressions, literal controls, duplicate keys, spreads before and after explicit values, accessors/methods, null/dynamic/missing values, unrelated literal keys and last-key precedence.

Owned plain-JavaScript directory fixtures exercise production application reconstruction without executing fixture code. They verify dynamic BrowserWindow coverage, absence of false BrowserWindow preload observations/loads associations, retained explicit preload links and unresolved utility labels.

Validation performed

After fast-forwarding the unchanged reviewed patch to main 9bab553:

Focused Electron/static-analysis and production reconstruction suites: 107 tests passed across six files.
Build and npm run check: passed; all five check tasks succeeded (109 warnings, zero errors; none in changed files).
npm run jscpd (2.36%), npm run scan:todos, and git diff --check: passed.
Independent review completed before publication with no blocking findings.

Node.js 24.19.0, npm 11.9.0, exact locked dependencies. Optional Turbo telemetry was disabled for local verification. Hosted CI is tracked separately; no full local-suite pass is claimed.

Compatibility, safety, and release

No schemas, dependency, package or inner-web-preference model changes. No binding evaluation is added. Static domain and production reconstruction tests establish this correction; no real Electron, Hopper/Ghidra, browser, compiled CLI or package-install lane was run.

Prepared with AI assistance.

_{🔄 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/710 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/6/2026 **Status:** ✅ Merged **Merged:** 10/6/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `fix/electron-option-property-keys` --- ### 📝 Commits (1) - [`98de338`](https://github.com/morluto/rea/commit/98de3380e3062cb25778263bfea07b215a09b39c) fix(electron): respect computed option keys and overrides ### 📊 Changes **4 files changed** (+340 additions, -29 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/electronStaticAnalysisBrowser.ts` (+13 -22) ➕ `src/domain/electronStaticAnalysisOptions.test.ts` (+157 -0) 📝 `src/domain/electronStaticAnalysisValues.ts` (+20 -7) ➕ `tests/composition/javascript-applications/electronOptionKeys.test.ts` (+150 -0) </details> ### 📄 Description ## Summary Respect computed keys and property precedence when observing Electron BrowserWindow and utility-process options. No separate issue: this is a reproduced static-analysis false positive. ## Problem and expected behavior For `const webPreferences = "title"; new BrowserWindow({ [webPreferences]: { preload: "./wrong.js" } })`, the options object has a `title` key. Static analysis nevertheless reports explicit webPreferences and a literal preload. The resulting BrowserWindow observation is marked exact with complete coverage and supports a false preload association. The shared lookup likewise reports a service name from `const serviceName = "env"; utilityProcess.fork("./worker.js", [], { [serviceName]: "wrong-name" })`. Computed identifier names do not establish their evaluated property names. Unresolved options must remain unknown. ## Change and scope Replace the two-caller named-property lookup with missing/dynamic/explicit results: - Reuse existing literal-key semantics without evaluating bindings. - Scan from the last property, preserving JavaScript's override order. - Preserve plain, quoted and computed string-literal keys. - Treat unresolved computed keys, spreads and matching methods/accessors as dynamic when they can override the requested value. - Allow a later explicit key to override earlier uncertainty. BrowserWindow dynamic results retain existing dynamic status, empty preferences and null preload. Unresolved utility service names remain null. Both paths contribute to partial-analysis coverage. ## Contract and boundary impact - Semantic owner: pure Electron option lookup before static findings are projected into application graphs. - CLI/MCP: shared analysis results stop asserting unsupported option values; no tool/schema/catalog changes. - Provider/platform: static AST interpretation only; no Electron or provider execution. - Evidence/provenance: BrowserWindow-specific observations and associations retain unknown coverage instead of a false literal premise. - Process/authorization/cleanup: unchanged. - Generated metadata/package/installation: unchanged. The independent generic `property:preload` heuristic is unchanged. Full reconstruction may still retain that heuristic's preload node or mapping; this fix removes the false BrowserWindow-specific premise and association. ## Evidence and regression coverage On unchanged main `c3bc38b`, 40 of 76 new domain cases and 7 of 11 reconstruction cases fail. The remaining cases pass as controls. Regressions cover both callers, computed identifiers/expressions, literal controls, duplicate keys, spreads before and after explicit values, accessors/methods, null/dynamic/missing values, unrelated literal keys and last-key precedence. Owned plain-JavaScript directory fixtures exercise production application reconstruction without executing fixture code. They verify dynamic BrowserWindow coverage, absence of false BrowserWindow preload observations/loads associations, retained explicit preload links and unresolved utility labels. ## Validation performed After fast-forwarding the unchanged reviewed patch to main `9bab553`: - Focused Electron/static-analysis and production reconstruction suites: 107 tests passed across six files. - Build and `npm run check`: passed; all five check tasks succeeded (109 warnings, zero errors; none in changed files). - `npm run jscpd` (2.36%), `npm run scan:todos`, and `git diff --check`: passed. - Independent review completed before publication with no blocking findings. Node.js 24.19.0, npm 11.9.0, exact locked dependencies. Optional Turbo telemetry was disabled for local verification. Hosted CI is tracked separately; no full local-suite pass is claimed. ## Compatibility, safety, and release No schemas, dependency, package or inner-web-preference model changes. No binding evaluation is added. Static domain and production reconstruction tests establish this correction; no real Electron, Hopper/Ghidra, browser, compiled CLI or package-install lane was run. Prepared with AI assistance. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-16 21:15:28 +00:00