morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 07:47:29 +00:00

[GH-ISSUE #5377] [Bug] JavaScript call graph resolves a callback before its var initializer executes #3260

New issue

Closed

opened 2026-10-31 00:38:30 +00:00 by bsakaguchi · 5 comments

bsakaguchi commented

2026-10-31 00:38:30 +00:00

Owner

Originally created by @morluto on GitHub (Oct 20, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/5377

Before submitting

I searched open and closed issues and related PRs.
I removed credentials, private paths, and sensitive target data.

Area

Evidence, artifacts, or reconstruction

Observed behavior

The public analyze-javascript-application CLI marks a nested cb() call as a resolved call to target even when the only top-level call to its containing function occurs before var cb = target executes:

function target() {}
function read() { cb(); }
read();
var cb = target;

Observed static output: the line 2 call-site has a calls relation to target with resolution: resolved. REA's internal resolver also reports that target set as complete: true. The control with read() moved after the declarator produces the same resolved edge. These are independently executed REA static-analysis observations; the target JavaScript was not executed.

Reproduction

At tested PR #5303 head db88f851e047d9a0e146f744a0091d40065a76fe, with locked dependencies and built runtime:

Create a directory with package.json containing {"name":"call-target-before","version":"1.0.0","main":"index.js"} and index.js containing the four lines above.
Run node scripts/rea.mjs analyze-javascript-application <directory> --format json.
Inspect the semantic graph's line 2 call-site relation: it resolves to target.
Move read(); after var cb = target; and repeat. That control also resolves to target.

Both CLI commands were executed once on that exact head. A separate source parser/indexer/resolver harness on main 799e8265b254e225dc1dabdd25e56f0ab7f86bd3 also reproduced both complete target sets. The resolver and initializer-ordering files have identical Git blobs at those revisions; no current-main CLI rerun is claimed.

Expected behavior

Do not upgrade an unordered cross-activation initializer into a complete callable target. Preserve the candidate with uncertainty, or prove the particular activation/read ordering before resolving it. Preserve normal post-initializer behavior where the analysis establishes it.

ECMAScript Variable Statement creates var bindings with undefined and assigns their initializer only when that declaration executes. EvaluateCall rejects non-callable values. Therefore, source-order reasoning says the first script reaches a call on undefined before the assignment; it cannot reach target there. This expected runtime outcome is language-semantics-derived, not a runtime observation.

Impact and scope

Observed: the static semantic Evidence overstates a callback target's certainty. NOT_ENOUGH_INFO: downstream trace/reconstruction impact, frequency in real applications, MCP delivery, other control-flow shapes and the first affected release. No target execution or source mutation occurs.

REA version

Package 6.7.0; public CLI at PR #5303 head db88f851e047d9a0e146f744a0091d40065a76fe. Source resolver probe at main 799e8265b254e225dc1dabdd25e56f0ab7f86bd3.

Runtime and platform

Node 24.18.0/npm 11.16.0; macOS/Darwin arm64.

Target and artifact context

Authored two-file JavaScript application directory; four-line source fixture and package metadata, parsed statically.

Interface

CLI

Earliest failure stage

Native analysis or reconstruction — static JavaScript semantic reconstruction

Relevant output or evidence

Selected from independently executed public Evidence:

{"scenario":"before-declarator","source_line":2,"source_kind":"call-site","target_label":"target","resolution":"resolved"}
{"scenario":"after-declarator","source_line":2,"source_kind":"call-site","target_label":"target","resolution":"resolved"}

The independent internal probe returned callableIds:["callable:function:0:20"], complete:true, and Unique local callable target. in both cases. The PR's existing focused export-shape suite also passed 51/51; those tests do not cover this separate call-identity trigger.

Typed outcome or recovery expected

An unresolved/candidate edge with a meaningful ordering limitation when the analysis cannot prove initialization at the read, rather than a uniquely resolved target.

Analysis or suspected cause

Source-derived: resolveIdentifier calculates coverage from read.hasDefiniteInitializer || semanticStableInitializer(...) !== undefined. A cross-activation unresolved read retains the full binding. The second arm treats a stable source initializer as execution proof, contrary to the initializer owner's documented contract.

Additional context

#5302/#5303 address value-lattice export shapes and leave this call-target path unchanged. Open #4173 handles conditional expression folding/deferred lexical reads, excludes var from that deferred logic and does not edit CallResolution. Merged #4243/#3880 covered conditional callback writes; closed #2562 covered skipped initializers. No exact duplicate for this nested-var read was found in the related open/closed searches. A fix is not included. AI-assisted report; REA analysis commands were executed as stated, without executing target JavaScript.

Originally created by @morluto on GitHub (Oct 20, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/5377 ### Before submitting - [x] I searched open and closed issues and related PRs. - [x] I removed credentials, private paths, and sensitive target data. ### Area Evidence, artifacts, or reconstruction ### Observed behavior The public `analyze-javascript-application` CLI marks a nested `cb()` call as a resolved call to `target` even when the only top-level call to its containing function occurs before `var cb = target` executes: ```js function target() {} function read() { cb(); } read(); var cb = target; ``` Observed static output: the line 2 call-site has a `calls` relation to `target` with `resolution: resolved`. REA's internal resolver also reports that target set as `complete: true`. The control with `read()` moved after the declarator produces the same resolved edge. These are independently executed REA static-analysis observations; the target JavaScript was not executed. ### Reproduction At tested PR #5303 head `db88f851e047d9a0e146f744a0091d40065a76fe`, with locked dependencies and built runtime: 1. Create a directory with `package.json` containing `{"name":"call-target-before","version":"1.0.0","main":"index.js"}` and `index.js` containing the four lines above. 2. Run `node scripts/rea.mjs analyze-javascript-application <directory> --format json`. 3. Inspect the semantic graph's line 2 call-site relation: it resolves to `target`. 4. Move `read();` after `var cb = target;` and repeat. That control also resolves to `target`. Both CLI commands were executed once on that exact head. A separate source parser/indexer/resolver harness on main `799e8265b254e225dc1dabdd25e56f0ab7f86bd3` also reproduced both complete target sets. The resolver and initializer-ordering files have identical Git blobs at those revisions; no current-main CLI rerun is claimed. ### Expected behavior Do not upgrade an unordered cross-activation initializer into a complete callable target. Preserve the candidate with uncertainty, or prove the particular activation/read ordering before resolving it. Preserve normal post-initializer behavior where the analysis establishes it. [ECMAScript Variable Statement](https://tc39.es/ecma262/multipage/ecmascript-language-statements-and-declarations.html#sec-variable-statement) creates `var` bindings with undefined and assigns their initializer only when that declaration executes. [EvaluateCall](https://tc39.es/ecma262/multipage/ecmascript-language-expressions.html#sec-evaluatecall) rejects non-callable values. Therefore, source-order reasoning says the first script reaches a call on undefined before the assignment; it cannot reach `target` there. This expected runtime outcome is language-semantics-derived, not a runtime observation. ### Impact and scope Observed: the static semantic Evidence overstates a callback target's certainty. NOT_ENOUGH_INFO: downstream trace/reconstruction impact, frequency in real applications, MCP delivery, other control-flow shapes and the first affected release. No target execution or source mutation occurs. ### REA version Package 6.7.0; public CLI at PR #5303 head `db88f851e047d9a0e146f744a0091d40065a76fe`. Source resolver probe at main `799e8265b254e225dc1dabdd25e56f0ab7f86bd3`. ### Runtime and platform Node 24.18.0/npm 11.16.0; macOS/Darwin arm64. ### Target and artifact context Authored two-file JavaScript application directory; four-line source fixture and package metadata, parsed statically. ### Interface CLI ### Earliest failure stage Native analysis or reconstruction — static JavaScript semantic reconstruction ### Relevant output or evidence Selected from independently executed public Evidence: ```json {"scenario":"before-declarator","source_line":2,"source_kind":"call-site","target_label":"target","resolution":"resolved"} {"scenario":"after-declarator","source_line":2,"source_kind":"call-site","target_label":"target","resolution":"resolved"} ``` The independent internal probe returned `callableIds:["callable:function:0:20"]`, `complete:true`, and `Unique local callable target.` in both cases. The PR's existing focused export-shape suite also passed 51/51; those tests do not cover this separate call-identity trigger. ### Typed outcome or recovery expected An unresolved/candidate edge with a meaningful ordering limitation when the analysis cannot prove initialization at the read, rather than a uniquely resolved target. ### Analysis or suspected cause Source-derived: [resolveIdentifier](https://github.com/morluto/rea/blob/799e8265b254e225dc1dabdd25e56f0ab7f86bd3/src/domain/javascript/javascriptSemanticCallResolution.ts#L104-L141) calculates coverage from `read.hasDefiniteInitializer || semanticStableInitializer(...) !== undefined`. A cross-activation unresolved read retains the full binding. The second arm treats a stable source initializer as execution proof, contrary to [the initializer owner's documented contract](https://github.com/morluto/rea/blob/799e8265b254e225dc1dabdd25e56f0ab7f86bd3/src/domain/javascript/javascriptSemanticState.ts#L190-L212). ### Additional context #5302/#5303 address value-lattice export shapes and leave this call-target path unchanged. Open #4173 handles conditional expression folding/deferred lexical reads, excludes var from that deferred logic and does not edit CallResolution. Merged #4243/#3880 covered conditional callback writes; closed #2562 covered skipped initializers. No exact duplicate for this nested-var read was found in the related open/closed searches. A fix is not included. AI-assisted report; REA analysis commands were executed as stated, without executing target JavaScript.

bsakaguchi

2026-10-31 00:38:30 +00:00

closed this issue
added the
triage

bug
labels

bsakaguchi commented

2026-10-31 00:38:31 +00:00

Author

Owner

@Faisal-Fayaz commented on GitHub (Oct 20, 2026):

Picking this up.

Plan

Reproduce both orderings (call site before vs. after var cb = target) on current main through the public analyze-javascript-application CLI and the internal resolver, so the baseline edge is confirmed rather than assumed.
Fix the producing boundary in src/domain/javascript/javascriptSemanticCallResolution.ts: a cross-activation var read that the analysis cannot prove was initialized at the read site should resolve to a candidate edge carrying an ordering limitation, not a completed unique target — matching the initializer owner's documented contract in src/domain/javascript/javascriptSemanticState.ts.
Preserve the post-initializer behavior: when the analysis does establish the read happens after the declarator, the edge stays resolved.

I'll add regression coverage for both orderings and report only the checks I actually run, plus anything I don't cover. Work will land on a fix/5377-... branch with a PR.

@Faisal-Fayaz commented on GitHub (Oct 20, 2026): Picking this up. **Plan** 1. Reproduce both orderings (call site before vs. after `var cb = target`) on current `main` through the public `analyze-javascript-application` CLI and the internal resolver, so the baseline edge is confirmed rather than assumed. 2. Fix the producing boundary in `src/domain/javascript/javascriptSemanticCallResolution.ts`: a cross-activation `var` read that the analysis cannot prove was initialized at the read site should resolve to a candidate edge carrying an ordering limitation, not a completed unique target — matching the initializer owner's documented contract in `src/domain/javascript/javascriptSemanticState.ts`. 3. Preserve the post-initializer behavior: when the analysis does establish the read happens after the declarator, the edge stays resolved. I'll add regression coverage for both orderings and report only the checks I actually run, plus anything I don't cover. Work will land on a `fix/5377-...` branch with a PR.

bsakaguchi commented

2026-10-31 00:38:31 +00:00

Author

Owner

@x1mm4nu3l commented on GitHub (Oct 20, 2026):

Reproduced on main 2538214e2, Linux/Node 24.18.0, through the CLI and stdio MCP. Both orderings resolve the line-2 cb() to target; running the tiny fixtures in Node throws TypeError only before initialization. A calls-only semantic trace also follows that edge without opting into ambiguous edges and reports found, with no relevant unknowns but partial overall coverage.

@x1mm4nu3l commented on GitHub (Oct 20, 2026): Reproduced on main 2538214e2, Linux/Node 24.18.0, through the CLI and stdio MCP. Both orderings resolve the line-2 `cb()` to `target`; running the tiny fixtures in Node throws TypeError only before initialization. A calls-only semantic trace also follows that edge without opting into ambiguous edges and reports found, with no relevant unknowns but partial overall coverage.

bsakaguchi commented

2026-10-31 00:38:32 +00:00

Author

Owner

@Faisal-Fayaz commented on GitHub (Oct 20, 2026):

Thanks for the reproduction — the runtime TypeError check is what turns the source-derived argument into an observed one, and the MCP stdio confirmation means this is not CLI-only.

I read the producing boundary on current main (07476890). Source-derived only so far; I have not executed anything yet.

src/domain/javascript/javascriptSemanticCallResolution.ts:131-133 computes coverage as read.hasDefiniteInitializer || semanticStableInitializer(binding, context.state) !== undefined.
semanticStableInitializer (src/domain/javascript/javascriptSemanticState.ts:196-212) documents that it "establishes a stable source fact only; it does not prove immutability or that the declaration has executed at a particular program point" — so the second arm is being used as execution proof.
For this shape read.hasDefiniteInitializer is already false: the read sits in read()'s body and the initializer in the top-level body, so positionPrecedes cannot relate them (javascriptSemanticMutationInitializers.ts:661).

Scope update from your trace finding: the calls-only trace follows that edge without opting into ambiguous edges and reports found with no relevant unknowns, so the over-confident target reaches trace/reconstruction answers. I'll cover the MCP stdio path and the calls-only trace alongside the analyze-javascript-application semantic graph, and make sure the downgraded edge surfaces as a stated limitation there rather than silence.

One nuance the fix must not break: semanticStableInitializer also covers the hoisted function/class arm, where the value genuinely is initialized before any statement runs. Only the var arm is wrong — var creates the binding with undefined and assigns at declarator execution. So the fix narrows the var arm rather than dropping the helper.

The harder half is the "after" control. Lexical position cannot establish it either (read and declarator are in different bodies), so proving read() runs after var cb = target needs call-graph-aware ordering rather than a position comparison. My inclination is to land the downgrade first — an unresolved/candidate edge with an explicit ordering limitation — and treat call-graph-ordered proof as a follow-up, unless you would rather it land in the same change.

Do you want to co-own this, or hand it over? If you prefer, we could split it: you take the call-graph ordering proof and I take the downgrade plus regression coverage.

@Faisal-Fayaz commented on GitHub (Oct 20, 2026): Thanks for the reproduction — the runtime `TypeError` check is what turns the source-derived argument into an observed one, and the MCP stdio confirmation means this is not CLI-only. I read the producing boundary on current `main` (`07476890`). Source-derived only so far; I have not executed anything yet. - `src/domain/javascript/javascriptSemanticCallResolution.ts:131-133` computes coverage as `read.hasDefiniteInitializer || semanticStableInitializer(binding, context.state) !== undefined`. - `semanticStableInitializer` (`src/domain/javascript/javascriptSemanticState.ts:196-212`) documents that it "establishes a stable source fact only; it does not prove immutability or that the declaration has executed at a particular program point" — so the second arm is being used as execution proof. - For this shape `read.hasDefiniteInitializer` is already false: the read sits in `read()`'s body and the initializer in the top-level body, so `positionPrecedes` cannot relate them (`javascriptSemanticMutationInitializers.ts:661`). **Scope update from your trace finding:** the calls-only trace follows that edge without opting into ambiguous edges and reports `found` with no relevant unknowns, so the over-confident target reaches trace/reconstruction answers. I'll cover the MCP stdio path and the calls-only trace alongside the `analyze-javascript-application` semantic graph, and make sure the downgraded edge surfaces as a stated limitation there rather than silence. **One nuance the fix must not break:** `semanticStableInitializer` also covers the hoisted function/class arm, where the value genuinely is initialized before any statement runs. Only the `var` arm is wrong — `var` creates the binding with `undefined` and assigns at declarator execution. So the fix narrows the `var` arm rather than dropping the helper. **The harder half is the "after" control.** Lexical position cannot establish it either (read and declarator are in different bodies), so proving `read()` runs after `var cb = target` needs call-graph-aware ordering rather than a position comparison. My inclination is to land the downgrade first — an unresolved/candidate edge with an explicit ordering limitation — and treat call-graph-ordered proof as a follow-up, unless you would rather it land in the same change. Do you want to co-own this, or hand it over? If you prefer, we could split it: you take the call-graph ordering proof and I take the downgrade plus regression coverage.

bsakaguchi commented

2026-10-31 00:38:32 +00:00

Author

Owner

@matbrgz commented on GitHub (Oct 20, 2026):

Cross-referencing with canonical issue #5302 ([Bug] Export return shapes give a var read before its initializer the initializer's value). Both identify the same core analysis defect: JavaScript var declarations hoisting without preserving uninitialized / undefined state prior to the initializer's execution, leading to premature symbol resolution in both call graph and export shape models. Cataloged under Cluster BM in consolidation tracker #4330.

@matbrgz commented on GitHub (Oct 20, 2026): Cross-referencing with canonical issue #5302 (`[Bug] Export return shapes give a var read before its initializer the initializer's value`). Both identify the same core analysis defect: JavaScript `var` declarations hoisting without preserving uninitialized / undefined state prior to the initializer's execution, leading to premature symbol resolution in both call graph and export shape models. Cataloged under Cluster BM in consolidation tracker #4330.

bsakaguchi commented

2026-10-31 00:38:32 +00:00

Author

Owner

@morluto commented on GitHub (Oct 21, 2026):

Closing as already fixed on main.

Verified on 71b696582 with npm run test:local -- src/domain/javascript/javascriptSemanticCallResolution.test.ts (25 passed): function read(){cb();} read(); var cb=target; stays complete: false; the same-activation call after var cb=target stays complete: true. Fix commit 3b0217956.

@morluto commented on GitHub (Oct 21, 2026): Closing as already fixed on main. Verified on `71b696582` with `npm run test:local -- src/domain/javascript/javascriptSemanticCallResolution.test.ts` (25 passed): `function read(){cb();} read(); var cb=target;` stays `complete: false`; the same-activation call after `var cb=target` stays `complete: true`. Fix commit `3b0217956`.

bsakaguchi referenced this issue

2026-10-31 01:08:20 +00:00

[PR #3227] [CLOSED] fix(ci): track compiler inputs and module scopes #5657

bsakaguchi referenced this issue

2026-10-31 01:08:36 +00:00

[PR #3260] [MERGED] test(domain): keep rejected Evidence payloads inert #5681