[PR #7053] [MERGED] fix(javascript): invalidate Electron reads after direct eval #7610

Closed
opened 2026-11-03 13:39:05 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

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

Base: main ← Head: fix/electron-direct-eval-read-identity-20261025


📝 Commits (1)

  • 4d2833d fix(javascript): invalidate Electron reads after direct eval

📊 Changes

6 files changed (+308 additions, -5 deletions)

View changed files

➕ src/domain/javascript/electronOpaqueBindingIdentity.test.ts (+131 -0)
📝 src/domain/javascript/javascriptSemanticAnalysis.eval_effects.test.ts (+21 -1)
📝 src/domain/javascript/javascriptSemanticAnalysis.ts (+17 -3)
📝 src/domain/javascript/javascriptSemanticMutationInitializers.ts (+46 -0)
📝 src/domain/javascript/javascriptSemanticOpaqueEffects.ts (+20 -1)
➕ tests/acceptance/applications/electronOpaqueBindingCli.test.ts (+73 -0)

📄 Description

Summary

Stop reporting an Electron BrowserWindow when direct eval may have rebound or shadowed its lexical root. Preserve proven earlier captures and their source locations.

Problem and expected behavior

A static fixture contains a genuine window before direct eval and a replacement constructor after it. Unpatched main reports both windows. The fix retains only the genuine pre-eval window. Target JavaScript is parsed, never executed.

Change and scope

Separate cached origin from per-read validity. Retain possible write targets even when positive read identity is uncertain. Extend the existing shared ordering owner only for fresh declaration initializers; loops, deferred effects, outer bindings and logical assignments remain conservative.

This is read admission for already-recognized Electron roots, not alias expansion. Related #6993 explicitly excludes the pre-existing direct-member/eval case; #2041's screened head retains that unguarded path. Their heads were rechecked unchanged before publication. No issue is claimed closed because this is a separately reproduced residual defect.

Contract and boundary impact

  • Semantic owners: JavaScript binding identity, opaque effects and shared initializer ordering.
  • Scope decision: not applicable; routine correctness fix within existing static-analysis authority.
  • CLI/MCP schemas, catalog, provider/platform support and target selection: unchanged.
  • Evidence: suppress unsupported window identity while preserving genuine capture provenance.
  • No new target execution, injection, mutation, network or cleanup authority.
  • Dependencies, generated metadata, installation and release behavior: unchanged.

Expected effect: read existing source files or captured data. Fixture code is never executed.

Evidence and regression coverage

  • Baseline: 7 regression failures, 33 passing controls before the production patch.
  • Coverage includes per-read cache validity, sloppy-eval shadowing, strict/indirect/optional/shadowed eval controls, constructor capture before argument evaluation, repeated activation/loops, deferred effects and possible write targets.
  • Compiled CLI and real stdio MCP both report one BrowserWindow, at the exact pre-eval source line (line 3), with zero parse failures.

Independent automated source review approved all six changed files. This is not human review.

Validation performed

Tested base: ab720b42163e33cba519518dedc67c7e11346b71.

  • Seven focused domain files: 371 tests passed.
  • Actual compiled CLI and stdio MCP acceptance: 2/2 passed.
  • Full TypeScript check after all six files, --noEmit --singleThreaded --checkers 1 --maxMemory 1G: passed.
  • Production compilation, catalog recording and runtime bundling: passed.
  • Scoped typed lint: six files, zero errors and 25 warnings; formatting passed.

Linux VM, Node 24.22.1 (repository pin), npm 11.9.0 (repository preference: 11.16.0). The source directory was reconstructed and hash-verified against the complete pinned superproject tree, not checked out as the original Git commit. All baseline blobs, directory trees, modes and five gitlinks were verified; submodule contents were not populated. Only the four intended baseline files changed, with two tests added.

The lock-pinned oxlint-tsgolint backend was installed separately with scripts disabled and integrity verified; repository package/lock files and shared dependencies were not changed.

Compatibility, safety, and release

No breaking schema or migration, package or release changes. This is Linux static-analysis validation, not a running Electron/native-provider test. The aggregate npm run check and full test suite were not run locally. Process-family RSS observations are incomplete samples, not precise peak/bound or independent cleanup proof. No performance improvement is claimed.

Published commit 4d2833d55ec0cf9c0702b0e19ba20c4de425beca has the exact reviewed six-file content and independently calculated full tree 3985faea75713d258462a237dd5b6363fecdd225. All six public blob hashes were verified.

Author readiness

Human owner: @WenyuChiou

  • 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.

This is a draft pending human 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/7053 **Author:** [@WenyuChiou](https://github.com/WenyuChiou) **Created:** 10/25/2026 **Status:** ✅ Merged **Merged:** 10/25/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/electron-direct-eval-read-identity-20261025` --- ### 📝 Commits (1) - [`4d2833d`](https://github.com/morluto/rea/commit/4d2833d55ec0cf9c0702b0e19ba20c4de425beca) fix(javascript): invalidate Electron reads after direct eval ### 📊 Changes **6 files changed** (+308 additions, -5 deletions) <details> <summary>View changed files</summary> ➕ `src/domain/javascript/electronOpaqueBindingIdentity.test.ts` (+131 -0) 📝 `src/domain/javascript/javascriptSemanticAnalysis.eval_effects.test.ts` (+21 -1) 📝 `src/domain/javascript/javascriptSemanticAnalysis.ts` (+17 -3) 📝 `src/domain/javascript/javascriptSemanticMutationInitializers.ts` (+46 -0) 📝 `src/domain/javascript/javascriptSemanticOpaqueEffects.ts` (+20 -1) ➕ `tests/acceptance/applications/electronOpaqueBindingCli.test.ts` (+73 -0) </details> ### 📄 Description ## Summary Stop reporting an Electron BrowserWindow when direct eval may have rebound or shadowed its lexical root. Preserve proven earlier captures and their source locations. ## Problem and expected behavior A static fixture contains a genuine window before direct eval and a replacement constructor after it. Unpatched main reports both windows. The fix retains only the genuine pre-eval window. Target JavaScript is parsed, never executed. ## Change and scope Separate cached origin from per-read validity. Retain possible write targets even when positive read identity is uncertain. Extend the existing shared ordering owner only for fresh declaration initializers; loops, deferred effects, outer bindings and logical assignments remain conservative. This is read admission for already-recognized Electron roots, not alias expansion. Related #6993 explicitly excludes the pre-existing direct-member/eval case; #2041's screened head retains that unguarded path. Their heads were rechecked unchanged before publication. No issue is claimed closed because this is a separately reproduced residual defect. ## Contract and boundary impact - Semantic owners: JavaScript binding identity, opaque effects and shared initializer ordering. - Scope decision: not applicable; routine correctness fix within existing static-analysis authority. - CLI/MCP schemas, catalog, provider/platform support and target selection: unchanged. - Evidence: suppress unsupported window identity while preserving genuine capture provenance. - No new target execution, injection, mutation, network or cleanup authority. - Dependencies, generated metadata, installation and release behavior: unchanged. Expected effect: read existing source files or captured data. Fixture code is never executed. ## Evidence and regression coverage - Baseline: **7 regression failures, 33 passing controls** before the production patch. - Coverage includes per-read cache validity, sloppy-eval shadowing, strict/indirect/optional/shadowed eval controls, constructor capture before argument evaluation, repeated activation/loops, deferred effects and possible write targets. - Compiled CLI and real stdio MCP both report **one BrowserWindow**, at the exact pre-eval source line (**line 3**), with **zero parse failures**. Independent automated source review approved all six changed files. This is not human review. ## Validation performed Tested base: `ab720b42163e33cba519518dedc67c7e11346b71`. - Seven focused domain files: **371 tests passed**. - Actual compiled CLI and stdio MCP acceptance: **2/2 passed**. - Full TypeScript check after all six files, `--noEmit --singleThreaded --checkers 1 --maxMemory 1G`: passed. - Production compilation, catalog recording and runtime bundling: passed. - Scoped typed lint: six files, **zero errors and 25 warnings**; formatting passed. Linux VM, Node 24.22.1 (repository pin), npm 11.9.0 (repository preference: 11.16.0). The source directory was reconstructed and hash-verified against the complete pinned superproject tree, not checked out as the original Git commit. All baseline blobs, directory trees, modes and five gitlinks were verified; submodule contents were not populated. Only the four intended baseline files changed, with two tests added. The lock-pinned oxlint-tsgolint backend was installed separately with scripts disabled and integrity verified; repository package/lock files and shared dependencies were not changed. ## Compatibility, safety, and release No breaking schema or migration, package or release changes. This is Linux static-analysis validation, not a running Electron/native-provider test. The aggregate `npm run check` and full test suite were not run locally. Process-family RSS observations are incomplete samples, not precise peak/bound or independent cleanup proof. No performance improvement is claimed. Published commit `4d2833d55ec0cf9c0702b0e19ba20c4de425beca` has the exact reviewed six-file content and independently calculated full tree `3985faea75713d258462a237dd5b6363fecdd225`. All six public blob hashes were verified. ## Author readiness Human owner: @WenyuChiou - [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. This is a draft pending human 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-11-03 13:39:05 +00:00
Sign in to join this conversation.
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#7610
No description provided.