morluto/rea

Fork 0

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

[PR #6986] [MERGED] fix(javascript): bound cyclic mutation captures by their paths #7548

New issue

Closed

opened 2026-11-03 13:38:38 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-03 13:38:38 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6986
Author: @gensokyo-dev
Created: 10/24/2026
Status: ✅ Merged
Merged: 10/24/2026
Merged by: @gensokyo-dev

Base: main ← Head: fix/javascript-cycle-closure-20261024

📝 Commits (1)

cdbe1d5 fix(javascript): bound cyclic mutation captures by their paths

📊 Changes

3 files changed (+158 additions, -67 deletions)

View changed files

📝 src/domain/javascript/javascriptSemanticMemberMutations.ts (+79 -57)
➕ src/domain/javascript/javascriptSemanticPropertyPaths.test.ts (+58 -0)
📝 src/domain/javascript/javascriptSemanticPropertyPaths.ts (+21 -10)

📄 Description

Problem and outcome

Main Source tests repeatedly exit in javascriptSemanticAnalysis.reference_lifetimes.test.ts (3ce311757 run, e3a6ce40f run). A cyclic parameter alias grows member-read capture history. The mutation collector puts that entire history outside its property-path prefix index, so a previously escaped receiver cannot cover a longer path with an additional capture suffix. Recursive widening restarts this work and exhausts the worker.

Index capture identity at each property-path edge, retaining distinct receiver captures, copied-slot fences, selectors and equal-depth write identity. Register the normalized prefix before recursive effect expansion so it owns its closure. Reuse the existing shared domain index; remove the superseded whole-capture-vector key from effect coverage. No timeout, heap limit, retry or correctness/performance assertion is relaxed. CLI, MCP and providers share the same analysis implementation; public schemas are unchanged. No separate issue was opened for this direct main-CI repair.

Executed correctness and resource checks

Linux x64, pinned Node 24.22.1 / npm 11.16.0, two CPUs. Tests below used merged base 48c203f35 plus this diff:

Original source-file runs crossed a 1.5 GiB sampled family stop before completing. A bounded diagnostic isolated cyclic member paths; the initial worker stderr marker was asynchronous and was replaced with synchronous markers before attributing the failing case. These stopped runs are not completed benchmarks, and their child CPU accounting is incomplete. No OOM was needed to reproduce the defect.
All original 113 reference-lifetime tests passed, preserving the 2 s assertions (6.41 s command wall / 6.41 s CPU / 572.2 MiB sampled family RSS). The formerly unbounded parameter cycle completed in 728 ms; the nine-way member-write cases also passed.
732 tests passed across 14 complete files, including six new index regressions and existing capture lifetimes, copy exclusions, stored receivers, getter/setter mutation, module interop, typed arrays and externally supplied parameter/catch values (19.10 s wall / 23.51 s CPU / 678.2 MiB).
Compiled real CLI + stdio MCP analyses of the 613-byte cyclic source and the unchanged 2770-byte installed dot-prop 5.3.0 source: passed named output schemas, CLI/MCP parent Evidence identity, retained summary/page/item lookups, source SHA-256 and unchanged-input checks. CLI summary delivery published complete Evidence bundles; unknowns remained explicit (38 and 109 respectively). This reads self-authored and real library source without executing targets. 18.88 s wall / 23.61 s CPU / 903.1 MiB for the completed two-target workflow; fresh CLI processes and one fresh MCP session with a warm second call.
npm run check and git diff --check passed (37.41 s wall / 66.38 s CPU / 2669.5 MiB). Required runtime build passed (38.91 s wall / 49.05 s CPU / 1100.0 MiB).

RSS sums descendants sampled every 100 ms; shared pages may double-count and short-lived/reparented children may be missed. Source probes and public verification had a 1.5 GiB sampled stop; build/static checks had an 8 GiB sampled stop and all commands had elapsed guards. No OOM occurred. Warm build artifacts were reused where stated. The failing baseline did not complete, so no equivalent-task speedup percentage is claimed. Agent token costs were not measured. Raw diagnostics remain outside the repository.

The branch is rebased onto latest main before publication. CI will verify that integrated head, including unrelated concurrent changes. Main CI must still finish successfully after merge; this PR does not claim to repair the earlier one-off Chrome startup timeout or parent-capacity observation that did not recur in #6978's eight successful built shards.

Completed CI and remaining main repair

CI 40083831021 completed on exact head cdbe1d5d6: static checks, build, all eight built shards, selected real checks and the complete 113-case reference-lifetime file passed (614 ms for that file). Source execution now completes all 970 files instead of losing its worker: 17585 passed / 1 failed / 13 skipped. Its sole failure is agentEvaluationReport.test.ts reporting Runner failure gate missing; a clean pre-change worktree d3044a2df reproduces that same failure (1 failed / 5 passed). The predicate test still assumes a top-level try block after verifier cleanup moved the body into an async callback. Portable/CI required propagate this independent failure, so this PR's overall CI is not green; the next isolated main-health change repairs that test.

Codex code review completed on cdbe1d5d6 with no findings or unresolved threads. PR title and readiness passed. Main has advanced with unrelated native/release changes; final post-merge main qualification remains required for the maintenance task.

Author readiness

Human owner: @gensokyo-dev

The owner explicitly delegated readiness declarations and agent-managed review. Inspection and execution were performed by Codex; no separate human inspection is claimed.

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/6986 **Author:** [@gensokyo-dev](https://github.com/gensokyo-dev) **Created:** 10/24/2026 **Status:** ✅ Merged **Merged:** 10/24/2026 **Merged by:** [@gensokyo-dev](https://github.com/gensokyo-dev) **Base:** `main` ← **Head:** `fix/javascript-cycle-closure-20261024` --- ### 📝 Commits (1) - [`cdbe1d5`](https://github.com/morluto/rea/commit/cdbe1d5d6dc5b421754b8d9c960b7cce39603b47) fix(javascript): bound cyclic mutation captures by their paths ### 📊 Changes **3 files changed** (+158 additions, -67 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/javascript/javascriptSemanticMemberMutations.ts` (+79 -57) ➕ `src/domain/javascript/javascriptSemanticPropertyPaths.test.ts` (+58 -0) 📝 `src/domain/javascript/javascriptSemanticPropertyPaths.ts` (+21 -10) </details> ### 📄 Description ## Problem and outcome Main Source tests repeatedly exit in `javascriptSemanticAnalysis.reference_lifetimes.test.ts` ([3ce311757 run](https://github.com/morluto/rea/actions/runs/40077564438/job/120824312850), [e3a6ce40f run](https://github.com/morluto/rea/actions/runs/40079461941/job/120829870212)). A cyclic parameter alias grows member-read capture history. The mutation collector puts that entire history outside its property-path prefix index, so a previously escaped receiver cannot cover a longer path with an additional capture suffix. Recursive widening restarts this work and exhausts the worker. Index capture identity **at each property-path edge**, retaining distinct receiver captures, copied-slot fences, selectors and equal-depth write identity. Register the normalized prefix before recursive effect expansion so it owns its closure. Reuse the existing shared domain index; remove the superseded whole-capture-vector key from effect coverage. No timeout, heap limit, retry or correctness/performance assertion is relaxed. CLI, MCP and providers share the same analysis implementation; public schemas are unchanged. No separate issue was opened for this direct main-CI repair. ## Executed correctness and resource checks Linux x64, pinned Node 24.22.1 / npm 11.16.0, two CPUs. Tests below used merged base `48c203f35` plus this diff: - Original source-file runs crossed a **1.5 GiB sampled family stop** before completing. A bounded diagnostic isolated cyclic member paths; the initial worker stderr marker was asynchronous and was replaced with synchronous markers before attributing the failing case. These stopped runs are not completed benchmarks, and their child CPU accounting is incomplete. No OOM was needed to reproduce the defect. - All original **113 reference-lifetime tests passed**, preserving the 2 s assertions (6.41 s command wall / 6.41 s CPU / 572.2 MiB sampled family RSS). The formerly unbounded parameter cycle completed in 728 ms; the nine-way member-write cases also passed. - **732 tests passed across 14 complete files**, including six new index regressions and existing capture lifetimes, copy exclusions, stored receivers, getter/setter mutation, module interop, typed arrays and externally supplied parameter/catch values (19.10 s wall / 23.51 s CPU / 678.2 MiB). - Compiled **real CLI + stdio MCP** analyses of the 613-byte cyclic source and the unchanged 2770-byte installed `dot-prop` 5.3.0 source: passed named output schemas, CLI/MCP parent Evidence identity, retained summary/page/item lookups, source SHA-256 and unchanged-input checks. CLI summary delivery published complete Evidence bundles; unknowns remained explicit (38 and 109 respectively). This reads self-authored and real library source without executing targets. 18.88 s wall / 23.61 s CPU / 903.1 MiB for the completed two-target workflow; fresh CLI processes and one fresh MCP session with a warm second call. - `npm run check` and `git diff --check` passed (37.41 s wall / 66.38 s CPU / 2669.5 MiB). Required runtime build passed (38.91 s wall / 49.05 s CPU / 1100.0 MiB). RSS sums descendants sampled every 100 ms; shared pages may double-count and short-lived/reparented children may be missed. Source probes and public verification had a 1.5 GiB sampled stop; build/static checks had an 8 GiB sampled stop and all commands had elapsed guards. No OOM occurred. Warm build artifacts were reused where stated. The failing baseline did not complete, so no equivalent-task speedup percentage is claimed. Agent token costs were not measured. Raw diagnostics remain outside the repository. The branch is rebased onto latest main before publication. CI will verify that integrated head, including unrelated concurrent changes. Main CI must still finish successfully after merge; this PR does not claim to repair the earlier one-off Chrome startup timeout or parent-capacity observation that did not recur in #6978's eight successful built shards. ## Completed CI and remaining main repair [CI 40083831021](https://github.com/morluto/rea/actions/runs/40083831021) completed on exact head `cdbe1d5d6`: static checks, build, **all eight built shards**, selected real checks and the complete 113-case reference-lifetime file passed (614 ms for that file). Source execution now completes all 970 files instead of losing its worker: **17585 passed / 1 failed / 13 skipped**. Its sole failure is `agentEvaluationReport.test.ts` reporting `Runner failure gate missing`; a clean pre-change worktree `d3044a2df` reproduces that same failure (1 failed / 5 passed). The predicate test still assumes a top-level try block after verifier cleanup moved the body into an async callback. Portable/CI required propagate this independent failure, so this PR's overall CI is **not green**; the next isolated main-health change repairs that test. Codex code review completed on `cdbe1d5d6` with no findings or unresolved threads. PR title and readiness passed. Main has advanced with unrelated native/release changes; final post-merge main qualification remains required for the maintenance task. ## Author readiness Human owner: @gensokyo-dev The owner explicitly delegated readiness declarations and agent-managed review. Inspection and execution were performed by Codex; no separate human inspection is claimed. - [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-03 13:38:38 +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#7548

No description provided.

Rows
Columns