Skip to content

fix(javascript): resolve module-level Electron options in functions - #8798

Open
ShuaixinHuang wants to merge 23 commits into
morluto:mainfrom
ShuaixinHuang:codex/fix-electron-module-options
Open

ShuaixinHuang wants to merge 23 commits into
morluto:mainfrom
ShuaixinHuang:codex/fix-electron-module-options

Conversation

@ShuaixinHuang

@ShuaixinHuang ShuaixinHuang commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Outer activation and prototype correction

Computed definition keys and class heritage retain their surrounding lexical-this owner; field values, static blocks and ordinary method/function bodies remain isolated. Tagged intrinsic invocations retain possible global activation. Array/function literal isolation now consults owner-specific prototype-effect bounds shared across reads; unrelated and later prototype effects retain their controls. Immutable statement indices are built once per owning body, including distinct functions.

Validation: four new early/late VM regressions initially failed (4 failed / 106 passed); final 238/238 tests pass across six complete files, including 40 real compiled CLI/stdio MCP cases, source-context controls, the unchanged import-cycle assertion and existing long-alias controls. The new distinct-function operation-count regression fails against the prior producer with 129 body indexOf calls, then passes an at-most-one bound while preserving 128 semantic results. This is an operation count, not a completed-workflow speed or memory measurement. check:fast 2/2 (62.206s); all 13 local files remain byte-exact after normal commit hooks. Exact-context application preserves upstream remote content and passes repository formatting. The complete integrated remote tree was not rerun locally; this new head requires its own hosted CI. Synthetic/inert JavaScript and local adapters do not establish real Electron or Linux/macOS behavior. Node 24.22.1 / existing npm 10.9.3; no dependency installation.

Effect-free receiver correction

When the completed receiver summary has no effects, preserve its ordinary-object identity without requiring a function/loop read position. Ordering is consulted only when an effect exists. This prevents unrelated deferred, repeating and aliased property reads from hiding proven Electron settings.

Validation: three new VM-backed controls failed before the fix; 171/171 tests across four files pass, including unchanged early-call protections, operation-count controls, cycle assertion and actual compiled CLI/stdio MCP fixtures with unrelated function/loop reads. check:fast 2/2 (36.243s); all three local files byte-exact after hooks. Exact-context patch preserves upstream contents; the complete integrated remote tree requires fresh hosted CI.

Aliased receiver and lexical-this correction

Propagate source-owned receiver effects through initializer aliases using one per-binding timing summary. Keep allocation identity separate from effect timing and retain uncertainty for cycles, deferred effects and ambiguous same-statement bounds. The shared source-context owner recognizes a possible classic-script top-level lexical this; modules, CommonJS wrappers and non-lexical function/class owners stay isolated.

Validation: three VM-backed early/late regressions failed before the fix. 293/293 tests across ten files now pass, including actual compiled CLI/stdio MCP aliases/computed constructors, unrelated-object controls, source parser, cycle and long-alias cases. check:fast 2/2 (30.512s); seven local files byte-exact after hooks. The 64/128 read-before-distinct-write regression counted 8192/32768 effect visits before, and passes bounds of 128/256 after. These are operation-count controls, not complete-workflow speed or memory measurements. Exact-context application preserved upstream remote contents and passed repository formatting; the complete integrated remote tree requires new hosted CI.

Constructor and receiver-effect timing correction

Normalize bound constructors and Reflect.construct through the shared intrinsic invocation owner. Receiver allocation identity stays cached separately from source-owned read-time effects; recompute module read bounds after the member-effect inventory completes. This retains early TDZ uncertainty for post-allocation Object.assign/setPrototypeOf while preserving unrelated own-method and nested-literal controls.

Validation: baseline four VM-backed early/late regressions failed; 252/252 tests across nine files now pass, including actual compiled CLI/stdio MCP cases, cycle check, identity and long-alias controls. check:fast 2/2 (31.93s), all eight local files byte-exact after hooks. Exact-context patch preserves integrated upstream contents; the complete remote integrated tree still needs fresh hosted CI.

Runtime-read and import-cycle correction

Pure copied-reference paths now have a separate owner, removing the Source/Portable test import cycle. Shared static property selection preserves unrelated nested receiver identity; erased TypeScript reads no longer activate module functions.

