[GH-ISSUE #6667] [Bug] CommonJS block functions leave stale literal return values #4046

New issue

Closed

opened 2026-10-31 00:46:20 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 00:46:20 +00:00

Owner

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

Before submitting

I searched the open and closed issues for a related report.
I removed credentials, tokens, private paths, and sensitive target data.

Area

Evidence, artifacts, or reconstruction

Observed behavior

For non-strict CommonJS source, analyzeJavaScriptArtifactFiles retains the old var initializer after a same-named block function can replace that binding. The exported read function is reported to return the definite literal 1 for var value = 1; { function value() {} } exports.read = function read(){return value;};.

This is observed analyzer output from the source application boundary. The expected binding replacement follows non-strict block-function language semantics; the target JavaScript was not executed. Public CLI/MCP output and the installed npm entrypoint were not tested.

Reproduction

With the pinned Node runtime on PATH and locked dependencies installed, run the following from the checkout root. This exact here-document ran once after preparation of the final reproduction; preceding exploratory application-boundary cases also showed the same result.

node --import ./node_modules/tsx/dist/loader.mjs --input-type=module <<'JS'
import { createHash } from 'node:crypto';
import { resolve } from 'node:path';
import { pathToFileURL } from 'node:url';
const { analyzeJavaScriptArtifactFiles } = await import(pathToFileURL(resolve('src/application/javascript/JavaScriptArtifactAnalysis.ts')).href);

function analyze(source, path = 'entry.cjs') {
  const sha256 = createHash('sha256').update(source).digest('hex');
  const file = {path, sha256, container_sha256: sha256,
    bytes: Buffer.byteLength(source), inventory_artifact_id: 'fixture',
    kind: 'javascript', unpacked: false, text: {included: true, value: source}};
  const result = analyzeJavaScriptArtifactFiles({files: [file], containers: [],
    unavailable_relevant_files: [], text_bytes_read: file.bytes, invalid_utf8_files: 0});
  return result.files[0].semantic.ir;
}

const base = 'var value = 1; { function value() {} } exports.read = function read(){return value;};';
for (const [name, source, path] of [
  ['sloppy', base, 'entry.cjs'],
  ['strict-control', '"use strict"; ' + base, 'entry.cjs'],
  ['lexical-control', base.replace('var value', 'let value'), 'entry.cjs'],
  ['module-control', 'var value = 1; { function value() {} } export function read(){return value;}', 'entry.mjs'],
]) {
  const ir = analyze(source, path);
  const read = ir.callables.find(c => c.name === 'read');
  console.log(JSON.stringify({name, returns: read.returnSites.map(s => s.value)}));
}
JS

Expected behavior

The non-strict fixture must not claim that the later read is definitely the earlier number. Under the legacy block-function semantics, the block function can replace the same-named outer var; if REA does not model that replacement, it should keep the value unknown. REA already guards against this exact unindexed copy in its expression-shape resolver, consistent with its uncertainty-preservation contract. Strict-mode, module, and outer-let controls should retain literal 1 because this replacement does not apply to those bindings.

Impact and scope

Observed with four non-strict fixture forms: a bare block, a block under if (true), a value copied after the block, and a block inside the exported function. Each retained literal 1. The final standalone reproduction includes the bare-block failure and three controls. This can misstate an exported function's return facts; frequency in real applications and downstream comparison or trace effects are unknown.

REA version

Package 6.7.0; source revision 19520d0008.

Runtime and platform

Node.js v24.22.1; Linux x64; dependencies from the repository lockfile. No Hopper, Ghidra, or other native provider was involved.

Target and artifact context

Authored, in-memory JavaScript source strings supplied as content-addressed artifact records. CommonJS fixtures use entry.cjs; the module control uses entry.mjs. The analyzer parses source without executing it.

Interface

Other

Earliest failure stage

Native analysis or reconstruction

Relevant output or evidence

The final reproduction exited successfully and printed:

{"name":"sloppy","returns":[{"status":"literal","value":1}]}
{"name":"strict-control","returns":[{"status":"literal","value":1}]}
{"name":"lexical-control","returns":[{"status":"literal","value":1}]}
{"name":"module-control","returns":[{"status":"literal","value":1}]}

Only sloppy is incorrect: the shared output does not distinguish its possible legacy var replacement from the controls. The analyzer result, rather than the target, was executed.

Expected source: REA preserves uncertainty; the existing Annex B guard; the language-semantics reference under Expected behavior.
Failure source: Executed analyzeJavaScriptArtifactFiles reproduction above, whose application entrypoint returns the semantic IR containing the literal return site.
Evidence provenance: observed for REA output; language-semantics-derived for the expected target value.
Local verification: reproduced at the source application API; no analyzed target execution or public CLI/MCP verification.
Reproduction completeness: complete for that application-boundary failure.

Typed outcome or recovery expected

Keep the non-strict same-named var read unknown, or recover the replacement function without publishing the stale number. Preserve literal 1 for the three controls. Applying a blanket loss of values to all block functions would discard these valid control results.

Analysis or suspected cause

Source-derived: the identifier evaluator sees the outer binding's original initializer and no indexed ordinary assignment. The block function owns a distinct lexical binding, while the Annex B copy into the outer var is absent from that value path. The expression-shape guard already documents this ownership gap; a complete diagnosis or implementation is not claimed.

Additional context

Open and closed issue searches covered Annex B, Annex, block-level functions, block functions, sloppy functions, and the saved open corpus. Related closed #3223 concerns uninitialized redeclarations of implicit CommonJS wrapper parameters; this fixture uses an ordinary local name and an initialized var. Open #5302 concerns reading before a later initializer; here initialization precedes the block and read, and an unindexed binding replacement changes the value. No exact duplicate was found in the open and closed searches refreshed before submission.

Triage assessment

Impact level: P2
Assessment status: supported
Impact basis: The executed application-boundary probe publishes a definite return literal contradicted by the source's language semantics. This is a bounded evidence-correctness defect; no severe downstream incident or widespread workflow failure was established.
Workaround status: unverified
Workaround basis: No existing analyzer option or equivalent automatic workflow preserving the required semantic result was demonstrated. Changing analyzed source or enabling strict mode can change the target's behavior and is not an established workaround.

Originally created by @coygeek on GitHub (Oct 23, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/6667 ## Before submitting - [x] I searched the open and closed issues for a related report. - [x] I removed credentials, tokens, private paths, and sensitive target data. ## Area Evidence, artifacts, or reconstruction ## Observed behavior For non-strict CommonJS source, `analyzeJavaScriptArtifactFiles` retains the old `var` initializer after a same-named block function can replace that binding. The exported `read` function is reported to return the definite literal `1` for `var value = 1; { function value() {} } exports.read = function read(){return value;};`. This is observed analyzer output from the source application boundary. The expected binding replacement follows non-strict block-function language semantics; the target JavaScript was not executed. Public CLI/MCP output and the installed npm entrypoint were not tested. ## Reproduction With the pinned Node runtime on `PATH` and locked dependencies installed, run the following from the checkout root. This exact here-document ran once after preparation of the final reproduction; preceding exploratory application-boundary cases also showed the same result. ```sh node --import ./node_modules/tsx/dist/loader.mjs --input-type=module <<'JS' import { createHash } from 'node:crypto'; import { resolve } from 'node:path'; import { pathToFileURL } from 'node:url'; const { analyzeJavaScriptArtifactFiles } = await import(pathToFileURL(resolve('src/application/javascript/JavaScriptArtifactAnalysis.ts')).href); function analyze(source, path = 'entry.cjs') { const sha256 = createHash('sha256').update(source).digest('hex'); const file = {path, sha256, container_sha256: sha256, bytes: Buffer.byteLength(source), inventory_artifact_id: 'fixture', kind: 'javascript', unpacked: false, text: {included: true, value: source}}; const result = analyzeJavaScriptArtifactFiles({files: [file], containers: [], unavailable_relevant_files: [], text_bytes_read: file.bytes, invalid_utf8_files: 0}); return result.files[0].semantic.ir; } const base = 'var value = 1; { function value() {} } exports.read = function read(){return value;};'; for (const [name, source, path] of [ ['sloppy', base, 'entry.cjs'], ['strict-control', '"use strict"; ' + base, 'entry.cjs'], ['lexical-control', base.replace('var value', 'let value'), 'entry.cjs'], ['module-control', 'var value = 1; { function value() {} } export function read(){return value;}', 'entry.mjs'], ]) { const ir = analyze(source, path); const read = ir.callables.find(c => c.name === 'read'); console.log(JSON.stringify({name, returns: read.returnSites.map(s => s.value)})); } JS ``` ## Expected behavior The non-strict fixture must not claim that the later read is definitely the earlier number. Under [the legacy block-function semantics](https://tc39.es/ecma262/multipage/additional-ecmascript-features-for-web-browsers.html#sec-block-level-function-declarations-web-legacy-compatibility-semantics), the block function can replace the same-named outer `var`; if REA does not model that replacement, it should keep the value unknown. REA already [guards against this exact unindexed copy in its expression-shape resolver](https://github.com/morluto/rea/blob/19520d00087bc92b5c1020918ea3b7c064768bb4/src/domain/javascript/javascriptSemanticExpressionShapes.ts#L108-L116), consistent with its [uncertainty-preservation contract](https://github.com/morluto/rea/blob/19520d00087bc92b5c1020918ea3b7c064768bb4/AGENTS.md#L19-L21). Strict-mode, module, and outer-`let` controls should retain literal `1` because this replacement does not apply to those bindings. ## Impact and scope Observed with four non-strict fixture forms: a bare block, a block under `if (true)`, a value copied after the block, and a block inside the exported function. Each retained literal `1`. The final standalone reproduction includes the bare-block failure and three controls. This can misstate an exported function's return facts; frequency in real applications and downstream comparison or trace effects are unknown. ## REA version Package 6.7.0; source revision 19520d00087bc92b5c1020918ea3b7c064768bb4. ## Runtime and platform Node.js v24.22.1; Linux x64; dependencies from the repository lockfile. No Hopper, Ghidra, or other native provider was involved. ## Target and artifact context Authored, in-memory JavaScript source strings supplied as content-addressed artifact records. CommonJS fixtures use `entry.cjs`; the module control uses `entry.mjs`. The analyzer parses source without executing it. ## Interface Other ## Earliest failure stage Native analysis or reconstruction ## Relevant output or evidence The final reproduction exited successfully and printed: ```jsonl {"name":"sloppy","returns":[{"status":"literal","value":1}]} {"name":"strict-control","returns":[{"status":"literal","value":1}]} {"name":"lexical-control","returns":[{"status":"literal","value":1}]} {"name":"module-control","returns":[{"status":"literal","value":1}]} ``` Only `sloppy` is incorrect: the shared output does not distinguish its possible legacy `var` replacement from the controls. The analyzer result, rather than the target, was executed. - Expected source: [REA preserves uncertainty](https://github.com/morluto/rea/blob/19520d00087bc92b5c1020918ea3b7c064768bb4/AGENTS.md#L19-L21); [the existing Annex B guard](https://github.com/morluto/rea/blob/19520d00087bc92b5c1020918ea3b7c064768bb4/src/domain/javascript/javascriptSemanticExpressionShapes.ts#L108-L116); the language-semantics reference under Expected behavior. - Failure source: Executed `analyzeJavaScriptArtifactFiles` reproduction above, whose [application entrypoint](https://github.com/morluto/rea/blob/19520d00087bc92b5c1020918ea3b7c064768bb4/src/application/javascript/JavaScriptArtifactAnalysis.ts#L87-L95) returns the semantic IR containing the literal return site. - Evidence provenance: observed for REA output; language-semantics-derived for the expected target value. - Local verification: reproduced at the source application API; no analyzed target execution or public CLI/MCP verification. - Reproduction completeness: complete for that application-boundary failure. ## Typed outcome or recovery expected Keep the non-strict same-named `var` read unknown, or recover the replacement function without publishing the stale number. Preserve literal `1` for the three controls. Applying a blanket loss of values to all block functions would discard these valid control results. ## Analysis or suspected cause Source-derived: the [identifier evaluator](https://github.com/morluto/rea/blob/19520d00087bc92b5c1020918ea3b7c064768bb4/src/domain/javascript/javascriptSemanticValues.ts#L657-L691) sees the outer binding's original initializer and no indexed ordinary assignment. The block function owns a distinct lexical binding, while the Annex B copy into the outer `var` is absent from that value path. The expression-shape guard already documents this ownership gap; a complete diagnosis or implementation is not claimed. ## Additional context Open and closed issue searches covered Annex B, Annex, block-level functions, block functions, sloppy functions, and the saved open corpus. Related closed [#3223](https://github.com/morluto/rea/issues/3223) concerns uninitialized redeclarations of implicit CommonJS wrapper parameters; this fixture uses an ordinary local name and an initialized `var`. Open [#5302](https://github.com/morluto/rea/issues/5302) concerns reading before a later initializer; here initialization precedes the block and read, and an unindexed binding replacement changes the value. No exact duplicate was found in the open and closed searches refreshed before submission. ## Triage assessment - Impact level: P2 - Assessment status: supported - Impact basis: The executed application-boundary probe publishes a definite return literal contradicted by the source's language semantics. This is a bounded evidence-correctness defect; no severe downstream incident or widespread workflow failure was established. - Workaround status: unverified - Workaround basis: No existing analyzer option or equivalent automatic workflow preserving the required semantic result was demonstrated. Changing analyzed source or enabling strict mode can change the target's behavior and is not an established workaround.

bsakaguchi closed this issue

2026-10-31 00:46:20 +00:00

bsakaguchi referenced this issue

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

[PR #4046] test(android): build split-set fixtures with aapt2 and read them with the manifest reader #6018