[PR #2041] fix(javascript): preserve builtin effects at their read sites #4982

New issue

Open

opened 2026-10-31 00:57:39 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 00:57:39 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2041
Author: @cyanseek
Created: 10/12/2026
Status: 🔄 Open

Base: main ← Head: codex/commonjs-builtin-effects

📝 Commits (10+)

9ba8498 fix(javascript): recover CommonJS builtin effects
37cf4bd fix(javascript): exclude mutated CommonJS builtin effects
aca5214 chore: merge main with synchronized documentation
0b46ef5 Merge remote-tracking branch 'origin/main' into codex/commonjs-builtin-effects
a67cf2b fix(javascript): retain builtin identity and effect ordering
7afb0e9 Merge remote-tracking branch 'origin/main' into codex/commonjs-builtin-effects
54acace fix(javascript): preserve builtin method capture ordering
4ff3e45 fix(javascript): preserve loader and builtin object identities
d141dea test(release): select the runnable preflight job
659b676 fix(javascript): share require imports and retain escaped callables

📊 Changes

47 files changed (+6391 additions, -322 deletions)

View changed files

📝 docs/javascript-application-workflows.md (+9 -0)
📝 src/android/JadxDoctor.ts (+4 -2)
📝 src/application/WebSourceLocationPorts.ts (+4 -2)
📝 src/application/javascript/JavaScriptSemanticGraphConstruction.ts (+2 -2)
📝 src/browser/PlaywrightBrowserScenarioProvider.test.ts (+20 -8)
📝 src/domain/javascript/electronModuleInterop.test.ts (+23 -0)
📝 src/domain/javascript/javascriptApplicationAnalysis.ts (+1 -2)
📝 src/domain/javascript/javascriptAstValues.ts (+84 -0)
➕ src/domain/javascript/javascriptCallableProperties.fixture.ts (+129 -0)
➕ src/domain/javascript/javascriptCommonJsExposure.fixture.ts (+48 -0)
📝 src/domain/javascript/javascriptModuleInterop.ts (+424 -61)
➕ src/domain/javascript/javascriptNamespaceReceiver.fixture.ts (+49 -0)
➕ src/domain/javascript/javascriptOpaqueMutators.fixture.ts (+218 -0)
📝 src/domain/javascript/javascriptRuntimeReconciliationSchemas.ts (+1 -2)
➕ src/domain/javascript/javascriptSemanticAnalysis.builtin_review.test.ts (+2010 -0)
📝 src/domain/javascript/javascriptSemanticAnalysis.empty_keys.test.ts (+7 -0)
📝 src/domain/javascript/javascriptSemanticAnalysis.recoveredArguments.test.ts (+2 -0)
📝 src/domain/javascript/javascriptSemanticDataEffectHelpers.ts (+229 -54)
➕ src/domain/javascript/javascriptSemanticEffectQueries.ts (+32 -0)
📝 src/domain/javascript/javascriptSemanticEvaluationOrder.ts (+55 -1)

...and 27 more files

📄 Description

Summary

Preserve Node built-in effects at their actual reads and captures after initial CommonJS support landed on main. Earlier native reads survive later reassignment; replacing the loader, a built-in method or a shared object prevents subsequent calls from being reported as native effects.

Change and scope

Share binding, alias, capture and effect-order analysis across loader provenance, saved methods and Object/Reflect intrinsics. Preserve separate receivers and captured object identities; read descriptor fields when their definition is invoked.
Track computed-key evaluation and coercion independently of scalar results, including nested/optional members, updates and compound assignments under typeof. Opaque changes invalidate ordinary property selections and Reflect own-data/null-prototype proofs at the relevant read.
Follow module exposure through descriptors, copied values, bound inputs, callable returns, await and yield. Keep inherited descriptor/then checks tied to each consumer. Rejected or skipped replacements cannot establish fresh allocation identity.
Select overlapping writes and escapes through a property-path index before applying the existing timing checks. Exact keys avoid rescanning unrelated writes; ancestor, descendant, wildcard and copied-property paths remain conservative.
Integrate main's current accessor, invocation-context, repeated-activation and cache owners. Preserve filesystem and synchronous child-process receiver links, TLS/net method distinctions and conservative argv selections.

Owner: JavaScript domain provenance and mutation analysis. No CLI/MCP schema, Evidence identity, dependency or authority change. Analyzed JavaScript is parsed, not executed.

Read existing files or captured data.

Validation performed

Commit d1cf35334, incorporating main 69bf61f7c, on Node 24.22.1.

Linux: 5,881 passing checks across 194 files, with one configured skip, covering the complete JavaScript domain, JavaScript adapters, public CLI/MCP, returned-reference cycles, semantic graph and web bundle consumers, browser worker/scenario integration, JEB configuration, JSON output ownership, Hermes setup and ADB/capture cleanup. The initial pass exposed duplicate setup in the merged JEB fixture; its complete three-case file passes after reusing the prepared environment.
All 939 builtin-review cases pass. 354 isolated native Node controls pass: the 322 existing controls plus 32 computed-key, coercion, opaque-object/Reflect and descriptor-timing cases. ESM namespace controls use imported values consistently with main's module model.
The seven public CLI/MCP cases pass on Windows and Linux. Complete analysis Evidence agrees across both adapters. The combined regression corpus explicitly provisions matching 16 MiB server/client response buffers under main's current transport budget.
The previously failing JSON output ownership suite passes all 15 cases after integrating main's existing cancellation/cleanup fixes (#5601 and #5852).
Build, full Linux check:ci, Windows typecheck/lint/format/module-boundary checks, generated-document checks and git diff --check pass. Hosted checks run against the new head after this push.

Coverage is static provenance and the affected public consumers; it does not establish observed target execution. Native controls execute isolated test snippets only. Complex descriptor histories without proven flags remain conservative.

Author readiness

Human owner: @cyanseek

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 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/2041 **Author:** [@cyanseek](https://github.com/cyanseek) **Created:** 10/12/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `codex/commonjs-builtin-effects` --- ### 📝 Commits (10+) - [`9ba8498`](https://github.com/morluto/rea/commit/9ba8498bf352e6b06d7411a858a73e862f415832) fix(javascript): recover CommonJS builtin effects - [`37cf4bd`](https://github.com/morluto/rea/commit/37cf4bd1014dcb2f3576257efb4a3a1c1027e96e) fix(javascript): exclude mutated CommonJS builtin effects - [`aca5214`](https://github.com/morluto/rea/commit/aca5214e4f0b4bc73ed61c7e813bc6ee8e504964) chore: merge main with synchronized documentation - [`0b46ef5`](https://github.com/morluto/rea/commit/0b46ef55453b7d7b806089ea885d783a41e7a29f) Merge remote-tracking branch 'origin/main' into codex/commonjs-builtin-effects - [`a67cf2b`](https://github.com/morluto/rea/commit/a67cf2bacdd1ace19312e06d4d86ca2d1b89de69) fix(javascript): retain builtin identity and effect ordering - [`7afb0e9`](https://github.com/morluto/rea/commit/7afb0e94e97faef293bee0ab078652c4ea9329f9) Merge remote-tracking branch 'origin/main' into codex/commonjs-builtin-effects - [`54acace`](https://github.com/morluto/rea/commit/54acace2cc6bb08ebfc92a75dd9b94a4c76ebc0d) fix(javascript): preserve builtin method capture ordering - [`4ff3e45`](https://github.com/morluto/rea/commit/4ff3e459bfd1e2391ca0d8cb86d1d2bfdbbf7f20) fix(javascript): preserve loader and builtin object identities - [`d141dea`](https://github.com/morluto/rea/commit/d141dea3085803649d90dfb01030d4b01101d07a) test(release): select the runnable preflight job - [`659b676`](https://github.com/morluto/rea/commit/659b676618fb12d9a6ff4fa1f4576d8a25f8e07d) fix(javascript): share require imports and retain escaped callables ### 📊 Changes **47 files changed** (+6391 additions, -322 deletions) <details> <summary>View changed files</summary> 📝 `docs/javascript-application-workflows.md` (+9 -0) 📝 `src/android/JadxDoctor.ts` (+4 -2) 📝 `src/application/WebSourceLocationPorts.ts` (+4 -2) 📝 `src/application/javascript/JavaScriptSemanticGraphConstruction.ts` (+2 -2) 📝 `src/browser/PlaywrightBrowserScenarioProvider.test.ts` (+20 -8) 📝 `src/domain/javascript/electronModuleInterop.test.ts` (+23 -0) 📝 `src/domain/javascript/javascriptApplicationAnalysis.ts` (+1 -2) 📝 `src/domain/javascript/javascriptAstValues.ts` (+84 -0) ➕ `src/domain/javascript/javascriptCallableProperties.fixture.ts` (+129 -0) ➕ `src/domain/javascript/javascriptCommonJsExposure.fixture.ts` (+48 -0) 📝 `src/domain/javascript/javascriptModuleInterop.ts` (+424 -61) ➕ `src/domain/javascript/javascriptNamespaceReceiver.fixture.ts` (+49 -0) ➕ `src/domain/javascript/javascriptOpaqueMutators.fixture.ts` (+218 -0) 📝 `src/domain/javascript/javascriptRuntimeReconciliationSchemas.ts` (+1 -2) ➕ `src/domain/javascript/javascriptSemanticAnalysis.builtin_review.test.ts` (+2010 -0) 📝 `src/domain/javascript/javascriptSemanticAnalysis.empty_keys.test.ts` (+7 -0) 📝 `src/domain/javascript/javascriptSemanticAnalysis.recoveredArguments.test.ts` (+2 -0) 📝 `src/domain/javascript/javascriptSemanticDataEffectHelpers.ts` (+229 -54) ➕ `src/domain/javascript/javascriptSemanticEffectQueries.ts` (+32 -0) 📝 `src/domain/javascript/javascriptSemanticEvaluationOrder.ts` (+55 -1) _...and 27 more files_ </details> ### 📄 Description ## Summary Preserve Node built-in effects at their actual reads and captures after initial CommonJS support landed on main. Earlier native reads survive later reassignment; replacing the loader, a built-in method or a shared object prevents subsequent calls from being reported as native effects. ## Change and scope - Share binding, alias, capture and effect-order analysis across loader provenance, saved methods and Object/Reflect intrinsics. Preserve separate receivers and captured object identities; read descriptor fields when their definition is invoked. - Track computed-key evaluation and coercion independently of scalar results, including nested/optional members, updates and compound assignments under typeof. Opaque changes invalidate ordinary property selections and Reflect own-data/null-prototype proofs at the relevant read. - Follow module exposure through descriptors, copied values, bound inputs, callable returns, await and yield. Keep inherited descriptor/then checks tied to each consumer. Rejected or skipped replacements cannot establish fresh allocation identity. - Select overlapping writes and escapes through a property-path index before applying the existing timing checks. Exact keys avoid rescanning unrelated writes; ancestor, descendant, wildcard and copied-property paths remain conservative. - Integrate main's current accessor, invocation-context, repeated-activation and cache owners. Preserve filesystem and synchronous child-process receiver links, TLS/net method distinctions and conservative argv selections. Owner: JavaScript domain provenance and mutation analysis. No CLI/MCP schema, Evidence identity, dependency or authority change. Analyzed JavaScript is parsed, not executed. - [x] Read existing files or captured data. ## Validation performed Commit **d1cf35334**, incorporating main **69bf61f7c**, on Node **24.22.1**. - Linux: **5,881 passing checks across 194 files**, with one configured skip, covering the complete JavaScript domain, JavaScript adapters, public CLI/MCP, returned-reference cycles, semantic graph and web bundle consumers, browser worker/scenario integration, JEB configuration, JSON output ownership, Hermes setup and ADB/capture cleanup. The initial pass exposed duplicate setup in the merged JEB fixture; its complete three-case file passes after reusing the prepared environment. - All **939 builtin-review cases** pass. **354 isolated native Node controls** pass: the 322 existing controls plus 32 computed-key, coercion, opaque-object/Reflect and descriptor-timing cases. ESM namespace controls use imported values consistently with main's module model. - The seven public CLI/MCP cases pass on Windows and Linux. Complete analysis Evidence agrees across both adapters. The combined regression corpus explicitly provisions matching 16 MiB server/client response buffers under main's current transport budget. - The previously failing JSON output ownership suite passes all **15 cases** after integrating main's existing cancellation/cleanup fixes (#5601 and #5852). - Build, full Linux check:ci, Windows typecheck/lint/format/module-boundary checks, generated-document checks and git diff --check pass. Hosted checks run against the new head after this push. Coverage is static provenance and the affected public consumers; it does not establish observed target execution. Native controls execute isolated test snippets only. Complex descriptor histories without proven flags remain conservative. ## Author readiness  Human owner: @cyanseek - [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. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi added the

pull-request

label

2026-10-31 00:57:39 +00:00

bsakaguchi referenced this issue

2026-11-01 21:42:29 +00:00

[PR #4990] [MERGED] perf(apktool): avoid duplicated primary strings on request #6713