morluto/rea

Fork 0

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

[PR #3394] [CLOSED] test(javascript): assert wrapper ownership in the correct source mode #5721

New issue

Closed

opened 2026-10-31 01:08:56 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/3394
Author: @N0zoM1z0
Created: 10/14/2026
Status: ❌ Closed

Base: main ← Head: fix/3341-wrapper-require-test-context

📝 Commits (1)

f62632e test(javascript): assert wrapper ownership in the correct source mode

📊 Changes

2 files changed (+18 additions, -2 deletions)

View changed files

📝 src/domain/javascript/javascriptSemanticAnalysis.require_links.test.ts (+16 -0)
📝 src/domain/javascript/javascriptStaticAnalysisModuleIndex.test.ts (+2 -2)

📄 Description

Summary

Fixes #3341. The untouched CommonJS wrapper test called a formatless static entrypoint whose semantic indexing uses import mode, then expected a CommonJS-owned loader. Main consequently failed this assertion even though real .cjs application analysis resolved the dependency correctly.

Preserve the same positive source/specifier oracle in the semantic require-link suite with an explicitly parsed .cjs file and its actual require resolution mode. Keep the formatless static case as a negative assertion: that declaration remains unresolved without CommonJS ownership context. Production ownership rules and adjacent reassignment/shadowing tests are unchanged.

Contract and boundary impact

Only two test files change. No runtime behavior, public CLI/MCP contracts, provider compatibility, evidence/provenance format, generated metadata, packages or releases change. Ordinary test correction within existing boundaries; a new capability scope decision is not applicable. Application validation reads isolated real JavaScript directory fixtures; the analyzer does not execute their target code. A separate trusted Node runtime control exercises CommonJS wrapper behavior.

Evidence and validation

Base: main 322506d0c. Linux x86_64 / Node 24.18.0 / npm 11.16.0 / Vitest 5.0.3.

Original clean-main reproduction: 26 passing / 1 failing case in the static module-index family. Hosted #3299 source-test job fails the same assertion. No Rizin change participates in this test group.
npm run test:focused -- src/domain/javascript/javascriptStaticAnalysisModuleIndex.test.ts src/domain/javascript/javascriptSemanticAnalysis.require_links.test.ts tests/boundary/filesystem/referenceSourceShadowedRequire.test.ts — 3 files / 61 tests passed in 2.16 s, including the actual filesystem source-mode/lexical-binding regression.
Actual analyzeJavaScriptApplication + owned artifact scope on two real local applications: .cjs/CommonJS resolves ./wrapper.cjs in require mode; .mjs/ESM does not infer that dependency. Both analyses succeed and resources close. This confirms a test-context correction for this input rather than an application producer fix.
Two controlled producer faults both fail the intended new assertion: removing CommonJS wrapper recognition fails the positive case; granting wrapper ownership in formatless mode fails the negative case. The producer was restored byte-for-byte; neither mutation is in this diff.
npm run check:ci — 8 tasks and architecture guards across 1,393 files passed.
git diff --check — passed.

No external applications, macOS/Windows execution, or runtime performance improvement is claimed. No timeout, retry, skip or mutation-score threshold changes.

Author readiness

Human owner: @N0zoM1z0

AI-assisted preparation; human declarations remain unchecked until the human owner confirms review of this PR's diff and validation evidence.

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/3394 **Author:** [@N0zoM1z0](https://github.com/N0zoM1z0) **Created:** 10/14/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/3341-wrapper-require-test-context` --- ### 📝 Commits (1) - [`f62632e`](https://github.com/morluto/rea/commit/f62632e7a6f38ad9cbdc2e61d636768161dbaa7b) test(javascript): assert wrapper ownership in the correct source mode ### 📊 Changes **2 files changed** (+18 additions, -2 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/javascript/javascriptSemanticAnalysis.require_links.test.ts` (+16 -0) 📝 `src/domain/javascript/javascriptStaticAnalysisModuleIndex.test.ts` (+2 -2) </details> ### 📄 Description ## Summary Fixes #3341. The untouched CommonJS wrapper test called a formatless static entrypoint whose semantic indexing uses import mode, then expected a CommonJS-owned loader. Main consequently failed this assertion even though real `.cjs` application analysis resolved the dependency correctly. Preserve the same positive source/specifier oracle in the semantic require-link suite with an explicitly parsed `.cjs` file and its actual `require` resolution mode. Keep the formatless static case as a negative assertion: that declaration remains unresolved without CommonJS ownership context. Production ownership rules and adjacent reassignment/shadowing tests are unchanged. ## Contract and boundary impact Only two test files change. No runtime behavior, public CLI/MCP contracts, provider compatibility, evidence/provenance format, generated metadata, packages or releases change. Ordinary test correction within existing boundaries; a new capability scope decision is not applicable. Application validation reads isolated real JavaScript directory fixtures; the analyzer does not execute their target code. A separate trusted Node runtime control exercises CommonJS wrapper behavior. ## Evidence and validation Base: main `322506d0c`. Linux x86_64 / Node 24.18.0 / npm 11.16.0 / Vitest 5.0.3. - Original clean-main reproduction: 26 passing / 1 failing case in the static module-index family. Hosted #3299 source-test job fails the same assertion. No Rizin change participates in this test group. - `npm run test:focused -- src/domain/javascript/javascriptStaticAnalysisModuleIndex.test.ts src/domain/javascript/javascriptSemanticAnalysis.require_links.test.ts tests/boundary/filesystem/referenceSourceShadowedRequire.test.ts` — **3 files / 61 tests passed** in 2.16 s, including the actual filesystem source-mode/lexical-binding regression. - Actual `analyzeJavaScriptApplication` + owned artifact scope on two real local applications: `.cjs`/CommonJS resolves `./wrapper.cjs` in `require` mode; `.mjs`/ESM does not infer that dependency. Both analyses succeed and resources close. This confirms a test-context correction for this input rather than an application producer fix. - Two controlled producer faults both fail the intended new assertion: removing CommonJS wrapper recognition fails the positive case; granting wrapper ownership in formatless mode fails the negative case. The producer was restored byte-for-byte; neither mutation is in this diff. - `npm run check:ci` — 8 tasks and architecture guards across 1,393 files passed. - `git diff --check` — passed. No external applications, macOS/Windows execution, or runtime performance improvement is claimed. No timeout, retry, skip or mutation-score threshold changes. ## Author readiness Human owner: @N0zoM1z0 AI-assisted preparation; human declarations remain unchecked until the human owner confirms review of this PR's diff and validation evidence. - [ ] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [ ] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [ ] I declare this PR ready for initial maintainer review. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

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

closed this issue
added the
pull-request
label

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

No description provided.

Rows
Columns