morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #4863] [CLOSED] fix(javascript): preserve ambient effects in Map endpoint proofs #6448

New issue

Closed

opened 2026-10-31 01:14:32 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:14:32 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4863
Author: @Abdooo2235
Created: 10/17/2026
Status: ❌ Closed

Base: main ← Head: fix/javascript-map-endpoint

📝 Commits (10+)

814d007 fix(javascript): distinguish Map lookups from network endpoints
c8eaf3e fix(javascript): address Map endpoint review findings
8f03ae1 fix(javascript): preserve Map uncertainty through opaque effects
be866e8 fix(javascript): distinguish private stores and shared prototype effects
c7f6218 chore: sync latest main
b7ca3d7 fix(javascript): converge ambient identities and reuse eval indexing
d56185a fix(process): retain kernel limit descriptor cleanup capability
831782b chore: sync latest main
2a36a74 fix(javascript): preserve callable identity unions and global effects
8b453ec test(process): preserve ownership tools in isolated Ghidra fixture

📊 Changes

11 files changed (+2032 additions, -189 deletions)

View changed files

📝 src/domain/javascript/javascriptElectronMemberWrites.ts (+6 -1)
📝 src/domain/javascript/javascriptIntrinsicInvocation.test.ts (+122 -0)
📝 src/domain/javascript/javascriptIntrinsicInvocation.ts (+194 -53)
📝 src/domain/javascript/javascriptModuleInterop.ts (+23 -6)
📝 src/domain/javascript/javascriptSemanticAmbientWrites.ts (+494 -63)
📝 src/domain/javascript/javascriptSemanticAnalysis.ts (+6 -5)
📝 src/domain/javascript/javascriptSemanticOpaqueEffects.ts (+553 -21)
📝 src/domain/javascript/javascriptStaticAnalysis.mapEndpoints.test.ts (+66 -0)
➕ src/domain/javascript/javascriptStaticAnalysis.mapEscapedValues.test.ts (+261 -0)
📝 src/domain/javascript/javascriptStaticAnalysis.mapOpaqueEffects.test.ts (+178 -1)
📝 tests/acceptance/applications/javascriptEndpointCandidateCli.test.ts (+129 -39)

📄 Description

Summary

Follow-up to #4853: keep Map lookup omission valid only while the shared semantic owner can prove collection behavior. Possible dynamic-code execution, escaped references and unknown alternatives remain inferred endpoint candidates; intact Map lookups stay omitted and known fetch observations remain exact. Analyzed source is never executed.

The original Map correction landed on main in fbfdb6a with shared application/bundle classification and author credit. This PR preserves it and addresses the remaining review findings.

Approach

Preserve plural plain/bound callable identities and argument/receiver uncertainty through the existing call/apply/Reflect normalizer. Possible identity unions find effects; only the shared strict intrinsic owner proves inert factory/bind/freeze or ordinary class-call exemptions.
Separate generated callable creation from execution, including inherited Function.constructor, Reflect.construct's first target, direct/bound eval and unknown-call/external-storage handoffs. Share capture-aware constructor canonicalization between possible and strict identities; retain unknown mutation sites and full property selections.
Carry possible identity through await without strengthening strict reads or destination privacy. Traverse escaped public static class members and distinguish generated superclass construction from ordinary class calls through the existing worklist and invocation facts.
Resolve stored eval aliases and escaped ordinary methods against the completed shared store graph. Suppress a store escape only when every selected destination is proven private; unknown leaves, widened projections and unresolved cycles stay unproven.
Preserve a known Object.freeze result through one shared identity fact. Require complete normalized argument coverage before suppressing raw carrier escapes, and query Function argument coercion hooks through the completed ambient resolver.
Follow invoked or potentially handed-off lexical arrows to constructor super effects using existing invocation records and Ambient identity resolution after stores/mutations complete. Share selected argument/coercion handoffs with opaque effects, and observe awaited then execution separately from await-result identity. Preserve unused-arrow, freeze/bind and newTarget controls.
Preserve selected values and remaining coercion projections through array spreads in the shared possible-write resolver. Known numeric prefixes stay precise; unknown spread offsets remain conservative and strict reads are unchanged.
Reuse existing traversal and interpretation owners; no second AST pass, call index, parser or dependency. Preserve recovered argument holes and the singular invocation contract.

Merged upstream main 847472a, including 1accac2. Its TOML serializer repair removes repeated terminal-LF regex scans while retaining capacity, backup, source-preservation and idempotence coverage. The stale plist and verifier fixtures now match upstream exactly; superseded local CI repairs were removed from this PR's final diff. Upstream's descriptor cleanup improvements are also preserved.

Contract and boundary impact

