Repository navigation
CommonJS var wrapper redeclarations lose semantic exports and require origins #3223
Description
On current main 58300885207133422af235e0f2b10658cf958b03, the semantic CommonJS graph loses normal exports when a .cjs source redeclares the implicit wrapper parameters with uninitialized top-level var declarations.
These declarations do not overwrite Node's wrapper parameters. Node 24.18.0 runs all three variants below successfully and returns the exported value 42. REA's public application CLI omits the commonjs-export / exposes facts for the exports and module variants. A control without the declarations correctly emits them. With var require;, the static require edge survives, but the semantic require origin/export dependency is lost, so two consumers disagree.
Reproduction after pinned npm ci and npm run build:cached:
mkdir -p /tmp/rea-wrapper-redeclaration
cat > /tmp/rea-wrapper-redeclaration/entry.cjs <<'JS'
var module, exports, require;
const value = require('./dep.cjs');
exports.value = value;
console.log(module.exports.value);
JS
printf 'module.exports = 42;\n' > /tmp/rea-wrapper-redeclaration/dep.cjs
node /tmp/rea-wrapper-redeclaration/entry.cjs
node scripts/rea.mjs analyze-javascript-application /tmp/rea-wrapper-redeclaration --format jsonNode control: exit 0, stdout 42. In the REA graph, the entry should retain a semantic require relationship to dep.cjs and the value CommonJS export with its source identity/range. Instead its export facts are missing. Replacing exports.value with module.exports.value has the same omission. Removing the var declarations restores the facts. Individually tested var require;, var exports;, and var module; variants show the same source-boundary gap.
Expected scope: retain the original wrapper binding for uninitialized program-owned var redeclarations in explicit CommonJS sources. Nested locals, lexical declarations, initialized declarations, assignments, uncertain/dynamic environments and non-CommonJS sources must remain properly shadowed or unknown. No application code should be executed during analysis.
Root cause: semantic scope collection creates an ordinary local binding for these redeclarations, while isUnshadowedGlobal and semanticRequireOrigin then reject any lexical binding as a wrapper/global origin. Static require handling already retains uninitialized top-level wrapper redeclarations, which makes the disagreement visible in the same public graph. The correction belongs at the common binding/default-value owner, not in graph consumers.
Related search checked redeclarations, var-module, var-exports and var-require reports. #2989 addresses false static edges from truly shadowed require calls and explicitly keeps top-level uninitialized wrapper declarations; it does not restore these semantic export/default-origin facts.
Verified with actual Node 24.18.0 and REA's compiled public CLI on macOS arm64, locked dependencies and unchanged main. Fixtures are self-owned CommonJS code; this is static-analysis verification, not a native-provider claim.
Activity
akushonkamen commentedon Oct 14, 2026
Claiming this. Scope I am taking: exactly the correction you scoped — the common binding/default-value owner retains the original wrapper binding for uninitialized program-owned top-level
varredeclarations ofmodule/exports/requirein explicit CommonJS sources, so the semantic export facts and require origins survive; nested locals, lexical declarations, initialized declarations, assignments, and non-CommonJS sources stay properly shadowed or unknown, and no graph consumer changes.Reproduced locally on
mainat500e32399before claiming (macOS arm64, Node 24.14.0): your fixture's entry module loses thecommonjs-exportfact in the public graph while the control without thevardeclarations emits it, and Node 24.14.0 runs the fixture and prints 42. Out of scope: the static require edge (already retained per #2989's split), and any behavior change for genuinely shadowed wrappers. I will validate with unit-level red→green tests at the semantic layer plus the focused JavaScript semantic suites and the public CLI/MCP paths for both the fixture and the control.akushonkamen commentedon Oct 14, 2026
Delivery update. Fixed and validated on branch
akushonkamen:fix/commonjs-wrapper-redeclarations(commitb4ed1c13608f21dea51303dd3b809c8ad73fd059, based onmainat500e32399, single commit). PR creation from this account remains repository-blocked (GraphQLCreatePullRequestpermission error; REST 404, retested), so the branch and this summary stand in for the PR.What the branch contains (both halves of the disagreement your analysis identified):
javascriptSemanticAnalysis.ts— the common binding owner:bindInnerDeclarationskips ordinary binding registration for an uninitializedvardeclarator that lands in the program scope (so block-levelvarredeclarations are covered and function-scoped ones are not) and namesmodule/exports/require, in a source analyzed in require mode. The wrapper binding is thereby retained and every existing gate (isUnshadowedGlobal,semanticRequireOrigin) works unchanged — no graph consumer changes.javascriptModuleResolutionMode.ts—.cjsnow selects require mode like.cts. This was the second, silent half: the application layer derives the semantic resolution mode per source path, and.cjsmapped to"import", so the retained-wrapper rule could never apply to the very sources your report targets even after (1). The parser already treats.cjsas CommonJS (runtimeSourceTypeForPath); the mode now agrees.Out of scope, unchanged: the static require edge (#2989's split), graph consumers, initialized/lexical/function-scoped shadowing (pinned by a new shadowing test), and non-CommonJS sources.
Executed validation (macOS arm64, Node 24.14.0, on the branch):
npm run test:focused -- src/domain/javascript/javascriptSemanticAnalysis.require_links.test.ts— new retention test fails on the unmodified tree (1 failed | 25 passed), green after (26 passed); the new shadowing test (initializedvar module = {},let require, nestedvar exports) passes before and after and pins the boundary.npm run test:focused -- src/domain/javascript/javascriptModuleResolutionMode.test.ts— new.cjsmode test fails with the one-line mapping stashed, green after (2 passed).npm run compile, thennode scripts/rea.mjs analyze-javascript-application): your fixture now emits the entry module'scommonjs-exportfact and the require edge with specifier./dep.cjs; the control without thevardeclarations is unchanged. One verification trap worth noting: identical fixture content replays the retained evidence snapshot (identicalevidence_id), so I re-verified with a fresh run and confirmed the pre-fix and post-fix graphs differ.npx vitest run --project domain src/domain/javascript— 65 files, 1530 passed;--project services src/application/javascript— 11 files, 291 passed;--project adapters src/javascript— 4 files, 43 passed;--project boundary tests/boundary/filesystem/javascript-applications— 17 files, 184 passed; acceptancejavascriptCommonJsDirectoryLookup+javascriptEndpointPayload+javascriptConfigurationReadsCli+applicationWorkflowCli— 41 passed; JS MCP boundary files — 5 passed/2 skipped; CLI boundaryjavascriptApplicationJsonOutput+javascriptApplicationProgress— 17 passed.npm run format:checkpass;npx tsc --noEmitclean;npm run lint0 errors (module boundaries pass);npx knipclean;npm run jscpd3.19% < 3.5%;npm run docs:checkpass.Not claimed: nothing runtime-related — this is static-analysis fidelity only, no code execution during analysis, matching your expected scope.
fix(javascript): retain CommonJS wrapper bindings across var redeclar…
akushonkamen commentedon Oct 14, 2026
Rebase + fresh validation (this session). PR creation retried just now and is still repository-blocked for this account (GraphQL
CreatePullRequestpermission error; REST 404) — the PR could not be opened from here.mainadvanced 25 commits since my delivery summary; the branch rebased cleanly (single commit6afc377dd, patch-id-identical to the announcedb4ed1c136, now based onmainat748440160) and was force-pushed toakushonkamen:fix/commonjs-wrapper-redeclarations. A maintainer can open the PR from that head (Closes #3223).Every gate re-run on the rebased branch this session:
748440160with this branch's test files:npm run test:focused -- src/domain/javascript/javascriptSemanticAnalysis.require_links.test.ts= 1 failed | 25 passed (26);npm run test:focused -- src/domain/javascript/javascriptModuleResolutionMode.test.ts= 1 failed | 1 passed (2). Green on the branch: 26 passed and 2 passed.npx vitest run --project domain src/domain/javascript— 67 files, 1563 passed (upstream additions grew this from the previously recorded 65/1530);--project services src/application/javascript— 11 files, 291 passed;--project adapters src/javascript— 4 files, 43 passed.npx vitest run --project boundary tests/boundary/filesystem/javascript-applications— 17 files, 190 passed, 1 failed. That one failure (javascriptFileUrlPaths.test.ts, "resolves a decoded literal file URL at the filesystem boundary") is pre-existing on unmodifiedmain: verified by detached checkout, it fails identically there (1 failed | 25 passed) and is unrelated to this branch.javascriptCommonJsDirectoryLookup+javascriptEndpointPayload+javascriptConfigurationReadsCli+applicationWorkflowCli) — 41 passed; JS MCP boundary (all sixtests/boundary/mcp/javascript*.test.ts) — 9 passed, 2 skipped (pre-existingjavascriptRecoveryMcpskips); CLI boundary (javascriptApplicationJsonOutput+javascriptApplicationProgress) — 17 passed.npm run compile+node scripts/rea.mjs analyze-javascript-application, fresh fixture content to avoid retained-evidence replay): your fixture now emits the entrycommonjs-exportfact and the require edge with specifier./dep.cjs; the control without thevardeclarations is unchanged.npm run format:checkpass;npx tsc --noEmitclean;npm run lint0 errors + module boundaries pass (1863 files);npx knipclean;npm run jscpd3.20% < 3.5%;npm run docs:check5 passed.Unchanged and not claimed: static-analysis fidelity only — no code execution during analysis, no runtime claim.
fix(javascript): preserve loader ownership and unavailable artifact f…