morluto/rea

Fork 0

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

[PR #4341] [MERGED] fix(javascript): preserve guarded factory fallthrough #6185

New issue

Closed

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

bsakaguchi commented

2026-10-31 01:12:31 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4341
Author: @N0zoM1z0
Created: 10/16/2026
Status: ✅ Merged
Merged: 10/16/2026
Merged by: @N0zoM1z0

Base: main ← Head: fix/javascript-factory-fallthrough-3963

📝 Commits (6)

316e45e fix(javascript): preserve guarded factory fallthrough
f3a570c fix(javascript): retain uncertainty for unhandled factory exits
7f19cd8 fix(javascript): bind namespace reads to source boundaries
5b257c3 fix(javascript): preserve completion ownership across aliases and calls
4c28734 fix(javascript): retain common export facts across factory exits
4f98121 fix(javascript): share invocation semantics and registration proofs

📊 Changes

8 files changed (+1292 additions, -131 deletions)

View changed files

📝 docs/javascript-artifact-reconstruction.md (+23 -0)
📝 src/domain/javascript/electronWrappedCallPaths.test.ts (+21 -0)
📝 src/domain/javascript/electronWrappedCallPaths.ts (+44 -3)
📝 src/domain/javascript/javascriptSymbolicBindings.test.ts (+184 -1)
📝 src/domain/javascript/javascriptSymbolicBindings.ts (+499 -124)
📝 src/domain/javascript/javascriptSymbolicControlFlow.ts (+2 -0)
📝 src/domain/javascript/javascriptSymbolicValues.ts (+22 -3)
➕ tests/acceptance/applications/javascriptFactoryCompletion.test.ts (+497 -0)

📄 Description

Summary

Fixes #3963. A factory containing if (false) return; before an Electron wrapper lost its literal IPC handler. Unknown guards lost fallthrough too. Preserve per-exit initialization: false guards recover the handler, unknown guards retain a !(flag) candidate with provenance/partial coverage, and definite exits cannot expose later state. Exports common to every proven exit remain unconditional. Guarded candidates intentionally do not increase unconditional IPC counts.

The shared symbolic owner reuses bounded return-flow analysis and source-positioned export writes, caching completion within one analysis. Completed aliases keep activation identity; lexical capture, synchronous callers and snapshot positions stay distinct. Promise/iterator identity and call-body semantics share that owner. The path consumer reuses registration proofs only for the same entry and trace, preserving separately guarded API entries and callback traversal. CLI/MCP schemas and catalog are unchanged.

Executed validation

1,933 tests /81 files passed: all 79 JavaScript domain files and two public acceptance files. Includes 39 factory scenarios, 4 invocation scenarios, CLI/stdio MCP Evidence-ID parity, authored Node VM oracles, cross-entry ordering, cache reuse and resource-bound expansion.
npm run check, npm run docs:check (including 5 tests), and 1,413-file test-discovery passed. Node 24.18/npm 11.16, serial two-core jobs; final static-check peak family RSS 4.64 GiB, no OOM/guard termination.
Frozen actual CLI controls reproduce the reviewed alias loss, impossible helper candidate and common-export regression. The common-export fixture yields 1 literal channel on main 1b97b0f37, 0 at prior head 5b257c3e8, and 1 after repair; Node registers for either flag.
Real artifact: official Electron 44.7.0 Linux archive matched its published GitHub asset SHA-256. Inert default_app.asar analysis parsed 3 JavaScript files, retained 43 findings/55 graph nodes and the same IPC counts, without parse failures. CLI and MCP complete Evidence agree within each version. One file's coverage becomes partial; source identities/properties otherwise match. Electron was not launched.
Exact-head Codex/Greptile reviews completed with no remaining findings. Final CI: 19 checks passed /16 intentionally skipped, with all five portable shards and required aggregation green.

Completed-task costs

Base 1b97b0f37 vs final 4f981211c, same artifacts. Each task lists the full catalog, runs cold/warm summary analyses, retrieves the exact parent via a filtered Evidence summary, then exports/verifies full Evidence locally (four tool calls). Cold includes startup/catalog; warm reanalyzes.

Workflow	Wall s	Family CPU s	Peak family MiB	Cold / warm s	Context proxy tokens
Control, two crossover runs/version, medians	18.57→19.18	22.72→23.63	1081.6→1143.5	13.00/4.72→13.45/4.83	809,995→809,995
Common pre-exit export, one pair	18.00→18.18	22.09→22.24	1075.9→1069.7	12.54/4.59→12.67/4.67	809,900→809,900
Real ASAR, one pair	12.60→12.13	15.16→14.67	947.1→929.3	10.54/1.61→10.06/1.61	810,466→810,423

Control and common-export full normalized Evidence/catalogs are exactly equal. The common-export prototype's redundant 236-node graph is back to 44, preserving separate entry semantics. The earlier six-run control block spans a shared-host timing transition (~37s to ~18s in both versions); all ten raw control runs are retained, and no general speedup is claimed.

The cl100k_base proxy counts the full catalog (798,917) and every request/SDK JSON reply, including text/structured duplication. It is not model billing. Initialization/wire framing and exported file contents are unmeasured/excluded. Sampled RSS can double-count shared pages or miss short processes. Single pairs are descriptive.

Limits: Linux inert-source verification with authored Electron stubs and a real static ASAR; no real Electron engine/non-Linux claim. Unsupported control flow, conditional/dynamic mutation and Promise/iterator member protocols remain conservative/unknown. Explicit generator advancement is partial, not proof of absent behavior. No new execution, installation, cleanup or provider authority.

Author readiness

Human owner: @N0zoM1z0. The human owner explicitly authorized delegated review, proxy completion of these declarations, ready publication and normal merge. The agent reviewed the diff/evidence; this records that delegation and does not assert a separate manual review.

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 or an agreed enhancement.
Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
The PR has one focused outcome and the title follows type(scope): outcome.
Related issue is linked, or the reason for not linking one is stated above.
Tests cover changed observable behavior and meaningful failure paths.
Owning docs, contracts, and generated metadata are updated where needed.
User-visible CLI/MCP changes include representative output.
I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

_{🔄 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/4341 **Author:** [@N0zoM1z0](https://github.com/N0zoM1z0) **Created:** 10/16/2026 **Status:** ✅ Merged **Merged:** 10/16/2026 **Merged by:** [@N0zoM1z0](https://github.com/N0zoM1z0) **Base:** `main` ← **Head:** `fix/javascript-factory-fallthrough-3963` --- ### 📝 Commits (6) - [`316e45e`](https://github.com/morluto/rea/commit/316e45e9f46c1ea832a13212dbee11c90b8fac36) fix(javascript): preserve guarded factory fallthrough - [`f3a570c`](https://github.com/morluto/rea/commit/f3a570cd7a14682054ca0cff1ec9467db0b07065) fix(javascript): retain uncertainty for unhandled factory exits - [`7f19cd8`](https://github.com/morluto/rea/commit/7f19cd8cdb97958c81a9cc69b1a5e89d38d9567a) fix(javascript): bind namespace reads to source boundaries - [`5b257c3`](https://github.com/morluto/rea/commit/5b257c3e8fa1720c44173acba9f70cb9d0c51afc) fix(javascript): preserve completion ownership across aliases and calls - [`4c28734`](https://github.com/morluto/rea/commit/4c28734da5f63fe01f3f4e1a36edacbabeeca3d0) fix(javascript): retain common export facts across factory exits - [`4f98121`](https://github.com/morluto/rea/commit/4f981211c70c477c1984cb25668cdcd150bc0eb5) fix(javascript): share invocation semantics and registration proofs ### 📊 Changes **8 files changed** (+1292 additions, -131 deletions) <details> <summary>View changed files</summary> 📝 `docs/javascript-artifact-reconstruction.md` (+23 -0) 📝 `src/domain/javascript/electronWrappedCallPaths.test.ts` (+21 -0) 📝 `src/domain/javascript/electronWrappedCallPaths.ts` (+44 -3) 📝 `src/domain/javascript/javascriptSymbolicBindings.test.ts` (+184 -1) 📝 `src/domain/javascript/javascriptSymbolicBindings.ts` (+499 -124) 📝 `src/domain/javascript/javascriptSymbolicControlFlow.ts` (+2 -0) 📝 `src/domain/javascript/javascriptSymbolicValues.ts` (+22 -3) ➕ `tests/acceptance/applications/javascriptFactoryCompletion.test.ts` (+497 -0) </details> ### 📄 Description ## Summary Fixes #3963. A factory containing `if (false) return;` before an Electron wrapper lost its literal IPC handler. Unknown guards lost fallthrough too. Preserve per-exit initialization: false guards recover the handler, unknown guards retain a `!(flag)` candidate with provenance/partial coverage, and definite exits cannot expose later state. Exports common to every proven exit remain unconditional. Guarded candidates intentionally do not increase unconditional IPC counts. The shared symbolic owner reuses bounded return-flow analysis and source-positioned export writes, caching completion within one analysis. Completed aliases keep activation identity; lexical capture, synchronous callers and snapshot positions stay distinct. Promise/iterator identity and call-body semantics share that owner. The path consumer reuses registration proofs only for the same entry and trace, preserving separately guarded API entries and callback traversal. CLI/MCP schemas and catalog are unchanged. ## Executed validation - **1,933 tests /81 files passed**: all 79 JavaScript domain files and two public acceptance files. Includes 39 factory scenarios, 4 invocation scenarios, CLI/stdio MCP Evidence-ID parity, authored Node VM oracles, cross-entry ordering, cache reuse and resource-bound expansion. - `npm run check`, `npm run docs:check` (including 5 tests), and 1,413-file test-discovery passed. Node 24.18/npm 11.16, serial two-core jobs; final static-check peak family RSS **4.64 GiB**, no OOM/guard termination. - Frozen actual CLI controls reproduce the reviewed alias loss, impossible helper candidate and common-export regression. The common-export fixture yields 1 literal channel on main `1b97b0f37`, 0 at prior head `5b257c3e8`, and 1 after repair; Node registers for either flag. - Real artifact: official [Electron 44.7.0](https://github.com/electron/electron/releases/tag/v44.7.0) Linux archive matched its published GitHub asset SHA-256. Inert default_app.asar analysis parsed 3 JavaScript files, retained 43 findings/55 graph nodes and the same IPC counts, without parse failures. CLI and MCP complete Evidence agree within each version. One file's coverage becomes partial; source identities/properties otherwise match. Electron was not launched. - Exact-head Codex/Greptile reviews completed with no remaining findings. Final CI: **19 checks passed /16 intentionally skipped**, with all five portable shards and required aggregation green. ## Completed-task costs Base `1b97b0f37` vs final `4f981211c`, same artifacts. Each task lists the full catalog, runs cold/warm summary analyses, retrieves the exact parent via a filtered Evidence summary, then exports/verifies full Evidence locally (four tool calls). Cold includes startup/catalog; warm reanalyzes. | Workflow | Wall s | Family CPU s | Peak family MiB | Cold / warm s | Context proxy tokens | | --- | ---: | ---: | ---: | ---: | ---: | | Control, two crossover runs/version, medians |18.57→19.18|22.72→23.63|1081.6→1143.5|13.00/4.72→13.45/4.83|809,995→809,995| | Common pre-exit export, one pair |18.00→18.18|22.09→22.24|1075.9→1069.7|12.54/4.59→12.67/4.67|809,900→809,900| | Real ASAR, one pair |12.60→12.13|15.16→14.67|947.1→929.3|10.54/1.61→10.06/1.61|810,466→810,423| Control and common-export full normalized Evidence/catalogs are exactly equal. The common-export prototype's redundant 236-node graph is back to 44, preserving separate entry semantics. The earlier six-run control block spans a shared-host timing transition (~37s to ~18s in both versions); all ten raw control runs are retained, and no general speedup is claimed. The `cl100k_base` proxy counts the full catalog (**798,917**) and every request/SDK JSON reply, including text/structured duplication. It is not model billing. Initialization/wire framing and exported file contents are unmeasured/excluded. Sampled RSS can double-count shared pages or miss short processes. Single pairs are descriptive. Limits: Linux inert-source verification with authored Electron stubs and a real static ASAR; no real Electron engine/non-Linux claim. Unsupported control flow, conditional/dynamic mutation and Promise/iterator member protocols remain conservative/unknown. Explicit generator advancement is partial, not proof of absent behavior. No new execution, installation, cleanup or provider authority. ## Author readiness Human owner: @N0zoM1z0. The human owner explicitly authorized delegated review, proxy completion of these declarations, ready publication and normal merge. The agent reviewed the diff/evidence; this records that delegation and does not assert a separate manual review. - [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 or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:12:31 +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#6185

No description provided.

Rows
Columns