[PR #3567] [MERGED] fix(javascript): track public CommonJS export receiver lifetimes #5830

Closed
opened 2026-10-31 01:09:44 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/3567
Author: @JNTMTMTM
Created: 10/14/2026
Status: ✅ Merged
Merged: 10/14/2026
Merged by: @morluto

Base: main ← Head: codex/fix-commonjs-export-receivers-3541


📝 Commits (2)

  • e4b5c6b fix(javascript): track public CommonJS export receiver lifetimes
  • 30a7043 fix(javascript): retain exports across uncertain identity effects

📊 Changes

10 files changed (+502 additions, -71 deletions)

View changed files

➕ src/domain/javascript/javascriptCommonJsExportReceivers.ts (+242 -0)
📝 src/domain/javascript/javascriptElectronMemberWrites.ts (+3 -1)
📝 src/domain/javascript/javascriptSemanticAmbientWrites.test.ts (+22 -0)
📝 src/domain/javascript/javascriptSemanticAmbientWrites.ts (+6 -0)
📝 src/domain/javascript/javascriptSemanticAnalysis.require_links.test.ts (+1 -0)
📝 src/domain/javascript/javascriptSemanticAnalysis.structure.test.ts (+98 -0)
📝 src/domain/javascript/javascriptSemanticAnalysis.ts (+3 -1)
📝 src/domain/javascript/javascriptSemanticProjection.ts (+11 -68)
📝 src/domain/javascript/javascriptSemanticState.ts (+1 -1)
📝 tests/acceptance/applications/exportedFunctionBindingsCli.test.ts (+115 -0)

📄 Description

Summary

Fixes #3541. CommonJS writes through an old exports receiver no longer create public export bindings or trusted return shapes after module.exports is replaced.

Problem and expected behavior

For module.exports = {}; exports.parse = () => ({ kind: "result", count: 1 });, Node returns an object without parse. REA previously exposed that callable and could report a definite cross-version /count change. Writes to provably detached objects must be excluded; unresolved identity or execution order must remain unknown.

Change and scope

Track receiver capture, allocation identity, rebinding and explicit reconnection in one cooperative semantic pass. The shared producer replaces the assignment-chain-only guard, so export bindings and callable consumers use the same facts. Preserve lexical shadows, source ranges, opaque identity reconnection and unknown control-flow outcomes. Update a dependency fixture to explicitly reconnect its exports alias.

The two review findings are valid and addressed at their producing boundaries:

  • Destructuring visits the pattern before its RHS in the AST, while runtime evaluation does the opposite. Mark the entire destructuring operation uncertain so a later tree-visited RHS cannot overwrite the unknown pattern effect with false identity certainty. Computed-key and default reconnects retain an unresolved export candidate instead of disappearing.
  • Visible mutator calls now consume the shared write index's canonical alias, invocation and property facts. Distinguish the CJS wrapper module from globalThis.module; record content mutation as an unknown child property rather than replacement of its target. A possible write to the wrapper's exports slot invalidates public identity, while unrelated keys, rebound aliases and public-object content mutations remain separate.

Contract and boundary impact

  • Semantic owner and earliest changed stage: javascriptCommonJsExportReceivers.ts, before semantic module-link collection.
  • Contribution scope: ordinary static-analysis correctness fix; advance maintainer scope decision is not applicable.
  • CLI/MCP and tool catalog: existing inputs and schemas; corrected export selection through both adapters.
  • Evidence/provenance: retain original source ranges and artifact identities; suppress detached links and clear callable/binding resolution for uncertain receivers.
  • Authority and cleanup: existing inert source-analysis workflow and owned worker lifecycle; no added target execution, injection, mutation or protection recovery.
  • Platform/package/generated metadata: no provider, installation, package metadata or generated catalog changes.

Expected workflow effects (existing behavior):

  • Read existing files or captured data.
  • Launch or attach to a process. Existing owned analysis worker only; no target process attach.
  • Inject instrumentation or run scripts/code.
  • Change process memory, registers, files, or configuration.
  • Contact network services.
  • Recover protected code or work around protections.
  • Documentation or usability only; no target interaction.
  • Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

Six detached-receiver acceptance cases failed before the original fix. Eight added review regressions failed on the previous PR implementation. The completed matrix checks actual Node require() results, compiled CLI/stdio MCP analysis parity, and comparison parity on full Evidence inputs.

Detached exports report missing with zero definite changes. Connected controls still report the /count change. The review examples genuinely export parse under Node; the static analyzer now retains an unresolved export and unknown comparison coverage rather than claiming it is missing. This deliberately preserves uncertainty rather than adding a general execution-order/call evaluator.

Source and public-workflow controls cover computed-key assignments, declaration and array-default destructuring, Object/Reflect mutators, callable and receiver aliases, lexical shadows, unrelated keys and mutations of the public object's contents. The producing-boundary regression verifies that wrapper and global module identities remain distinct. Node executes only test-owned source fixtures as an oracle; REA analysis remains inert.

Validation performed

Executed locally on macOS arm64 with Node 24.18.0:

  • npm run test:local — 2,450 affected source tests passed.
  • npm run test:focused -- tests/acceptance/applications/exportedFunctionBindingsCli.test.ts tests/boundary/filesystem/javascriptExportShapeComparison.test.ts tests/boundary/filesystem/javascriptExportContainerCoverage.test.ts — 156 tests passed (92 real Node/compiled CLI/stdio MCP export tests and 64 downstream boundary tests).
  • npm run test:focused -- tests/acceptance/applications/electronModuleInteropCli.test.ts — 2 existing CLI/stdio MCP workflows passed, covering the shared identity-write consumers with inert source fixtures.
  • npm run check — typecheck, lint, formatting, knip, metadata, module boundaries and architecture guards passed.
  • Current-head pre-push npm run check:fast — passed.

Performance sample comparing the pre-review implementation with this update: identical complete CLI analysis of 20 owned files / 1,000 intact exports, three alternating pairs with equal normalized results. Excluding first runs, repeat medians were 2.375 → 2.240 s elapsed, 2.765 → 2.615 s process-family CPU, and approximately 524.6 → 518.0 MiB sampled family peak RSS. Whole-response tokenx estimates were identical at 3,157,413 tokens. All processes were fresh on a warm host; first runs were retained separately. These small synthetic samples do not establish an improvement; RSS sampling can miss peaks. Controlled cold-host behavior and total agent/schema/input/retry token usage were not measured.

Compatibility, safety, and release

No input/schema migration or package/release metadata change. Static local-alias and cross-function identity remains conservative. Windows/Linux and native/external analysis providers were not exercised by these checks.

Author readiness

Human owner: @JNTMTMTM

  • 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/3567 **Author:** [@JNTMTMTM](https://github.com/JNTMTMTM) **Created:** 10/14/2026 **Status:** ✅ Merged **Merged:** 10/14/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `codex/fix-commonjs-export-receivers-3541` --- ### 📝 Commits (2) - [`e4b5c6b`](https://github.com/morluto/rea/commit/e4b5c6b507e65432580b9deab637dead9b3cdabd) fix(javascript): track public CommonJS export receiver lifetimes - [`30a7043`](https://github.com/morluto/rea/commit/30a704336174b342e5e4f5bdccbcfdc658b7d79c) fix(javascript): retain exports across uncertain identity effects ### 📊 Changes **10 files changed** (+502 additions, -71 deletions) <details> <summary>View changed files</summary> ➕ `src/domain/javascript/javascriptCommonJsExportReceivers.ts` (+242 -0) 📝 `src/domain/javascript/javascriptElectronMemberWrites.ts` (+3 -1) 📝 `src/domain/javascript/javascriptSemanticAmbientWrites.test.ts` (+22 -0) 📝 `src/domain/javascript/javascriptSemanticAmbientWrites.ts` (+6 -0) 📝 `src/domain/javascript/javascriptSemanticAnalysis.require_links.test.ts` (+1 -0) 📝 `src/domain/javascript/javascriptSemanticAnalysis.structure.test.ts` (+98 -0) 📝 `src/domain/javascript/javascriptSemanticAnalysis.ts` (+3 -1) 📝 `src/domain/javascript/javascriptSemanticProjection.ts` (+11 -68) 📝 `src/domain/javascript/javascriptSemanticState.ts` (+1 -1) 📝 `tests/acceptance/applications/exportedFunctionBindingsCli.test.ts` (+115 -0) </details> ### 📄 Description ## Summary Fixes #3541. CommonJS writes through an old `exports` receiver no longer create public export bindings or trusted return shapes after `module.exports` is replaced. ## Problem and expected behavior For `module.exports = {}; exports.parse = () => ({ kind: "result", count: 1 });`, Node returns an object without `parse`. REA previously exposed that callable and could report a definite cross-version `/count` change. Writes to provably detached objects must be excluded; unresolved identity or execution order must remain unknown. ## Change and scope Track receiver capture, allocation identity, rebinding and explicit reconnection in one cooperative semantic pass. The shared producer replaces the assignment-chain-only guard, so export bindings and callable consumers use the same facts. Preserve lexical shadows, source ranges, opaque identity reconnection and unknown control-flow outcomes. Update a dependency fixture to explicitly reconnect its `exports` alias. The two review findings are valid and addressed at their producing boundaries: - Destructuring visits the pattern before its RHS in the AST, while runtime evaluation does the opposite. Mark the entire destructuring operation uncertain so a later tree-visited RHS cannot overwrite the unknown pattern effect with false identity certainty. Computed-key and default reconnects retain an unresolved export candidate instead of disappearing. - Visible mutator calls now consume the shared write index's canonical alias, invocation and property facts. Distinguish the CJS wrapper module from `globalThis.module`; record content mutation as an unknown child property rather than replacement of its target. A possible write to the wrapper's `exports` slot invalidates public identity, while unrelated keys, rebound aliases and public-object content mutations remain separate. ## Contract and boundary impact - Semantic owner and earliest changed stage: `javascriptCommonJsExportReceivers.ts`, before semantic module-link collection. - Contribution scope: ordinary static-analysis correctness fix; advance maintainer scope decision is not applicable. - CLI/MCP and tool catalog: existing inputs and schemas; corrected export selection through both adapters. - Evidence/provenance: retain original source ranges and artifact identities; suppress detached links and clear callable/binding resolution for uncertain receivers. - Authority and cleanup: existing inert source-analysis workflow and owned worker lifecycle; no added target execution, injection, mutation or protection recovery. - Platform/package/generated metadata: no provider, installation, package metadata or generated catalog changes. Expected workflow effects (existing behavior): - [x] Read existing files or captured data. - [x] Launch or attach to a process. Existing owned analysis worker only; no target process attach. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage Six detached-receiver acceptance cases failed before the original fix. Eight added review regressions failed on the previous PR implementation. The completed matrix checks actual Node `require()` results, compiled CLI/stdio MCP analysis parity, and comparison parity on full Evidence inputs. Detached exports report `missing` with zero definite changes. Connected controls still report the `/count` change. The review examples genuinely export `parse` under Node; the static analyzer now retains an unresolved export and unknown comparison coverage rather than claiming it is missing. This deliberately preserves uncertainty rather than adding a general execution-order/call evaluator. Source and public-workflow controls cover computed-key assignments, declaration and array-default destructuring, Object/Reflect mutators, callable and receiver aliases, lexical shadows, unrelated keys and mutations of the public object's contents. The producing-boundary regression verifies that wrapper and global module identities remain distinct. Node executes only test-owned source fixtures as an oracle; REA analysis remains inert. ## Validation performed Executed locally on macOS arm64 with Node 24.18.0: - `npm run test:local` — 2,450 affected source tests passed. - `npm run test:focused -- tests/acceptance/applications/exportedFunctionBindingsCli.test.ts tests/boundary/filesystem/javascriptExportShapeComparison.test.ts tests/boundary/filesystem/javascriptExportContainerCoverage.test.ts` — 156 tests passed (92 real Node/compiled CLI/stdio MCP export tests and 64 downstream boundary tests). - `npm run test:focused -- tests/acceptance/applications/electronModuleInteropCli.test.ts` — 2 existing CLI/stdio MCP workflows passed, covering the shared identity-write consumers with inert source fixtures. - `npm run check` — typecheck, lint, formatting, knip, metadata, module boundaries and architecture guards passed. - Current-head pre-push `npm run check:fast` — passed. Performance sample comparing the pre-review implementation with this update: identical complete CLI analysis of 20 owned files / 1,000 intact exports, three alternating pairs with equal normalized results. Excluding first runs, repeat medians were 2.375 → 2.240 s elapsed, 2.765 → 2.615 s process-family CPU, and approximately 524.6 → 518.0 MiB sampled family peak RSS. Whole-response tokenx estimates were identical at 3,157,413 tokens. All processes were fresh on a warm host; first runs were retained separately. These small synthetic samples do not establish an improvement; RSS sampling can miss peaks. Controlled cold-host behavior and total agent/schema/input/retry token usage were not measured. ## Compatibility, safety, and release No input/schema migration or package/release metadata change. Static local-alias and cross-function identity remains conservative. Windows/Linux and native/external analysis providers were not exercised by these checks. ## Author readiness Human owner: @JNTMTMTM - [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 2026-10-31 01:09:44 +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#5830
No description provided.