morluto/rea

Fork 0

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

[PR #7131] [MERGED] fix(javascript): close member-path cycles through self stores #7662

New issue

Closed

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

bsakaguchi commented

2026-11-03 13:39:28 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/7131
Author: @kaoru0822-kitauji
Created: 10/25/2026
Status: ✅ Merged
Merged: 10/25/2026
Merged by: @kaoru0822-kitauji

Base: main ← Head: fix/js-heap-stylus

📝 Commits (1)

448b83d fix(javascript): close member-path cycles through self stores

📊 Changes

3 files changed (+196 additions, -24 deletions)

View changed files

📝 src/domain/javascript/javascriptSemanticMemberMutations.ts (+42 -23)
📝 src/domain/javascript/javascriptSemanticMutationCycles.test.ts (+93 -1)
➕ tests/boundary/cli/javascriptSelfStoreCycles.test.ts (+61 -0)

📄 Description

What broke: Since 6.8.0, a write below a slot that stores a value read from the same slot, such as s.c = s.c.p; s.c.i = 1 on a parameter, makes member-mutation indexing loop forever and exhaust the worker heap. CodeMirror 5's mode/stylus/stylus.js (42 KB) returns resource_constraint after 20 s and 1.19 GB.
Fix: The cycle closure in markValue already escaped a segment repeated at the front of the path (node = node[key]). It now also recognizes a segment repeated after a stable base, [c] + p^k + [i], and escapes through it once.
Risk: Low. Front-insertion cycles keep their order and result. A differential over 4,299 sources found no output differences, and timing was unchanged.

Summary

Fixes #7130

Regressing commit: eb721ebbf fix(javascript): retain stored receiver capture authority (first released in 6.8.0). It made markValue follow receiver.storedSources for a write below a stored slot. Each call can see the previous call's store, so s.c may legitimately be c0.p^k for any k. The work-list therefore revisits binding s at [c,p,i], [c,p,p,i], …, and no path ever repeats. Each turn also copies the alias-history set, whose keys contain the whole path, so memory grows superlinearly.

Problem and expected behavior

Smallest trigger: function f(s) { s.c = s.c.p; s.c.i = 1; }. On main it runs out of a 512 MiB heap in about 9 s. d3a33fcfc (eb721ebbf^) finished it in 8 ms.
Violated invariant: a repeated alias/store transfer must close, either on an exact repeated path or through the existing widening when the repetition can never be pruned.
New behavior: the revisit is recognized and the receiver escapes once at base + segment, the same widening the self-alias rule uses. Analysis completes with coverage: complete.

Change and scope

markValue (javascriptSemanticMemberMutations.ts): on a revisited capture, the repeated-prefix search now also tries a contiguous segment removed after a nonzero base. Base 0, the existing case, is searched first and in the same order, so self-alias cycles behave exactly as before.
observableMutationPaths.repeatsUnpruned(bindingId, base, segment): projects the base, then repeats the segment until an origin signature repeats. It decides the escape only when normalization can never prune the repetition. Local objects whose projections run out of origins still terminate through normalization, unchanged.
Not included: #6127/#6146 and #2498 cover other member-mutation costs (distinct histories, generic bounds) and are not touched. pug.js (#7115/#7126) remains slow on main for its own reason.
Contribution scope: an ordinary fix within existing boundaries. A maintainer scope decision is not applicable.

Contract and boundary impact

Semantic owner and earliest changed stage: collectSemanticMemberMutationsSteps cycle closure (javascriptSemanticMemberMutations.ts). Consumers of member writes and escapes are unchanged.
Contribution scope fit and linked maintainer decision: not applicable (regression fix).
CLI and MCP/tool-catalog contract: none.
Provider, bridge, target-format, or platform compatibility: none.
Evidence, artifact, provenance, or reconstruction contract: no differences on the differential corpus. Inputs that previously exhausted the heap now produce complete results.
Target selection, authority, upstream behavior, and cleanup details: not applicable.
Generated metadata, package, or installation impact: none.

Expected effects:

Read existing files or captured data.
Launch or attach to a process.
Inject instrumentation or run scripts/code.
Change process memory, registers, files, or configuration.
Contact network services.
Recover protected code or work around protections.
Documentation or usability only; no target interaction.
Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

Tests added or updated:
- src/domain/javascript/javascriptSemanticMutationCycles.test.ts, six self-store shapes on a parameter: one key, a nested key, a computed key, a two-key segment, two alternating slots, and a CodeMirror-style context stack. Each asserts complete coverage. A getter on every MemberExpression.object fails the test after 5,000 reads, so a regression fails within about 2 s (measured 0.04–1.8 s on main) instead of exhausting the worker. With the fix the shapes need 332–652 reads. On main all six fail ("member-object reads exceeded"). A further case checks that a local stack popped in a loop still leaves seed.keep literal and the reachable slot unknown. It passes on both main and the fix.
- tests/boundary/cli/javascriptSelfStoreCycles.test.ts: the public CLI analyzes a 641-byte CodeMirror-style context stack with --max-heap-mb 256 and asserts parsed_javascript_files: 1, skipped_javascript_files: 0. With the fix it passes in 1.5 s. With the main build, the same fixture exhausts the 256 MiB worker heap in 4.1 s and reports parsed_javascript_files: 0, skipped_javascript_files: 1.
Base reproduction: listed in #7130 (bisect, work-list trace, V8 tick profile: 47% of ticks in markValue, most of the rest in semanticProjectedSourceFactsAtRead and GC).
Differential (main dist vs fix dist, same harness): ambient-write facts, analyzeParsedJavaScriptStaticSourceSteps output and the projected module semantic IR were compared per source. Inputs were 4,140 JS snippets extracted from the JavaScript/Electron test files, 24 synthetic and 16 real bundles, and all 121 codemirror@5.65.18 mode files. Result: 0 differences in 4,299 comparable sources. Two were not comparable: pug.js times out on both (the #7115 chain), and stylus.js aborts on main but completes with the fix. Five files that shared main's aborted chunk and eight that shared the pug chunk were rerun one at a time, and all were identical. Total time over the 4,299 sources was semantic 30.3 s → 30.5 s, static 18.2 s → 18.1 s, index 9.7 s → 9.7 s (noise).

Measurements, node scripts/rea.mjs analyze-javascript-application <dir> --format json --analysis-timeout-ms 120000, /usr/bin/time -l (CPU = user+sys; RSS = the largest single process), macOS arm64, Node 26.5.0:

Input	main	fix
`codemirror@5.65.18` `mode/stylus/stylus.js` (42,246 B)	`resource_constraint` (1024 MiB heap), 20.3 s wall, 24.3 s CPU, 1.19 GB RSS	complete, 2.5–3.8 s wall, 2.4 s CPU, 390–397 MB RSS (2 runs)
641 B context-stack fixture, `--max-heap-mb 256`	`resource_constraint`, 4.1 s, 354 MB	complete (CLI test, 1.5 s)

In-process semantic phase for stylus.js: did not finish on main within a 1.5 GiB heap, versus 46 ms with the fix.

Remaining proof gaps: Linux/Windows not run. Agent token cost is not affected (the output shape is unchanged).

For boundary fixes: this changes analysis termination inside the semantic owner. Lifecycle stages are unaffected. The CLI test covers the worker heap boundary through the public workflow.

For evidence-bearing changes:

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 performed

npm run test:local -- <211 JavaScript/Electron source test files>: 5,047 passed, 1 skipped, 1 failed. The failure is src/inspector/JavaScriptRuntimeScope.cancellation.test.ts ("stops Inspector target authorization after the current stat settles"), which also fails locally on unmodified main. It is an Inspector adapter test that this change does not touch.
npm run test:focused -- tests/boundary/cli/javascript*.test.ts (13 files, including the new one): 52 passed.
npm run test:local -- javascriptSemanticMutationCycles / member_mutations / MemberMutationCoverage: 233 passed.
New tests against unmodified source: the 6 self-store cases fail. The CLI fixture against the main build reports the heap exhaustion.
npm run check:fast: passed. npm run format:check: passed.

Compatibility, safety, and release

Breaking changes or migration steps: none.
Real Hopper/Ghidra, browser, or OS coverage: not applicable. macOS arm64 only.
Package or release metadata impact: none.
Security, privacy, process, or containment review: no new process, file or network behavior. The package under test was read only as data; none of its scripts were run.

Author readiness

Human owner: @kaoru0822-kitauji

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.

🤖 Generated with Claude Code

_{🔄 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/7131 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/25/2026 **Status:** ✅ Merged **Merged:** 10/25/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `fix/js-heap-stylus` --- ### 📝 Commits (1) - [`448b83d`](https://github.com/morluto/rea/commit/448b83d73216a308f8ee1da5d9d7f86fed0c7ca4) fix(javascript): close member-path cycles through self stores ### 📊 Changes **3 files changed** (+196 additions, -24 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/javascript/javascriptSemanticMemberMutations.ts` (+42 -23) 📝 `src/domain/javascript/javascriptSemanticMutationCycles.test.ts` (+93 -1) ➕ `tests/boundary/cli/javascriptSelfStoreCycles.test.ts` (+61 -0) </details> ### 📄 Description **What broke:** Since 6.8.0, a write below a slot that stores a value read from the same slot, such as `s.c = s.c.p; s.c.i = 1` on a parameter, makes member-mutation indexing loop forever and exhaust the worker heap. CodeMirror 5's `mode/stylus/stylus.js` (42 KB) returns `resource_constraint` after 20 s and 1.19 GB. **Fix:** The cycle closure in `markValue` already escaped a segment repeated at the front of the path (`node = node[key]`). It now also recognizes a segment repeated after a stable base, `[c] + p^k + [i]`, and escapes through it once. **Risk:** Low. Front-insertion cycles keep their order and result. A differential over 4,299 sources found no output differences, and timing was unchanged. ## Summary Fixes #7130 Regressing commit: eb721ebbf `fix(javascript): retain stored receiver capture authority` (first released in 6.8.0). It made `markValue` follow `receiver.storedSources` for a write below a stored slot. Each call can see the previous call's store, so `s.c` may legitimately be `c0.p^k` for any k. The work-list therefore revisits binding `s` at `[c,p,i]`, `[c,p,p,i]`, …, and no path ever repeats. Each turn also copies the alias-history set, whose keys contain the whole path, so memory grows superlinearly. ## Problem and expected behavior - Smallest trigger: `function f(s) { s.c = s.c.p; s.c.i = 1; }`. On main it runs out of a 512 MiB heap in about 9 s. `d3a33fcfc` (eb721ebbf^) finished it in 8 ms. - Violated invariant: a repeated alias/store transfer must close, either on an exact repeated path or through the existing widening when the repetition can never be pruned. - New behavior: the revisit is recognized and the receiver escapes once at `base + segment`, the same widening the self-alias rule uses. Analysis completes with `coverage: complete`. ## Change and scope - `markValue` (`javascriptSemanticMemberMutations.ts`): on a revisited capture, the repeated-prefix search now also tries a contiguous segment removed after a nonzero base. Base 0, the existing case, is searched first and in the same order, so self-alias cycles behave exactly as before. - `observableMutationPaths.repeatsUnpruned(bindingId, base, segment)`: projects the base, then repeats the segment until an origin signature repeats. It decides the escape only when normalization can never prune the repetition. Local objects whose projections run out of origins still terminate through normalization, unchanged. - Not included: #6127/#6146 and #2498 cover other member-mutation costs (distinct histories, generic bounds) and are not touched. `pug.js` (#7115/#7126) remains slow on main for its own reason. - Contribution scope: an ordinary fix within existing boundaries. A maintainer scope decision is not applicable. ## Contract and boundary impact - Semantic owner and earliest changed stage: `collectSemanticMemberMutationsSteps` cycle closure (`javascriptSemanticMemberMutations.ts`). Consumers of member writes and escapes are unchanged. - Contribution scope fit and linked maintainer decision: not applicable (regression fix). - CLI and MCP/tool-catalog contract: none. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: no differences on the differential corpus. Inputs that previously exhausted the heap now produce complete results. - Target selection, authority, upstream behavior, and cleanup details: not applicable. - Generated metadata, package, or installation impact: none. Expected effects: - [x] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage - Tests added or updated: - `src/domain/javascript/javascriptSemanticMutationCycles.test.ts`, six self-store shapes on a parameter: one key, a nested key, a computed key, a two-key segment, two alternating slots, and a CodeMirror-style context stack. Each asserts complete coverage. A getter on every `MemberExpression.object` fails the test after 5,000 reads, so a regression fails within about 2 s (measured 0.04–1.8 s on main) instead of exhausting the worker. With the fix the shapes need 332–652 reads. On main all six fail ("member-object reads exceeded"). A further case checks that a local stack popped in a loop still leaves `seed.keep` literal and the reachable slot unknown. It passes on both main and the fix. - `tests/boundary/cli/javascriptSelfStoreCycles.test.ts`: the public CLI analyzes a 641-byte CodeMirror-style context stack with `--max-heap-mb 256` and asserts `parsed_javascript_files: 1, skipped_javascript_files: 0`. With the fix it passes in 1.5 s. With the main build, the same fixture exhausts the 256 MiB worker heap in 4.1 s and reports `parsed_javascript_files: 0, skipped_javascript_files: 1`. - Base reproduction: listed in #7130 (bisect, work-list trace, V8 tick profile: 47% of ticks in `markValue`, most of the rest in `semanticProjectedSourceFactsAtRead` and GC). - Differential (main dist vs fix dist, same harness): ambient-write facts, `analyzeParsedJavaScriptStaticSourceSteps` output and the projected module semantic IR were compared per source. Inputs were 4,140 JS snippets extracted from the JavaScript/Electron test files, 24 synthetic and 16 real bundles, and all 121 `codemirror@5.65.18` mode files. Result: **0 differences in 4,299 comparable sources.** Two were not comparable: `pug.js` times out on both (the #7115 chain), and `stylus.js` aborts on main but completes with the fix. Five files that shared main's aborted chunk and eight that shared the pug chunk were rerun one at a time, and all were identical. Total time over the 4,299 sources was semantic 30.3 s → 30.5 s, static 18.2 s → 18.1 s, index 9.7 s → 9.7 s (noise). - Measurements, `node scripts/rea.mjs analyze-javascript-application <dir> --format json --analysis-timeout-ms 120000`, `/usr/bin/time -l` (CPU = user+sys; RSS = the largest single process), macOS arm64, Node 26.5.0: | Input | main | fix | |---|---|---| | `codemirror@5.65.18` `mode/stylus/stylus.js` (42,246 B) | `resource_constraint` (1024 MiB heap), 20.3 s wall, 24.3 s CPU, 1.19 GB RSS | complete, 2.5–3.8 s wall, 2.4 s CPU, 390–397 MB RSS (2 runs) | | 641 B context-stack fixture, `--max-heap-mb 256` | `resource_constraint`, 4.1 s, 354 MB | complete (CLI test, 1.5 s) | In-process semantic phase for stylus.js: did not finish on main within a 1.5 GiB heap, versus 46 ms with the fix. - Remaining proof gaps: Linux/Windows not run. Agent token cost is not affected (the output shape is unchanged). For boundary fixes: this changes analysis termination inside the semantic owner. Lifecycle stages are unaffected. The CLI test covers the worker heap boundary through the public workflow. For evidence-bearing changes: - [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 performed - `npm run test:local -- <211 JavaScript/Electron source test files>`: 5,047 passed, 1 skipped, 1 failed. The failure is `src/inspector/JavaScriptRuntimeScope.cancellation.test.ts` ("stops Inspector target authorization after the current stat settles"), which also fails locally on unmodified main. It is an Inspector adapter test that this change does not touch. - `npm run test:focused -- tests/boundary/cli/javascript*.test.ts` (13 files, including the new one): 52 passed. - `npm run test:local -- javascriptSemanticMutationCycles / member_mutations / MemberMutationCoverage`: 233 passed. - New tests against unmodified source: the 6 self-store cases fail. The CLI fixture against the main build reports the heap exhaustion. - `npm run check:fast`: passed. `npm run format:check`: passed. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: not applicable. macOS arm64 only. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no new process, file or network behavior. The package under test was read only as data; none of its scripts were run. ## Author readiness Human owner: @kaoru0822-kitauji - [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. - [ ] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-11-03 13:39:28 +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#7662

No description provided.

Rows
Columns