Validation: 231/231 tests across eight files including unchanged cycle checks and compiled CLI/stdio MCP; check:fast 2/2 (22.329s). All nine local files byte-exact after hooks. Two integrated upstream files preserve their remote contents with import-only migration. The full integrated remote tree was not locally rerun; fresh hosted CI is required.

Problem and outcome

Keep module-level Electron const options uncertain when a hoisted function can be read or activated before initialization. Use parser-owned runtime context, lexical direct-eval facts and conservative global-code bounds rather than Babel syntax alone.

Pattern property reads now come from the existing semanticPropertyReadSources owner, covering declarations, assignment destructuring and for-of patterns. The specialized declarator-only decoder is removed. Nested/iteration projections without a disjointness proof retain possible activation rather than proving global isolation. Ordinary unrelated receiver controls remain.

Immutable enclosing-function positions are cached with the existing parent-map facts. The shared source-read index, incremental opaque effects and per-binding receiver summaries remain; distinct initializer identities are preserved.

Validation and limits

  • Assignment-pattern reproduction against previous producer: 1 failed / 88 passed, with VM early-throw / late-success controls.
  • Function-position regression against previous producer: 1 failed / 29 passed; 64 captured reads repeated the same function's body lookup 64 times. The fixed semantic regression permits at most one such lookup and checks all 64 initializer results. No general speedup claim.
  • Final exact changed-file selection: 226 passed in seven complete files, including real compiled CLI/stdio MCP assignment/for-of cases and existing ESM/TypeScript, alias, opaque-effect and long-chain controls. check:fast: 2/2 passed in 56.193s.
  • All four committed file bytes match the checked files after hooks. Full integrated-tree validation was not repeated locally; current-head hosted CI must complete.
  • Current head: 5b8594a6c806d0fd7dc9386f38f3ea7ba47d7f5c; new hosted CI pending. Previous complete green runs cover earlier heads only. Human declarations remain intact, and no target Electron application execution is claimed.
Earlier validation records and scope limits

Runtime-context and lexical-eval corrections

Use the parser-owned runtime context for hoisted-function global activation: erased TypeScript imports do not establish module isolation. Include the already indexed, scope-owned direct-eval sites in initialization timing even in ESM/CommonJS; indirect eval and Function construction retain their separate global-visibility rules.

Receiver summaries now share a representative by binding identity while keeping initializer-expression identities separate. Repeated member reads reuse the same initializer summary instead of expanding the binding again.

Validation: previous producer fails both new P1 regressions (2 failed / 114 passed); the fixed final selection passes 217 tests in seven complete files, including compiled CLI/stdio MCP TypeScript and ESM fixtures, lexical early/late controls and prior identity/alias/opaque-effect cases. Independent Node ESM controls throw ReferenceError before initialization and return 1 after initialization. check:fast passes 2/2 in 29.693s.

The owned diagnostic for 64/128/256 receiver reads recorded 4160/16512/65792 initializer visits before and 65/129/257 after. This establishes removed repeated expansion; one warm in-process sample per size does not establish general end-to-end speed or memory improvement. Logs include wall time, CPU and cumulative process peak RSS. No probe instrumentation is committed.

Current head: 1016f6c9d433727e1cc3b2ef8dba1461458958bb; new hosted CI pending. All four changed files match the checked local bytes after hooks. Upstream's existing call-site presence assertion is preserved. Other checkout files remain from earlier integration, so complete current-tree validation still requires hosted CI. The ESM positive public control omits opaque eval because eval may mutate the options object even after initialization; the dedicated lexical-binding regression separately checks early versus late direct-eval ordering.

Problem and outcome

Keep hoisted Electron option reads uncertain when classic-script global code may activate the function before its captured const is initialized. The shared opaque-global observer now recognizes Function construction as well as eval through the existing global identity and intrinsic call owner, including new, aliases, call/apply and bound calls. Local shadowed constructors retain definite options.

Constructed code may escape, so construction supplies a conservative possible-activation bound. This does not assert that merely creating a function executed it; unused constructed functions can therefore retain uncertainty. No source strings or target Electron programs are executed by the analyzer.

The source timing index remains shared and incremental. Internal discovery facts now name global code rather than eval alone; call positions and retained effect facts include constructor nodes.

