morluto/rea

Fork 0

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

[GH-ISSUE #2648] fix(javascript): unconditional loop initializers hide Electron constructors #950

New issue

Closed

opened 2026-10-15 18:29:50 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-15 18:29:50 +00:00

Owner

Originally created by @coygeek on GitHub (Oct 13, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/2648

Summary

Static JavaScript source analysis omits a BrowserWindow construction when its Electron module binding has its single initializer in a top-level for initializer or the first statement of a do…while(false) body. The equivalent plain var declaration recovers the construction. Literal preload options are also lost with the omitted finding. This affects the existing static lexical identity recovery rule for unconditional initializers.

Steps to reproduce

Use the source checkout and locked dependencies for the baseline below. Save this focused test as src/domain/javascript/electronLoopInitializer.repro.test.ts. The source strings are inert input to the analyzer; they must not be evaluated as application code. These are the smaller constructor inputs observed during local verification after removing the preload options.

import { expect, it } from "vitest";
import { analyzeJavaScriptStaticSource } from "./javascriptStaticAnalysis.js";

const declarations = [
  ["plain", 'var electron = require("electron");'],
  ["for-initializer", 'for (var electron = require("electron"); false;) {}'],
  ["do-once", 'do {var electron = require("electron");} while(false);'],
] as const;

for (const [name, declaration] of declarations) {
  it(name, () => {
    const result = analyzeJavaScriptStaticSource(
      declaration + 'new electron.BrowserWindow({});',
    );
    console.log(JSON.stringify({
      name,
      parse_errors: result.parse_error_count,
      windows: result.electron.browser_windows.length,
    }));
    expect(result.parse_error_count).toBe(0);
    expect(result.electron.browser_windows).toHaveLength(1);
  });
}

Run the repository source-test workflow:

npm run test:local -- src/domain/javascript/electronLoopInitializer.repro.test.ts

Local verification used the same producer, source inputs and assertions through a serialized focused-test launcher. The ordinary command above is the portable rerun procedure; it was not separately executed during verification.

Expected behavior

Recover one static BrowserWindow finding for each of the three sources. Each binding has one literal require("electron") initializer, no reassignment, no shadowed require, and no namespace overwrite. The for initializer precedes the first loop test, and the supplied do body executes its declaration before its false test. Their initialization cannot be skipped in these top-level forms. This is the existing stable unconditional-initializer case; it does not require proving runtime execution or exhaustive recovery of arbitrary Electron code.

With the constructor options replaced by {webPreferences:{preload:"./preload.js"}}, retain preload_path: "./preload.js" in the finding, as the plain declaration does.

Actual behavior

The locally executed smaller inputs produced these counts, with zero parser errors in every case:

plain:                  windows=1
for-initializer:        windows=0
do-once:                windows=0

Both loop assertions failed with expected [] to have a length of 1 but got +0. The variants with literal preload options produced ["./preload.js"] for the plain declaration and [] for both loop forms. All three full-option inputs reported parse_status: "complete" and retained the ordinary limitation that missing findings do not establish absence. That parse status is not used as an exhaustive-recovery guarantee here.

In the complete focused run, nine tests executed: four positive assertions failed and five controls passed. The skipped for body, skipped while body, and an outer-condition-controlled for initializer remained excluded, as expected. The other two passing controls were the plain declaration with and without preload options.

Evidence

Expected source: docs/javascript-artifact-reconstruction.md:216-228 documents static recovery of explicit BrowserWindow options and resolvable preload entrypoints; its lexical module-alias and single-unconditional-initializer rules are at lines 328-370. src/domain/javascript/javascriptSemanticState.ts:87-108 defines a stable initializer as one unconditional source initializer, without proving execution at a particular program point.
Failure source: The focused source test directly observed the window and preload omissions above. src/domain/javascript/javascriptSemanticAnalysis.ts:644-651 marks the initializer using the ancestor test at lines 723-739, which treats any loop ancestor as conditional without distinguishing the supplied initializer positions. The marker is rejected at src/domain/javascript/javascriptSemanticState.ts:106 and the Electron identity lookup consequently fails at src/domain/javascript/javascriptSemanticAnalysis.ts:489-530.
Evidence provenance: observed
Local verification: reproduced
Reproduction completeness: complete

The primary violation evidence comes from the focused analyzer source test. The explanation of the guard chain and downstream consumer behavior is source-derived.

The executed source-analyzer tests establish the missing constructor findings and preload paths. Application summary effects are source-derived: the included-source entry point is src/application/javascript/JavaScriptArtifactAnalysis.ts:350-380, constructor recovery is at src/domain/javascript/electronStaticAnalysisBrowser.ts:43-51, and the summary-count consumer is src/application/javascript/ElectronBoundaryAnalysis.ts:86-126. Application-service execution and public CLI/MCP transport were not exercised for this report.

Restoration check

Both supplied loop forms must recover the same single constructor as the plain declaration, including the literal preload path when those options are present. Keep the plain declaration control passing. Preserve exclusion for for (;false;) {var electron = require("electron");}, while(false) {var electron = require("electron");}, and if(flag) {for (var electron = require("electron"); false;) {}}, each followed by the same constructor. These controls distinguish unconditional initializer recovery from accepting declarations that can be skipped. Retain the existing static-analysis limitations and leave implementation choices open.

Runtime or environment

Verified source revision: a2d8b94dd50dab8d966239dd63ab84718aa8b4b4. Source tests used Node 24.18.0, npm 11.16.0 and the repository's locked Vitest 5.0.3. The published package was not executed. No Electron process or application source evaluation was needed.

Current-upstream source recheck at commit 3ae826ceba found the loop-initializer classification, Electron identity guards, static constructor recovery and findings-to-summary path unchanged. New source-map resolution changes do not affect these direct-source fixtures. Runtime reproduction was not rerun at this revision.

Additional context

The earlier Electron alias-recovery report concerns API identity propagated through a second local binding. It is related background. The current reproduction has a direct literal module initializer, and moving that same initializer outside the loop restores recovery in the observed controls. Its closed state does not establish that these loop-input omissions are fixed.

Related: #2562 concerns incorrect definite values when initialization can be skipped, while this report concerns missing module identity when initialization is unconditional. #1802 concerns namespace and interop constructor recovery; the plain namespace control already passes in this report. Those reports have different restoration checks.

Triage assessment

Impact level: P2
Assessment status: supported
Impact basis: Two bounded, valid source forms lose static constructor findings and literal preload paths while the equivalent plain declaration succeeds. The source consumer derives summary counts from those findings. No application runtime failure, lost artifact data or universal recovery failure was observed.
Workaround status: unverified
Workaround basis: The observed plain-declaration control shows that moving the initializer outside the loop restores this fixture's result. Rewriting a separate analysis copy of a shipped artifact would change its bytes and identity; whether that is a practical way to preserve the caller's required evidence was not verified.

Originally created by @coygeek on GitHub (Oct 13, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/2648 ## Summary Static JavaScript source analysis omits a BrowserWindow construction when its Electron module binding has its single initializer in a top-level `for` initializer or the first statement of a `do…while(false)` body. The equivalent plain `var` declaration recovers the construction. Literal preload options are also lost with the omitted finding. This affects the existing static lexical identity recovery rule for unconditional initializers. ## Steps to reproduce Use the source checkout and locked dependencies for the baseline below. Save this focused test as `src/domain/javascript/electronLoopInitializer.repro.test.ts`. The source strings are inert input to the analyzer; they must not be evaluated as application code. These are the smaller constructor inputs observed during local verification after removing the preload options. ```ts import { expect, it } from "vitest"; import { analyzeJavaScriptStaticSource } from "./javascriptStaticAnalysis.js"; const declarations = [ ["plain", 'var electron = require("electron");'], ["for-initializer", 'for (var electron = require("electron"); false;) {}'], ["do-once", 'do {var electron = require("electron");} while(false);'], ] as const; for (const [name, declaration] of declarations) { it(name, () => { const result = analyzeJavaScriptStaticSource( declaration + 'new electron.BrowserWindow({});', ); console.log(JSON.stringify({ name, parse_errors: result.parse_error_count, windows: result.electron.browser_windows.length, })); expect(result.parse_error_count).toBe(0); expect(result.electron.browser_windows).toHaveLength(1); }); } ``` Run the repository source-test workflow: ```sh npm run test:local -- src/domain/javascript/electronLoopInitializer.repro.test.ts ``` Local verification used the same producer, source inputs and assertions through a serialized focused-test launcher. The ordinary command above is the portable rerun procedure; it was not separately executed during verification. ## Expected behavior Recover one static BrowserWindow finding for each of the three sources. Each binding has one literal `require("electron")` initializer, no reassignment, no shadowed `require`, and no namespace overwrite. The `for` initializer precedes the first loop test, and the supplied `do` body executes its declaration before its false test. Their initialization cannot be skipped in these top-level forms. This is the existing stable unconditional-initializer case; it does not require proving runtime execution or exhaustive recovery of arbitrary Electron code. With the constructor options replaced by `{webPreferences:{preload:"./preload.js"}}`, retain `preload_path: "./preload.js"` in the finding, as the plain declaration does. ## Actual behavior The locally executed smaller inputs produced these counts, with zero parser errors in every case: ```text plain: windows=1 for-initializer: windows=0 do-once: windows=0 ``` Both loop assertions failed with `expected [] to have a length of 1 but got +0`. The variants with literal preload options produced `["./preload.js"]` for the plain declaration and `[]` for both loop forms. All three full-option inputs reported `parse_status: "complete"` and retained the ordinary limitation that missing findings do not establish absence. That parse status is not used as an exhaustive-recovery guarantee here. In the complete focused run, nine tests executed: four positive assertions failed and five controls passed. The skipped `for` body, skipped `while` body, and an outer-condition-controlled `for` initializer remained excluded, as expected. The other two passing controls were the plain declaration with and without preload options. ## Evidence - Expected source: `docs/javascript-artifact-reconstruction.md:216-228` documents static recovery of explicit BrowserWindow options and resolvable preload entrypoints; its lexical module-alias and single-unconditional-initializer rules are at lines 328-370. `src/domain/javascript/javascriptSemanticState.ts:87-108` defines a stable initializer as one unconditional source initializer, without proving execution at a particular program point. - Failure source: The focused source test directly observed the window and preload omissions above. `src/domain/javascript/javascriptSemanticAnalysis.ts:644-651` marks the initializer using the ancestor test at lines 723-739, which treats any loop ancestor as conditional without distinguishing the supplied initializer positions. The marker is rejected at `src/domain/javascript/javascriptSemanticState.ts:106` and the Electron identity lookup consequently fails at `src/domain/javascript/javascriptSemanticAnalysis.ts:489-530`. - Evidence provenance: observed - Local verification: reproduced - Reproduction completeness: complete The primary violation evidence comes from the focused analyzer source test. The explanation of the guard chain and downstream consumer behavior is source-derived. The executed source-analyzer tests establish the missing constructor findings and preload paths. Application summary effects are source-derived: the included-source entry point is `src/application/javascript/JavaScriptArtifactAnalysis.ts:350-380`, constructor recovery is at `src/domain/javascript/electronStaticAnalysisBrowser.ts:43-51`, and the summary-count consumer is `src/application/javascript/ElectronBoundaryAnalysis.ts:86-126`. Application-service execution and public CLI/MCP transport were not exercised for this report. ## Restoration check Both supplied loop forms must recover the same single constructor as the plain declaration, including the literal preload path when those options are present. Keep the plain declaration control passing. Preserve exclusion for `for (;false;) {var electron = require("electron");}`, `while(false) {var electron = require("electron");}`, and `if(flag) {for (var electron = require("electron"); false;) {}}`, each followed by the same constructor. These controls distinguish unconditional initializer recovery from accepting declarations that can be skipped. Retain the existing static-analysis limitations and leave implementation choices open. ## Runtime or environment Verified source revision: `a2d8b94dd50dab8d966239dd63ab84718aa8b4b4`. Source tests used Node 24.18.0, npm 11.16.0 and the repository's locked Vitest 5.0.3. The published package was not executed. No Electron process or application source evaluation was needed. Current-upstream source recheck at commit 3ae826ceba6f25b08bce50bcb51001209bc16b57 found the loop-initializer classification, Electron identity guards, static constructor recovery and findings-to-summary path unchanged. New source-map resolution changes do not affect these direct-source fixtures. Runtime reproduction was not rerun at this revision. ## Additional context The earlier [Electron alias-recovery report](https://github.com/morluto/rea/issues/1656) concerns API identity propagated through a second local binding. It is related background. The current reproduction has a direct literal module initializer, and moving that same initializer outside the loop restores recovery in the observed controls. Its closed state does not establish that these loop-input omissions are fixed. Related: #2562 concerns incorrect definite values when initialization can be skipped, while this report concerns missing module identity when initialization is unconditional. #1802 concerns namespace and interop constructor recovery; the plain namespace control already passes in this report. Those reports have different restoration checks. ## Triage assessment - Impact level: P2 - Assessment status: supported - Impact basis: Two bounded, valid source forms lose static constructor findings and literal preload paths while the equivalent plain declaration succeeds. The source consumer derives summary counts from those findings. No application runtime failure, lost artifact data or universal recovery failure was observed. - Workaround status: unverified - Workaround basis: The observed plain-declaration control shows that moving the initializer outside the loop restores this fixture's result. Rewriting a separate analysis copy of a shipped artifact would change its bytes and identity; whether that is a practical way to preserve the caller's required evidence was not verified.

bsakaguchi

2026-10-15 18:29:50 +00:00

closed this issue
added the
area: runtime

bug
labels

bsakaguchi commented

2026-10-15 18:29:51 +00:00

Author

Owner

@morluto commented on GitHub (Oct 14, 2026):

Addressed on main in 896696b2e761e34c267977bb5c6982c607a622d0.

Unconditional for initializers and do-while bodies retain definite binding edges, while enclosing conditionality remains preserved.

Validation: Affected suites passed in the merged 65-file run (1169 passed, 2 explicit skips), with full static/architecture checks and documentation checks passing. Final installed-package verification is recorded separately in the audit.

Verification scope: Controlled source and production-boundary verification on macOS with pinned Node 24.18.0; real external engines and Windows hosts were not independently rerun. Closure covers the described source defect, not broader provider conformance or performance.

@morluto commented on GitHub (Oct 14, 2026): Addressed on main in [896696b2e761e34c267977bb5c6982c607a622d0](https://github.com/morluto/rea/commit/896696b2e761e34c267977bb5c6982c607a622d0). Unconditional for initializers and do-while bodies retain definite binding edges, while enclosing conditionality remains preserved. Validation: Affected suites passed in the merged 65-file run (1169 passed, 2 explicit skips), with full static/architecture checks and documentation checks passing. Final installed-package verification is recorded separately in the audit. Verification scope: Controlled source and production-boundary verification on macOS with pinned Node 24.18.0; real external engines and Windows hosts were not independently rerun. Closure covers the described source defect, not broader provider conformance or performance.

bsakaguchi referenced this issue

2026-10-17 21:07:11 +00:00

[PR #950] [MERGED] fix(ida): share concurrent session cleanup #2527

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

No description provided.

Rows
Columns