morluto/rea

Fork 0

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

[PR #4941] [MERGED] fix(javascript): preserve runtime path bytes and Electron ownership #6683

New issue

Closed

opened 2026-11-01 21:42:16 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4941
Author: @ShuaixinHuang
Created: 10/18/2026
Status: ✅ Merged
Merged: 10/20/2026
Merged by: @morluto

Base: main ← Head: codex/fix-runtime-path-ownership-resubmit

📝 Commits (10+)

d4de95c fix(javascript): preserve literal percent runtime paths
f675800 Merge upstream/main into codex/fix-runtime-literal-percent
b3fcaab fix(javascript): ignore resolve prefixes before the final anchor
b3f0400 fix(electron): retain application roots for loadFile declarations
ee25b9a fix(electron): resolve loadFile from enclosing application package
ca427f9 Merge remote-tracking branch 'origin/main' into codex/fix-runtime-literal-percent
0fbd037 fix(electron): retain outer application root across package scopes
522fc05 fix(electron): derive application ownership from inventoried packages
31dd95e fix(electron): retain resolved package entrypoint ownership
f0f721a fix(electron): preserve competing application ownership

📊 Changes

149 files changed (+16874 additions, -981 deletions)

View changed files

➕ src/application/javascript/JavaScriptArtifactAnalysis.packageMain.test.ts (+64 -0)
📝 src/application/javascript/JavaScriptArtifactAnalysis.ts (+9 -0)
📝 src/application/javascript/JavaScriptArtifactAnalysisTypes.ts (+5 -0)
📝 src/application/javascript/JavaScriptArtifactGraphBuilder.ts (+64 -1)
📝 src/application/javascript/JavaScriptArtifactGraphContext.ts (+124 -4)
📝 src/application/javascript/JavaScriptArtifactGraphFindings.ts (+185 -17)
📝 src/application/javascript/JavaScriptArtifactGraphStructure.test.ts (+230 -0)
📝 src/application/javascript/JavaScriptArtifactGraphStructure.ts (+87 -19)
➕ src/application/javascript/JavaScriptArtifactPathResolution.syntheticRootEscape.test.ts (+67 -0)
📝 src/application/javascript/JavaScriptArtifactPathResolution.test.ts (+1205 -17)
📝 src/application/javascript/JavaScriptArtifactPathResolution.ts (+371 -135)
📝 src/application/javascript/JavaScriptSemanticGraphBuilder.test.ts (+31 -0)
📝 src/application/javascript/JavaScriptSemanticGraphBuilder.ts (+2 -2)
➕ src/contracts/canonicalBase64Metadata.test.ts (+68 -0)
📝 src/domain/javascript/electronStaticAnalysis.ts (+26 -1)
➕ src/domain/javascript/electronStaticAnalysisAppPathSelection.test.ts (+127 -0)
📝 src/domain/javascript/electronStaticAnalysisBrowser.ts (+19 -6)
➕ src/domain/javascript/electronStaticAnalysisExportedAppPath.test.ts (+136 -0)
📝 src/domain/javascript/electronStaticAnalysisIdentity.test.ts (+124 -0)
➕ src/domain/javascript/electronStaticAnalysisPublishedApp.test.ts (+112 -0)

...and 80 more files

📄 Description

Problem and outcome

Electron renderer/preload paths require the original filename bytes and a proven application owner. The shared JavaScript analysis now retains unsupported loads, competing owners and possible app-path mutations as explicit uncertainty instead of resolving them to a decoy. This replaces #4327.

The current fixes use the existing source and export owners:

Preserve possible app-path setters when a native Electron app object escapes through an export, while retaining definite local replacements.
Model Object.assign exports as the own enumerable fields actually copied, including later-source overwrites, and retain shared module provenance.
Share mutable path/URL default-import and native child-object identities across aliases.
Preserve opaque alternatives in conditional createRequire factories.
Normalize TypeScript-wrapped member-store targets at the shared iteration-source producer. A typed assignment now retains the same loader RHS and write-target identity as its untyped counterpart; local replacements remain inert. This follow-up adds two production lines.

Synced upstream main at 833136507dd79d959340b09862a53af344e4f1b3. Its process UID/exit-scope/lifecycle implementation and recorded-crash trace policy replace the superseded local observer. The upstream canonical base64 schema replaces the separate schema translator; MCP metadata/intersection controls remain. Process production and recorded-crash verification match that upstream base. Its readiness fork-association fix (#5316) also applies.

Executed validation

Current revision: 35e2cad5bdbbf90f15adb5c814297b25e36f4c79, Windows x64, Node 24.18.0/npm 11.16.0.

Final combined source and ownership regressions: 173 files, 4,153 passed and 8 existing platform skips. This includes the JavaScript/application/worker, native ownership, trace-policy, canonical-schema and four typed-store controls.
Final compiled CLI/MCP indirect-loader checks: 82/82 passed locally, including all 78 existing cases and four added typed-store CLI/MCP controls (252.85 s). The same 82 tests also pass on hosted Linux (178.329 s)..
The typed-store source controls reproduced 2 failures with 2 passing local controls on the previous revision. The compiled CLI/MCP controls independently reproduced 2 failures and 2 passes on 31d6970b, with 78 unrelated cases excluded by the name filter.
The Linux-only CI failure in the literal-punctuation fixture used an unbound path identifier. The fixture now supplies actual CommonJS native-path and Electron bindings and checks the same exact preload%20?#.cjs result through full static analysis. The filtered test failed before and passes after on Windows (1 executed, 26 excluded). The current hosted Linux run passes all 27 tests in this file; shard 1 passes 1,935 tests with zero failures.
Final npm run check: six tasks passed, 2,277 resolved module boundaries, 1,594 architecture guards, zero lint errors. Build, MCP catalog and diff checks passed. Test discovery registered all 1,766 files exactly once.
Representative completed-task cost evidence: the same 38 samples in two rounds produced 1 mismatch before and 0 after, comparing 31d6970b to this committed source (83 input hashes; artifact 48cf3aee8d13290c4bdb603071d894b6268aa8aefdef2fff056a414d9ed38baf; workload df41bf234b49076137628cd8f858807acc8fde439ca5996078cd669f9e7b5af9). Before/after wall times were 134.852/210.824 ms and 70.360/149.873 ms; CPU 187/265 ms and 140/94 ms; whole-child high-water RSS 84,532/85,048 KiB and 90,892/89,576 KiB. Mixed results under uncontrolled desktop load establish neither a speedup nor a definite regression. Imports/bundling and complete CLI/native process-family costs are excluded.

Current-head hosted Linux executes all 16 jadx controls successfully (12.453 s), including cancellation of an in-flight Java readiness probe and joining cancelled Java until its owned child exits. These are actual Linux passes, independent of the 16 POSIX-only local skips. Earlier validation on the immediately preceding 31d6970b revision also passed 148 compiled public/Node/schema controls across nine files; that historical count is not added to the current-head totals.

Current-head hosted status: SUCCESS on this exact revision: https://github.com/morluto/rea/actions/runs/39350036839. All five portable test shards and CI required passed: 10,251 tests passed, zero failed, 162 existing skips; 1,025 test files passed. Source tests, static checks, build, docs, real JavaScript recovery, Apple artifact E2E, Windows package/capability checks and all three native package baselines passed. Independently scheduled sigrok/tshark checks also passed. Scope-excluded lanes remain skipped and are not counted as passes. PR readiness is SUCCESS (https://github.com/morluto/rea/actions/runs/39350091288). GitHub reports mergeable=true and mergeable_state=clean against upstream 83313650..

The four reviewed inline threads were replied to with evidence and resolved at 31d6970b. The typed-store follow-up also addresses the older outside-diff finding in https://github.com/morluto/rea/pull/4941#issuecomment-6263011538.

No analyzed target or real Electron application is executed by the public analysis fixtures. Authored Node oracle fixtures establish only the native semantics they exercise. Real provider execution, complete CLI process-family costs and token usage remain unmeasured unless explicitly reported above. Prepared with AI assistance; the original human initial-readiness declaration is preserved.

Author readiness

Human owner: @ShuaixinHuang

The human owner explicitly confirmed review, evidence, responsibility and initial readiness for this PR on 2026-10-18.

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/4941 **Author:** [@ShuaixinHuang](https://github.com/ShuaixinHuang) **Created:** 10/18/2026 **Status:** ✅ Merged **Merged:** 10/20/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `codex/fix-runtime-path-ownership-resubmit` --- ### 📝 Commits (10+) - [`d4de95c`](https://github.com/morluto/rea/commit/d4de95c71707f22f5039c9aa99788719a293d854) fix(javascript): preserve literal percent runtime paths - [`f675800`](https://github.com/morluto/rea/commit/f675800bdb599fabe46e283b83fcdec1510ce5d7) Merge upstream/main into codex/fix-runtime-literal-percent - [`b3fcaab`](https://github.com/morluto/rea/commit/b3fcaab706249089e9c2e9f67c5a8bcf87ae358a) fix(javascript): ignore resolve prefixes before the final anchor - [`b3f0400`](https://github.com/morluto/rea/commit/b3f04002f5f37d4a858777e66e33e6145805b37c) fix(electron): retain application roots for loadFile declarations - [`ee25b9a`](https://github.com/morluto/rea/commit/ee25b9a539ce4de7a4587d8ed0977a8e5092ec2e) fix(electron): resolve loadFile from enclosing application package - [`ca427f9`](https://github.com/morluto/rea/commit/ca427f9ff56d707f86f3a2afd8482c34657a129a) Merge remote-tracking branch 'origin/main' into codex/fix-runtime-literal-percent - [`0fbd037`](https://github.com/morluto/rea/commit/0fbd037603272d91b03554efc92ce5aabb0aeed7) fix(electron): retain outer application root across package scopes - [`522fc05`](https://github.com/morluto/rea/commit/522fc0566a239c69747a88e0a0159b1c020e6180) fix(electron): derive application ownership from inventoried packages - [`31dd95e`](https://github.com/morluto/rea/commit/31dd95e8e4f93fc43cc94aa3d7a9afc914afd56e) fix(electron): retain resolved package entrypoint ownership - [`f0f721a`](https://github.com/morluto/rea/commit/f0f721acb761ca0b51d435174987355924cd44c8) fix(electron): preserve competing application ownership ### 📊 Changes **149 files changed** (+16874 additions, -981 deletions) <details> <summary>View changed files</summary> ➕ `src/application/javascript/JavaScriptArtifactAnalysis.packageMain.test.ts` (+64 -0) 📝 `src/application/javascript/JavaScriptArtifactAnalysis.ts` (+9 -0) 📝 `src/application/javascript/JavaScriptArtifactAnalysisTypes.ts` (+5 -0) 📝 `src/application/javascript/JavaScriptArtifactGraphBuilder.ts` (+64 -1) 📝 `src/application/javascript/JavaScriptArtifactGraphContext.ts` (+124 -4) 📝 `src/application/javascript/JavaScriptArtifactGraphFindings.ts` (+185 -17) 📝 `src/application/javascript/JavaScriptArtifactGraphStructure.test.ts` (+230 -0) 📝 `src/application/javascript/JavaScriptArtifactGraphStructure.ts` (+87 -19) ➕ `src/application/javascript/JavaScriptArtifactPathResolution.syntheticRootEscape.test.ts` (+67 -0) 📝 `src/application/javascript/JavaScriptArtifactPathResolution.test.ts` (+1205 -17) 📝 `src/application/javascript/JavaScriptArtifactPathResolution.ts` (+371 -135) 📝 `src/application/javascript/JavaScriptSemanticGraphBuilder.test.ts` (+31 -0) 📝 `src/application/javascript/JavaScriptSemanticGraphBuilder.ts` (+2 -2) ➕ `src/contracts/canonicalBase64Metadata.test.ts` (+68 -0) 📝 `src/domain/javascript/electronStaticAnalysis.ts` (+26 -1) ➕ `src/domain/javascript/electronStaticAnalysisAppPathSelection.test.ts` (+127 -0) 📝 `src/domain/javascript/electronStaticAnalysisBrowser.ts` (+19 -6) ➕ `src/domain/javascript/electronStaticAnalysisExportedAppPath.test.ts` (+136 -0) 📝 `src/domain/javascript/electronStaticAnalysisIdentity.test.ts` (+124 -0) ➕ `src/domain/javascript/electronStaticAnalysisPublishedApp.test.ts` (+112 -0) _...and 80 more files_ </details> ### 📄 Description ## Problem and outcome Electron renderer/preload paths require the original filename bytes and a proven application owner. The shared JavaScript analysis now retains unsupported loads, competing owners and possible app-path mutations as explicit uncertainty instead of resolving them to a decoy. This replaces #4327. The current fixes use the existing source and export owners: - Preserve possible app-path setters when a native Electron app object escapes through an export, while retaining definite local replacements. - Model Object.assign exports as the own enumerable fields actually copied, including later-source overwrites, and retain shared module provenance. - Share mutable path/URL default-import and native child-object identities across aliases. - Preserve opaque alternatives in conditional createRequire factories. - Normalize TypeScript-wrapped member-store targets at the shared iteration-source producer. A typed assignment now retains the same loader RHS and write-target identity as its untyped counterpart; local replacements remain inert. This follow-up adds two production lines. Synced upstream main at `833136507dd79d959340b09862a53af344e4f1b3`. Its process UID/exit-scope/lifecycle implementation and recorded-crash trace policy replace the superseded local observer. The upstream canonical base64 schema replaces the separate schema translator; MCP metadata/intersection controls remain. Process production and recorded-crash verification match that upstream base. Its readiness fork-association fix (#5316) also applies. ## Executed validation Current revision: `35e2cad5bdbbf90f15adb5c814297b25e36f4c79`, Windows x64, Node 24.18.0/npm 11.16.0. - Final combined source and ownership regressions: 173 files, 4,153 passed and 8 existing platform skips. This includes the JavaScript/application/worker, native ownership, trace-policy, canonical-schema and four typed-store controls. - Final compiled CLI/MCP indirect-loader checks: 82/82 passed locally, including all 78 existing cases and four added typed-store CLI/MCP controls (252.85 s). The same 82 tests also pass on hosted Linux (178.329 s).. - The typed-store source controls reproduced 2 failures with 2 passing local controls on the previous revision. The compiled CLI/MCP controls independently reproduced 2 failures and 2 passes on `31d6970b`, with 78 unrelated cases excluded by the name filter. - The Linux-only CI failure in the literal-punctuation fixture used an unbound `path` identifier. The fixture now supplies actual CommonJS native-path and Electron bindings and checks the same exact `preload%20?#.cjs` result through full static analysis. The filtered test failed before and passes after on Windows (1 executed, 26 excluded). The current hosted Linux run passes all 27 tests in this file; shard 1 passes 1,935 tests with zero failures. - Final `npm run check`: six tasks passed, 2,277 resolved module boundaries, 1,594 architecture guards, zero lint errors. Build, MCP catalog and diff checks passed. Test discovery registered all 1,766 files exactly once. - Representative completed-task cost evidence: the same 38 samples in two rounds produced 1 mismatch before and 0 after, comparing `31d6970b` to this committed source (83 input hashes; artifact `48cf3aee8d13290c4bdb603071d894b6268aa8aefdef2fff056a414d9ed38baf`; workload `df41bf234b49076137628cd8f858807acc8fde439ca5996078cd669f9e7b5af9`). Before/after wall times were 134.852/210.824 ms and 70.360/149.873 ms; CPU 187/265 ms and 140/94 ms; whole-child high-water RSS 84,532/85,048 KiB and 90,892/89,576 KiB. Mixed results under uncontrolled desktop load establish neither a speedup nor a definite regression. Imports/bundling and complete CLI/native process-family costs are excluded. Current-head hosted Linux executes all 16 jadx controls successfully (12.453 s), including cancellation of an in-flight Java readiness probe and joining cancelled Java until its owned child exits. These are actual Linux passes, independent of the 16 POSIX-only local skips. Earlier validation on the immediately preceding `31d6970b` revision also passed 148 compiled public/Node/schema controls across nine files; that historical count is not added to the current-head totals. Current-head hosted status: SUCCESS on this exact revision: https://github.com/morluto/rea/actions/runs/39350036839. All five portable test shards and CI required passed: 10,251 tests passed, zero failed, 162 existing skips; 1,025 test files passed. Source tests, static checks, build, docs, real JavaScript recovery, Apple artifact E2E, Windows package/capability checks and all three native package baselines passed. Independently scheduled sigrok/tshark checks also passed. Scope-excluded lanes remain skipped and are not counted as passes. PR readiness is SUCCESS (https://github.com/morluto/rea/actions/runs/39350091288). GitHub reports mergeable=true and mergeable_state=clean against upstream 83313650.. The four reviewed inline threads were replied to with evidence and resolved at `31d6970b`. The typed-store follow-up also addresses the older outside-diff finding in https://github.com/morluto/rea/pull/4941#issuecomment-6263011538. No analyzed target or real Electron application is executed by the public analysis fixtures. Authored Node oracle fixtures establish only the native semantics they exercise. Real provider execution, complete CLI process-family costs and token usage remain unmeasured unless explicitly reported above. Prepared with AI assistance; the original human initial-readiness declaration is preserved. ## Author readiness Human owner: @ShuaixinHuang The human owner explicitly confirmed review, evidence, responsibility and initial readiness for this PR on 2026-10-18. - [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-11-01 21:42:16 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:47:30 +00:00

[PR #6683] test(cli): cover production doctor output option ordering #7389

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

No description provided.

Rows
Columns