Validation and limits

  • Reproduced original constructor case: 1 failed / 79 passed, with inert VM early-throw / late-success controls.
  • Final exact changed files: 210 passed in seven complete files, including real compiled CLI/stdio MCP construction cases, aliases/call/apply/bind, a shadowed-constructor control, eval identity/mutation controls and existing long-alias coverage.
  • npm run check:fast: 2/2 passed in 43.954s. Precommit preserved all six checked files byte-for-byte.
  • Current head: 13f5340e8bfe199766d6af982ff4342520a11626. New hosted CI is pending; the previous head's full CI success does not cover this push.
  • The six published file contents match the checked local files and preserve upstream changes (including the previously removed opaque-effects facade export). Other local files remain from the earlier checkout; full integrated-tree verification relies on the new hosted run. No new real Electron/provider/platform claim.
Earlier evidence and verification limits

Problem and outcome

Hoisted Electron functions could treat module-level const options as initialized despite an earlier possible classic-script activation. Preserve uncertainty for early global aliases, destructuring, logical assignment, prototype/spread receivers and indirect eval; retain concrete values where initialization is actually ordered.

The shared initializer owner builds the source-read index once. It records possible call positions during that walk, then consumes newly observed opaque global calls incrementally. This keeps callee-resolution prewarming from freezing incomplete timing facts and avoids rebuilding the entire source index after each eval. Existing per-node receiver facts, unrelated-object controls and function-boundary rules remain.

Validation and limits

  • Previous producer: VM-backed cache-prewarming regression failed (1 failed / 77 passed). Other earlier reproduced cases and evidence remain below.
  • Current changed files: 127 tests passed across four complete files, including compiled CLI/stdio MCP, VM early/late controls, repeated opaque calls and the existing 6000-alias control. npm run check:fast passed 2/2 in 62.068s.
  • An isolated in-process diagnostic counted source-index builds for 64/128/256 eval-helper pairs: previously 64/128/256; now 1/1/1. Windows Node 24.22.1, one warm sample per size; instrumented only in the loader, no instrumentation committed. Logged wall time, CPU and cumulative process peak RSS are diagnostic observations, not an end-to-end performance or memory improvement claim.
  • The two published file blobs exactly match the locally checked files. Existing main integration and the value-equivalent Wrangler formatting correction remain. The complete integrated tree was not rerun locally; current-head hosted CI is required.
  • Current head: e4cf01fa4a9ccfa766c5ad9427be4fd0c40f60c3; new hosted CI pending. Existing human declarations are preserved. No target Electron application was executed, and no new real-platform/provider coverage is claimed.
Earlier validation records and execution limits

Current CI integration — 2026-11-03

Current head: e2e3576f238df648d9fd714b1425d0769a9be153. The first cache-fix run failed only the formatting gate: Static checks identifies the upstream website/wrangler.toml watch-directory array. The same exact blob (7c64886b3005bda755a580e36eb058d3f84d7755) exists on main d496d803a4f50dc82248db0630042894c1136d00. Integrated that main normally, preserving its GitHub-computed merge tree, then applied only the formatter's array wrapping. TOML parsing before/after produces identical values; the exact affected file fails formatting before and passes afterward. #7716 already contains the same incidental gate correction, so no duplicate standalone PR was opened.

The 126 tests and check:fast below cover the local cache correction before upstream integration. The full integrated tree has not been rerun locally; its new hosted CI is pending. No runtime/configuration values or test expectations were weakened.

Activation-cache lifetime correction — 2026-11-03

Callee resolution could populate module-function timing before the ambient observer had recorded a later indirect eval. That stale cache then incorrectly treated an early classic-script function call as occurring after const initialization. Cache entries now include the append-only opaque-global-site count and are rebuilt when that producing index grows; unchanged effect prefixes still reuse the shared timing facts.

Reproduction against the previous published producer: 1 failed / 77 passed, including a VM-backed early-throw / late-success control. Fixed validation: 126 passed in four complete files, including compiled CLI and stdio MCP coverage of this cache-prewarming case; npm run check:fast passed 2/2 in 31.009s. Existing unrelated-object, late-call and 6000-alias controls remain. No performance improvement or new real Electron runtime claim. Current head: c347f9ad462f812a128a514dc35954b370c3d10d; its new hosted CI is pending. Precommit only wrapped a condition; remote tree exactly matches the committed local tree.

Projected and opaque activation correction — 2026-11-03

