Repository navigation
fix(javascript): resolve module-level Electron options in functions - #8087
ShuaixinHuang wants to merge 5 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
[Medium impact] The PR appears safe to merge; the loop regression is fixed without dropping the early-call guard. SummaryThis PR lets Electron analysis retain module-level
Diagram%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Read a module-level const] --> B[Find the containing function]
B --> C[Check initialization before activation]
C -->|Proven| D[Retain initialization fact]
C -->|Unproven| E[Keep options dynamic]
D --> F[Keep capture ordering unresolved]
F --> G[Electron checks object mutations]
Reviews (5) · Last reviewed commit: "fix(javascript): retain function ownersh..." · Reviewed by Greptile |
Superseded by #8250 at the author's request: #8250 . The replacement uses the same verified head commit; this PR retains the original discussion and validation record.
Problem and resulting behavior
Fixes #7227. Electron static analysis loses module-level immutable
webPreferencesread inside a later activation. Retain those settings when initialization precedes the function's module-level uses, while preserving uncertainty when a hoisted function can run earlier.For example, this source must remain dynamic rather than report a proven
nodeIntegration: true:Moving the call after the const initializer yields
object-literalwith the literal setting, including when the function declaration appears before the initializer.Owning change and scope
Initialization coverage stays in
javascriptSemanticMutationInitializers.ts; Electron consumers use that fact without reinterpreting declaration order. A hoisted declaration's position does not prove activation timing. The owner indexes module-level function references once per immutable module body and treats earlier aliases, indirect activations, live exports and unproven timing conservatively. This is a bounded reference-order check, not a general call graph or runtime execution proof.The existing ancestry cache also retains the enclosing function, avoiding repeated walks from deep reads. Initialization coverage remains separate from cross-function capture ordering: the latter stays unresolved so named child-process argv shapes retain their existing uncertainty. No public schema, provider, execution authority, deadline, resource ceiling or assertion was weakened.
Validation at the current follow-up
Windows, Node 24.22.1 / npm 11.16.0:
fb2532ad61ceb4cce7d233131fc1edd4305925ff, with the new source regressions: 4 failed / 47 passed. Three cases expose early direct, aliased and forwarding calls; the fourth exposes a late-call positive control.9bafc838629358fb8ebc0ce86abefa5c82c8a0ab: 112/112 passed across six complete files: Electron options 51, existing scope-depth/resource controls 14, binding initializers 13, argument shapes 30, existing child-process compiled CLI/stdio MCP 2, and new Electron initialization compiled CLI/stdio MCP 2. Three build prerequisite tasks passed.The earlier head's independent hosted CI passed, but it did not cover this hoisted-declaration review case. The new head requires its own hosted CI; an earlier green run does not validate it. Local checks use synthetic JavaScript through production static analysis and real local compiled CLI/stdio MCP. No real Electron application or Linux/macOS execution is claimed.
Loop ownership review follow-up
Commit
cc2bb1242a5611527bfb99935bc7dbcf85e902f7keeps enclosing-function identity in its own immutable ancestry cache. Loop ordering remains uncertain; entering a loop no longer erases which function owns the read. This retains initialized module-level Electron settings inside for-of, for and while loops while keeping early calls dynamic. No downstream reinterpretation or call graph expansion was added.On exact preceding head
9bafc838, the three new loop source cases fail while the existing 51 pass. With the correction, six complete files pass 115/115 (Electron 54, scope-depth 14, initializers 13, shapes 30, existing child-process CLI/MCP 2, Electron initialization CLI/MCP 2). The public adapter cases include both early and late calls inside loops and outside loops, with source provenance. All three committed files match the tested bytes exactly after normal precommit hooks. Three build tasks passed; normal prepush typecheck/lint passed 2/2, 46.166 seconds. No assertions or resource controls were relaxed.The earlier
9bafc838own hosted workflow41009833080completed successfully; the newcc2bb124head requires independent hosted validation. Validation uses synthetic inert JavaScript and real local compiled CLI/stdio MCP on Windows, Node 24.22.1 / npm 11.16.0; no real Electron application or Linux/macOS run is claimed.Author readiness
Human owner: @ShuaixinHuang
Before merge
type(scope): outcome.