morluto/rea

Fork 0

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

[PR #2952] [MERGED] fix(javascript-semantics): keep emitted data out of dispatch listener fields #5511

New issue

Closed

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

bsakaguchi commented

2026-10-31 01:07:13 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2952
Author: @0x-saurabh
Created: 10/13/2026
Status: ✅ Merged
Merged: 10/13/2026
Merged by: @morluto

Base: main ← Head: fix/javascript-event-dispatch-listener

📝 Commits (2)

1cbea3b fix(javascript-semantics): keep emitted data out of dispatch listener fields
2b508fc fix(javascript-semantics): read listener arguments only for listener-taking methods

📊 Changes

2 files changed (+81 additions, -1 deletions)

View changed files

📝 src/domain/javascript/javascriptSemanticAnalysis.runtime_boundaries.test.ts (+71 -0)
📝 src/domain/javascript/javascriptSemanticAsyncEffects.ts (+10 -1)

📄 Description

Summary

Fixes #2574.

The EventEmitter operation collector read argument position 1 as the listener for every event method, including emit. A dispatch such as bus.emit("ready", data) therefore reported data as its listenerBindingId and the data argument's range as listenerLocation, giving IR consumers a false listener association for what is event payload.

Problem and expected behavior

Smallest trigger:

import { EventEmitter } from "node:events";
const bus = new EventEmitter();
const data = 1;
bus.emit("ready", data);

The dispatch operation carried listenerBindingId: "...:binding:data" and a listenerLocation covering data. emit passes its remaining arguments to listeners that already exist; only registration (on, once, ...) and removal (off, removeListener) name a listener in argument position 1. Expected: the dispatch keeps eventName: "ready" and resolution: "complete" with both listener fields null, while on and off keep reporting the bound handler and its location.

Change and scope

In collectEvent (src/domain/javascript/javascriptSemanticAsyncEffects.ts), the listener argument is read only for the methods whose argument position 1 names a listener (EVENT_LISTENER_METHODS: on, once, addListener, prependListener, prependOnceListener, off, removeListener). emit passes its remaining arguments as data, and removeAllListeners takes only an optional event name, so neither reports a listener. Nothing else changes: event identity, emitter resolution, event-name resolution, and the dispatches-candidate relation are untouched.

The graph projection (JavaScriptSemanticGraphAsyncProjection.ts) already builds listener nodes only for non-dispatch operations, so graph output does not change; the defect was confined to the IR, as the issue states.

Follow the contribution scope.

Contract and boundary impact

Semantic owner and earliest changed stage: collectEvent in src/domain/javascript/javascriptSemanticAsyncEffects.ts (domain static analysis, IR production).
Contribution scope fit and linked maintainer decision: ordinary static-analysis correctness fix within existing boundaries; not applicable.
CLI and MCP/tool-catalog contract: none.
Provider, bridge, target-format, or platform compatibility: none.
Evidence, artifact, provenance, or reconstruction contract: JavaScriptSemanticEventOperation for dispatch now has null listener fields, matching the type's documented nullable contract. No graph node or relation changes.
Target selection, authority, upstream behavior, and cleanup details (if applicable): not applicable; static source parsing only.
Generated metadata (docs/public/product-catalog.json), 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: src/domain/javascript/javascriptSemanticAnalysis.runtime_boundaries.test.ts gains one case with an explicit EventEmitter: on and off report the handler binding and a listener location; emit("ready", data) and emit("ready") both report null listener fields; removeAllListeners("ready", data) is a remove with null listener fields; all five operations keep eventName: "ready" and resolution: "complete".
Base reproduction or other evidence: executed. With the source file reverted to upstream main and only the test applied, the new case fails on the emit("ready", data) entry (listener binding and location populated). With the fix applied the file's 5 tests pass.
User-visible CLI/MCP output (if applicable): not applicable.
Remaining proof gaps: none known.
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 -- src/domain/javascript/javascriptSemanticAnalysis.runtime_boundaries.test.ts — 1 file, 5 tests passed (at 2b508fcaf).
Same focused run with javascriptSemanticAsyncEffects.ts reverted to upstream main (test only) — the new case fails on the emit("ready", data) entry; the other 4 pass. Confirms the regression reproduces on the base.
npm run check:fast — typecheck and lint, 2 tasks successful.
npm run check:changed — 453 test files passed, 2 failed: src/domain/reconstructionVerification.functionSources.test.ts (5 tests) and tests/conformance/providers/hopper/realHopperSearch.test.ts. Both fail identically with this change reverted on upstream main at 8fef2aa64; the first is a pre-existing failure on main, the second requires a real Hopper installation that this host does not have. Neither imports the changed module.
git diff --check — clean.
Toolchain: Node 24.18.0 (.nvmrc), npm 11.16.0, macOS arm64. No real-provider lane applies.

Compatibility, safety, and release

Breaking changes or migration steps: none.
Real Hopper/Ghidra, browser, or OS coverage: not applicable; pure static JavaScript analysis.
Package or release metadata impact: none.
Security, privacy, process, or containment review: no new I/O, process, or network behavior.

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.

🤖 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/2952 **Author:** [@0x-saurabh](https://github.com/0x-saurabh) **Created:** 10/13/2026 **Status:** ✅ Merged **Merged:** 10/13/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/javascript-event-dispatch-listener` --- ### 📝 Commits (2) - [`1cbea3b`](https://github.com/morluto/rea/commit/1cbea3bb0e222b1bf266a7360ad87f2b3f511a72) fix(javascript-semantics): keep emitted data out of dispatch listener fields - [`2b508fc`](https://github.com/morluto/rea/commit/2b508fcaf69e85efba50c311c0bd0c76ed97a5bd) fix(javascript-semantics): read listener arguments only for listener-taking methods ### 📊 Changes **2 files changed** (+81 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/javascript/javascriptSemanticAnalysis.runtime_boundaries.test.ts` (+71 -0) 📝 `src/domain/javascript/javascriptSemanticAsyncEffects.ts` (+10 -1) </details> ### 📄 Description ## Summary Fixes #2574. The EventEmitter operation collector read argument position 1 as the listener for every event method, including `emit`. A dispatch such as `bus.emit("ready", data)` therefore reported `data` as its `listenerBindingId` and the data argument's range as `listenerLocation`, giving IR consumers a false listener association for what is event payload. ## Problem and expected behavior Smallest trigger: ```js import { EventEmitter } from "node:events"; const bus = new EventEmitter(); const data = 1; bus.emit("ready", data); ``` The `dispatch` operation carried `listenerBindingId: "...:binding:data"` and a `listenerLocation` covering `data`. `emit` passes its remaining arguments to listeners that already exist; only registration (`on`, `once`, ...) and removal (`off`, `removeListener`) name a listener in argument position 1. Expected: the dispatch keeps `eventName: "ready"` and `resolution: "complete"` with both listener fields `null`, while `on` and `off` keep reporting the bound handler and its location. ## Change and scope In `collectEvent` (`src/domain/javascript/javascriptSemanticAsyncEffects.ts`), the listener argument is read only for the methods whose argument position 1 names a listener (`EVENT_LISTENER_METHODS`: `on`, `once`, `addListener`, `prependListener`, `prependOnceListener`, `off`, `removeListener`). `emit` passes its remaining arguments as data, and `removeAllListeners` takes only an optional event name, so neither reports a listener. Nothing else changes: event identity, emitter resolution, event-name resolution, and the `dispatches-candidate` relation are untouched. The graph projection (`JavaScriptSemanticGraphAsyncProjection.ts`) already builds listener nodes only for non-dispatch operations, so graph output does not change; the defect was confined to the IR, as the issue states. Follow the [contribution scope](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope). ## Contract and boundary impact - Semantic owner and earliest changed stage: `collectEvent` in `src/domain/javascript/javascriptSemanticAsyncEffects.ts` (domain static analysis, IR production). - Contribution scope fit and linked maintainer decision: ordinary static-analysis correctness fix within existing boundaries; not applicable. - CLI and MCP/tool-catalog contract: none. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: `JavaScriptSemanticEventOperation` for `dispatch` now has `null` listener fields, matching the type's documented nullable contract. No graph node or relation changes. - Target selection, authority, upstream behavior, and cleanup details (if applicable): not applicable; static source parsing only. - Generated metadata (`docs/public/product-catalog.json`), 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: `src/domain/javascript/javascriptSemanticAnalysis.runtime_boundaries.test.ts` gains one case with an explicit `EventEmitter`: `on` and `off` report the `handler` binding and a listener location; `emit("ready", data)` and `emit("ready")` both report `null` listener fields; `removeAllListeners("ready", data)` is a `remove` with `null` listener fields; all five operations keep `eventName: "ready"` and `resolution: "complete"`. - Base reproduction or other evidence: executed. With the source file reverted to upstream main and only the test applied, the new case fails on the `emit("ready", data)` entry (listener binding and location populated). With the fix applied the file's 5 tests pass. - User-visible CLI/MCP output (if applicable): not applicable. - Remaining proof gaps: none known. - [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 -- src/domain/javascript/javascriptSemanticAnalysis.runtime_boundaries.test.ts` — 1 file, 5 tests passed (at 2b508fcaf). - Same focused run with `javascriptSemanticAsyncEffects.ts` reverted to upstream `main` (test only) — the new case fails on the `emit("ready", data)` entry; the other 4 pass. Confirms the regression reproduces on the base. - `npm run check:fast` — typecheck and lint, 2 tasks successful. - `npm run check:changed` — 453 test files passed, 2 failed: `src/domain/reconstructionVerification.functionSources.test.ts` (5 tests) and `tests/conformance/providers/hopper/realHopperSearch.test.ts`. Both fail identically with this change reverted on upstream `main` at `8fef2aa64`; the first is a pre-existing failure on `main`, the second requires a real Hopper installation that this host does not have. Neither imports the changed module. - `git diff --check` — clean. - Toolchain: Node 24.18.0 (`.nvmrc`), npm 11.16.0, macOS arm64. No real-provider lane applies. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: not applicable; pure static JavaScript analysis. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no new I/O, process, or network behavior. ## Before merge - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. 🤖 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:07:13 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

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

[PR #5511] [MERGED] feat(desktop): suggest managed members for Unity script assemblies #6947

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

No description provided.

Rows
Columns