Retain classic-script function reads through global destructuring, logical assignment, prototype/spread receivers and opaque global eval. Reuse the shared write-read and opaque-effects facts. Receiver aliases now use an iterative per-node postorder cache: each initializer retains its own answer, cycles stay uncertain, and shared alias prefixes are reused.

Previous production fails all five new VM-backed cases (5 failed / 72 passed). Fixed focused validation passes 123 tests in four complete files, including compiled CLI/stdio MCP; final option grouping passes 77 tests. The 6000-alias control now reads every intermediate receiver. check:fast passes 2/2 in 36.912s. Precommit removed one blank line only; production and assertions remain the tested bytes. No measured performance speedup is claimed. Fresh hosted CI is required for this HEAD.

Receiver identity follow-up — 2026-11-02

An alias with both an ordinary object and the global object as possible origins previously cached its combined answer on the ordinary object's initializer. A later unrelated method call then incorrectly made safely initialized Electron preferences dynamic. Cache the combined answer only on the queried receiver. Unknown computed global properties now share one earliest-read/uncertainty fact, applied to module functions once after the source scan, removing the repeated function scan per access.

The strengthened unrelated-object runtime control passes while the previous production code fails its static assertion (1 failed / 71 passed). Fixed four-file focused validation: 118 passed, including compiled CLI and stdio MCP; all 3 prerequisite tasks pass. check:fast: 2/2 passed, 45.104s. No measured performance speedup is claimed. This new HEAD requires fresh hosted CI.

Latest conservative activation correction

The previous correction still missed classic-script receiver projections, computed global keys and uncertain with-scope binding ownership. The shared first-activation owner now keeps these possible reads in its timing bound instead of treating unsupported identity as absence. Aliases use an iterative visited-node worklist with conservative cached may-alias results, so alias depth does not become JavaScript recursion depth. Callback statement lookup stops at intervening functions, including hoisted default-parameter callbacks. Existing unrelated object and shadowed-local controls remain.

Executed evidence: replacing only the production owner with the previous committed version causes 5 failures / 67 passes in the expanded option file; restoring the final owner yields 118 passing tests across options, initializer ordering, call resolution and actual compiled CLI/stdio MCP, plus 277 passing sibling binding/loader/fingerprint tests. check:fast passed 2/2 in 41.573s; diff check passed. A 6000-link flat alias fixture passes; it also passed on the previous version on this host, so stack exhaustion is source-supported rather than reproduced. No target source was executed by static analysis. Real Electron/platform coverage remains limited as recorded below.

Current HEAD: 507b50a97ca70bab1e25bbb3c201aff1097c522c. Hosted code CI for this new head is pending; previous green checks below belong to previous heads. Human declarations are preserved. No review thread was replied to or resolved.

Supersedes #8634 at the author's explicit request to resubmit all current PRs. Same published branch and HEAD 4cd804c. Original discussion and historical validation remain at #8634. This new PR requires its own hosted CI and new human declarations for this PR ID.

Current review correction

Current HEAD: 4cd804cbcc17dc76f02c27f327f577d2fc7b3665.

Reproduced the global-object-alias early-call case and the unrelated same-named parameter case (3 new cases failed on the prior head). The shared initializer timing owner now consumes existing lexical binding state: identifier reads match the actual function binding, and classic-script global receivers follow binding-owned initializer aliases with a cycle guard. All production callers now pass their existing analysis state rather than only its parent map; low-level parent-map-only tests retain their API. No target source is executed by analysis. Validation: 112 passed across initializer, call-resolution, Electron options and compiled CLI/stdio MCP files, plus 277 sibling binding/loader/fingerprint cases; check:fast 2/2 passed (43.349s). The VM controls confirm the early alias call reaches the temporal dead zone while the late control succeeds. These are synthetic source/VM and actual compiled CLI/MCP checks, not a real Electron application run.

The new HEAD requires its own hosted CI; earlier green runs below belong to earlier commits. Existing human declarations remain in force. The original replacement description and its execution points follow as historical context.

Supersedes #8495 at the author's explicit request to resubmit all current PRs. Same published branch and HEAD 9611e5cbe8d0d3639f87df529d2989e69a63d1f2. Original discussion and historical validation remain at #8495. This new PR requires its own hosted CI and a new human declaration for this PR ID; prior review declarations are historical.

