[PR #4072] [CLOSED] fix(reference): drop replaced CommonJS loaders from reference-source edges #6036

Closed
opened 2026-10-31 01:11:22 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4072
Author: @VarunGore36
Created: 10/15/2026
Status: ❌ Closed

Base: main ← Head: fix/3982-shadowed-require-reference-source


📝 Commits (7)

  • f16dffd fix(reference): drop replaced CommonJS loaders from reference-source edges
  • 40d7a86 fix(reference): shield unknown writes behind local require bindings
  • 9e7f223 fix(reference): scope write shields to evaluated bindings and empty loops
  • 74f9c43 fix(reference): evaluate write shields in the write's own scope
  • 3299ed1 fix(reference): prove empty loop heads and prototype state before trusting them
  • a297eae fix(reference): shield captured writes and split pollution by loop kind
  • c04ab26 fix(reference): close shield gaps for unscopables, discriminants, and keyed protos

📊 Changes

2 files changed (+669 additions, -7 deletions)

View changed files

📝 src/domain/referenceSourceRequireBindings.ts (+417 -7)
📝 tests/boundary/filesystem/referenceSourceShadowedRequire.test.ts (+252 -0)

📄 Description

Summary

Fixes #3982.

import-reference-source reported a parsed internal requires edge when the apparent loader call targeted a replaced local function: a for-in/of var head initializes its binding on every iteration, and an assignment inside with leaves the binding identity unknown. Both now fail closed like a plain assignment, while a truly uninitialized top-level var require retains its edge.

Problem and expected behavior

Trigger: for (var require of [() => "local"]) {} or with ({}) { require = () => "local"; } followed by require("./dep.cjs") in a .cjs file. Node prints only local; the importer reported a parsed internal edge to dep.cjs.

  • Wrong outcome: {"from_path": "main.cjs", "to": "dep.cjs", "kind": "requires", "resolution": "internal", "parse_state": "parsed"}.
  • Expected: no edge, as the existing ordinary-assignment control produces. The uninitialized var require control keeps its edge.

Change and scope

Two changes in src/domain/referenceSourceRequireBindings.ts, the single owner of loader attribution:

  • hasUninitializedTopLevelVarRequire treats a var declarator binding require as initialized when it is the head of a for-in/of statement. A bare for (var require;;) head stays uninitialized.
  • requireLoaderWasWritten counts unknown-resolution writes alongside unbound ones, so an assignment under dynamic scope (with) fails closed instead of reading as an intact loader.

Genuinely absent bindings, shadowing declarations, member overwrites, and calls lexically inside with are unchanged. Test-only additions otherwise.

Contribution scope fit and linked maintainer decision: ordinary fix within existing boundaries; not applicable.

Contract and boundary impact

  • Semantic owner and earliest changed stage: referenceSourceRequireBindings.unshadowedReferenceSourceRequireCalls (reference-source import parsing).
  • CLI and MCP/tool-catalog contract: fewer false requires relationships; no schema changes.
  • Provider, bridge, target-format, or platform compatibility: none.
  • Evidence, artifact, provenance, or reconstruction contract: replaced loaders no longer produce dependency facts.
  • Target selection, authority, upstream behavior, and cleanup details: not applicable.
  • Generated metadata, package, or installation impact: none.

Expected effects:

  • Read existing files or captured data.
  • Launch or attach to a process.
  • Inject instrumentation or run scripts/code.
  • Change process memory, registers, files, or configuration.
  • Contact network services.
  • Recover protected code or work around protections.
  • Documentation or usability only; no target interaction.
  • Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

  • Tests added or updated: for-of-var, for-in-var, and with-write fixtures in tests/boundary/filesystem/referenceSourceShadowedRequire.test.ts, each with a Node-oracle stdout check, excluded from the expected dependency list. The existing valid controls are unchanged.
  • Base reproduction: the extended test fails on the base revision (false edges reported); it passes with the fix. Verified via the issue's CLI reproduction (4-case matrix) before and after.
  • User-visible CLI/MCP output: relationships array in the import result; no CLI flag changes.
  • Remaining proof gaps: MCP path not executed (same import service; source-derived). Only Linux x64 exercised.

For evidence-bearing changes:

  • Observed, derived, and inferred claims remain distinguishable.
  • Artifact identity, source provenance, and failed attempts remain preserved.
  • Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

  • npm run test:focused -- tests/boundary/filesystem/referenceSourceShadowedRequire.test.ts — 2 passed.
  • npm run test:focused -- <referenceSourceImport, ReferenceSourceImportEntries, referenceSourceGraph, referenceSourceSummary> — 51 passed.
  • npm run check:fast (typecheck + lint) — green.
  • git diff --check — clean.
  • Real Hopper/Ghidra, browser, or OS coverage beyond Linux x64: unavailable.

Compatibility, safety, and release

  • Breaking changes or migration steps: none. Callers matching on the two false edges will stop seeing them.
  • Real Hopper/Ghidra, browser, or OS coverage: none.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: none; no target execution, no new I/O.

Author readiness

Human owner:

  • 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.

Before merge

  • This PR addresses a concrete problem or an agreed enhancement.
  • Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
  • The PR has one focused outcome and the title follows type(scope): outcome.
  • Related issue is linked, or the reason for not linking one is stated above.
  • Tests cover changed observable behavior and meaningful failure paths.
  • Owning docs, contracts, and generated metadata are updated where needed.
  • User-visible CLI/MCP changes include representative output.
  • I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

🔄 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/4072 **Author:** [@VarunGore36](https://github.com/VarunGore36) **Created:** 10/15/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/3982-shadowed-require-reference-source` --- ### 📝 Commits (7) - [`f16dffd`](https://github.com/morluto/rea/commit/f16dffda12ff9891b26bbe8888b0f2df8689da71) fix(reference): drop replaced CommonJS loaders from reference-source edges - [`40d7a86`](https://github.com/morluto/rea/commit/40d7a8634743e80d0b0e010a25deed63ecfd18c1) fix(reference): shield unknown writes behind local require bindings - [`9e7f223`](https://github.com/morluto/rea/commit/9e7f223d14b00460755704fd66253eff8cb6bd08) fix(reference): scope write shields to evaluated bindings and empty loops - [`74f9c43`](https://github.com/morluto/rea/commit/74f9c436d295f8315873dd27a3972e90f59d4da8) fix(reference): evaluate write shields in the write's own scope - [`3299ed1`](https://github.com/morluto/rea/commit/3299ed11d87d24be63865f647fd7ee3c234d9ab2) fix(reference): prove empty loop heads and prototype state before trusting them - [`a297eae`](https://github.com/morluto/rea/commit/a297eaecee8396dfd27e021722e941114fb40d1c) fix(reference): shield captured writes and split pollution by loop kind - [`c04ab26`](https://github.com/morluto/rea/commit/c04ab26a49622a218869ebc83c6deca486963f72) fix(reference): close shield gaps for unscopables, discriminants, and keyed protos ### 📊 Changes **2 files changed** (+669 additions, -7 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/referenceSourceRequireBindings.ts` (+417 -7) 📝 `tests/boundary/filesystem/referenceSourceShadowedRequire.test.ts` (+252 -0) </details> ### 📄 Description ## Summary Fixes #3982. `import-reference-source` reported a parsed internal `requires` edge when the apparent loader call targeted a replaced local function: a `for`-`in`/`of` `var` head initializes its binding on every iteration, and an assignment inside `with` leaves the binding identity unknown. Both now fail closed like a plain assignment, while a truly uninitialized top-level `var require` retains its edge. ## Problem and expected behavior Trigger: `for (var require of [() => "local"]) {}` or `with ({}) { require = () => "local"; }` followed by `require("./dep.cjs")` in a `.cjs` file. Node prints only `local`; the importer reported a parsed internal edge to `dep.cjs`. - Wrong outcome: `{"from_path": "main.cjs", "to": "dep.cjs", "kind": "requires", "resolution": "internal", "parse_state": "parsed"}`. - Expected: no edge, as the existing ordinary-assignment control produces. The uninitialized `var require` control keeps its edge. ## Change and scope Two changes in `src/domain/referenceSourceRequireBindings.ts`, the single owner of loader attribution: - `hasUninitializedTopLevelVarRequire` treats a `var` declarator binding `require` as initialized when it is the head of a `for`-`in`/`of` statement. A bare `for (var require;;)` head stays uninitialized. - `requireLoaderWasWritten` counts `unknown`-resolution writes alongside unbound ones, so an assignment under dynamic scope (`with`) fails closed instead of reading as an intact loader. Genuinely absent bindings, shadowing declarations, member overwrites, and calls lexically inside `with` are unchanged. Test-only additions otherwise. Contribution scope fit and linked maintainer decision: ordinary fix within existing boundaries; not applicable. ## Contract and boundary impact - Semantic owner and earliest changed stage: `referenceSourceRequireBindings.unshadowedReferenceSourceRequireCalls` (reference-source import parsing). - CLI and MCP/tool-catalog contract: fewer false `requires` relationships; no schema changes. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: replaced loaders no longer produce dependency facts. - Target selection, authority, upstream behavior, and cleanup details: not applicable. - Generated metadata, package, or installation impact: none. Expected effects: - [x] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage - Tests added or updated: `for-of-var`, `for-in-var`, and `with-write` fixtures in `tests/boundary/filesystem/referenceSourceShadowedRequire.test.ts`, each with a Node-oracle stdout check, excluded from the expected dependency list. The existing valid controls are unchanged. - Base reproduction: the extended test fails on the base revision (false edges reported); it passes with the fix. Verified via the issue's CLI reproduction (4-case matrix) before and after. - User-visible CLI/MCP output: `relationships` array in the import result; no CLI flag changes. - Remaining proof gaps: MCP path not executed (same import service; source-derived). Only Linux x64 exercised. For evidence-bearing changes: - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed - `npm run test:focused -- tests/boundary/filesystem/referenceSourceShadowedRequire.test.ts` — 2 passed. - `npm run test:focused -- <referenceSourceImport, ReferenceSourceImportEntries, referenceSourceGraph, referenceSourceSummary>` — 51 passed. - `npm run check:fast` (typecheck + lint) — green. - `git diff --check` — clean. - Real Hopper/Ghidra, browser, or OS coverage beyond Linux x64: unavailable. ## Compatibility, safety, and release - Breaking changes or migration steps: none. Callers matching on the two false edges will stop seeing them. - Real Hopper/Ghidra, browser, or OS coverage: none. - Package or release metadata impact: none. - Security, privacy, process, or containment review: none; no target execution, no new I/O. ## Author readiness Human owner: - [x] 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. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. ## Before merge - [ ] This PR addresses a concrete problem or an agreed enhancement. - [ ] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [ ] The PR has one focused outcome and the title follows `type(scope): outcome`. - [ ] Related issue is linked, or the reason for not linking one is stated above. - [ ] Tests cover changed observable behavior and meaningful failure paths. - [ ] Owning docs, contracts, and generated metadata are updated where needed. - [ ] User-visible CLI/MCP changes include representative output. - [ ] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-10-31 01:11:22 +00:00
Sign in to join this conversation.
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#6036
No description provided.