morluto/rea

Fork 0

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

[PR #4364] [CLOSED] fix(javascript): bound returned-reference expansion on reassignment webs #6208

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4364
Author: @SaigyoujYuyuko
Created: 10/16/2026
Status: ❌ Closed

Base: main ← Head: fix/4348-returned-reference-budget

📝 Commits (9)

6ef3345 fix(javascript): bound returned-reference expansion on reassignment webs
9e1016b test(javascript): pin returned-reference outcomes across the expansion budget
38ee0e3 fix(javascript): reserve returned-reference budget before allocating candidates
421560c fix(javascript): share entered-initializer ancestry across candidates
3c23aa7 fix(javascript): intern returned-reference projection paths
ace5867 fix(javascript): key path interning by link identity, not serialized keys
e8efbaf fix(javascript): budget path links created per returned-reference query
cf070b9 fix(javascript): bound blocker propagation over shared ancestry
d32a703 fix(javascript): disclose which returned-reference budget stopped expansion

📊 Changes

7 files changed (+596 additions, -85 deletions)

View changed files

📝 src/application/javascript/JavaScriptSemanticGraphBuilder.ts (+10 -3)
➕ src/domain/javascript/javascriptSemanticAnalysis.returned_reference_expansion.test.ts (+282 -0)
📝 src/domain/javascript/javascriptSemanticCoverage.ts (+15 -0)
📝 src/domain/javascript/javascriptSemanticResourceLimits.ts (+19 -0)
📝 src/domain/javascript/javascriptSemanticReturnedReferences.ts (+263 -81)
📝 src/domain/javascript/javascriptSemanticValueTypes.ts (+4 -1)
📝 src/javascript/analysis/JavaScriptAnalysisWorkerSchemas.ts (+3 -0)

📄 Description

Summary

Refs #4348 (part 2). Since #4243, one createSemanticReturnedReferences query
on a recursive reassignment web (React-reconciler style: a fiber local
reassigned across dozens of switch arms) enumerates every simple initializer
chain as a distinct projection path, so the member-mutation phase exhausts the
worker heap on the two playwright-core@1.58.2 bundles reported in the issue.

This PR bounds the states one call-target query may expand and discloses the
bound through the existing semantic resource-limit coverage channel, following
the getter-expansion precedent. Part 1 (call resolution) is #4349 and is not
included here.

Problem and expected behavior

Trigger: rea analyze-javascript-application on an app containing
lib/vite/recorder/assets/index-BhTWtUlo.js (or the trace viewer bundle from
the issue). Instrumented run on 42fc6c53a (semantic phase in-process, Windows
x64, Node 24.19.0): 1466 of 1467 returned-reference queries complete in at most
~1.5k expansions, then a single member-callee query (a fiber-derived
d.push(...) receiver) enumerates >1M expansions — one leaf CallExpression
alone is reached under 206,008 distinct projection paths — and the worker dies
at the 4 GiB heap limit after ~17 s.

Invariant violated: analysis must stay within safe resource bounds; an
unlucky input must not turn into heap exhaustion with zero evidence.

Change and scope

createSemanticReturnedReferences now gives each query an admission reserve of
SEMANTIC_RETURNED_REFERENCE_EXPANSION_LIMIT (16384) candidates: every child
expansion reserves budget before its path/history representation is
constructed, so pending candidates, visits, and allocations share one bound.
The query keeps the references it has already proven and records a new
"returned-reference-expansion" value in state.valueResourceLimits,
which flows into ir.coverage.resourceLimits. Entered-initializer ancestry is kept as a persistent chain extended in O(1) per candidate instead of a copied Set, so the retained representation itself stays within the same bound. Projection paths are interned shared links extended and consumed in O(1), which also keys completed expansions without per-visit serialization.
The graph builder projects it as a resource-limit unknown exactly like
getter-expansion (query effect without tagged values). The worker IR schema
enum accepts the new value.

Intentionally not included:

No change to #4243's initializer-level cycle semantics. Path-growing cycles
(x = x.next) terminate exactly as before; below the budget, outputs are
unchanged (see digest evidence).
No change to the per-query completed-expansion cache.
Budget value (16384, ~10x the largest legitimate query observed) and the
keep-partial-references fallback are a design proposal; the issue notes part
2 needs a maintainer design decision, so both are called out for review.

Contribution scope: ordinary fix within an existing analysis pass; no new
provider or capability — "not applicable".

Contract and boundary impact

Semantic owner and earliest changed stage: src/domain/javascript/javascriptSemanticReturnedReferences.ts (domain semantic pass); disclosure wiring in javascriptSemanticCoverage.ts and JavaScriptSemanticGraphBuilder.ts.
Contribution scope fit and linked maintainer decision: not applicable (existing boundary; design decision flagged in #4348).
CLI and MCP/tool-catalog contract: no new/renamed tools or schemas; coverage.resourceLimits can now report the additional enum value "returned-reference-expansion" (worker IR zod enum extended).
Provider, bridge, target-format, or platform compatibility: none; analysis behavior only changes on inputs that previously exhausted the heap (no output at all before).
Evidence, artifact, provenance, or reconstruction contract: bounded queries keep proven references and mark the file's coverage partial with the limit disclosed; no silent truncation.
Target selection, authority, upstream behavior, and cleanup details (if applicable): 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.

(Expected-effects checklist: static analysis of caller-selected files only —
closest to "read existing files", left unchecked for the human owner to
confirm wording.)

Evidence and regression coverage

Tests added or updated: src/domain/javascript/javascriptSemanticAnalysis.returned_reference_expansion.test.ts — a generated 24-arm reassignment web trips the budget, discloses the limit, and preserves unrelated facts; a small resolution case stays under budget with no limit recorded.

Base reproduction or other evidence (executed, Windows x64, Node 24.19.0, semantic phase in-process):

Input	`42fc6c53a` (base)	this PR
`index-BhTWtUlo.js` (363,108 B)	heap exhausted (2 GiB cap) after ~45 s	mutations phase 2.6 s, 385 MiB RSS, limit disclosed
`defaultSettingsView-CJSZINFr.js` (640,870 B)	heap exhausted (2 GiB cap)	mutations phase 3.0 s, 445 MiB RSS, limit disclosed

Output-equivalence evidence (executed): sha-256 digests of
requiredModuleEffects + recorded resource limits over 311 real files
(playwright-core@1.58.2 lib/, excluding the two previously-OOM bundles)
are byte-identical between base and this PR; no file below the budget
records the new limit.
User-visible CLI/MCP output (if applicable): bounded files now report
coverage.resourceLimits: ["returned-reference-expansion"] instead of the
worker dying with "Owned JavaScript worker exhausted its heap".
Remaining proof gaps: Linux/macOS measurements not run locally (covered by
CI on the fork); CLI acceptance tests requiring packaged Windows native
controls do not run in this dev environment and are covered by CI.

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.

(Left to the human owner to re-check.)

Validation performed

Repro harness (vitest, .cache scratch) on both issue bundles, base vs PR — table above.
Digest harness over 311 files, base vs PR — identical, 0 limits triggered.
npx vitest run src/domain/javascript — 80 files / 1922 tests passed (incl. 4 new).
npm run check:fast — typecheck + lint passed.
npx prettier --check <changed files> — clean.
npm run test:focused -- tests/acceptance/applications/javascriptCallbackTargets.test.ts — fails in this environment for an unrelated reason (missing packaged Windows native controls, native/windows/build/manifest.json); delegated to CI.
Fork CI (workflow_dispatch, branch fix/4348-returned-reference-budget on Patchouli-CN/rea, latest run 38863240193 on the final head) — all JavaScript suites passed; the only failures are the pre-existing main breakage also red on upstream main (PlaywrightBrowserScenarioProvider cancellation, browserScenarioCancellationCli SIGINT/SIGTERM, adbCli reinstall, signalCleanup retry — fixed separately by #4345). A sessionMcp failure under the Apple E2E lane appeared twice and cleared on rerun both times (flaky; also green on upstream main).

Compatibility, safety, and release

Breaking changes or migration steps: none. coverage.resourceLimits gains one enum value; consumers validating it closed-world should accept "returned-reference-expansion" (worker schema already does).
Real Hopper/Ghidra, browser, or OS coverage: not applicable; Windows-only local runs, Linux via CI.
Package or release metadata impact: none.
Security, privacy, process, or containment review: converts an unbounded-memory failure mode into a bounded, disclosed one; no new target interaction.

Author readiness

Human owner: @Patchouli-CN

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/4364 **Author:** [@SaigyoujYuyuko](https://github.com/SaigyoujYuyuko) **Created:** 10/16/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/4348-returned-reference-budget` --- ### 📝 Commits (9) - [`6ef3345`](https://github.com/morluto/rea/commit/6ef3345adbddad2b27178e30b7d1187e4076a28b) fix(javascript): bound returned-reference expansion on reassignment webs - [`9e1016b`](https://github.com/morluto/rea/commit/9e1016bd2503a1b64e121c7e1d2a0a2e39087806) test(javascript): pin returned-reference outcomes across the expansion budget - [`38ee0e3`](https://github.com/morluto/rea/commit/38ee0e3b6a61f41b2c85cc9b840bf0769853b3fa) fix(javascript): reserve returned-reference budget before allocating candidates - [`421560c`](https://github.com/morluto/rea/commit/421560cce1bb82750ec3a155b54dd7abea9cca0c) fix(javascript): share entered-initializer ancestry across candidates - [`3c23aa7`](https://github.com/morluto/rea/commit/3c23aa712770f6a99009939ef36a7506bba8b365) fix(javascript): intern returned-reference projection paths - [`ace5867`](https://github.com/morluto/rea/commit/ace5867ebc9817f11d3d87e224b7a0ee3750b22d) fix(javascript): key path interning by link identity, not serialized keys - [`e8efbaf`](https://github.com/morluto/rea/commit/e8efbafa171f94319855e56950c10074bdf632e6) fix(javascript): budget path links created per returned-reference query - [`cf070b9`](https://github.com/morluto/rea/commit/cf070b9ad437888e35da6ceea9e3d260a917b74d) fix(javascript): bound blocker propagation over shared ancestry - [`d32a703`](https://github.com/morluto/rea/commit/d32a703bf9637d67ae91aae0f48ade816c5c957c) fix(javascript): disclose which returned-reference budget stopped expansion ### 📊 Changes **7 files changed** (+596 additions, -85 deletions) <details> <summary>View changed files</summary> 📝 `src/application/javascript/JavaScriptSemanticGraphBuilder.ts` (+10 -3) ➕ `src/domain/javascript/javascriptSemanticAnalysis.returned_reference_expansion.test.ts` (+282 -0) 📝 `src/domain/javascript/javascriptSemanticCoverage.ts` (+15 -0) 📝 `src/domain/javascript/javascriptSemanticResourceLimits.ts` (+19 -0) 📝 `src/domain/javascript/javascriptSemanticReturnedReferences.ts` (+263 -81) 📝 `src/domain/javascript/javascriptSemanticValueTypes.ts` (+4 -1) 📝 `src/javascript/analysis/JavaScriptAnalysisWorkerSchemas.ts` (+3 -0) </details> ### 📄 Description ## Summary Refs #4348 (part 2). Since #4243, one `createSemanticReturnedReferences` query on a recursive reassignment web (React-reconciler style: a fiber local reassigned across dozens of `switch` arms) enumerates every simple initializer chain as a distinct projection path, so the member-mutation phase exhausts the worker heap on the two `playwright-core@1.58.2` bundles reported in the issue. This PR bounds the states one call-target query may expand and discloses the bound through the existing semantic resource-limit coverage channel, following the `getter-expansion` precedent. Part 1 (call resolution) is #4349 and is not included here. ## Problem and expected behavior Trigger: `rea analyze-javascript-application` on an app containing `lib/vite/recorder/assets/index-BhTWtUlo.js` (or the trace viewer bundle from the issue). Instrumented run on `42fc6c53a` (semantic phase in-process, Windows x64, Node 24.19.0): 1466 of 1467 returned-reference queries complete in at most ~1.5k expansions, then a single member-callee query (a fiber-derived `d.push(...)` receiver) enumerates >1M expansions — one leaf `CallExpression` alone is reached under 206,008 distinct projection paths — and the worker dies at the 4 GiB heap limit after ~17 s. Invariant violated: analysis must stay within safe resource bounds; an unlucky input must not turn into heap exhaustion with zero evidence. ## Change and scope `createSemanticReturnedReferences` now gives each query an admission reserve of `SEMANTIC_RETURNED_REFERENCE_EXPANSION_LIMIT` (16384) candidates: every child expansion reserves budget before its path/history representation is constructed, so pending candidates, visits, and allocations share one bound. The query keeps the references it has already proven and records a new `"returned-reference-expansion"` value in `state.valueResourceLimits`, which flows into `ir.coverage.resourceLimits`. Entered-initializer ancestry is kept as a persistent chain extended in O(1) per candidate instead of a copied Set, so the retained representation itself stays within the same bound. Projection paths are interned shared links extended and consumed in O(1), which also keys completed expansions without per-visit serialization. The graph builder projects it as a resource-limit unknown exactly like `getter-expansion` (query effect without tagged values). The worker IR schema enum accepts the new value. Intentionally not included: - No change to #4243's initializer-level cycle semantics. Path-growing cycles (`x = x.next`) terminate exactly as before; below the budget, outputs are unchanged (see digest evidence). - No change to the per-query completed-expansion cache. - Budget value (16384, ~10x the largest legitimate query observed) and the keep-partial-references fallback are a design proposal; the issue notes part 2 needs a maintainer design decision, so both are called out for review. Contribution scope: ordinary fix within an existing analysis pass; no new provider or capability — "not applicable". ## Contract and boundary impact - Semantic owner and earliest changed stage: `src/domain/javascript/javascriptSemanticReturnedReferences.ts` (domain semantic pass); disclosure wiring in `javascriptSemanticCoverage.ts` and `JavaScriptSemanticGraphBuilder.ts`. - Contribution scope fit and linked maintainer decision: not applicable (existing boundary; design decision flagged in #4348). - CLI and MCP/tool-catalog contract: no new/renamed tools or schemas; `coverage.resourceLimits` can now report the additional enum value `"returned-reference-expansion"` (worker IR zod enum extended). - Provider, bridge, target-format, or platform compatibility: none; analysis behavior only changes on inputs that previously exhausted the heap (no output at all before). - Evidence, artifact, provenance, or reconstruction contract: bounded queries keep proven references and mark the file's coverage partial with the limit disclosed; no silent truncation. - Target selection, authority, upstream behavior, and cleanup details (if applicable): 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. - [x] Not sure yet; investigation or maintainer guidance needed. (Expected-effects checklist: static analysis of caller-selected files only — closest to "read existing files", left unchecked for the human owner to confirm wording.) ## Evidence and regression coverage - Tests added or updated: `src/domain/javascript/javascriptSemanticAnalysis.returned_reference_expansion.test.ts` — a generated 24-arm reassignment web trips the budget, discloses the limit, and preserves unrelated facts; a small resolution case stays under budget with no limit recorded. - Base reproduction or other evidence (executed, Windows x64, Node 24.19.0, semantic phase in-process): | Input | `42fc6c53a` (base) | this PR | | --- | --- | --- | | `index-BhTWtUlo.js` (363,108 B) | heap exhausted (2 GiB cap) after ~45 s | mutations phase 2.6 s, 385 MiB RSS, limit disclosed | | `defaultSettingsView-CJSZINFr.js` (640,870 B) | heap exhausted (2 GiB cap) | mutations phase 3.0 s, 445 MiB RSS, limit disclosed | - Output-equivalence evidence (executed): sha-256 digests of `requiredModuleEffects` + recorded resource limits over 311 real files (`playwright-core@1.58.2` `lib/`, excluding the two previously-OOM bundles) are byte-identical between base and this PR; no file below the budget records the new limit. - User-visible CLI/MCP output (if applicable): bounded files now report `coverage.resourceLimits: ["returned-reference-expansion"]` instead of the worker dying with "Owned JavaScript worker exhausted its heap". - Remaining proof gaps: Linux/macOS measurements not run locally (covered by CI on the fork); CLI acceptance tests requiring packaged Windows native controls do not run in this dev environment and are covered by CI. 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. (Left to the human owner to re-check.) ## Validation performed - Repro harness (vitest, `.cache` scratch) on both issue bundles, base vs PR — table above. - Digest harness over 311 files, base vs PR — identical, 0 limits triggered. - `npx vitest run src/domain/javascript` — 80 files / 1922 tests passed (incl. 4 new). - `npm run check:fast` — typecheck + lint passed. - `npx prettier --check <changed files>` — clean. - `npm run test:focused -- tests/acceptance/applications/javascriptCallbackTargets.test.ts` — fails in this environment for an unrelated reason (missing packaged Windows native controls, `native/windows/build/manifest.json`); delegated to CI. - Fork CI (`workflow_dispatch`, branch `fix/4348-returned-reference-budget` on `Patchouli-CN/rea`, latest run 38863240193 on the final head) — all JavaScript suites passed; the only failures are the pre-existing main breakage also red on upstream `main` (`PlaywrightBrowserScenarioProvider` cancellation, `browserScenarioCancellationCli` SIGINT/SIGTERM, `adbCli` reinstall, `signalCleanup` retry — fixed separately by #4345). A `sessionMcp` failure under the Apple E2E lane appeared twice and cleared on rerun both times (flaky; also green on upstream `main`). ## Compatibility, safety, and release - Breaking changes or migration steps: none. `coverage.resourceLimits` gains one enum value; consumers validating it closed-world should accept `"returned-reference-expansion"` (worker schema already does). - Real Hopper/Ghidra, browser, or OS coverage: not applicable; Windows-only local runs, Linux via CI. - Package or release metadata impact: none. - Security, privacy, process, or containment review: converts an unbounded-memory failure mode into a bounded, disclosed one; no new target interaction. ## Author readiness Human owner: @Patchouli-CN - [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. - [ ] 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. - [ ] 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. --- <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:42 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:46:30 +00:00

[PR #6280] [MERGED] fix(javascript): read utilityProcess.fork options from the argv slot #7253

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

No description provided.

Rows
Columns