morluto/rea

Fork 0

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

[GH-ISSUE #3341] fix(javascript): main fails the untouched wrapper require redeclaration regression #1210

New issue

Closed

opened 2026-10-16 21:02:49 +00:00 by bsakaguchi · 3 comments

bsakaguchi commented

2026-10-16 21:02:49 +00:00

Owner

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

Originally assigned to: @N0zoM1z0 on GitHub.

Main 0351328e5 fails the existing regression keeps a top-level wrapper require redeclaration in src/domain/javascript/javascriptStaticAnalysisModuleIndex.test.ts:80. Hosted source-test job 115769597003 fails the same case: 512 other files pass.

Local reproduction on a clean main checkout (Linux x86_64, Node 24.18.0, npm 11.16.0, Vitest 5.0.3):

npm run test:local -- src/domain/javascript/javascriptStaticAnalysisModuleIndex.test.ts

Result: 26 passed / 1 failed, exit 1, 1.48 s. Actual analyzeJavaScriptStaticSource('var require; require("./wrapper.cjs");') reference has kind: require, specifier: null, module_key: null; the regression expects specifier ./wrapper.cjs.

Runtime control using Node's real CommonJS wrapper:

node --input-type=commonjs -e 'var require; console.log(JSON.stringify({requireType: typeof require, loaded: require("node:path").basename("/tmp/wrapper.cjs")}))'

Observed: {"requireType":"function","loaded":"wrapper.cjs"}. This supports the test's untouched-wrapper-redeclaration contract; it does not establish that every source with an unavailable/foreign loader should be resolved.

Source context: 7e5cffa2a says it preserves untouched CJS wrapper redeclarations while conservatively handling loader ownership and assignments. The specific producer cause is NOT_ENOUGH_INFO; no fix or broader impact is asserted here. Preserve the adjacent reassignment, shadowing and unknown-loader negative cases when correcting this failure.

This is independent of #3299, whose only diff is a Rizin shell fixture in a different test group. No assertion/deadline change or blind CI retry is proposed. The reproduced main failure currently blocks that PR's required CI gate.

Originally created by @N0zoM1z0 on GitHub (Oct 14, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/3341 Originally assigned to: @N0zoM1z0 on GitHub. Main `0351328e5` fails the existing regression `keeps a top-level wrapper require redeclaration` in `src/domain/javascript/javascriptStaticAnalysisModuleIndex.test.ts:80`. Hosted source-test job [115769597003](https://github.com/morluto/rea/actions/runs/38539192025/job/115769597003) fails the same case: 512 other files pass. Local reproduction on a clean main checkout (Linux x86_64, Node 24.18.0, npm 11.16.0, Vitest 5.0.3): ```sh npm run test:local -- src/domain/javascript/javascriptStaticAnalysisModuleIndex.test.ts ``` Result: **26 passed / 1 failed**, exit 1, 1.48 s. Actual `analyzeJavaScriptStaticSource('var require; require("./wrapper.cjs");')` reference has `kind: require`, `specifier: null`, `module_key: null`; the regression expects specifier `./wrapper.cjs`. Runtime control using Node's real CommonJS wrapper: ```sh node --input-type=commonjs -e 'var require; console.log(JSON.stringify({requireType: typeof require, loaded: require("node:path").basename("/tmp/wrapper.cjs")}))' ``` Observed: `{"requireType":"function","loaded":"wrapper.cjs"}`. This supports the test's untouched-wrapper-redeclaration contract; it does not establish that every source with an unavailable/foreign loader should be resolved. Source context: `7e5cffa2a` says it preserves untouched CJS wrapper redeclarations while conservatively handling loader ownership and assignments. The specific producer cause is **NOT_ENOUGH_INFO**; no fix or broader impact is asserted here. Preserve the adjacent reassignment, shadowing and unknown-loader negative cases when correcting this failure. This is independent of #3299, whose only diff is a Rizin shell fixture in a different test group. No assertion/deadline change or blind CI retry is proposed. The reproduced main failure currently blocks that PR's required CI gate.

bsakaguchi closed this issue

2026-10-16 21:02:50 +00:00

bsakaguchi commented

2026-10-16 21:02:51 +00:00

Author

Owner

@N0zoM1z0 commented on GitHub (Oct 14, 2026):

Additional source evidence (not a completed diagnosis): isUnmodifiedCommonJsWrapperBinding in javascriptSemanticState.ts requires state.staticImportResolutionMode === "require". The failing test calls the formatless analyzeJavaScriptStaticSource(source) entrypoint; semantic state construction defaults its resolution mode to "import".

The observed test failure is established. Missing mode propagation and a fixture using the wrong context remain competing explanations. The Node runtime control establishes behavior specifically inside a real CommonJS wrapper, not behavior for formatless/ESM input. Do not remove the ownership guard merely to satisfy this assertion; preserve unknown/foreign loader outcomes and use an actual .cjs workflow plus the appropriate semantic entrypoint to decide the owning correction.

@N0zoM1z0 commented on GitHub (Oct 14, 2026): Additional source evidence (not a completed diagnosis): `isUnmodifiedCommonJsWrapperBinding` in `javascriptSemanticState.ts` requires `state.staticImportResolutionMode === "require"`. The failing test calls the formatless `analyzeJavaScriptStaticSource(source)` entrypoint; semantic state construction defaults its resolution mode to `"import"`. The observed test failure is established. Missing mode propagation and a fixture using the wrong context remain competing explanations. The Node runtime control establishes behavior specifically inside a real CommonJS wrapper, not behavior for formatless/ESM input. Do not remove the ownership guard merely to satisfy this assertion; preserve unknown/foreign loader outcomes and use an actual `.cjs` workflow plus the appropriate semantic entrypoint to decide the owning correction.

bsakaguchi commented

2026-10-16 21:02:51 +00:00

Author

Owner

@N0zoM1z0 commented on GitHub (Oct 14, 2026):

Executed the actual analyzeJavaScriptApplication service and artifact resource scope on main 0351328e5, with two isolated directory applications containing var require; require("./wrapper.cjs"); and a real local wrapper.cjs:

package.json type commonjs, entry main.cjs: succeeds; graph edge specifier ./wrapper.cjs, resolution mode require, status resolved.
Type module, entry main.mjs: succeeds without inferring that dependency.
Both owned artifact scopes and temporary directories were closed/removed.

Thus the real application contract is working for this input. The failed source-only test currently requests an owned-wrapper result from a formatless static entrypoint whose semantic indexing defaults to import mode. Proposed fixture correction: retain a conservative formatless-input assertion and move the positive untouched-wrapper assertion to the existing semantic require-link family using a parsed .cjs file and its actual require resolution mode. Preserve the same source literal/specifier oracle, all adjacent reassignment/shadowing cases, and the production ownership guard. This is evidence for a context-drift correction, not grounds to weaken the producer's uncertainty policy.

@N0zoM1z0 commented on GitHub (Oct 14, 2026): Executed the actual `analyzeJavaScriptApplication` service and artifact resource scope on main `0351328e5`, with two isolated directory applications containing `var require; require("./wrapper.cjs");` and a real local `wrapper.cjs`: - `package.json` type `commonjs`, entry `main.cjs`: succeeds; graph edge specifier `./wrapper.cjs`, resolution mode `require`, status `resolved`. - Type `module`, entry `main.mjs`: succeeds without inferring that dependency. - Both owned artifact scopes and temporary directories were closed/removed. Thus the real application contract is working for this input. The failed source-only test currently requests an owned-wrapper result from a formatless static entrypoint whose semantic indexing defaults to import mode. Proposed fixture correction: retain a conservative formatless-input assertion and move the positive untouched-wrapper assertion to the existing semantic require-link family using a parsed `.cjs` file and its actual `require` resolution mode. Preserve the same source literal/specifier oracle, all adjacent reassignment/shadowing cases, and the production ownership guard. This is evidence for a context-drift correction, not grounds to weaken the producer's uncertainty policy.

bsakaguchi commented

2026-10-16 21:02:52 +00:00

Author

Owner

@N0zoM1z0 commented on GitHub (Oct 14, 2026):

Resolved by main's producer correction dae1e5538: wrapper ownership now follows the parsed program's script/module context rather than the specifier-resolution hint. The original static-entrypoint positive assertion remains intact.

My earlier fixture-only diagnosis was premature: a working application-level graph does not rule out a defect in the lower static entrypoint. I am withdrawing #3394's test-only proposal; none of its test changes were merged into main. The owning producer fix preserves the stronger original oracle.

Verified on main ba33c706d integrated with the independent Hopper fixture correction: 8 relevant files / 145 tests pass. Direct current-production controls also pass for (1) script/CJS with an import-resolution hint, (2) explicit ESM even with a require-resolution hint, (3) nested shadows, (4) reassigned wrappers, and (5) the original static entrypoint. Thus both the positive recovery and explicit-module/ownership rejection paths remain protected. #3410 now has only its Hopper fixture delta.

@N0zoM1z0 commented on GitHub (Oct 14, 2026): Resolved by main's producer correction `dae1e5538`: wrapper ownership now follows the parsed program's script/module context rather than the specifier-resolution hint. The original static-entrypoint positive assertion remains intact. My earlier fixture-only diagnosis was premature: a working application-level graph does not rule out a defect in the lower static entrypoint. I am withdrawing #3394's test-only proposal; none of its test changes were merged into main. The owning producer fix preserves the stronger original oracle. Verified on main `ba33c706d` integrated with the independent Hopper fixture correction: 8 relevant files / 145 tests pass. Direct current-production controls also pass for (1) script/CJS with an import-resolution hint, (2) explicit ESM even with a require-resolution hint, (3) nested shadows, (4) reassigned wrappers, and (5) the original static entrypoint. Thus both the positive recovery and explicit-module/ownership rejection paths remain protected. #3410 now has only its Hopper fixture delta.

bsakaguchi referenced this issue

2026-10-17 21:09:30 +00:00

[PR #1210] [CLOSED] fix(cli): show usage for rea mcp doctor --help #2723

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

No description provided.

Rows
Columns