morluto/rea

Fork 0

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

[PR #1767] [MERGED] fix(javascript): bound self-alias mutation paths that never prune #4805

New issue

Closed

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

bsakaguchi commented

2026-10-31 00:55:14 +00:00

Owner

📋 Pull Request Information

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

Base: main ← Head: fix/javascript-self-alias-mutation-paths

📝 Commits (2)

d036668 fix(javascript): bound self-alias mutation paths that never prune
f2dac07 perf(javascript): share repeated-prefix checks across alias revisits

📊 Changes

2 files changed (+113 additions, -11 deletions)

View changed files

📝 src/domain/javascript/javascriptSemanticAnalysis.reference_lifetimes.test.ts (+24 -0)
📝 src/domain/javascript/javascriptSemanticMemberMutations.ts (+89 -11)

📄 Description

Summary

Since the getter-receiver fix for #1665 (79569349), analyze_javascript_application can exhaust its semantic worker heap on code that walks a self alias such as node = node[key], or node = node.next on a class instance. jquery-3.2.0.min.js (Sizzle's :nth-child matcher) loses its whole semantic graph, even with --max-heap-mb 4096. This PR makes such cycles terminate while keeping every possibly affected fact unknown.

Follow-up to #1665. I did not open a separate issue because the PR carries the two-line reproduction below.

Problem and expected behavior

Smallest trigger:

var m = {};
while ((m = m[p])) m.nodeName.toLowerCase();

collectSemanticMemberMutationsSteps closes an alias cycle only when a binding is revisited at the same capture with exactly the same path. 79569349 added the path to that key so that getter receivers can revisit a capture with a shorter path. A self alias, however, prepends its projection on every cycle: ["nodeName"], then [null, "nodeName"], then [null, null, "nodeName"], and so on. The key therefore never repeats. When the binding's origin is opaque (a dynamic-key read or a class instance) or cyclic, observableMutationPaths keeps every path, so the traversal grows until the worker heap is exhausted.

Expected: the analysis terminates, and values that the loop may reach stay unknown.

Change and scope

The fix applies to a revisit whose earlier path on the same alias chain is a suffix of the current path. In that case observableMutationPaths.repeatsUnpruned projects the added prefix repeatedly. Projection origins depend only on the source origins and the key, so once an origin set repeats, normalization can never prune a longer path. The collector then records one escape at the repeated prefix and stops the cycle. That escape covers every longer write or escape, including writes below shared children. Finite cycles, where a projection runs out of origins, keep their exact paths as before.

Alternatives considered:

Closing the cycle on any longer revisit, as before 79569349, terminates but drops deeper writes. For example, alias = flag ? alias.child : alias; alias.value = 2 would then report shared.child.value as the stale literal 1. A new regression case covers this.
Escaping on every longer revisit is sound, but it also widens finite static-key loops. The existing case "bounds a loop that reassigns one reference through many members" would then lose its literal.

Known precision cost: a dynamic-key self alias escapes its repeated [null] prefix, so every child becomes unknown. In node = node[key]; node.name.toLowerCase(), seed.child.value is now unknown; before this change the analysis did not terminate. A static key without that slot keeps its literal.

Not included: no changes to the semantic graph budgets or the output size.

Contract and boundary impact

Semantic owner and earliest changed stage: domain JavaScript member-mutation collection (src/domain/javascript/javascriptSemanticMemberMutations.ts), before IR projection.
CLI and MCP/tool-catalog contract: none; schemas and tools are unchanged.
Provider, bridge, target-format, or platform compatibility: none.
Evidence, artifact, provenance, or reconstruction contract: affected effects are recorded as an escape at the repeated prefix, and values below it become unknown. Previously the file fell back to resource-limit unknowns for every semantic family.
Process execution, authorization, cleanup, or containment impact: none; the owned worker no longer exhausts its heap on these inputs.
Generated metadata (docs/public/product-catalog.json), package, or installation impact: none.

Evidence and regression coverage

Tests added or updated: src/domain/javascript/javascriptSemanticAnalysis.reference_lifetimes.test.ts
- Four self-alias shapes with a 2 s bound: dynamic key with receiver escape, call escape and write, and a class instance with a static-key write.
- One deeper-write soundness case (shared.child.value through repeated alias = flag ? alias.child : alias).
Base reproduction: running the self-alias cases on the base revision crashes the Vitest worker with heap exhaustion (NODE_OPTIONS=--max-old-space-size=768). The deeper-write case passes on the base revision; a probe build with a plain cycle cut reported the stale literal for this shape.
User-visible CLI/MCP output: rea analyze-javascript-application <dir containing jquery-3.2.0.min.js> --artifact-format directory --format json
- Before (21650455): the limitation Owned JavaScript worker exhausted its heap while analyzing jquery-3.2.0.min.js during semantic, and a semantic graph with only resource-limit unknowns. This took 20 s at the default heap, and 134 s with 4.5 GB RSS at --max-heap-mb 4096.
- After: completes in 5.5 s with 555 MB RSS, has no resource limitation, and produces 20,000 nodes and 27,947 relations. The semantic IR step alone takes 0.28 s with 186 MB RSS.
Remaining proof gaps: growth whose earlier path is not a suffix (cycles that strip and re-prepend keys) still relies on normalization for termination; I found no trigger for it. Directories that also contain other large bundles can now reach the shared semantic graph budget with jQuery, which is reported as a resource limit for the later file (existing behavior).

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

On macOS (Darwin 27.0, Apple Silicon), Node 24.18.0:

npm run test:fast — 375 files, 4274 passed, 7 skipped (before the rebase onto 062723e4; the intervening commits do not touch JavaScript analysis)
npx vitest run src/domain/javascript/ after the rebase — 43 files, 821 passed
npm run check — passed
npm run verify:test-discovery — 813 test files discovered exactly once
npm run build:cached and the CLI run above — passed

Review follow-up

f2dac079 caches the repeated-prefix check per binding and prefix; 7663df90 skips the suffix scan on first capture visits.
Measured (macOS, Node 24.18.0), finite self-alias chain of 200 links: 24.9 s on origin/main, about 27 s here. That cost predates this PR. 300 aliases × depth 300: 1.75 s on main, 1.73 s here.
npx vitest run src/domain/javascript/ — 821 passed; npm run check:fast — passed.

Compatibility, safety, and release

Breaking changes or migration steps: none.
Real Hopper/Ghidra, browser, or OS coverage: not applicable (static JavaScript analysis); macOS only.
Package or release metadata impact: none.
Security, privacy, process, or containment review: removes an unbounded memory path that a small untrusted script could trigger; no new process or file access.

Review checklist

This PR addresses a concrete problem or an agreed enhancement.
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/1767 **Author:** [@flitzrrr](https://github.com/flitzrrr) **Created:** 10/11/2026 **Status:** ✅ Merged **Merged:** 10/11/2026 **Merged by:** [@N0zoM1z0](https://github.com/N0zoM1z0) **Base:** `main` ← **Head:** `fix/javascript-self-alias-mutation-paths` --- ### 📝 Commits (2) - [`d036668`](https://github.com/morluto/rea/commit/d0366686acdca50b3f5349370bd18fb3a676148b) fix(javascript): bound self-alias mutation paths that never prune - [`f2dac07`](https://github.com/morluto/rea/commit/f2dac0792ce4b82b6f1d341d288785e14d69edd4) perf(javascript): share repeated-prefix checks across alias revisits ### 📊 Changes **2 files changed** (+113 additions, -11 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/javascript/javascriptSemanticAnalysis.reference_lifetimes.test.ts` (+24 -0) 📝 `src/domain/javascript/javascriptSemanticMemberMutations.ts` (+89 -11) </details> ### 📄 Description ## Summary Since the getter-receiver fix for #1665 (79569349), `analyze_javascript_application` can exhaust its semantic worker heap on code that walks a self alias such as `node = node[key]`, or `node = node.next` on a class instance. `jquery-3.2.0.min.js` (Sizzle's `:nth-child` matcher) loses its whole semantic graph, even with `--max-heap-mb 4096`. This PR makes such cycles terminate while keeping every possibly affected fact unknown. Follow-up to #1665. I did not open a separate issue because the PR carries the two-line reproduction below. ## Problem and expected behavior Smallest trigger: ```js var m = {}; while ((m = m[p])) m.nodeName.toLowerCase(); ``` `collectSemanticMemberMutationsSteps` closes an alias cycle only when a binding is revisited at the same capture with exactly the same path. 79569349 added the path to that key so that getter receivers can revisit a capture with a shorter path. A self alias, however, prepends its projection on every cycle: `["nodeName"]`, then `[null, "nodeName"]`, then `[null, null, "nodeName"]`, and so on. The key therefore never repeats. When the binding's origin is opaque (a dynamic-key read or a class instance) or cyclic, `observableMutationPaths` keeps every path, so the traversal grows until the worker heap is exhausted. Expected: the analysis terminates, and values that the loop may reach stay unknown. ## Change and scope The fix applies to a revisit whose earlier path on the same alias chain is a suffix of the current path. In that case `observableMutationPaths.repeatsUnpruned` projects the added prefix repeatedly. Projection origins depend only on the source origins and the key, so once an origin set repeats, normalization can never prune a longer path. The collector then records one escape at the repeated prefix and stops the cycle. That escape covers every longer write or escape, including writes below shared children. Finite cycles, where a projection runs out of origins, keep their exact paths as before. Alternatives considered: - Closing the cycle on any longer revisit, as before 79569349, terminates but drops deeper writes. For example, `alias = flag ? alias.child : alias; alias.value = 2` would then report `shared.child.value` as the stale literal `1`. A new regression case covers this. - Escaping on every longer revisit is sound, but it also widens finite static-key loops. The existing case "bounds a loop that reassigns one reference through many members" would then lose its literal. Known precision cost: a dynamic-key self alias escapes its repeated `[null]` prefix, so every child becomes unknown. In `node = node[key]; node.name.toLowerCase()`, `seed.child.value` is now unknown; before this change the analysis did not terminate. A static key without that slot keeps its literal. Not included: no changes to the semantic graph budgets or the output size. ## Contract and boundary impact - Semantic owner and earliest changed stage: domain JavaScript member-mutation collection (`src/domain/javascript/javascriptSemanticMemberMutations.ts`), before IR projection. - CLI and MCP/tool-catalog contract: none; schemas and tools are unchanged. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: affected effects are recorded as an escape at the repeated prefix, and values below it become unknown. Previously the file fell back to resource-limit unknowns for every semantic family. - Process execution, authorization, cleanup, or containment impact: none; the owned worker no longer exhausts its heap on these inputs. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact: none. ## Evidence and regression coverage - Tests added or updated: `src/domain/javascript/javascriptSemanticAnalysis.reference_lifetimes.test.ts` - Four self-alias shapes with a 2 s bound: dynamic key with receiver escape, call escape and write, and a class instance with a static-key write. - One deeper-write soundness case (`shared.child.value` through repeated `alias = flag ? alias.child : alias`). - Base reproduction: running the self-alias cases on the base revision crashes the Vitest worker with heap exhaustion (`NODE_OPTIONS=--max-old-space-size=768`). The deeper-write case passes on the base revision; a probe build with a plain cycle cut reported the stale literal for this shape. - User-visible CLI/MCP output: `rea analyze-javascript-application <dir containing jquery-3.2.0.min.js> --artifact-format directory --format json` - Before (21650455): the limitation `Owned JavaScript worker exhausted its heap while analyzing jquery-3.2.0.min.js during semantic`, and a semantic graph with only resource-limit unknowns. This took 20 s at the default heap, and 134 s with 4.5 GB RSS at `--max-heap-mb 4096`. - After: completes in 5.5 s with 555 MB RSS, has no resource limitation, and produces 20,000 nodes and 27,947 relations. The semantic IR step alone takes 0.28 s with 186 MB RSS. - Remaining proof gaps: growth whose earlier path is not a suffix (cycles that strip and re-prepend keys) still relies on normalization for termination; I found no trigger for it. Directories that also contain other large bundles can now reach the shared semantic graph budget with jQuery, which is reported as a resource limit for the later file (existing behavior). 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 On macOS (Darwin 27.0, Apple Silicon), Node 24.18.0: - `npm run test:fast` — 375 files, 4274 passed, 7 skipped (before the rebase onto 062723e4; the intervening commits do not touch JavaScript analysis) - `npx vitest run src/domain/javascript/` after the rebase — 43 files, 821 passed - `npm run check` — passed - `npm run verify:test-discovery` — 813 test files discovered exactly once - `npm run build:cached` and the CLI run above — passed ### Review follow-up - f2dac079 caches the repeated-prefix check per binding and prefix; 7663df90 skips the suffix scan on first capture visits. - Measured (macOS, Node 24.18.0), finite self-alias chain of 200 links: 24.9 s on origin/main, about 27 s here. That cost predates this PR. 300 aliases × depth 300: 1.75 s on main, 1.73 s here. - `npx vitest run src/domain/javascript/` — 821 passed; `npm run check:fast` — passed. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: not applicable (static JavaScript analysis); macOS only. - Package or release metadata impact: none. - Security, privacy, process, or containment review: removes an unbounded memory path that a small untrusted script could trigger; no new process or file access. ## Review checklist - [x] This PR addresses a concrete problem or an agreed enhancement. - [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. 🤖 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-10-31 00:55:14 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

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

[PR #4824] [MERGED] fix(ci): handle cache eviction and composed cleanup assertions #6426

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

No description provided.

Rows
Columns