Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
041a506
fix(javascript): emit no event operation for a missing listener argument
gongfpp Nov 2, 2026
f1d30fe
fix(javascript): hold only a proven Node emitter to the listener cont…
gongfpp Nov 3, 2026
4d36250
fix(javascript): require the native listener method itself, not just …
gongfpp Nov 3, 2026
b6917aa
chore: merge main to pick up the Wrangler config format fix
gongfpp Nov 3, 2026
9617f1a
fix(javascript): keep the listener operation when the method or the r…
gongfpp Nov 3, 2026
cb36de8
fix(javascript): read prototype writes by slot, not by exact key
gongfpp Nov 3, 2026
e9e604a
fix(javascript): withdraw the listener proof when the prototype itsel…
gongfpp Nov 3, 2026
0bd6c11
fix(javascript): require the events class surface to be untouched
gongfpp Nov 3, 2026
c964e98
fix(javascript): scope the class-surface proof to the events class
gongfpp Nov 3, 2026
87b7be8
Merge upstream main into missing-listener fix
gongfpp Nov 5, 2026
c9427ea
fix(javascript): keep listener candidates after shared mutations
gongfpp Nov 5, 2026
f038fdc
fix(javascript): retain constructed receiver class references
gongfpp Nov 5, 2026
47e3b9b
fix(javascript): time ambient prototype effects at XHR reads
gongfpp Nov 5, 2026
ad13261
perf(javascript): share ambient path effect matching
gongfpp Nov 5, 2026
94dc27a
merge: sync main into missing-listener fix
gongfpp Nov 9, 2026
98316a1
perf(javascript): index shared EventEmitter effects
gongfpp Nov 9, 2026
c719fb1
fix(javascript): track EventEmitter initializer escapes
gongfpp Nov 9, 2026
88d5732
fix(javascript): honor initializer binding provenance
gongfpp Nov 9, 2026
514a306
fix(javascript): preserve timed native receiver proofs
gongfpp Nov 9, 2026
9a7b483
fix(javascript): retain builtin acquisition mutation effects
gongfpp Nov 9, 2026
3c3684c
fix(javascript): retain awaited module acquisition effects
gongfpp Nov 9, 2026
65d74b0
fix(javascript): keep intrinsic Map calls from escaping
gongfpp Nov 9, 2026
b6cc762
fix(javascript): preserve pristine XHR and getter alias effects
gongfpp Nov 9, 2026
5944b69
chore: merge upstream main to refresh CI base
gongfpp Nov 9, 2026
676c01d
fix(javascript): share mutable events default effects
gongfpp Nov 9, 2026
25cc26b
fix(javascript): track inherited initializer call effects
gongfpp Nov 9, 2026
95cd68c
fix(javascript): preserve iterator and initializer effect boundaries
gongfpp Nov 9, 2026
e118fc9
fix(javascript): retain pristine Array receiver certainty
gongfpp Nov 9, 2026
ba119f3
fix(javascript): preserve builtin receiver and getter effects
gongfpp Nov 9, 2026
6b18b70
fix(javascript): preserve pristine Array read certainty
gongfpp Nov 9, 2026
9b39943
fix(javascript): preserve Array iteration read timing
gongfpp Nov 10, 2026
09001af
merge: sync main and preserve timed iteration effects
gongfpp Nov 10, 2026
be61d5c
fix(javascript): preserve native inspection and discarded-result proofs
gongfpp Nov 10, 2026
048274c
fix(javascript): preserve inherited native method proofs
gongfpp Nov 10, 2026
486aade
fix(javascript): reconcile listener proofs with upstream main
gongfpp Nov 11, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ const eventsFor = (call: string) =>
eventName,
}));

const eventsForSource = (source: string) =>
analyzeJavaScriptSemantics(source).eventOperations.map(
({ kind, method, eventName }) => ({ kind, method, eventName }),
);

