morluto/rea

Fork 0

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

[PR #3141] [CLOSED] fix(javascript): resolve a growing ambient alias at its own read #5638

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

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

Base: main ← Head: fix/javascript-ambient-alias-write-capture

📝 Commits (1)

ce21fee fix(javascript): resolve a growing ambient alias at its own read

📊 Changes

1 file changed (+25 additions, -13 deletions)

View changed files

📝 src/domain/javascript/javascriptSemanticAmbientWrites.ts (+25 -13)

📄 Description

Summary

Resolve each ambient alias initializer at its own read position, and follow the
reference initializers a mutation selection keeps, so a growing self-reference
alias (alias = alias.globalThis) retains the ambient identity its earlier read
selected.

This repairs one of the four JavaScript regressions introduced by
04672f2b
("fix(javascript): preserve builtin identity and share alias traversal").

Problem and expected behavior

04672f2b carried write sites into ambient alias resolution and narrowed the
initializers a possible write can observe:

const initializers = possibleWrites
  ? capture === undefined
    ? binding.initializers
    : semanticMutationInitializers(binding, capture, state.parentsByNode).initializers
  : ...;

semanticMutationInitializers returns only the initializers at the latest
preceding statement, and it splits them: an assignment such as
alias = alias.globalThis is reported through referenceInitializers, while
initializers holds the declarator that preceded it. The write lookup read only
.initializers and never followed referenceInitializers, so for

let alias = globalThis;
alias = alias.globalThis;
alias.URL = replacement;

the selection returned initializers: [], the ambient identity from
let alias = globalThis was filtered out, and no path was pushed. The resolver
therefore retained nothing and alias.URL = replacement was no longer recorded
as a write through the global object.

Expected behavior: URL is covered by an ambient write, so
isUnmodifiedGlobalReference is false and a new URL("./worker.js", import.meta.url)
specifier is not trusted. The owning test,
src/domain/javascript/javascriptSemanticAmbientWrites.test.ts >
"preserves growing self-reference projections with an ambient initializer",
asserts exactly this.

Change and scope

src/domain/javascript/javascriptSemanticAmbientWrites.ts only:

Each resolver step now carries the node whose position decided it. An alias
initializer is resolved at its own read rather than at the later write site
that followed it, so the earlier read selects the initializers visible where
it occurred.
A mutation selection now contributes both initializers and
referenceInitializers to the write lookup. The cycle/diamond guard already
makes re-entering the same read terminate, so following a self-reference stays
finite.
Sub-expression descent (continueWith) keeps the current step's position, so
conditional, logical, and object-literal branches resolve where their value
was read.

An assignment of an unrelated value still replaces an earlier initializer, which
is the behavior 04672f2b intended: for let a = globalThis; a = {}; a.URL = x;
the selection returns the object literal and no ambient path is retained.

Contract and boundary impact

No schema, emitted-fact, or public API change. The change is confined to the
query "which ambient identity could this write have reached", which remains an
over-approximation for possible writes. Intrinsic reads (possibleWrites: false)
are untouched and still use semanticStableInitializer.

Evidence and regression coverage

Bisect on origin/main (58300885):

At 04672f2b^ (0f4c484b): the four affected files pass, 71 passed (71).
At 04672f2b: the four affected files fail, 4 failed | 68 passed (72).
At 04672f2b with this patch: 3 failed | 69 passed (72) — the ambient-writes
test now passes and no previously passing test regressed.

Full JavaScript semantic suite (76 files) with this patch, run through
npm run test:focused:

1810 passed | 3 failed | 1813 total. The three failures are the unfixed
regressions listed below; none is new.
npm run check:fast: 2 successful.
npm run format:check: clean.

Unavailable coverage

This patch does not make the Source tests check green. Three further
JavaScript regressions from the same commit remain, each with a distinct
mechanism, and one further failure in src/ida/IdaNativeProtocol.test.ts:

src/domain/javascript/javascriptSemanticAnalysis.native_bindings.test.ts:88
— "recovers configuration, requests, response consumers, and boundaries": the
{ kind: "environment", key: "PORT" } operation from Number(process.env.PORT)
is missing.
src/application/javascript/JavaScriptSemanticGraphResources.test.ts:190
— "projects config precedence, request fields, and boundaries": a
supplies-request-field relation is absent.
src/domain/javascript/electronWrappedCallPaths.test.ts:150 — "retains the
original expression for a dynamic isolated world id": the world_id is no
longer reported as { status: "dynamic", expression: "worldId" }.
src/ida/IdaNativeProtocol.test.ts:790 — "does not admit the headless-only
containment and document operations": expected true to be false.

All four also fail at 04672f2b on origin/main and in the Source tests CI
job for b3c5672c, so they are independent of this change.

_{🔄 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/3141 **Author:** [@XiaoBinGan](https://github.com/XiaoBinGan) **Created:** 10/14/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/javascript-ambient-alias-write-capture` --- ### 📝 Commits (1) - [`ce21fee`](https://github.com/morluto/rea/commit/ce21feead3e29bf691142555f6eb1b13c81bd58d) fix(javascript): resolve a growing ambient alias at its own read ### 📊 Changes **1 file changed** (+25 additions, -13 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/javascript/javascriptSemanticAmbientWrites.ts` (+25 -13) </details> ### 📄 Description ## Summary Resolve each ambient alias initializer at its own read position, and follow the reference initializers a mutation selection keeps, so a growing self-reference alias (`alias = alias.globalThis`) retains the ambient identity its earlier read selected. This repairs one of the four JavaScript regressions introduced by [`04672f2b`](https://github.com/morluto/rea/commit/04672f2b917d7ae3d71424a6abbcc2da0f24fa0b) ("fix(javascript): preserve builtin identity and share alias traversal"). ## Problem and expected behavior `04672f2b` carried write sites into ambient alias resolution and narrowed the initializers a possible write can observe: ```ts const initializers = possibleWrites ? capture === undefined ? binding.initializers : semanticMutationInitializers(binding, capture, state.parentsByNode).initializers : ...; ``` `semanticMutationInitializers` returns only the initializers at the latest preceding statement, and it splits them: an assignment such as `alias = alias.globalThis` is reported through `referenceInitializers`, while `initializers` holds the declarator that preceded it. The write lookup read only `.initializers` and never followed `referenceInitializers`, so for ```js let alias = globalThis; alias = alias.globalThis; alias.URL = replacement; ``` the selection returned `initializers: []`, the ambient identity from `let alias = globalThis` was filtered out, and no path was pushed. The resolver therefore retained nothing and `alias.URL = replacement` was no longer recorded as a write through the global object. Expected behavior: `URL` is covered by an ambient write, so `isUnmodifiedGlobalReference` is false and a `new URL("./worker.js", import.meta.url)` specifier is not trusted. The owning test, `src/domain/javascript/javascriptSemanticAmbientWrites.test.ts` > "preserves growing self-reference projections with an ambient initializer", asserts exactly this. ## Change and scope `src/domain/javascript/javascriptSemanticAmbientWrites.ts` only: - Each resolver step now carries the node whose position decided it. An alias initializer is resolved at its own read rather than at the later write site that followed it, so the earlier read selects the initializers visible where it occurred. - A mutation selection now contributes both `initializers` and `referenceInitializers` to the write lookup. The cycle/diamond guard already makes re-entering the same read terminate, so following a self-reference stays finite. - Sub-expression descent (`continueWith`) keeps the current step's position, so conditional, logical, and object-literal branches resolve where their value was read. An assignment of an unrelated value still replaces an earlier initializer, which is the behavior `04672f2b` intended: for `let a = globalThis; a = {}; a.URL = x;` the selection returns the object literal and no ambient path is retained. ## Contract and boundary impact No schema, emitted-fact, or public API change. The change is confined to the query "which ambient identity could this write have reached", which remains an over-approximation for possible writes. Intrinsic reads (`possibleWrites: false`) are untouched and still use `semanticStableInitializer`. ## Evidence and regression coverage Bisect on `origin/main` (`58300885`): - At `04672f2b^` (`0f4c484b`): the four affected files pass, 71 passed (71). - At `04672f2b`: the four affected files fail, 4 failed | 68 passed (72). - At `04672f2b` with this patch: 3 failed | 69 passed (72) — the ambient-writes test now passes and no previously passing test regressed. Full JavaScript semantic suite (76 files) with this patch, run through `npm run test:focused`: - 1810 passed | 3 failed | 1813 total. The three failures are the unfixed regressions listed below; none is new. - `npm run check:fast`: 2 successful. - `npm run format:check`: clean. ## Unavailable coverage This patch does not make the `Source tests` check green. Three further JavaScript regressions from the same commit remain, each with a distinct mechanism, and one further failure in `src/ida/IdaNativeProtocol.test.ts`: - `src/domain/javascript/javascriptSemanticAnalysis.native_bindings.test.ts:88` — "recovers configuration, requests, response consumers, and boundaries": the `{ kind: "environment", key: "PORT" }` operation from `Number(process.env.PORT)` is missing. - `src/application/javascript/JavaScriptSemanticGraphResources.test.ts:190` — "projects config precedence, request fields, and boundaries": a `supplies-request-field` relation is absent. - `src/domain/javascript/electronWrappedCallPaths.test.ts:150` — "retains the original expression for a dynamic isolated world id": the `world_id` is no longer reported as `{ status: "dynamic", expression: "worldId" }`. - `src/ida/IdaNativeProtocol.test.ts:790` — "does not admit the headless-only containment and document operations": `expected true to be false`. All four also fail at `04672f2b` on `origin/main` and in the `Source tests` CI job for `b3c5672c`, so they are independent of this change. --- <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:10 +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#5638

No description provided.

Rows
Columns