Repository navigation
fix(javascript): bound module source payload before deep semantic analysis - #1610
akushonkamen wants to merge 2 commits into
Conversation
…lysis Deep semantic analysis of one oversized module could expand past the whole host heap and terminate the CLI/MCP process instead of reporting a bounded result (issue morluto#1563; public Claude Code 2.1.296 member chunk-kasbfbhj.js, 4762637 bytes, dies with a fatal heap OOM at a 768 MiB old space). The existing semantic resource limits only bound single primitive values. Modules above a 2 MiB source-payload budget now skip deep semantics with an explicit failed coverage carrying the named module-source-bytes limit and a disclosed limitation, while static analysis, parsing and every smaller module continue unaffected. Coverage stays partial and names the limiting bound; no semantic absence is claimed for the degraded module. Validation: 5 new domain/services tests pass. The JavaScript domain and services subset (npx vitest run --project domain --project services src/domain/javascript src/application/javascript) observed 1018 passed / 6 failed (1024 tests, 59 files) on this branch and 1013 passed / 6 failed (1019 tests, 57 files) on clean c1b5ccf; the same 6 tests fail in both runs, so they predate this change. The adapters JavaScript and artifacts slice (npx vitest run --project adapters src/artifacts src/javascript) passes 107 tests. The public failing member now exits 0 with partial coverage naming javascript_semantic_module_source_bytes where main exited 134.
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
TL;DR: Fixed: at a 768 MiB heap, the 4.76 MB public module no longer kills the CLI or the MCP server. It returns I tested head 808ded3 against main c1217e7 (#1577). Main moved to b22d817 while I ran, so I rebuilt that too. The PR applies cleanly to both. The module is the member from the issue: 4,762,637 bytes, SHA-256
Not run
Thanks, akushonkamen, for the clear root cause, the named limit, and a test that drives both the CLI and MCP. Heap sweep and memoryNode 24 unless noted. Memory is peak resident size in MiB. Time is wall seconds. Load was 2.9 to 5.3. Load counts programs running or waiting, out of 24 CPU threads. Another test job ran alongside. Each cell is one run.
The PR on top of main, at 768 MiB: exit 0, same bytes as the head. The parent 11e3fae at 768 MiB: exit 134. Main b22d817 at 768 MiB: exit 134. The PR gave the same bytes on every Node and heap (SHA-256 Over MCP on Node 24 at 768 MiB, I sent a ping, the analysis, a ping, and a small analysis. Main's server dropped the connection, and every later call failed. The PR's server answered all four, and the analysis result was 2.9 MB. Boundary filesEach case is a folder with one module file and a small
"Same" means the same output SHA-256. The check counts UTF-8 bytes. The two-byte file at 2,097,153 bytes has only 1,048,654 characters, and it is skipped. In the comment case at 2,097,153 bytes, the app graph loses edges. Imports go from 4 to 2. Exports go from 3 to 1. Module nodes go from 7 to 3. The Plain code just under the cap also passed at a 768 MiB heap: 819 MiB peak. The minified style peaked at 794 MiB. The limitation texts, as printed:
Real inputs: IDs and run timeSix inputs, Node 24, heap 4,096 MiB. One is the Beyond All Reason launcher. Its code, spring-launcher, is MIT (LICENSE). Its Flathub build is GPL-2.0-or-later, as its metadata file says. One is a vendor chunk from Hermes Desktop: hermes-agent, MIT. The other four are small files from our scripts. They are the 1,200-function file, the #1495 file with N = 2,000, and two more files with 2,000 functions each. Shape A is exported functions that return objects. Shape B is plain functions that call No input has a JavaScript file over 2 MiB. The launcher has two entries over 2 MiB: a native binary and a source map. IDs, first 8 to 14 characters. The PR on top of main gave the same bytes as the head on all five.
The launcher's semantic graph has the same 100,000 nodes, 93,437 relations and 33,444 unknowns in both trees. It has the same coverage too. Only the limitations differ. The PR adds five lines. One of them says the parser recovered from syntax errors. Main b22d817 gave the same bytes as c1217e7 (#1577) on the five inputs I re-ran. The N = 2,000 file matches my earlier IDs for it on main 11e3fae. Wall time in seconds, median of 3 interleaved runs. Peak memory in MiB.
Load was 1.7 to 4.2 and fell during the run. For the N = 2,000 file it was 1.1 to 1.4. The launcher gap is 4%. The spread between its runs was 4% to 7%. The PR's launcher peak was higher in all 3 runs. I do not know why. CommandsPATH=$HOME/.local/opt/node24/bin:$PATH
git archive <sha> | tar -x -C <dir>
npm ci && npm run build
npm pack --ignore-scripts <package and version from the issue>
python3 -I slice.py <tgz> <outdir>
NODE_OPTIONS=--max-old-space-size=768 node scripts/rea.mjs analyze-javascript-application <dir> --artifact-format directory --json
/usr/bin/time -f "%e %M" <the command above>
npm run test:focused -- <the 4 test files>
npm run check:fast
Raw outputTests, Node 22.23.1, 24.18.0 and 26.11.1, on the PR head: 4 files and 61 tests passed each time.
Main, 768 MiB, Node 24, end of stderr: Node 22 prints PR, same command, coverage in the JSON: Test setup
🤖 Generated with Claude Code (Sonnet 5.5, verified by Opus 5.5) 🦊 |
N0zoM1z0
left a comment
N0zoM1z0
left a comment
There was a problem hiding this comment.
Independent review of exact head 808ded32c4b18691365ca5fcc54c090621c92c25, built and exercised on Linux x64 with Node 24.18.0/npm 11.16.0. The explicit failed-module coverage, global unknown frontiers and preserved static-fact intent are useful, but the resource boundary does not yet protect the real CLI/MCP host. Requesting changes for the remaining #1563 termination path.
Actual public workflow evidence (CPUs 13/15, nice 10, one heavy lane, 2304 MiB process-family RSS guard, 2 GiB host reserve; no guard stop in these runs):
- Original complete extracted Claude Code 2.1.296 directory, 2351 relevant files,
NODE_OPTIONS=--max-old-space-size=768 node scripts/rea.mjs analyze-javascript-application INPUT --json: exit 134, fatal heap exhaustion atchunk-kasbfbhj.js(1432/2351), zero CLI stdout, 170.64 seconds; peak family RSS 1,165,242,368 bytes. Same official bytes as #1563; the 4,762,637-byte member's SHA256 is64516393b8f06dce5b74140781d43b1df4e743de3e283526a533508ca1c58147. The isolated-member green in the PR does not establish complete-directory recovery. - Dense input below the new cutoff:
function f(){return 1;}\nplus'f();'.repeat(400000), UTF-8 1,600,024 bytes, SHA256fd861fc0c984035c6c9a41df7bf389f6e82a69404f7bf91323159e0aa56f6efe. Actual CLI at 768 MiB: exit 134, fatal heap exhaustion, zero stdout, 34.44 seconds; peak family RSS 969,363,456 bytes. - Real pinned-SDK stdio MCP on that same below-cutoff input: complete 170-tool catalog and initial ping succeed; analysis fails CONNECTION_CLOSED, fatal heap exhaustion is captured; subsequent ping and a separate small analysis both fail Not connected. Server 768 MiB/parent 512 MiB; peak family RSS 1,297,494,016 bytes. Source SHA unchanged.
- Real pinned-SDK MCP on the original complete directory: the 170-tool catalog and initial ping succeed, then analysis fails CONNECTION_CLOSED after 174.60 seconds with fatal heap exhaustion. Subsequent ping and a separate small analysis both fail Not connected. Server 768 MiB/parent 512 MiB; peak family RSS 1,411,112,960 bytes, no guard stop. Culprit source SHA unchanged.
- Positive control: a 2,097,199-byte file made of an inert comment plus
export const answer=42; fetch("/answer")returns explicit partial coverage with the named limit and no semantic bindings. Subsequent ping and small analysis succeed; source SHA unchanged. This verifies the disclosed skip policy, rather than a bound on parsing/semantic expansion.
The source-byte check runs after parsing, open-receiver classification and static analysis, and does not bound accumulated representations from earlier modules. Please use a supervised, genuinely bounded analysis boundary/admission policy that preserves observations and returns an actionable resource failure while keeping the parent alive. Moving the same fixed source check earlier or lowering it does not resolve the below-cutoff expansion counterexample. Keep #1563 open until the full original workflow and connection recovery are verified; adjust the deterministic-upper-bound/process-survival claims accordingly.
No new tools, provider installations or target execution were required. macOS/Windows, real Hopper/Ghidra and higher-heap full-tree behavior were not verified in this review. CPU sampled counters are not used as performance evidence.
| const semantics = | ||
| analysis.parse_status === "complete" || analysis.parse_status === "partial" | ||
| ? yield* analyzeParsedJavaScriptSemanticsSteps(parsed) | ||
| ? exceedsSemanticModuleSourceBytesBudget(file.text.value) |
There was a problem hiding this comment.
[P1] Contain parsing and accumulated analysis before claiming host survival
The new check runs after Babel parsing, receiver classification and static analysis, and is a per-file source-length cutoff rather than a bound on expanded/retained representations. On this exact head, the original full Claude Code directory still exits 134 at chunk-kasbfbhj.js (1432/2351), zero CLI stdout, with a 768 MiB old space; the real 170-tool stdio MCP server also dies with CONNECTION_CLOSED and subsequent ping/small analysis fail Not connected. No external RSS/time guard stopped either run. A 1,600,024-byte module (function f(){return 1;}\n + f(); repeated 400000 times) is below this limit and independently kills both public workflows too. Thus isolated-member success cannot close #1563 or establish the stated deterministic resource bound. Please supervise/bound the actual parsing and analysis work plus retained representations, preserve observed facts, and verify full-directory failure recovery and the below-cutoff case before making process-survival claims.
N0zoM1z0
added
area: cli
|
TL;DR: Byte size alone does not decide this. Files with few syntax nodes per byte pass at the full 2 MiB cutoff. The file in the review has over four times more nodes per byte, and it runs out of memory at 768 MiB. Rebasing onto current main moves the threshold, but dense files still die. My earlier comment found boundary files of exactly 2,097,152 bytes passing at 768 MiB. The review's file is smaller and fails. Both are true because the files have different shapes. I rebuilt the review's file (
Each cell is one run: seconds, then peak memory in MiB. A "call" is
This agrees with the review: a per-module byte cap does not bound memory. The rows for main + #1610 show dense files under or at the cap that still abort. Not run
Commands# input: a directory holding only mod.js, default --artifact-format (auto)
# the review does not say how the file was packaged
env -i HOME=/tmp/p1610b/home PATH=$HOME/.local/opt/node24/bin:/usr/bin:/bin \
TMPDIR=/tmp/p1610b/tmp NODE_OPTIONS=--max-old-space-size=768 \
nice -n 19 /usr/bin/time -f "%e %M" timeout 600 \
node scripts/rea.mjs analyze-javascript-application <dir> --json
# "main + #1610" is main with the diff 11e3fae69..808ded32c applied by patch -p1 (0 rejects)
# nodes per KiB: @babel/parser, sourceType unambiguous, errorRecovery on, every AST node countedTest setup🤖 Generated with Claude Code (Sonnet 5.5, verified by Opus 5.5) 🦊 |
|
Closing as superseded by merged #1651, commit 4999e4b. Source parsing and semantic projection now run in an owned worker with independent heap/deadline controls, validated static checkpoints, explicit partial coverage, and retained Evidence through CLI/MCP. This addresses the host-termination boundary tracked in #1563 without imposing this PR's fixed 2 MiB source cutoff. The below-cutoff dense-source and full-directory counterexamples recorded in the review are covered by the replacement approach. Thank you for the reproduction and proposed fix; this branch/history is retained. |
|
Thanks @morluto — agreed, and thanks for carrying this into #1651. The worker-based isolation with independent heap/deadline controls addresses the #1563 host-termination boundary more robustly than this PR's fixed 2 MiB byte cutoff, which — as the reviews above showed — dense low-nodes-per-byte sources could still defeat. Treating this branch as superseded; no further work here. |
Summary
Deep semantic analysis of one oversized module could expand past the whole host heap and terminate the CLI/MCP process instead of returning a bounded result. Fixes #1563.
Modules above a 2 MiB source-payload budget now skip deep semantics with an explicit failed coverage that carries the named
javascript_semantic_module_source_byteslimit and a disclosed limitation. Static analysis, parsing, and every smaller module continue unaffected; coverage stays partial and names the bounding limit, and no semantic absence is claimed for the degraded module.Problem and expected behavior
The existing semantic resource limits (
src/domain/javascript/javascriptSemanticResourceLimits.ts) only bound single primitive values (256 candidates, 1 MiB per byte value); there was no module-payload-level budget. The semantic main path (src/application/javascript/JavaScriptArtifactAnalysis.ts,analyzeArtifactFileSteps→analyzeParsedJavaScriptSemanticsSteps) expanded multi-MiB module sources inside the host heap, so one 4,762,637-byte module (public Claude Code 2.1.296 memberchunk-kasbfbhj.js) exhausted a 768 MiB old space and killed the whole process with a fatal OOM before any JSON output. Expected behavior: the process survives, returns partial coverage, and names the limit that bound the result.Change and scope
SEMANTIC_MODULE_SOURCE_BYTES_LIMIT= 2 MiB payload budget in the JavaScript semantic resource limits.moduleSourceBudgetJavaScriptSemanticIr()into an explicit failed coverage carrying the named module-source-bytes limit plus a limitation disclosure.Contract and boundary impact
JavaScriptArtifactAnalysissteps.analyze-javascript-applicationno longer terminates on oversized members; it exits 0 with partial coverage, the named limit and the limitation.parse_statusstayscomplete; semantic absence is not claimed.Evidence and regression coverage
src/domain/javascript/javascriptSemanticResourceLimits.test.ts, 1 module-budget test insrc/application/javascript/JavaScriptArtifactAnalysisModuleBudget.test.ts).@anthropic-ai/claude-code-linux-x64@2.1.296(never executed);chunk-kasbfbhj.js, 4,762,637 bytes, SHA25664516393b8f06dce5b74140781d43b1df4e743de3e283526a533508ca1c58147. Red was re-verified on current main11e3fae6after this branch was rebased.limits=[{"name":"javascript_semantic_module_source_bytes","value":2097152,"unit":"bytes"}]and limitation "Module source-payload budget exceeded (maximum 2097152 bytes); deep semantic analysis was skipped for this module."For evidence-bearing changes:
Validation performed
11e3fae6(rebased base, dist rebuilt):timeout 300 env NODE_OPTIONS=--max-old-space-size=768 node scripts/rea.mjs analyze-javascript-application /tmp/rea1563/rea-claude-memory-repro --json→ exit 134, stderrFATAL ERROR: Ineffective mark-compacts near heap limit Allocation failed - JavaScript heap out of memory, 0 bytes of JSON output.coverage.status=partial,limits=[{"name":"javascript_semantic_module_source_bytes","value":2097152,"unit":"bytes"}], the budget limitation disclosed; the failing member's staticparse_status=complete. At 2048 MiB → exit 0 in 6.39 s with the same deterministic degradation.npx vitest run --project domain --project services src/domain/javascript src/application/javascript→ 1024 passed / 0 failed (59 files), including the 5 new tests. (On the earlier acceptance base the same slice was 1023 passed / 6 failed, with the same 6 failures on cleanc1b5ccf0; current main has since fixed those 6.)npx vitest run --project adapters src/artifacts src/javascript→ 107 passed / 0 failed.npx tsc --noEmit→ pass;npx oxlint→ 0 errors (627 pre-existing warnings);npx oxfmt --check→ pass;npm run verify:module-boundaries→ 1411 files pass;git diff --check→ clean.Compatibility, safety, and release
🤖 Generated with Claude Code