Read-only shared JavaScript interpretation. CLI/MCP arguments, schemas, providers, target authority and package installation are unchanged.

Read existing files or captured data.
Observed, derived and inferred claims remain distinguishable.
Artifact identity, source provenance and failed attempts remain preserved.
Unsupported, incomplete, unavailable or uncertain outcomes remain visible.

Validation

Current head: fb131da; pinned Node 24.18.0 / npm 11.16.0.

JavaScript/Web/bundle source suite: 2,798 tests passed in 99 files.
Rebuilt focused endpoint CLI, public MCP, artifact graph, verifier resources, keyed archives and TOML capacity: 33 passed, four Windows platform skips in six files. The Codex capacity case passed in 7.492s without changing its 30s timeout.
npm run check — six tasks plus module/architecture guards passed; zero lint errors, 1,698 repository warnings (including changed functions).
npm run check:ci — all eight tasks, including duplicate/debt checks, plus architecture guards passed.
npm run docs:check — passed, including five README tests.
npm run verify:test-discovery — all 1,576 files exactly once; diff and commit/push hooks passed.
Current-head CI: running; no completed pass is claimed. The preceding 41f7567 run completed with all 17 checks passing.
Separate bare native Linux npm test on exact preceding 833b0a8 completed with 19,051 passed, seven failed and 173 skipped (exit 1, 3,239.949s), using default settings. Six environment failures were diagnosed without product changes: missing genuine Git metadata, zip/unzip prerequisites and a WSL shell-level interaction. Their combined rerun passed 16/16 on unchanged source. The remaining local Hopper case requires /usr/bin/Xvfb, unavailable in that environment; the corresponding supported Ubuntu CI case passed on 7ad92c8. These are versioned results, not a passing current-head full local suite.

Reported boundary regressions failed before correction. The latest focused source set passed 384 cases; independent review passed 12 additional probes. This cycle covers nested/aliased spread carriers and constructor/coercion projections, including precise prefixes, uncertain spread destinations and inert factory/newTarget controls. Earlier bound constructor, lexical-arrow and awaited-thenable regressions remain covered. The expanded public CLI regression failed against the preceding published build before rebuilding the correction. The exact Function.prototype.constructor review example already worked on published 833b0a8; execution/unused-creation controls were added without claiming a new production fix. The public CLI asserts candidate certainty and factory-only omission.

Earlier CI failed the stale plist expectation and then the TOML capacity timeout. The latter reproduced locally before upstream's serializer fix. Earlier superseded native attempts were explicitly interrupted; they are not completed suite results.

Cost measurements and limits

Source fb131da was compared with main 847472a in two serial isolated Node 24.18.0 processes using the same locked dependencies. Four samples per fixture complete analysis and validate endpoint count/candidate interpretation; the last three form warm medians. Source, HEAD and lockfile hashes stayed unchanged across all 32 samples. Project tests/builds were stopped.

Fixture	Warm elapsed baseline/current (ms)	Process CPU baseline/current (ms)
250 ordinary Map lookups	67.818 / 53.297	140 / 126
250 lookups with eval	66.298 / 72.506	109 / 109
1,000 ordinary Map lookups	208.099 / 177.711	344 / 329
1,000 lookups with eval	210.764 / 311.859	344 / 407

The largest eval case was about 48% slower with 18% more process CPU in this run; no general speedup is claimed. Cumulative process peak RSS was 237,052 / 241,816 KiB. Module loading, source hashing and explicit GC are outside task timers; cumulative RSS includes imports/transpilation and prior tasks, not task-local or process-family memory. OS/background load is uncontrolled. Tokens and packaged CLI/process-family costs remain unmeasured. Earlier measurements also showed increased eval cost with variable CPU ratios; their raw output is preserved.

Author readiness

Human owner: @Abdooo2235

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 within the existing contribution scope.
The PR has one focused outcome and links the related issue.
Tests cover changed behavior and meaningful failure paths.
The final diff was checked for unrelated changes and unsupported claims.
Human readiness and required CI/review checks are complete.