it.each([
["a numeric listener", `e.on("x", 1)`],
["a string listener", `e.on("x", "fn")`],
Expand Down Expand Up @@ -67,6 +72,158 @@ it.each([
]);
});

/**
* A listener method called without a listener argument passes `undefined` to
* Node's `checkListener`, which throws before anything is registered or
* removed. The argument position is empty, so no function can arrive there.
*
* Measured on Node 22.23.3:
*
* emitter.on() -> THREW ERR_INVALID_ARG_TYPE
* emitter.on("x") -> THREW ERR_INVALID_ARG_TYPE
* emitter.once("x") -> THREW ERR_INVALID_ARG_TYPE
* emitter.addListener("x") -> THREW ERR_INVALID_ARG_TYPE
* emitter.prependListener("x") -> THREW ERR_INVALID_ARG_TYPE
* emitter.prependOnceListener("x") -> THREW ERR_INVALID_ARG_TYPE
* emitter.removeListener("x") -> THREW ERR_INVALID_ARG_TYPE
* emitter.off("x") / emitter.off() -> THREW ERR_INVALID_ARG_TYPE
*
* A spread still reaches the position through the call, so it is untouched.
*/
it.each([
["a missing listener", `e.on("x")`],
["a missing once listener", `e.once("x")`],
["a missing addListener listener", `e.addListener("x")`],
["a missing prependListener listener", `e.prependListener("x")`],
["a missing prependOnceListener listener", `e.prependOnceListener("x")`],
["a missing removeListener listener", `e.removeListener("x")`],
["a missing off listener", `e.off("x")`],
["no arguments at all", `e.on()`],
["no off arguments at all", `e.off()`],
] as const)("emits no event operation for %s", (_name, call) => {
expect(eventsFor(call)).toEqual([]);
});

it("keeps the event operation for a spread call with no fixed listener", () => {
expect(eventsFor(`e.on(...pairs)`)).toEqual([
{
kind: expect.any(String) as unknown as string,
method: "on",
eventName: null,
},
]);
});

it.each([
[
"a named import",
`import { EventEmitter } from "node:events";`,
"EventEmitter",
],
["a default import", `import EE from "node:events";`, "EE"],
[
"a destructured require",
`const { EventEmitter } = require("events");`,
"EventEmitter",
],
[
"a member require",
`const events = require("node:events");`,
"events.EventEmitter",
],
] as const)(
"drops a missing listener on a receiver built from %s",
(_name, header, constructor) => {
expect(
eventsForSource(`${header}
const e = new ${constructor}();
e.on("x");`),
).toEqual([]);
},
);

it("keeps a missing listener when the receiver was reassigned", () => {
expect(
eventsForSource(`import { EventEmitter } from "node:events";
const e = new EventEmitter();
e = other;
e.on("x");`),
).toContainEqual({ kind: "register", method: "on", eventName: "x" });
});

/**
* `checkListener` is Node's own contract. An EventEmitter-style receiver that
* this module does not construct from `events.EventEmitter` may define a
* one-argument form: Socket.IO's `socket.off("x")` removes every listener for
* the event, so the operation must survive as an unresolved candidate.
*/
it("keeps a one-argument removal on a receiver with no proven Node emitter", () => {
expect(
eventsForSource(`function g(socket) { socket.off("x"); }`),
).toContainEqual({ kind: "remove", method: "off", eventName: "x" });
});

/**
* A subclass may default the listener position, which makes `bus.on("ready")`
* a real registration even though the module never proves Node's rejection.
* Measured on Node 22.23.3: this class stores `"ready"` and does not throw.
*/
it("keeps the registration of a subclass that defaults its listener", () => {
const events = eventsForSource(
`import { EventEmitter } from "node:events";
class Bus extends EventEmitter {
on(name, fn = () => {}) { return super.on(name, fn); }
}
const bus = new Bus();
bus.on("ready");`,
);
expect(events).toContainEqual({
kind: "register",
method: "on",
eventName: "ready",
});
});