Supersedes #8250 at the author's explicit request to resubmit the existing PRs. This replacement reuses the identical published head cc2bb1242a5611527bfb99935bc7dbcf85e902f7 and full three-file patch. Prior discussion and historical verification remain at #8250 (earlier #8087). No code changes or new local test runs were made for this resubmission. This new PR requires its own hosted CI. The owner has specifically confirmed review of this replacement PR's final diff, evidence, limitations and unresolved P1, accepted full responsibility, and approved it for initial maintainer review. The P1 remains a before-merge blocker.

Outstanding review follow-up — addressed in the current published revision

The known P1 review finding was addressed in the current head. In a classic script, an early globalThis.createWindow() followed by a later bare call can make module-constant initialization appear definite even though the early activation encounters the temporal dead zone. The previously reported 115-test verification below does not establish that this additional case is fixed.

A separate local draft follow-up has 67 focused tests and three build tasks passing, plus check:fast; expanded verification and commit/push are not complete. That draft is not part of this PR's published commit or diff, and those draft results are not claimed as a fix in this revision. The prior finding is covered by the current-head regression; hosted CI and maintainer review still apply; the original review is preserved rather than marked resolved.

Supersedes #8087 at the author's request. This replacement reuses verified head cc2bb12 and preserves the original discussion and validation record at #8087. Historical revision-specific validation below remains at its stated execution points. This new PR must pass its own hosted CI.

Problem and resulting behavior

Fixes #7227. Electron static analysis loses module-level immutable webPreferences read 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:

const { BrowserWindow } = require("electron");
createWindow();
const webPreferences = { nodeIntegration: true };
function createWindow() {
  return new BrowserWindow({ webPreferences });
}

Moving the call after the const initializer yields object-literal with 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:

  • Exact preceding head 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.
  • Inert Node VM controls reproduce a temporal-dead-zone ReferenceError for the three early calls. They substitute a stub BrowserWindow and do not run Electron.
  • Final committed follow-up 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.
  • Both public adapters report dynamic/empty preferences for the early-call source and object-literal/literal true for the late-call source. Source ranges distinguish the two files. Fixtures remain inert.
  • Test discovery passed: all 2,866 test files discovered exactly once. Normal precommit passed; typecheck/lint passed 2/2. Normal prepush typecheck/lint passed 2/2, 26.986 seconds; the normal fast-forward push to this PR's existing fork branch completed.

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 cc2bb1242a5611527bfb99935bc7dbcf85e902f7 keeps 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 9bafc838 own hosted workflow 41009833080 completed successfully; the new cc2bb124 head 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

  • 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. The known early-global-call P1 below still needs its published regression/fix.
  • 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.

Current-head P1 correction

Commit 9611e5cbe8d0d3639f87df529d2989e69a63d1f2 covers the classic-script global activation case. Focused domain and CLI/stdio MCP validation passed 67/67 tests; check:fast passed 2/2. The local regression covers early and late globalThis/window/self/this activations without expanding call-graph or authority semantics.

This replacement preserves the prior PR discussion and validation history at #8331. It reuses the verified published head and requires its own hosted CI.

Supersedes #8749.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-03T16:07:52.435547Z 2d32d98 New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@greptile-apps

greptile-apps Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 4/5 Tier: plus

[Medium impact] The changes affect JavaScript source analysis and its shared callers.

Fix the unrelated-write regression before merging so known Electron settings remain available.

Findings

  1. P1 Unrelated writes hide Electron settings ▶
Fix with agent prompt
### Issue 1
src/domain/javascript/javascriptSemanticMutationInitializers.ts:1138-1139
`prototypeBounds` treats every `Object` entry as a possible change to array and function inheritance. But the producer also records harmless static writes such as `Object.extra = 1` under that owner.

Put that write and `function unused() { [].createWindow; }` before the `webPreferences` initializer, then call `createWindow()` afterward. The unrelated read now makes initialization uncertain, so proven Electron settings become `dynamic` in CLI and MCP results.

Keep static constructor writes separate from changes to inherited properties before using these sites to discard receiver isolation.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

The PR resolves initialized module-level Electron options inside functions while preserving uncertainty for possible early calls.

  • Electron options stay literal only when a function cannot read them too early.
  • Copied reference paths now live in a separate helper.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Captured module constant] --> B[Shared initialization check]
  B --> C[Function read timing]
  C --> D[Lexical reads and property reads]
  C --> E[Eval and Function sites]
  D --> F[Receiver identity and effect timing]
  F --> G{Initialization precedes possible use?}
  E --> G
  G -->|Yes| H[Keep constant options]
  G -->|No or unknown| I[Keep options dynamic]