_{🔄 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/4863 **Author:** [@Abdooo2235](https://github.com/Abdooo2235) **Created:** 10/17/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/javascript-map-endpoint` --- ### 📝 Commits (10+) - [`814d007`](https://github.com/morluto/rea/commit/814d007b709c90efdda81ca020e0c24baa412d32) fix(javascript): distinguish Map lookups from network endpoints - [`c8eaf3e`](https://github.com/morluto/rea/commit/c8eaf3efe35c284a6dc0b3544059ebe337c484ec) fix(javascript): address Map endpoint review findings - [`8f03ae1`](https://github.com/morluto/rea/commit/8f03ae1df09eb4fa8984d3768d3fae6e87b64374) fix(javascript): preserve Map uncertainty through opaque effects - [`be866e8`](https://github.com/morluto/rea/commit/be866e8a57db3dc34d447c42aca9d39b217db9a2) fix(javascript): distinguish private stores and shared prototype effects - [`c7f6218`](https://github.com/morluto/rea/commit/c7f62187fcc6e19d25683338dff47cfc708dff6b) chore: sync latest main - [`b7ca3d7`](https://github.com/morluto/rea/commit/b7ca3d7ecfb194120be85e1206e4ad2b851492c5) fix(javascript): converge ambient identities and reuse eval indexing - [`d56185a`](https://github.com/morluto/rea/commit/d56185a23c590a21a2905b276d1cda80944a1ba6) fix(process): retain kernel limit descriptor cleanup capability - [`831782b`](https://github.com/morluto/rea/commit/831782bdd236e4bf1dd49b38b149a0a505b4ea1c) chore: sync latest main - [`2a36a74`](https://github.com/morluto/rea/commit/2a36a74036f6d83ae43c21802d80ce32aa0924eb) fix(javascript): preserve callable identity unions and global effects - [`8b453ec`](https://github.com/morluto/rea/commit/8b453ec8248a1b3bf67183d32d079514850cc0ff) test(process): preserve ownership tools in isolated Ghidra fixture ### 📊 Changes **11 files changed** (+2032 additions, -189 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/javascript/javascriptElectronMemberWrites.ts` (+6 -1) 📝 `src/domain/javascript/javascriptIntrinsicInvocation.test.ts` (+122 -0) 📝 `src/domain/javascript/javascriptIntrinsicInvocation.ts` (+194 -53) 📝 `src/domain/javascript/javascriptModuleInterop.ts` (+23 -6) 📝 `src/domain/javascript/javascriptSemanticAmbientWrites.ts` (+494 -63) 📝 `src/domain/javascript/javascriptSemanticAnalysis.ts` (+6 -5) 📝 `src/domain/javascript/javascriptSemanticOpaqueEffects.ts` (+553 -21) 📝 `src/domain/javascript/javascriptStaticAnalysis.mapEndpoints.test.ts` (+66 -0) ➕ `src/domain/javascript/javascriptStaticAnalysis.mapEscapedValues.test.ts` (+261 -0) 📝 `src/domain/javascript/javascriptStaticAnalysis.mapOpaqueEffects.test.ts` (+178 -1) 📝 `tests/acceptance/applications/javascriptEndpointCandidateCli.test.ts` (+129 -39) </details> ### 📄 Description ## Summary Follow-up to #4853: keep Map lookup omission valid only while the shared semantic owner can prove collection behavior. Possible dynamic-code execution, escaped references and unknown alternatives remain inferred endpoint candidates; intact Map lookups stay omitted and known fetch observations remain exact. Analyzed source is never executed. The original Map correction landed on `main` in fbfdb6a with shared application/bundle classification and author credit. This PR preserves it and addresses the remaining review findings. ## Approach - Preserve plural plain/bound callable identities and argument/receiver uncertainty through the existing call/apply/Reflect normalizer. Possible identity unions find effects; only the shared strict intrinsic owner proves inert factory/bind/freeze or ordinary class-call exemptions. - Separate generated callable creation from execution, including inherited Function.constructor, Reflect.construct's first target, direct/bound eval and unknown-call/external-storage handoffs. Share capture-aware constructor canonicalization between possible and strict identities; retain unknown mutation sites and full property selections. - Carry possible identity through await without strengthening strict reads or destination privacy. Traverse escaped public static class members and distinguish generated superclass construction from ordinary class calls through the existing worklist and invocation facts. - Resolve stored eval aliases and escaped ordinary methods against the completed shared store graph. Suppress a store escape only when every selected destination is proven private; unknown leaves, widened projections and unresolved cycles stay unproven. - Preserve a known Object.freeze result through one shared identity fact. Require complete normalized argument coverage before suppressing raw carrier escapes, and query Function argument coercion hooks through the completed ambient resolver. - Follow invoked or potentially handed-off lexical arrows to constructor super effects using existing invocation records and Ambient identity resolution after stores/mutations complete. Share selected argument/coercion handoffs with opaque effects, and observe awaited then execution separately from await-result identity. Preserve unused-arrow, freeze/bind and newTarget controls. - Preserve selected values and remaining coercion projections through array spreads in the shared possible-write resolver. Known numeric prefixes stay precise; unknown spread offsets remain conservative and strict reads are unchanged. - Reuse existing traversal and interpretation owners; no second AST pass, call index, parser or dependency. Preserve recovered argument holes and the singular invocation contract. Merged upstream `main` 847472a, including 1accac2. Its TOML serializer repair removes repeated terminal-LF regex scans while retaining capacity, backup, source-preservation and idempotence coverage. The stale plist and verifier fixtures now match upstream exactly; superseded local CI repairs were removed from this PR's final diff. Upstream's descriptor cleanup improvements are also preserved. ## Contract and boundary impact Read-only shared JavaScript interpretation. CLI/MCP arguments, schemas, providers, target authority and package installation are unchanged. - [x] Read existing files or captured data. - [x] Observed, derived and inferred claims remain distinguishable. - [x] Artifact identity, source provenance and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable or uncertain outcomes remain visible. ## Validation Current head: `fb131da`; pinned Node 24.18.0 / npm 11.16.0. - JavaScript/Web/bundle source suite: **2,798 tests passed in 99 files**. - Rebuilt focused endpoint CLI, public MCP, artifact graph, verifier resources, keyed archives and TOML capacity: **33 passed, four Windows platform skips in six files**. The Codex capacity case passed in 7.492s without changing its 30s timeout. - `npm run check` — six tasks plus module/architecture guards passed; zero lint errors, 1,698 repository warnings (including changed functions). - `npm run check:ci` — all eight tasks, including duplicate/debt checks, plus architecture guards passed. - `npm run docs:check` — passed, including five README tests. - `npm run verify:test-discovery` — all 1,576 files exactly once; diff and commit/push hooks passed. - Current-head CI: [running](https://github.com/morluto/rea/actions/runs/39092097955); no completed pass is claimed. The preceding 41f7567 run completed with all 17 checks passing. - Separate bare native Linux `npm test` on exact preceding `833b0a8` completed with 19,051 passed, seven failed and 173 skipped (exit 1, 3,239.949s), using default settings. Six environment failures were diagnosed without product changes: missing genuine Git metadata, zip/unzip prerequisites and a WSL shell-level interaction. Their combined rerun passed **16/16** on unchanged source. The remaining local Hopper case requires `/usr/bin/Xvfb`, unavailable in that environment; the corresponding supported Ubuntu CI case passed on 7ad92c8. These are versioned results, not a passing current-head full local suite. Reported boundary regressions failed before correction. The latest focused source set passed 384 cases; independent review passed 12 additional probes. This cycle covers nested/aliased spread carriers and constructor/coercion projections, including precise prefixes, uncertain spread destinations and inert factory/newTarget controls. Earlier bound constructor, lexical-arrow and awaited-thenable regressions remain covered. The expanded public CLI regression failed against the preceding published build before rebuilding the correction. The exact Function.prototype.constructor review example already worked on published 833b0a8; execution/unused-creation controls were added without claiming a new production fix. The public CLI asserts candidate certainty and factory-only omission. Earlier CI failed the stale plist expectation and then the TOML capacity timeout. The latter reproduced locally before upstream's serializer fix. Earlier superseded native attempts were explicitly interrupted; they are not completed suite results. ## Cost measurements and limits Source `fb131da` was compared with main `847472a` in two serial isolated Node 24.18.0 processes using the same locked dependencies. Four samples per fixture complete analysis and validate endpoint count/candidate interpretation; the last three form warm medians. Source, HEAD and lockfile hashes stayed unchanged across all 32 samples. Project tests/builds were stopped. | Fixture | Warm elapsed baseline/current (ms) | Process CPU baseline/current (ms) | | --- | ---: | ---: | | 250 ordinary Map lookups | 67.818 / 53.297 | 140 / 126 | | 250 lookups with eval | 66.298 / 72.506 | 109 / 109 | | 1,000 ordinary Map lookups | 208.099 / 177.711 | 344 / 329 | | 1,000 lookups with eval | 210.764 / 311.859 | 344 / 407 | The largest eval case was about **48% slower with 18% more process CPU** in this run; no general speedup is claimed. Cumulative process peak RSS was 237,052 / 241,816 KiB. Module loading, source hashing and explicit GC are outside task timers; cumulative RSS includes imports/transpilation and prior tasks, not task-local or process-family memory. OS/background load is uncontrolled. Tokens and packaged CLI/process-family costs remain unmeasured. Earlier measurements also showed increased eval cost with variable CPU ratios; their raw output is preserved. ## Author readiness Human owner: @Abdooo2235 - [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. ## Before merge - [x] This PR addresses a concrete problem within the existing contribution scope. - [x] The PR has one focused outcome and links the related issue. - [x] Tests cover changed behavior and meaningful failure paths. - [x] The final diff was checked for unrelated changes and unsupported claims. - [ ] Human readiness and required CI/review checks are complete. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:14:32 +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#6448

No description provided.

Rows
Columns