morluto/rea

Fork 0

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

[PR #915] [MERGED] fix(javascript): handle deep AST traversal and addition without stack overflow #2343

New issue

Closed

opened 2026-10-16 21:17:33 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-16 21:17:33 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/915
Author: @morluto
Created: 10/7/2026
Status: ✅ Merged
Merged: 10/7/2026
Merged by: @morluto

Base: main ← Head: codex/javascript-ast-depth

📝 Commits (3)

c19baa1 fix(javascript): avoid recursion and eager ancestry copies in deep ASTs
5dc9469 fix(javascript): traverse static analysis and bundler ASTs iteratively
f698933 chore: merge main and preserve JavaScript depth fixes after module move

📊 Changes

9 files changed (+203 additions, -40 deletions)

View changed files

📝 src/domain/javascript/javascriptAstFingerprint.ts (+14 -8)
📝 src/domain/javascript/javascriptSemanticAnalysis.dataflow.test.ts (+3 -0)
📝 src/domain/javascript/javascriptSemanticAnalysis.rejection.test.ts (+50 -1)
📝 src/domain/javascript/javascriptSemanticProjection.ts (+2 -2)
➕ src/domain/javascript/javascriptSemanticTraversal.test.ts (+53 -0)
📝 src/domain/javascript/javascriptSemanticTraversal.ts (+29 -12)
📝 src/domain/javascript/javascriptSemanticValues.ts (+32 -6)
📝 src/domain/javascript/javascriptStaticAnalysis.ts (+8 -4)
📝 src/domain/javascript/javascriptStaticAnalysisBundler.ts (+12 -7)

📄 Description

Summary

Deep, parser-admitted addition expressions can overflow REA's AST traversal, constant evaluator, and static-analysis walks. Traverse these trees with explicit stacks and avoid copying the complete ancestor chain for every node. This fix was discovered while profiling the codebase; no separate issue is linked.

Problem and expected behavior

A valid const result = 1 + 1 + ...; result; expression can throw RangeError after parsing, causing application analysis to fail. Admitted syntax should retain its facts and constant value. Preserve AST grouping, coercion, lexical scope, source-tree callback order, and meaningful unknown values.

Change and scope

Use iterative source-tree traversal and create detached ancestor snapshots when a visitor requests them during enter.
Evaluate addition trees iteratively, retaining left-before-right evaluation and each binary expression's grouping.
Reuse the traversal for application static inspection, bundler argument collection, fingerprints, and export collection, where Babel's recursive traverseFast also overflowed.

Babel's parser limits and other recursive expression families are outside the demonstrated scope. No analysis cap is introduced.

Contract and boundary impact

Semantic owner and earliest changed stage: pure domain AST traversal and constant evaluation, after parsing.
CLI and MCP/tool-catalog contract: schemas unchanged; application analysis can return recovered facts for these admitted trees.
Provider, bridge, target-format, or platform compatibility: inert JavaScript analysis only.
Evidence, artifact, provenance, or reconstruction contract: result ordering, source identity, and unknowns preserved.
Process execution, authorization, cleanup, or containment impact: none; inspected JavaScript is not executed.
Generated metadata, package, or installation impact: none.

Evidence and regression coverage

Executed on macOS arm64, Node 24.18.0. Three samples per workload, full semantic analysis including parsing, excluding fixture construction and result hashing:

Workload	Base median	Candidate median	Result
1,000-term addition	24.5 ms	6.7 ms	identical complete IR
3,000-term addition	308.2 ms	24.0 ms	identical complete IR
6,000-term addition	`RangeError`	36.0 ms	complete IR and recovered value
Babel parser 8.0.4 source, 480,937 bytes	524.1 ms	495.5 ms	identical complete IR

The harness analyzes the Babel parser source three times before the chains, uses --max-old-space-size=512, and compares full result SHA-256 hashes. Parser optimization can change which deep sources Babel admits; the regression builds admitted 12,000-term ASTs directly in both directions to isolate REA's failures. A standalone compiled Node process also recovers both values as 12,000.

Additional comparison: 202 seeded fixtures produce identical complete semantic IR, static findings, fingerprints, and export names. Fixtures include mixed/grouped additions, unknowns, logical unions, an alias cycle, and webpack registration.

Tests added/updated: ordered enter/exit and retained ancestor snapshots; deep static/semantic analysis; grouped number/string coercion.
CLI/application evidence: compiled CLI recovers the 3,000-term result; configured application-workflow verification recovers the 3,000→3,001 export-shape change and preserves partial/unknown coverage.
Remaining proof gaps: no claim of unrestricted Babel parsing depth or universal expression-depth support.
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 build:cached and npm run check:fast — passed.
npm run test:focused -- <24 exact paths> — 413 tests passed across all JavaScript semantic/static tests, fingerprints, public CLI acceptance, and Electron MCP boundaries.
npm run verify:application-workflows -- --left <source-owned-left-app> --right <source-owned-right-app> — passed on distinct 3,000/3,001-term application directories.
Compiled CLI analysis, standalone compiled AST verification, and baseline/candidate profiling/equivalence harnesses — passed.
Independent Standards and Spec reviews of the final diff — both clean.

Compatibility, safety, and release

Breaking changes or migration steps: none; the ancestor-reader interface is internal to domain traversal.
Real Hopper/Ghidra, browser, or OS coverage: not needed for this inert-analysis change; no new provider claim.
Package or release metadata impact: none.
Security, privacy, process, or containment review: no new execution authority or evidence redaction.

Review checklist

One focused outcome and Conventional Commit title.
Reason for no separate issue stated above.
Tests cover recovered facts, ordering, coercion, and unknowns.
No owning contract or generated metadata changes required.
CLI/application behavior verified.
Final diff checked for unrelated edits 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/915 **Author:** [@morluto](https://github.com/morluto) **Created:** 10/7/2026 **Status:** ✅ Merged **Merged:** 10/7/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `codex/javascript-ast-depth` --- ### 📝 Commits (3) - [`c19baa1`](https://github.com/morluto/rea/commit/c19baa118435446b6343ce1e642c966674acf9aa) fix(javascript): avoid recursion and eager ancestry copies in deep ASTs - [`5dc9469`](https://github.com/morluto/rea/commit/5dc9469348de577b1fe99d0729350fa397c43551) fix(javascript): traverse static analysis and bundler ASTs iteratively - [`f698933`](https://github.com/morluto/rea/commit/f69893314da85e54177e805711379ee20ca5c02a) chore: merge main and preserve JavaScript depth fixes after module move ### 📊 Changes **9 files changed** (+203 additions, -40 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/javascript/javascriptAstFingerprint.ts` (+14 -8) 📝 `src/domain/javascript/javascriptSemanticAnalysis.dataflow.test.ts` (+3 -0) 📝 `src/domain/javascript/javascriptSemanticAnalysis.rejection.test.ts` (+50 -1) 📝 `src/domain/javascript/javascriptSemanticProjection.ts` (+2 -2) ➕ `src/domain/javascript/javascriptSemanticTraversal.test.ts` (+53 -0) 📝 `src/domain/javascript/javascriptSemanticTraversal.ts` (+29 -12) 📝 `src/domain/javascript/javascriptSemanticValues.ts` (+32 -6) 📝 `src/domain/javascript/javascriptStaticAnalysis.ts` (+8 -4) 📝 `src/domain/javascript/javascriptStaticAnalysisBundler.ts` (+12 -7) </details> ### 📄 Description ## Summary Deep, parser-admitted addition expressions can overflow REA's AST traversal, constant evaluator, and static-analysis walks. Traverse these trees with explicit stacks and avoid copying the complete ancestor chain for every node. This fix was discovered while profiling the codebase; no separate issue is linked. ## Problem and expected behavior A valid `const result = 1 + 1 + ...; result;` expression can throw `RangeError` after parsing, causing application analysis to fail. Admitted syntax should retain its facts and constant value. Preserve AST grouping, coercion, lexical scope, source-tree callback order, and meaningful unknown values. ## Change and scope - Use iterative source-tree traversal and create detached ancestor snapshots when a visitor requests them during `enter`. - Evaluate addition trees iteratively, retaining left-before-right evaluation and each binary expression's grouping. - Reuse the traversal for application static inspection, bundler argument collection, fingerprints, and export collection, where Babel's recursive `traverseFast` also overflowed. Babel's parser limits and other recursive expression families are outside the demonstrated scope. No analysis cap is introduced. ## Contract and boundary impact - Semantic owner and earliest changed stage: pure domain AST traversal and constant evaluation, after parsing. - CLI and MCP/tool-catalog contract: schemas unchanged; application analysis can return recovered facts for these admitted trees. - Provider, bridge, target-format, or platform compatibility: inert JavaScript analysis only. - Evidence, artifact, provenance, or reconstruction contract: result ordering, source identity, and unknowns preserved. - Process execution, authorization, cleanup, or containment impact: none; inspected JavaScript is not executed. - Generated metadata, package, or installation impact: none. ## Evidence and regression coverage Executed on macOS arm64, Node 24.18.0. Three samples per workload, full semantic analysis including parsing, excluding fixture construction and result hashing: | Workload | Base median | Candidate median | Result | | --- | ---: | ---: | --- | | 1,000-term addition | 24.5 ms | 6.7 ms | identical complete IR | | 3,000-term addition | 308.2 ms | 24.0 ms | identical complete IR | | 6,000-term addition | `RangeError` | 36.0 ms | complete IR and recovered value | | Babel parser 8.0.4 source, 480,937 bytes | 524.1 ms | 495.5 ms | identical complete IR | The harness analyzes the Babel parser source three times before the chains, uses `--max-old-space-size=512`, and compares full result SHA-256 hashes. Parser optimization can change which deep sources Babel admits; the regression builds admitted 12,000-term ASTs directly in both directions to isolate REA's failures. A standalone compiled Node process also recovers both values as 12,000. Additional comparison: 202 seeded fixtures produce identical complete semantic IR, static findings, fingerprints, and export names. Fixtures include mixed/grouped additions, unknowns, logical unions, an alias cycle, and webpack registration. - Tests added/updated: ordered enter/exit and retained ancestor snapshots; deep static/semantic analysis; grouped number/string coercion. - CLI/application evidence: compiled CLI recovers the 3,000-term result; configured application-workflow verification recovers the 3,000→3,001 export-shape change and preserves partial/unknown coverage. - Remaining proof gaps: no claim of unrestricted Babel parsing depth or universal expression-depth support. - [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 build:cached` and `npm run check:fast` — passed. - `npm run test:focused -- <24 exact paths>` — 413 tests passed across all JavaScript semantic/static tests, fingerprints, public CLI acceptance, and Electron MCP boundaries. - `npm run verify:application-workflows -- --left <source-owned-left-app> --right <source-owned-right-app>` — passed on distinct 3,000/3,001-term application directories. - Compiled CLI analysis, standalone compiled AST verification, and baseline/candidate profiling/equivalence harnesses — passed. - Independent Standards and Spec reviews of the final diff — both clean. ## Compatibility, safety, and release - Breaking changes or migration steps: none; the ancestor-reader interface is internal to domain traversal. - Real Hopper/Ghidra, browser, or OS coverage: not needed for this inert-analysis change; no new provider claim. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no new execution authority or evidence redaction. ## Review checklist - [x] One focused outcome and Conventional Commit title. - [x] Reason for no separate issue stated above. - [x] Tests cover recovered facts, ordering, coercion, and unknowns. - [x] No owning contract or generated metadata changes required. - [x] CLI/application behavior verified. - [x] Final diff checked for unrelated edits 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-16 21:17:33 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-10-31 01:04:43 +00:00

[PR #2384] [MERGED] fix(formats): preserve high bits in remaining magic-byte checks #5196

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

No description provided.

Rows
Columns