morluto/rea

Fork 0

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

[PR #2886] [MERGED] fix(browser): retain optional source-map requires #5469

New issue

Closed

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

bsakaguchi commented

2026-10-31 01:06:53 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2886
Author: @JFSAS
Created: 10/13/2026
Status: ✅ Merged
Merged: 10/13/2026
Merged by: @morluto

Base: main ← Head: codex/source-map-optional-require

📝 Commits (1)

1319c19 fix(browser): retain optional source-map requires

📊 Changes

2 files changed (+79 additions, -5 deletions)

View changed files

📝 src/browser/WebSourceMapFetcher.ts (+10 -5)
➕ tests/boundary/browser/optionalSourceMapRequire.test.ts (+69 -0)

📄 Description

Summary

Fixes #2883. A source-map original containing require?.("./dep.cjs") now produces the same literal dependency candidate as the ordinary call. Locally shadowed and computed calls remain excluded.

Problem and expected behavior

On 3e7c6302, a fetched map containing ordinary and optional calls reports included but retains only the ordinary dependency. Real Chrome runs through compiled CLI and stdio MCP reproduce that omission. After this change both dependencies are retained with their existing resolved URLs and unchanged source-map bytes/digest.

Change and scope

The source-map adapter accepts OptionalCallExpression and returns its callee together with the dependency. The consumer validates that retained callee through the existing lexical reference analysis, rather than independently checking the AST call kind. This keeps optional calls subject to the same shadowing gate and leaves TypeScript import-equals declarations on their existing path.

Ordinary static-analysis correctness fix; maintainer scope decision: not applicable. #2842 changes literal reading in the adjacent extractor and addresses a separate trigger; the two fixes may need a small hunk reconciliation when merging.

Contract and boundary impact

Semantic owner and earliest changed stage: source-map original dependency extraction in WebSourceMapFetcher.
Contribution scope: existing read-only captured-source analysis.
CLI/MCP catalog: unchanged schemas, names and arguments.
Provider/platform compatibility: shared browser adapter; no new engine or host requirement.
Evidence: additional supported dependency edges; retained source-map text, digest, original source text and URL resolution preserved.
Target selection/authority/cleanup: unchanged. No new fetch, execution, mutation or resource owner.
Generated metadata/package/installation: none.

Expected effects:

Read existing files or captured data.

Evidence and regression coverage

A real loopback HTTP boundary test covers ordinary and optional literal calls, a shadowed optional call, a computed argument, resolved URLs and retained map/original evidence. It failed before the producer fix and passes afterward.

The same map (SHA-256 d9d6a38307044d56516840ff08aef4a583f37a188fd228ebdd21e2db69ef82c1) was served to Chrome 155.0.8059.40 and inspected by both public interfaces on macOS arm64:

Build	CLI and stdio MCP specifiers
Base `3e7c6302`	`./normal.cjs`
Fixed	`./normal.cjs`, `./optional.cjs`

The shadowed call is absent in both builds. REA does not execute the original CommonJS text, and the edges do not establish runtime execution. Windows/Linux and other browsers were not run.

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

Node 24.18.0 / npm 11.16.0:

npm run test:focused -- tests/boundary/browser/optionalSourceMapRequire.test.ts tests/boundary/browser/webSourceMapFetcher.test.ts tests/boundary/browser/webSourceMapNestedCoordinates.test.ts tests/boundary/browser/webSourceMapHttpStatus.test.ts tests/boundary/browser/webSourceMapResponseCleanup.test.ts: 104/104 passed. The strengthened final regression was rerun and passed.
Before/after compiled CLI and stdio MCP against real Chrome and loopback HTTP: passed the respective baseline/fixed assertions, including hash retention and owned-browser cleanup.
npm run build:cached, npm run check:fast, npm run knip, npm run verify:architecture-guards, npm run verify:test-discovery: passed. The pre-push hook also passed check:fast on the committed version.
npm run check: blocked by existing formatting errors in unchanged src/domain/functionComparison.test.ts and src/ida/IdaOperationRunner.ts. Formatting for both changed files passed. The separate fast checks, knip and architecture guards above completed successfully.

Compatibility, safety, and release

No breaking schema or installation change. No native decompiler claim; browser validation is limited to the macOS/Chrome configuration stated above. No original-source execution or additional network access is introduced.

Before merge

Concrete reproduced problem and linked issue.
Scope and authority questions resolved for this ordinary fix.
One focused outcome and Conventional Commit title.
Regression and relevant failure/negative controls verified.
No owning contract or generated metadata change required.
Representative public CLI/MCP behavior documented.
Final diff checked for unrelated changes, secrets and unsupported claims.

Prepared with AI assistance; all reported executions were run locally.

_{🔄 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/2886 **Author:** [@JFSAS](https://github.com/JFSAS) **Created:** 10/13/2026 **Status:** ✅ Merged **Merged:** 10/13/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `codex/source-map-optional-require` --- ### 📝 Commits (1) - [`1319c19`](https://github.com/morluto/rea/commit/1319c19fc9348e6ebdb94bfd1905c43996a4d1fd) fix(browser): retain optional source-map requires ### 📊 Changes **2 files changed** (+79 additions, -5 deletions) <details> <summary>View changed files</summary> 📝 `src/browser/WebSourceMapFetcher.ts` (+10 -5) ➕ `tests/boundary/browser/optionalSourceMapRequire.test.ts` (+69 -0) </details> ### 📄 Description ## Summary Fixes #2883. A source-map original containing `require?.("./dep.cjs")` now produces the same literal dependency candidate as the ordinary call. Locally shadowed and computed calls remain excluded. ## Problem and expected behavior On `3e7c6302`, a fetched map containing ordinary and optional calls reports `included` but retains only the ordinary dependency. Real Chrome runs through compiled CLI and stdio MCP reproduce that omission. After this change both dependencies are retained with their existing resolved URLs and unchanged source-map bytes/digest. ## Change and scope The source-map adapter accepts `OptionalCallExpression` and returns its callee together with the dependency. The consumer validates that retained callee through the existing lexical reference analysis, rather than independently checking the AST call kind. This keeps optional calls subject to the same shadowing gate and leaves TypeScript import-equals declarations on their existing path. Ordinary static-analysis correctness fix; maintainer scope decision: not applicable. #2842 changes literal reading in the adjacent extractor and addresses a separate trigger; the two fixes may need a small hunk reconciliation when merging. ## Contract and boundary impact - Semantic owner and earliest changed stage: source-map original dependency extraction in `WebSourceMapFetcher`. - Contribution scope: existing read-only captured-source analysis. - CLI/MCP catalog: unchanged schemas, names and arguments. - Provider/platform compatibility: shared browser adapter; no new engine or host requirement. - Evidence: additional supported dependency edges; retained source-map text, digest, original source text and URL resolution preserved. - Target selection/authority/cleanup: unchanged. No new fetch, execution, mutation or resource owner. - Generated metadata/package/installation: none. Expected effects: - [x] Read existing files or captured data. ## Evidence and regression coverage A real loopback HTTP boundary test covers ordinary and optional literal calls, a shadowed optional call, a computed argument, resolved URLs and retained map/original evidence. It failed before the producer fix and passes afterward. The same map (SHA-256 `d9d6a38307044d56516840ff08aef4a583f37a188fd228ebdd21e2db69ef82c1`) was served to **Chrome 155.0.8059.40** and inspected by both public interfaces on macOS arm64: | Build | CLI and stdio MCP specifiers | | --- | --- | | Base `3e7c6302` | `./normal.cjs` | | Fixed | `./normal.cjs`, `./optional.cjs` | The shadowed call is absent in both builds. REA does not execute the original CommonJS text, and the edges do not establish runtime execution. Windows/Linux and other browsers were not run. - [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 Node 24.18.0 / npm 11.16.0: - `npm run test:focused -- tests/boundary/browser/optionalSourceMapRequire.test.ts tests/boundary/browser/webSourceMapFetcher.test.ts tests/boundary/browser/webSourceMapNestedCoordinates.test.ts tests/boundary/browser/webSourceMapHttpStatus.test.ts tests/boundary/browser/webSourceMapResponseCleanup.test.ts`: **104/104 passed**. The strengthened final regression was rerun and passed. - Before/after compiled CLI and stdio MCP against real Chrome and loopback HTTP: passed the respective baseline/fixed assertions, including hash retention and owned-browser cleanup. - `npm run build:cached`, `npm run check:fast`, `npm run knip`, `npm run verify:architecture-guards`, `npm run verify:test-discovery`: passed. The pre-push hook also passed `check:fast` on the committed version. - `npm run check`: blocked by existing formatting errors in unchanged `src/domain/functionComparison.test.ts` and `src/ida/IdaOperationRunner.ts`. Formatting for both changed files passed. The separate fast checks, knip and architecture guards above completed successfully. ## Compatibility, safety, and release No breaking schema or installation change. No native decompiler claim; browser validation is limited to the macOS/Chrome configuration stated above. No original-source execution or additional network access is introduced. ## Before merge - [x] Concrete reproduced problem and linked issue. - [x] Scope and authority questions resolved for this ordinary fix. - [x] One focused outcome and Conventional Commit title. - [x] Regression and relevant failure/negative controls verified. - [x] No owning contract or generated metadata change required. - [x] Representative public CLI/MCP behavior documented. - [x] Final diff checked for unrelated changes, secrets and unsupported claims. Prepared with AI assistance; all reported executions were run locally. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:06:53 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:44:42 +00:00

[PR #5651] [MERGED] fix(javascript): preserve path reasons, module coverage, and property provenance #7014

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

No description provided.

Rows
Columns