/**
* A member write replaces the method the native contract belongs to. Measured
* on Node 22.23.3: `e.on = (name, fn = () => {}) => ...` stores `"ready"`, so
* `checkListener` never runs and the one-argument call is a real registration.
*/
it("keeps the registration of a receiver whose method was replaced", () => {
expect(
eventsForSource(`import { EventEmitter } from "node:events";
const e = new EventEmitter();
e.on = (name, fn = () => {}) => EventEmitter.prototype.on.call(e, name, fn);
e.on("ready");`),
).toContainEqual({ kind: "register", method: "on", eventName: "ready" });
});

/**
* Every instance shares the constructor's prototype, so replacing the method
* there replaces it for this receiver too.
*/
it("keeps the registration of a receiver whose prototype was replaced", () => {
expect(
eventsForSource(`import { EventEmitter } from "node:events";
EventEmitter.prototype.on = function (name, fn = () => {}) { return this; };
const e = new EventEmitter();
e.on("ready");`),
).toContainEqual({ kind: "register", method: "on", eventName: "ready" });
});

/**
* The proof is keyed on the selected method, so a write to an unrelated member
* must not weaken it: `tag` never answers `on`.
*/
it("still rejects a missing listener beside an unrelated member write", () => {
expect(
eventsForSource(`import { EventEmitter } from "node:events";
const e = new EventEmitter();
e.tag = 1;
e.on("ready");`),
).toEqual([]);
});

/**
* `emit` passes its remaining arguments to listeners that already exist, so it
* does not validate them the way a registration does.
Expand Down
106 changes: 102 additions & 4 deletions src/domain/javascript/javascriptSemanticAsyncEffects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,20 @@ import {
import {
resolveSemanticBindingState,
semanticResolutionBlocked,
semanticStableInitializer,
type JavaScriptSemanticAnalysisState,
type JavaScriptSemanticBindingState,
} from "./javascriptSemanticState.js";
import { semanticBindingAtRead } from "./javascriptSemanticMutationInitializers.js";
import {
semanticBindingAtRead,
semanticTopLevelEffectFollowsCapture,
} from "./javascriptSemanticMutationInitializers.js";
import { traverseJavaScriptValueAst } from "./javascriptSemanticTraversal.js";
import { isUnmodifiedGlobalReference } from "./javascriptSemanticAmbientWrites.js";
import { range } from "./javascriptStaticAnalysisHelpers.js";
import {
affectsBuiltinPath,
builtinDataEffectCallablePath,
builtinDataEffectMethod,
dataEffectArgumentBindingId,
dataEffectLiteralString,
Expand Down Expand Up @@ -161,6 +167,88 @@ const rejectsEventListener = (listener: t.Node): boolean => {
);
};

/**
* Node's `events` module, whose `EventEmitter` validates every listener
* argument before it registers or removes anything.
*/
const NODE_EVENTS_SPECIFIERS = ["events", "node:events"] as const;

/**
* Report whether a constructor reference resolves to Node's `EventEmitter`.
* Named and default imports, destructured requires, and member requires all
* denote that class.
*/
const isNodeEventEmitterConstructor = (
node: t.Node,
state: JavaScriptSemanticAnalysisState,
): boolean => {
const value = unwrapJavaScriptExpression(node).node;
if (!t.isNewExpression(value)) return false;
const selected = builtinDataEffectCallablePath(
value.callee,
state,
NODE_EVENTS_SPECIFIERS,
);
Comment on lines +230 to +234

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Share events-module mutations across independent imports

With import A from "events"; import B from "node:events", both bindings reference the same mutable constructor. After A.EventEmitter = class extends A { on(name, fn = () => {}) { return super.on(name, fn); } }, const e = new B.EventEmitter(); e.on("ready") registers successfully (verified on Node 24.15.0). However, semanticMutableBuiltinOrigin excludes events from MUTABLE_BUILTIN_DEFAULTS, so the write never enters shared module effects and this lookup through B still reports ["EventEmitter"]. The prototype scan also ignores the constructor-slot write, allowing the new guard to discard the real registration. Register the shared mutable identity at the builtin-mutation owner before using this resolution to prove absence.

AGENTS.md reference: AGENTS.md:L19-L19

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Valid. Both specifiers resolve to one mutable module at runtime, so the proof cannot be scoped to the binding the construction happens to use.

Writes are now matched by what they name rather than by which binding owns them: any write whose path selects EventEmitter withdraws the proof, whichever import performed it, and the module records are read the same way for the inline require form.

Validation: first.EventEmitter = class extends first.EventEmitter { ... } followed by new second.EventEmitter() keeps the registration. Measured on Node 22.22.2: e.on("ready") does not throw afterwards. Reverting the class-surface check fails the new case.

// An empty selected path is the module's default export, which `events`
// defines as `EventEmitter`.
return (
selected !== null &&
(selected.length === 0 || selected.at(-1) === "EventEmitter")
);
};

