[PR #3801] [MERGED] fix(reference): stop attributing overwritten require members to Node #5908

New issue

Closed

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

bsakaguchi commented

2026-10-31 01:10:21 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/3801
Author: @kaoru0822-kitauji
Created: 10/15/2026
Status: ✅ Merged
Merged: 10/15/2026
Merged by: @kaoru0822-kitauji

Base: main ← Head: fix/3393-overwritten-require-members

📝 Commits (2)

60424ab fix(reference): stop attributing overwritten require members to Node
f3941db fix(reference): keep optional and wrapped loader uses intact

📊 Changes

2 files changed (+151 additions, -0 deletions)

View changed files

📝 src/domain/referenceSourceRequireBindings.ts (+87 -0)
📝 tests/boundary/filesystem/referenceSourceShadowedRequire.test.ts (+64 -0)

📄 Description

Fixes #3393

Root cause

unshadowedReferenceSourceRequireCalls (src/domain/referenceSourceRequireBindings.ts) decides which calls are Node's loader. For require.resolve(...) and require.main(...) it checked only the require binding: unbound or wrapper-owned, and never reassigned. It never checked whether the member had been replaced. After require.resolve = () => "custom", the literal argument still became a parsed internal requires edge, although the call never resolves a module.

Change

Member calls are attributed only while the loader's resolve and main members keep Node's identity (requireLoaderMembersIntact). Their identity is unknown, and the member calls are not attributed, when either holds:

A write to require.resolve, require.main or a computed require[...]. Write sites come from the shared semanticWriteSiteTarget / semanticWriteTargets helpers, so assignment, compound assignment, update, delete, for…in/of and destructuring targets are all covered.
require used as a value other than a direct call, a member read or typeof, for example Object.defineProperty(require, …), Object.assign(require, …) or const r = require. A mutator or alias can then replace members without a visible member write.

This matches how the boundary already excludes shadowed or reassigned loaders. Bare require("…") calls are unaffected.

Validation

The issue's repro and controls, public CLI, main vs this PR:

`main.cjs`	`main`	this PR
`require.resolve = () => "custom"; require.resolve("./dep.cjs")`	`requires dep.cjs` (false)	none
`Object.defineProperty(require, "resolve", …); require.resolve("./dep.cjs")`	`requires dep.cjs` (false)	none
`patch(require); require.resolve("./dep.cjs")`	`requires dep.cjs`	none (member identity unknown)
`require.resolve("./dep.cjs")` (control)	`requires dep.cjs`	`requires dep.cjs`
`patch(require); require("./dep.cjs")` (bare call)	`requires dep.cjs`	`requires dep.cjs`

Real sources, main vs this PR: relationships are identical and the rest of the graph is byte-identical. node_modules/pino has 575 relationships (0.55 s / 0.53 s), playwright-core/lib 1,362 (2.24 s / 2.26 s) and REA src/ 11,637 (2.97 s / 2.99 s). Output size is unchanged.

Tests:

New does not attribute overwritten require members to Node's resolver in referenceSourceShadowedRequire.test.ts covers assigned, defineProperty and computed-key overwrites plus a control. Each is first executed under Node to establish the oracle (custom or string), and only the control keeps its edge. The test fails on main; I checked by restoring main's classifier and rebuilding.
All 22 reference-source boundary test files pass (154 tests), along with the reference domain unit tests and npm run check:fast.

🤖 Generated with Claude Code

_{🔄 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/3801 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/15/2026 **Status:** ✅ Merged **Merged:** 10/15/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `fix/3393-overwritten-require-members` --- ### 📝 Commits (2) - [`60424ab`](https://github.com/morluto/rea/commit/60424abbffcc976340eb7478094fce5a0f11ef2f) fix(reference): stop attributing overwritten require members to Node - [`f3941db`](https://github.com/morluto/rea/commit/f3941db05a13b9add70fce9d15f1c2446a5a0238) fix(reference): keep optional and wrapped loader uses intact ### 📊 Changes **2 files changed** (+151 additions, -0 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/referenceSourceRequireBindings.ts` (+87 -0) 📝 `tests/boundary/filesystem/referenceSourceShadowedRequire.test.ts` (+64 -0) </details> ### 📄 Description Fixes #3393 ## Root cause `unshadowedReferenceSourceRequireCalls` (`src/domain/referenceSourceRequireBindings.ts`) decides which calls are Node's loader. For `require.resolve(...)` and `require.main(...)` it checked only the `require` binding: unbound or wrapper-owned, and never reassigned. It never checked whether the **member** had been replaced. After `require.resolve = () => "custom"`, the literal argument still became a parsed internal `requires` edge, although the call never resolves a module. ## Change Member calls are attributed only while the loader's `resolve` and `main` members keep Node's identity (`requireLoaderMembersIntact`). Their identity is unknown, and the member calls are not attributed, when either holds: - **A write to `require.resolve`, `require.main` or a computed `require[...]`.** Write sites come from the shared `semanticWriteSiteTarget` / `semanticWriteTargets` helpers, so assignment, compound assignment, update, `delete`, `for…in/of` and destructuring targets are all covered. - **`require` used as a value** other than a direct call, a member read or `typeof`, for example `Object.defineProperty(require, …)`, `Object.assign(require, …)` or `const r = require`. A mutator or alias can then replace members without a visible member write. This matches how the boundary already excludes shadowed or reassigned loaders. Bare `require("…")` calls are unaffected. ## Validation **The issue's repro and controls**, public CLI, `main` vs this PR: | `main.cjs` | `main` | this PR | |---|---|---| | `require.resolve = () => "custom"; require.resolve("./dep.cjs")` | `requires dep.cjs` (false) | none | | `Object.defineProperty(require, "resolve", …); require.resolve("./dep.cjs")` | `requires dep.cjs` (false) | none | | `patch(require); require.resolve("./dep.cjs")` | `requires dep.cjs` | none (member identity unknown) | | `require.resolve("./dep.cjs")` (control) | `requires dep.cjs` | `requires dep.cjs` | | `patch(require); require("./dep.cjs")` (bare call) | `requires dep.cjs` | `requires dep.cjs` | **Real sources, `main` vs this PR:** relationships are identical and the rest of the graph is byte-identical. `node_modules/pino` has 575 relationships (0.55 s / 0.53 s), `playwright-core/lib` 1,362 (2.24 s / 2.26 s) and REA `src/` 11,637 (2.97 s / 2.99 s). Output size is unchanged. **Tests:** - New `does not attribute overwritten require members to Node's resolver` in `referenceSourceShadowedRequire.test.ts` covers assigned, `defineProperty` and computed-key overwrites plus a control. Each is first executed under Node to establish the oracle (`custom` or `string`), and only the control keeps its edge. The test **fails on `main`**; I checked by restoring main's classifier and rebuilding. - All 22 reference-source boundary test files pass (154 tests), along with the reference domain unit tests and `npm run check:fast`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:10:21 +00:00