[PR #4347] [CLOSED] fix(javascript): escape repeated prefixes in front of copied-slot selections #6192

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4347
Author: @kaoru0822-kitauji
Created: 10/16/2026
Status: ❌ Closed

Base: main ← Head: fix/member-mutation-selection-repeat

📝 Commits (1)

733c160 fix(javascript): escape repeated prefixes in front of copied-slot selections

📊 Changes

2 files changed (+36 additions, -4 deletions)

View changed files

📝 src/domain/javascript/javascriptSemanticAnalysis.reference_lifetimes.test.ts (+21 -0)
📝 src/domain/javascript/javascriptSemanticMemberMutations.ts (+15 -4)

📄 Description

Summary

A five-statement function can exhaust any worker heap in the JavaScript member-mutation pass:

function f(e) { let t = { ...e.opts }; e = e.value; h(t); }

This exhausts a 256 MiB heap in 0.5 s. Real code hits it too: Fluent's FluentDateTime constructor (t = {...e.opts, ...t}; e = e.value), bundled in pdf.js's web viewer, makes Obsidian 1.12.7's lib/pdfjs/pdf.viewer.min.mjs exhaust a 6 GB in-process heap in 36 s. This PR makes the repeated-prefix rule treat a selection-carrying path as never pruned, so that function finishes. No issue existed; I found it while profiling #2498's Obsidian workload.

Problem and expected behavior

observableMutationPaths owns two rules that disagreed:

normalize returns any path that carries a copied-slot selection (from a spread or rest) unchanged, so it never prunes such a path.
In markValue, a self alias that re-enters its capture with a repeated prefix escapes that prefix once ("escaping it once covers every longer effect"), but only when repeatsUnpruned says normalization can never prune it. repeatsUnpruned looked only at the prefix's projected origins. For a parameter e, those are empty, so it answered "prunable".

Escaping t therefore reached e at [opts, <selection>], and each lap through e = e.value prepended value. That growth happens at the front, so prefix coverage never catches it and normalization never prunes it. The path grew without bound: I instrumented a trace and saw length 5,231 and climbing, with the work list growing in step.

Expected: the repeated prefix escapes once, as for selection-free paths.

Change and scope

carriesSelection(path) is now the single predicate both rules use.
repeatsUnpruned(bindingId, prefix, path) returns true when path carries a selection, because normalize will never prune it however often the prefix repeats. Its cached origin-signature logic is unchanged for every other path.
The escape this enables is the existing conservative rule: escaping the prefix covers every longer effect beneath it.
Not included: #2498's typed-array escape volume, and the #4243 returned-reference regression on playwright-core's recorder and trace-viewer bundles. Both are reported separately.

Contract and boundary impact

Semantic owner and earliest changed stage: observableMutationPaths in javascriptSemanticMemberMutations.ts (domain).
Contribution scope fit and linked maintainer decision: not applicable (bug fix inside the existing analysis boundary).
CLI and MCP/tool-catalog contract: none.
Provider, bridge, target-format, or platform compatibility: none.
Evidence, artifact, provenance, or reconstruction contract: affected files now complete and retain semantic results where they were dropped as heap exhaustion. Elsewhere outputs are byte-identical (below).
Target selection, authority, upstream behavior, and cleanup details: not applicable.
Generated metadata, package, or installation impact: none.
Read existing files or captured data.

Evidence and regression coverage

Executed on macOS 26 arm64, Node 26.5.0, main at 42fc6c53a.

Workload	main	This PR
Minimal snippet above, semantic phase, 256 MiB heap	heap exhausted after 0.5 s	60 ms
Obsidian `lib/pdfjs/pdf.viewer.min.mjs` (488,821 B), semantic phase in-process, 6 GB heap	heap exhausted after 36 s	0.88 s, 23.7 MB IR
`rea analyze-javascript-application <dir with that file>` (defaults)	3.8 s, 1.23 GB RSS: `Owned JavaScript worker exhausted its heap … during semantic`; 396 KB JSON (static facts only)	5.9 s, 515 MB RSS, no resource limitation; 44.9 MB JSON (semantic graph retained)
Semantic IR over 153 real files (VS Code app bundle, `node_modules`, Obsidian sources), one child per file	—	0 of 151 digests differ; the other 2 files exhaust the heap on both revisions (#4243 regression, reported separately)

Tests added: javascriptSemanticAnalysis.reference_lifetimes.test.ts adds self aliases in front of copied-slot selections. A shallow copy of e.opts escapes, and then e = e.value. The shared children seed.opts.inner.token and seed.value.opts.inner.token must be unknown, and the unrelated primitive seed.keep must stay literal, each within 2 s. Against main's implementation the vitest worker crashes with heap exhaustion; with the fix, 116/116 pass.
User-visible CLI/MCP output: affected sources now return their semantic graph instead of a heap-exhaustion limitation.
Remaining proof gaps: a single run per configuration; no Linux or Windows measurements. The Obsidian file is from an installed app, not a public download. The minimal snippet and test fixture reproduce the same root cause publicly.
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 -- <94 JavaScript domain/application/worker/reference-source test files>: 94 files, 2256 tests passed.
npm run test:local -- src/domain/javascript/javascriptSemanticAnalysis.reference_lifetimes.test.ts with main's javascriptSemanticMemberMutations.ts: worker exited (heap exhausted). With the fix: 116 passed.
npm run check:fast: typecheck and lint passed.
Per-file semantic comparison (child process, 4 GiB heap, 120 s cap) over 153 real files, main vs fix: 0 differing digests among the 151 that complete.
rea analyze-javascript-application on the Obsidian viewer file, main vs fix: results above.

Compatibility, safety, and release

Breaking changes or migration steps: none.
Real Hopper/Ghidra, browser, or OS coverage: macOS 26 arm64 only.
Package or release metadata impact: none.
Security, privacy, process, or containment review: removes an attacker-controllable unbounded memory growth on inert parsing; no new process, network, or file behavior.

Author readiness

Human owner:

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/4347 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/16/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/member-mutation-selection-repeat` --- ### 📝 Commits (1) - [`733c160`](https://github.com/morluto/rea/commit/733c160d4f2548469e2dca01be09bf48b5ce7965) fix(javascript): escape repeated prefixes in front of copied-slot selections ### 📊 Changes **2 files changed** (+36 additions, -4 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/javascript/javascriptSemanticAnalysis.reference_lifetimes.test.ts` (+21 -0) 📝 `src/domain/javascript/javascriptSemanticMemberMutations.ts` (+15 -4) </details> ### 📄 Description ## Summary A five-statement function can exhaust any worker heap in the JavaScript member-mutation pass: ```js function f(e) { let t = { ...e.opts }; e = e.value; h(t); } ``` This exhausts a 256 MiB heap in 0.5 s. Real code hits it too: Fluent's `FluentDateTime` constructor (`t = {...e.opts, ...t}; e = e.value`), bundled in pdf.js's web viewer, makes Obsidian 1.12.7's `lib/pdfjs/pdf.viewer.min.mjs` exhaust a 6 GB in-process heap in 36 s. This PR makes the repeated-prefix rule treat a selection-carrying path as never pruned, so that function finishes. No issue existed; I found it while profiling #2498's Obsidian workload. ## Problem and expected behavior `observableMutationPaths` owns two rules that disagreed: - `normalize` returns any path that carries a copied-slot selection (from a spread or rest) unchanged, so it never prunes such a path. - In `markValue`, a self alias that re-enters its capture with a repeated prefix escapes that prefix once ("escaping it once covers every longer effect"), but only when `repeatsUnpruned` says normalization can never prune it. `repeatsUnpruned` looked only at the prefix's projected origins. For a parameter `e`, those are empty, so it answered "prunable". Escaping `t` therefore reached `e` at `[opts, <selection>]`, and each lap through `e = e.value` prepended `value`. That growth happens at the front, so prefix coverage never catches it and normalization never prunes it. The path grew without bound: I instrumented a trace and saw length 5,231 and climbing, with the work list growing in step. Expected: the repeated prefix escapes once, as for selection-free paths. ## Change and scope - `carriesSelection(path)` is now the single predicate both rules use. - `repeatsUnpruned(bindingId, prefix, path)` returns `true` when `path` carries a selection, because `normalize` will never prune it however often the prefix repeats. Its cached origin-signature logic is unchanged for every other path. - The escape this enables is the existing conservative rule: escaping the prefix covers every longer effect beneath it. - Not included: #2498's typed-array escape volume, and the #4243 returned-reference regression on `playwright-core`'s recorder and trace-viewer bundles. Both are reported separately. ## Contract and boundary impact - Semantic owner and earliest changed stage: `observableMutationPaths` in `javascriptSemanticMemberMutations.ts` (domain). - Contribution scope fit and linked maintainer decision: not applicable (bug fix inside the existing analysis boundary). - CLI and MCP/tool-catalog contract: none. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: affected files now complete and retain semantic results where they were dropped as heap exhaustion. Elsewhere outputs are byte-identical (below). - Target selection, authority, upstream behavior, and cleanup details: not applicable. - Generated metadata, package, or installation impact: none. - [x] Read existing files or captured data. ## Evidence and regression coverage Executed on macOS 26 arm64, Node 26.5.0, main at `42fc6c53a`. | Workload | main | This PR | | --- | ---: | ---: | | Minimal snippet above, semantic phase, 256 MiB heap | heap exhausted after 0.5 s | 60 ms | | Obsidian `lib/pdfjs/pdf.viewer.min.mjs` (488,821 B), semantic phase in-process, 6 GB heap | heap exhausted after 36 s | **0.88 s**, 23.7 MB IR | | `rea analyze-javascript-application <dir with that file>` (defaults) | 3.8 s, 1.23 GB RSS: `Owned JavaScript worker exhausted its heap … during semantic`; 396 KB JSON (static facts only) | 5.9 s, 515 MB RSS, no resource limitation; 44.9 MB JSON (semantic graph retained) | | Semantic IR over 153 real files (VS Code app bundle, `node_modules`, Obsidian sources), one child per file | — | **0 of 151 digests differ**; the other 2 files exhaust the heap on both revisions (#4243 regression, reported separately) | - Tests added: `javascriptSemanticAnalysis.reference_lifetimes.test.ts` adds `self aliases in front of copied-slot selections`. A shallow copy of `e.opts` escapes, and then `e = e.value`. The shared children `seed.opts.inner.token` and `seed.value.opts.inner.token` must be `unknown`, and the unrelated primitive `seed.keep` must stay literal, each within 2 s. Against main's implementation the vitest worker crashes with heap exhaustion; with the fix, 116/116 pass. - User-visible CLI/MCP output: affected sources now return their semantic graph instead of a heap-exhaustion limitation. - Remaining proof gaps: a single run per configuration; no Linux or Windows measurements. The Obsidian file is from an installed app, not a public download. The minimal snippet and test fixture reproduce the same root cause publicly. - [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 -- <94 JavaScript domain/application/worker/reference-source test files>`: 94 files, 2256 tests passed. - `npm run test:local -- src/domain/javascript/javascriptSemanticAnalysis.reference_lifetimes.test.ts` with main's `javascriptSemanticMemberMutations.ts`: worker exited (heap exhausted). With the fix: 116 passed. - `npm run check:fast`: typecheck and lint passed. - Per-file semantic comparison (child process, 4 GiB heap, 120 s cap) over 153 real files, main vs fix: 0 differing digests among the 151 that complete. - `rea analyze-javascript-application` on the Obsidian viewer file, main vs fix: results above. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: macOS 26 arm64 only. - Package or release metadata impact: none. - Security, privacy, process, or containment review: removes an attacker-controllable unbounded memory growth on inert parsing; no new process, network, or file behavior. ## Author readiness Human owner:  - [ ] 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](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 01:12:34 +00:00