/**
* Report whether the receiver is a value this module constructs from Node's
* `EventEmitter`. Only that class is bound by `checkListener`; other
* EventEmitter-style APIs define one-argument forms, such as Socket.IO's
* `socket.off("x")`, which removes every listener for the event. A receiver
* that is not proven this way keeps its operation as an unresolved candidate.
*/
const nodeEventEmitterReceiver = (
node: t.Expression | t.Super,
method: string,
state: JavaScriptSemanticAnalysisState,
): boolean => {
if (!t.isIdentifier(node)) return false;
const binding = resolveSemanticBindingState(state, node, node.name);
if (binding === undefined) return false;
const initializer = semanticStableInitializer(binding, state);
if (
initializer === undefined ||
initializer.projection.length !== 0 ||
!isNodeEventEmitterConstructor(initializer.node, state)
)
return false;
// Constructing the class proves where the receiver came from, not which
// definition answers the call. A member write replaces the selected method,
// so it no longer keeps the native contract. An unrelated write such as
// `emitter.tag = 1` leaves the proof intact.
if (
binding.memberWrites.some(
({ path, node: write }) =>
affectsBuiltinPath(path, [method]) &&
!semanticTopLevelEffectFollowsCapture(node, write, state.parentsByNode),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Include receiver prototype writes in method validation

After native construction, e.__proto__ = { on(name, fn = () => {}) { return saved.call(this, name, fn); } }, with saved holding the native on, makes e.on("ready") register successfully (verified on Node 24.15.0). Unlike the direct method writes already covered, this write is recorded as ["__proto__"]: it does not affect [method] under this predicate, and the subsequent prototype scan recognizes only paths containing prototype, not __proto__. Neither check withdraws the native proof, so the guard incorrectly removes a real registration from eventOperations. Check receiver-prototype mutations as well as selected-member mutations, as the sibling Map proof already does, and preserve the candidate when either may have changed.

AGENTS.md reference: AGENTS.md:L21-L21

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Valid. The receiver's own prototype chain is the other way to select a different method, and it is recorded as a __proto__ member write, which neither the selected-method check nor the prototype scan matched.

A __proto__ write on the receiver now withdraws the proof on the same terms as a direct method write, ordering included. The prototype-object escape check added in the thread above covers the shared case.

Validation: e.__proto__ = { on(name, fn = () => {}) { ... } } now keeps the registration. Measured on Node 22.22.2: e.on("ready") does not throw afterwards. Reverting the check fails the new case.

)
)
return false;
Comment thread
greptile-apps[bot] marked this conversation as resolved.
Resolved
// Every instance shares the constructor's prototype, so a write there
// replaces the method for this receiver as well.
const constructed = (initializer.node as t.NewExpression).callee;
const structural = unwrapJavaScriptExpression(constructed).node;
if (!t.isIdentifier(structural)) return true;
Comment thread
greptile-apps[bot] marked this conversation as resolved.
Resolved

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Validate prototype writes for member-expression constructors

With import * as events from "node:events" and const e = new events.EventEmitter(), replacing events.EventEmitter.prototype.on after construction with function (name, fn = () => {}) { return this.addListener(name, fn); } makes e.on("ready") register a real listener (verified on Node 24.15.0). The constructor callee is a MemberExpression, so this return skips the prototype-write check. The earlier builtinDataEffectCallablePath check proves only the constructor, not prototype.on; consequently, collectEvent discards the sole literal ready operation and projectSemanticEvents loses the event. This member-constructor bypass is fresh evidence beyond the direct-identifier mutation case now covered by tests. Check the selected prototype method at the call site for every accepted constructor form, or retain the operation when that proof is unavailable.

AGENTS.md reference: AGENTS.md:L21-L21

Useful? React with 👍 / 👎.

const constructorBinding = resolveSemanticBindingState(
state,
structural,
structural.name,
);
if (constructorBinding === undefined) return false;
return !constructorBinding.memberWrites.some(
({ path, node: write }) =>
affectsBuiltinPath(path, ["prototype", method]) &&
!semanticTopLevelEffectFollowsCapture(node, write, state.parentsByNode),
);
};