Loading

Reviews (2) · Last reviewed commit: "fix(javascript): retain outer activation..." · Reviewed by Greptile

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9da5935aff

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +296 to +299
(t.isFunction(current) && !t.isArrowFunctionExpression(current)) ||
t.isClass(current)
)
return false;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve enclosing this in definition-time computed expressions

In a classic script with a hoisted createWindow that captures webPreferences, placing ({ [this.eval('createWindow()')]() {} }); before the preferences initializer invokes the function early: computed method keys use the enclosing script's this. This ancestor check nevertheless stops at the ObjectMethod and similarly rejects class keys and heritage expressions, so createAmbientPathResolver never records that eval. The new activation check can then accept a later bare call as evidence of definite initialization and report literal Electron preferences despite the early temporal-dead-zone read. Reuse the existing computed-key evaluation-context facts when deciding this ownership.

AGENTS.md reference: AGENTS.md:L19-L19

Useful? React with 👍 / 👎.

Comment on lines +1008 to +1010
!t.isArrayExpression(node) &&
!t.isLiteral(node) &&
!t.isFunction(node)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Check prototype effects before excluding literal receivers

In a classic script, Object.setPrototypeOf(Array.prototype, globalThis); [].createWindow(); before the preferences declaration reaches the global hoisted function and reads its captured webPreferences before initialization. These exclusions classify the literal receiver as isolated without consulting ambient prototype mutations, so the early activation is skipped and a later bare createWindow() makes hasDefiniteInitializer true. Function literals have the same problem after changing Function.prototype. Consult the shared prototype/mutation facts before excluding these receivers; their local identity does not prove that lookup cannot reach the global function. Otherwise, the analyzer can report literal preferences for an uninitialized read.

AGENTS.md reference: AGENTS.md:L19-L19

Useful? React with 👍 / 👎.

Comment on lines +1138 to +1139
for (const effect of state?.prototypeMutationSites ?? []) {
if (effect.owner !== owner && effect.owner !== "Object") continue;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Unrelated writes hide Electron settings

prototypeBounds treats every Object entry as a possible change to array and function inheritance. But the producer also records harmless static writes such as Object.extra = 1 under that owner.

Put that write and function unused() { [].createWindow; } before the webPreferences initializer, then call createWindow() afterward. The unrelated read now makes initialization uncertain, so proven Electron settings become dynamic in CLI and MCP results.

Keep static constructor writes separate from changes to inherited properties before using these sites to discard receiver isolation.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/domain/javascript/javascriptSemanticMutationInitializers.ts
Line: 1138-1139

Comment:
**Unrelated writes hide Electron settings**

`prototypeBounds` treats every `Object` entry as a possible change to array and function inheritance. But the producer also records harmless static writes such as `Object.extra = 1` under that owner.

Put that write and `function unused() { [].createWindow; }` before the `webPreferences` initializer, then call `createWindow()` afterward. The unrelated read now makes initialization uncertain, so proven Electron settings become `dynamic` in CLI and MCP results.

Keep static constructor writes separate from changes to inherited properties before using these sites to discard receiver isolation.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2d32d9804c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

opaquePositions.set(node, active.size > 0 ? null : index);
}
if (classicScript) {
for (const source of semanticPropertyReadSources(node, parent)) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Index implicit global reads before proving initialization

In a classic script with a hoisted createWindow that captures webPreferences, both Reflect.get(globalThis, 'createWindow')() and try { throw globalThis; } catch ({ createWindow: start }) { start(); } can activate the function before the preferences declaration. Neither operation produces a read in semanticPropertyReadSources, and the remaining name scan does not associate get or start with createWindow. If a later bare createWindow() also exists, this index retains only that later read and incorrectly establishes hasDefiniteInitializer: true, despite the early invocation reading the captured const in its temporal dead zone. Extend the source-owned read facts to cover these implicit property reads, reusing the catch/throw projections already collected by collectDefinitionsSteps, or retain unknown timing rather than admitting literal Electron preferences.

AGENTS.md reference: AGENTS.md:L19-L19

Useful? React with 👍 / 👎.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Module-level webPreferences constants are reported as dynamic when the window is created inside a function

1 participant