morluto/rea

Fork 0

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

[PR #4889] [CLOSED] fix(javascript): omit resolved keyed-collection get/delete reads from network endpoints #6456

New issue

Closed

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

bsakaguchi commented

2026-10-31 01:14:36 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4889
Author: @piyushpcs
Created: 10/17/2026
Status: ❌ Closed

Base: main ← Head: fix/map-get-keyed-collection-receiver

📝 Commits (3)

c3cb855 fix(javascript): omit resolved keyed-collection get/delete reads from network endpoints
cc8ea86 fix(javascript): verify keyed-collection reads against mutation and ambient facts
8d7e878 fix(javascript): keep prototype-overridden keyed-collection reads as endpoints

📊 Changes

5 files changed (+273 additions, -1 deletions)

View changed files

📝 src/domain/javascript/javascriptStaticAnalysis.ts (+3 -0)
📝 src/domain/javascript/javascriptStaticAnalysisCalleeBoundary.test.ts (+113 -0)
📝 src/domain/javascript/javascriptStaticAnalysisCalls.ts (+149 -0)
📝 src/domain/javascript/javascriptStaticAnalysisHelpers.ts (+2 -1)
📝 src/domain/javascript/javascriptStaticAnalysisState.ts (+6 -0)

📄 Description

Drafted with AI assistance: I read the full diff, walked through the approach, and ran every command listed below. Ask me anything about the code.

Problem

analyze-javascript-application classifies Map.get string arguments as network endpoints.

From #4853, the whole program:

const records = new Map([['/accounts', { name: 'Alice' }]]);
console.log(records.get('/accounts'));

The result contains an endpoint node call:records.get with endpoint_kind: "network", confidence: "exact", and no limitations. That's an in-memory lookup. It never touched the network.

Approach

The call site can prove more than the receiver spelling does, so resolution happens where the full facts live. During the traversal, a .get/.delete call with a string-literal argument whose receiver resolves through immutable bindings (one identifier alias deep) to a new Map/Set/WeakMap/WeakSet/Headers/FormData/URLSearchParams construction queues the read instead of classifying it. The constructor must be an unmodified global, so a local class Map {} or globalThis.Map = ApiClient never selects the path.

After the traversal, the shared member-mutation facts are indexed (the same on-demand phase pendingNetworkCalls uses) and finishKeyedCollectionReads re-verifies each queued read with referenceMayHaveChangedAt along the alias chain. Overwritten methods (records.get = client.get.bind(client)), later reassignments, or unresolved effects keep the previous endpoint classification — the suppression only applies when the call still targets the built-in method.

One conservatism this inherits from collectSemanticMemberMutations: every member call marks its receiver escaped, so only the first observable read of a never-called resolved collection suppresses; reads after any earlier call on the same binding keep the old behavior. The spelling-only path (new Headers().get("/api/header-shaped")) is unchanged, matching the existing tests. The issue's broader suggestion (downgrade every unresolved receiver to candidate) is follow-up work, not this PR.

The web bundle analyzer keeps its current spelling-based behavior; wiring this suppression through its endpointArgument call is follow-up if maintainers want it.

What I ran

New regression tests in src/domain/javascript/javascriptStaticAnalysisCalleeBoundary.test.ts: direct and alias bindings, .delete, overwritten methods, overwritten globals, the timing boundary (later.get("/before") suppressed, later.get("/after") kept), and the conservative stays (direct new spelling, reassigned binding, conditional receiver, shadowed constructor).
npm run test:focused -- src/domain/javascript/javascriptStaticAnalysisCalleeBoundary.test.ts → 18 passed.
Related focused suites (partial endpoints, RPC endpoints, send_beacon, event_source, getter receivers) → 157 passed.
npm run check:changed → 126 files, 3075 tests passed.
npm run check:fast, npm run verify:test-discovery (1556 test files), npm run check → all pass.
Real CLI: npm run build:cached, then node scripts/rea.mjs analyze-javascript-application <repro> on a directory with the issue's main.js, a fetch('/accounts') control, and a mutated receiver. The repro now emits no endpoint node; the fetch control and the mutated clientRecords.get read each emit the expected endpoint.

Not run: the published npm package, the full check:pr suite, and real-provider lanes. CI owns the complete deterministic verification for the rest.

Author readiness

I have reviewed the final diff, verified the evidence above, and I'm 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/4889 **Author:** [@piyushpcs](https://github.com/piyushpcs) **Created:** 10/17/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/map-get-keyed-collection-receiver` --- ### 📝 Commits (3) - [`c3cb855`](https://github.com/morluto/rea/commit/c3cb85538f9148193a8afa8a7af167858c7c0797) fix(javascript): omit resolved keyed-collection get/delete reads from network endpoints - [`cc8ea86`](https://github.com/morluto/rea/commit/cc8ea86ceb8b2287137148a89e637e40696c79f2) fix(javascript): verify keyed-collection reads against mutation and ambient facts - [`8d7e878`](https://github.com/morluto/rea/commit/8d7e87801eca335cd8909166c9db5c29f54e4ae8) fix(javascript): keep prototype-overridden keyed-collection reads as endpoints ### 📊 Changes **5 files changed** (+273 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/javascript/javascriptStaticAnalysis.ts` (+3 -0) 📝 `src/domain/javascript/javascriptStaticAnalysisCalleeBoundary.test.ts` (+113 -0) 📝 `src/domain/javascript/javascriptStaticAnalysisCalls.ts` (+149 -0) 📝 `src/domain/javascript/javascriptStaticAnalysisHelpers.ts` (+2 -1) 📝 `src/domain/javascript/javascriptStaticAnalysisState.ts` (+6 -0) </details> ### 📄 Description Drafted with AI assistance: I read the full diff, walked through the approach, and ran every command listed below. Ask me anything about the code. ## Problem `analyze-javascript-application` classifies `Map.get` string arguments as network endpoints. From [#4853](https://github.com/morluto/rea/issues/4853), the whole program: ```js const records = new Map([['/accounts', { name: 'Alice' }]]); console.log(records.get('/accounts')); ``` The result contains an endpoint node `call:records.get` with `endpoint_kind: "network"`, `confidence: "exact"`, and no limitations. That's an in-memory lookup. It never touched the network. ## Approach The call site can prove more than the receiver spelling does, so resolution happens where the full facts live. During the traversal, a `.get`/`.delete` call with a string-literal argument whose receiver resolves through immutable bindings (one identifier alias deep) to a `new Map/Set/WeakMap/WeakSet/Headers/FormData/URLSearchParams` construction queues the read instead of classifying it. The constructor must be an unmodified global, so a local `class Map {}` or `globalThis.Map = ApiClient` never selects the path. After the traversal, the shared member-mutation facts are indexed (the same on-demand phase `pendingNetworkCalls` uses) and `finishKeyedCollectionReads` re-verifies each queued read with `referenceMayHaveChangedAt` along the alias chain. Overwritten methods (`records.get = client.get.bind(client)`), later reassignments, or unresolved effects keep the previous endpoint classification — the suppression only applies when the call still targets the built-in method. One conservatism this inherits from `collectSemanticMemberMutations`: every member call marks its receiver escaped, so only the first observable read of a never-called resolved collection suppresses; reads after any earlier call on the same binding keep the old behavior. The spelling-only path (`new Headers().get("/api/header-shaped")`) is unchanged, matching the existing tests. The issue's broader suggestion (downgrade every unresolved receiver to `candidate`) is follow-up work, not this PR. The web bundle analyzer keeps its current spelling-based behavior; wiring this suppression through its `endpointArgument` call is follow-up if maintainers want it. ## What I ran - New regression tests in `src/domain/javascript/javascriptStaticAnalysisCalleeBoundary.test.ts`: direct and alias bindings, `.delete`, overwritten methods, overwritten globals, the timing boundary (`later.get("/before")` suppressed, `later.get("/after")` kept), and the conservative stays (direct `new` spelling, reassigned binding, conditional receiver, shadowed constructor). - `npm run test:focused -- src/domain/javascript/javascriptStaticAnalysisCalleeBoundary.test.ts` → 18 passed. - Related focused suites (partial endpoints, RPC endpoints, send_beacon, event_source, getter receivers) → 157 passed. - `npm run check:changed` → 126 files, 3075 tests passed. - `npm run check:fast`, `npm run verify:test-discovery` (1556 test files), `npm run check` → all pass. - Real CLI: `npm run build:cached`, then `node scripts/rea.mjs analyze-javascript-application <repro>` on a directory with the issue's `main.js`, a `fetch('/accounts')` control, and a mutated receiver. The repro now emits no endpoint node; the `fetch` control and the mutated `clientRecords.get` read each emit the expected endpoint. Not run: the published npm package, the full `check:pr` suite, and real-provider lanes. CI owns the complete deterministic verification for the rest. ## Author readiness - [x] I have reviewed the final diff, verified the evidence above, and I'm 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:14:36 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:47:19 +00:00

[PR #6478] [CLOSED] fix(javascript): classify distinct source-to-bundle occurrences #7361

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

No description provided.

Rows
Columns