const collectEvent = (
node: t.CallExpression | t.OptionalCallExpression,
ownerCallableId: string | null,
Expand All @@ -177,9 +265,19 @@ const collectEvent = (
if (method === undefined) return;
const kind = eventKind(method);
const eventArgument = node.arguments[0];
const listenerArgument = EVENT_LISTENER_METHODS.includes(method)
? node.arguments[1]
: undefined;
const listenerMethod = EVENT_LISTENER_METHODS.includes(method);
const listenerArgument = listenerMethod ? node.arguments[1] : undefined;
// A listener method called without a listener argument passes `undefined` to
// Node's `checkListener`, which throws before it registers or removes
// anything. Only a spread argument can still reach that position, and only a
// receiver proven to be Node's `EventEmitter` is held to that contract.
if (
listenerMethod &&
listenerArgument === undefined &&
!node.arguments.some((argument) => t.isSpreadElement(argument)) &&
nodeEventEmitterReceiver(member.object, method, state)
)
return;
Comment thread
greptile-apps[bot] marked this conversation as resolved.
Resolved
Comment on lines +682 to +687

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Restrict missing-listener rejection to verified Node methods

For an EventEmitter subclass defining on(name, fn = () => {}) { return super.on(name, fn); }, bus.on("ready") successfully registers a listener, but this new branch discards its literal event operation. collectEvent recognizes only the property name and never proves that the receiver uses Node's native listener validation; optional-listener off implementations are affected for the same reason. The remaining super.on(name, fn) candidate has an unknown event name, so it does not preserve the ready event that previously reached projectSemanticEvents. Apply this rejection only when the native method contract is established, and retain an uncertain candidate otherwise.

AGENTS.md reference: AGENTS.md:L21-L21

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed on the same root cause as the other thread, and the subclass case is now a regression test.

The guard keyed on the property name, so it applied Node's checkListener contract to receivers that never agreed to it. It now requires proof that the receiver is constructed from events.EventEmitter, so bus.on("ready") on a subclass with a defaulted listener keeps its "ready" registration, and off implementations with an optional listener are no longer discarded.

I also kept the uncertainty explicit rather than resolving it toward absence: a receiver that cannot be proven keeps its operation as an unresolved candidate, matching the behavior before this PR.

Measured on Node 22.23.3 — the subclass defaults the listener, registers "ready", and does not throw; the inherited off still throws.

// A leading spread shifts the runtime argument positions, so argument 1 is not
// provably the listener: `e.on(...["x", fn], 1)` registers `fn` and ignores the
// trailing `1`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -580,7 +580,7 @@ const builtinNamespaceOrigin = (
: { ...origin, specifier, importedPath };
};

const affectsBuiltinPath = (
export const affectsBuiltinPath = (
path: JavaScriptSemanticPropertyPath,
selected: readonly string[],
): boolean =>
Expand Down
Loading