[PR #1914] [CLOSED] fix(javascript): preserve intrinsic invocation read identity #4904

New issue

Closed

opened 2026-10-31 00:56:37 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 00:56:37 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/1914
Author: @morluto
Created: 10/11/2026
Status: ❌ Closed

Base: main ← Head: codex/fix-intrinsic-invocation-boundaries

📝 Commits (5)

1a322ab fix(javascript): preserve intrinsic invocation read identity
a6de554 chore: merge main into intrinsic invocation fixes
af2fe0a chore: merge main into intrinsic invocation fixes
55ca7d8 fix(javascript): retain write identity and operand evaluation order
0a66e59 fix(javascript): retain destructuring effects in evaluation order

📊 Changes

9 files changed (+528 additions, -124 deletions)

View changed files

📝 docs/javascript-artifact-reconstruction.md (+8 -4)
➕ src/domain/javascript/electronIntrinsicCapture.test.ts (+176 -0)
📝 src/domain/javascript/electronModuleInterop.test.ts (+5 -0)
📝 src/domain/javascript/javascriptElectronMemberWrites.ts (+157 -75)
📝 src/domain/javascript/javascriptIntrinsicInvocation.ts (+10 -0)
📝 src/domain/javascript/javascriptModuleInterop.ts (+150 -39)
📝 src/domain/javascript/javascriptSemanticAnalysis.ts (+3 -2)
📝 src/domain/javascript/javascriptSemanticTraversal.ts (+12 -3)
📝 tests/acceptance/applications/electronModuleInteropCli.test.ts (+7 -1)

📄 Description

A comma-expression callee such as (0, Object.defineProperty)(Object, "create", descriptor) hid an intrinsic write, allowing an unverified Electron namespace to be attributed as real APIs. Conversely, invoking an earlier captured function through .call, .apply, or .bind could lose that capture after its global property was replaced.

This follow-up to #1857 fixes both at the shared invocation and mutation owners. Sequence callees retain their final value and the complete effect traversal; calls are classified after callee/argument traversal while actual property-read positions preserve selection timing. Invocation members are checked separately from the earlier base-function capture. Bound calls use the same checks. Single-write records retain their path directly instead of allocating a separate coverage trie, reusing the existing property matcher.

Validation: eight regressions fail on the original base 097f0d54a and pass afterward. After merging main 9d8e42bdd and retaining #1912's nested-receiver correction, all 868 tests across 24 files pass, including public Electron/package-import CLI and production stdio MCP. Both Electron adapters retain seven construction anchors, parse thirteen files, exclude mutation controls and preserve dynamic preload uncertainty. Two comma-detached .call/.apply regressions fail without the merged receiver correction and pass afterward; Reflect.apply still supplies an explicit receiver. Full npm run check, runtime/skill build and documentation checks pass. npm run verify:package passes on merge commit a6de554e2, including installation/update, packaged CLI/MCP, setup/uninstall, evidence and cleanup. Independent Luna review found no remaining blocker.

Separately, the earlier combined preflight with five local boundary-cleanup commits passed 1,035 tests, with 25 platform-specific skips, plus full checks, package verification, docs and pack dry run. That preflight is pinned to main 097f0d54a and the original follow-up 1a322ab75; the five cleanup commits remain outside this PR.

External DeepWiki research was checked against the ECMAScript comma-operator semantics. No CLI/MCP schema, target execution, provider authority or package metadata changes. Arbitrary cross-call effects remain unexamined; no real Electron execution or deep-provider behavior is claimed.

Addresses #1857 review findings sequence callees and alias capture timing. The five broader cleanup commits and two validation follow-ups are now published on main through 6407718f3; this PR retains the separate intrinsic-invocation fixes.

_{🔄 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/1914 **Author:** [@morluto](https://github.com/morluto) **Created:** 10/11/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `codex/fix-intrinsic-invocation-boundaries` --- ### 📝 Commits (5) - [`1a322ab`](https://github.com/morluto/rea/commit/1a322ab75b7c5a0ce09fd200b9c3e580a72fab6b) fix(javascript): preserve intrinsic invocation read identity - [`a6de554`](https://github.com/morluto/rea/commit/a6de554e2a0fa7032219d53de9f1ca01d296758f) chore: merge main into intrinsic invocation fixes - [`af2fe0a`](https://github.com/morluto/rea/commit/af2fe0a8cc66a3432ca49997b15aa2ad8adfb9c8) chore: merge main into intrinsic invocation fixes - [`55ca7d8`](https://github.com/morluto/rea/commit/55ca7d83472c040b8699adedf13fe09e33487461) fix(javascript): retain write identity and operand evaluation order - [`0a66e59`](https://github.com/morluto/rea/commit/0a66e595bbfdd10d0c876a3c37d264382f9acbd4) fix(javascript): retain destructuring effects in evaluation order ### 📊 Changes **9 files changed** (+528 additions, -124 deletions) <details> <summary>View changed files</summary> 📝 `docs/javascript-artifact-reconstruction.md` (+8 -4) ➕ `src/domain/javascript/electronIntrinsicCapture.test.ts` (+176 -0) 📝 `src/domain/javascript/electronModuleInterop.test.ts` (+5 -0) 📝 `src/domain/javascript/javascriptElectronMemberWrites.ts` (+157 -75) 📝 `src/domain/javascript/javascriptIntrinsicInvocation.ts` (+10 -0) 📝 `src/domain/javascript/javascriptModuleInterop.ts` (+150 -39) 📝 `src/domain/javascript/javascriptSemanticAnalysis.ts` (+3 -2) 📝 `src/domain/javascript/javascriptSemanticTraversal.ts` (+12 -3) 📝 `tests/acceptance/applications/electronModuleInteropCli.test.ts` (+7 -1) </details> ### 📄 Description A comma-expression callee such as `(0, Object.defineProperty)(Object, "create", descriptor)` hid an intrinsic write, allowing an unverified Electron namespace to be attributed as real APIs. Conversely, invoking an earlier captured function through `.call`, `.apply`, or `.bind` could lose that capture after its global property was replaced. This follow-up to #1857 fixes both at the shared invocation and mutation owners. Sequence callees retain their final value and the complete effect traversal; calls are classified after callee/argument traversal while actual property-read positions preserve selection timing. Invocation members are checked separately from the earlier base-function capture. Bound calls use the same checks. Single-write records retain their path directly instead of allocating a separate coverage trie, reusing the existing property matcher. Validation: eight regressions fail on the original base `097f0d54a` and pass afterward. After merging main `9d8e42bdd` and retaining #1912's nested-receiver correction, all 868 tests across 24 files pass, including public Electron/package-import CLI and production stdio MCP. Both Electron adapters retain seven construction anchors, parse thirteen files, exclude mutation controls and preserve dynamic preload uncertainty. Two comma-detached `.call`/`.apply` regressions fail without the merged receiver correction and pass afterward; `Reflect.apply` still supplies an explicit receiver. Full `npm run check`, runtime/skill build and documentation checks pass. `npm run verify:package` passes on merge commit `a6de554e2`, including installation/update, packaged CLI/MCP, setup/uninstall, evidence and cleanup. Independent Luna review found no remaining blocker. Separately, the earlier combined preflight with five local boundary-cleanup commits passed 1,035 tests, with 25 platform-specific skips, plus full checks, package verification, docs and pack dry run. That preflight is pinned to main `097f0d54a` and the original follow-up `1a322ab75`; the five cleanup commits remain outside this PR. External [DeepWiki research](https://deepwiki.com/search/for-static-effectidentity-anal_435e8354-f715-4bc4-90e4-fb4fc30d7f77) was checked against the [ECMAScript comma-operator semantics](https://tc39.es/ecma262/multipage/ecmascript-language-expressions.html#sec-comma-operator). No CLI/MCP schema, target execution, provider authority or package metadata changes. Arbitrary cross-call effects remain unexamined; no real Electron execution or deep-provider behavior is claimed. Addresses #1857 review findings [sequence callees](https://github.com/morluto/rea/pull/1857#discussion_r4242445686) and [alias capture timing](https://github.com/morluto/rea/pull/1857#discussion_r4242445687). The five broader cleanup commits and two validation follow-ups are now published on main through [6407718f3](https://github.com/morluto/rea/commit/6407718f3020a34cc087163492e3f9a465e2e841); this PR retains the separate intrinsic-invocation fixes. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 00:56:37 +00:00

closed this issue
added the
pull-request
label

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

No description provided.

Rows
Columns