Migrate core event system from EventEmitter3 to Eventa#567
Migrate core event system from EventEmitter3 to Eventa#567luoling8192 wants to merge 4 commits intomainfrom
Conversation
Replace eventemitter3-based event bus with @moeru/eventa for type-safe events and RPC patterns in the core package. Phase 1: Define all Eventa events across 14 domain files in events/ Phase 2: Replace CoreContext.emitter with EventContext from Eventa Phase 3: Migrate all event handlers, services, and tests - Convert 13 implicit request-response pairs to defineInvokeEventa RPC - Convert ~40 fire-and-forget events to defineEventa - Update all 10 handler files, 6 service files, and 7 test files - Install @moeru/eventa in server and client packages Old CoreEventType enum and types/events.ts retained for server/client packages (to be removed in later phases). https://claude.ai/code/session_01JgqUCyTk9N37FSGY6nc97W
Migrate server WebSocket bridge, bot package, and client core-bridge to
use @moeru/eventa event system, completing the Phase 4 refactor:
- Create shared event dispatch maps (fireAndForgetEvents, rpcEvents,
notificationEvents) in packages/core/src/events/dispatch.ts
- Refactor server app.ts and event-dispatch.ts to use shared maps
for dispatching WS messages to Eventa events
- Refactor server account.ts with notification event broadcasting
via typed Eventa event objects
- Migrate bot bridge.ts, export.ts, and summary.ts to Eventa events
(summary.ts uses defineInvoke for RPC)
- Fix client core-bridge.ts to use Eventa context (ctx.ctx) instead
of removed emitter property
- Add onEvent/onceEvent/waitForEvent helpers in utils/promise.ts for
safe body unwrapping (Eventa wraps payload in { body?: P })
- Fix CoreContext.ctx type to EventContext<any, any> for
InvocableEventContext compatibility
- Fix void event emits to pass explicit undefined
- Remove unused memory-leak-detector.ts
- Simplify server events.ts types with Parameters<> pattern
- All event handler files updated to use onEvent helper
- All tests passing, typecheck clean across all packages
https://claude.ai/code/session_01JgqUCyTk9N37FSGY6nc97W
|
Warning Gemini encountered an error creating the summary. You can try again by commenting |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c4386ea346
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| function cleanup() { | ||
| logger.debug('Cleaning up CoreContext') | ||
|
|
||
| // Clean up memory leak detector first | ||
| cleanupMemoryLeakDetector() | ||
|
|
||
| // Remove all event listeners | ||
| emitter.removeAllListeners() | ||
|
|
||
| // Clear event sets | ||
| toCoreEvents.clear() | ||
| fromCoreEvents.clear() | ||
| useLogger('core:context').debug('Cleaning up CoreContext') | ||
|
|
||
| // Clear client reference | ||
| // @ts-expect-error - Allow setting to undefined for cleanup |
There was a problem hiding this comment.
destroyCoreInstance() still treats ctx.cleanup() as the final teardown path, but cleanup() now only clears local references and never unregisters Eventa subscriptions. Because many handlers run async fire-and-forget (for example logout/takeout flows), in-flight work can still emit into this context after destruction and trigger stale handlers/broadcast closures, causing post-logout side effects and retained account-scoped references that the previous removeAllListeners() cleanup prevented.
Useful? React with 👍 / 👎.
| { event: messageDataEvent, wireName: 'message:data' }, | ||
| { event: messageFetchProgressEvent, wireName: 'message:fetch:progress' }, | ||
| { event: dialogAvatarDataEvent, wireName: 'dialog:avatar:data' }, |
There was a problem hiding this comment.
The new server-side static notificationEvents list omits message:processed, even though this commit still defines it as a core notification in the shared dispatch map. In WebSocket/server mode, clients that register for message:processed will no longer receive batch completion events, creating a behavior regression and inconsistency versus core-bridge mode where this notification is still wired.
Useful? React with 👍 / 👎.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
|
|
||
| logger.withFields({ accountId: dbAccount.id }).verbose('Set current account ID') | ||
|
|
||
| ctx.emitter.emit(CoreEventType.AccountReady, { accountId: dbAccount.id }) |
There was a problem hiding this comment.
Bootstrap fetchDialogs call no longer persists dialogs to database
High Severity
The refactored fetchDialogs now only returns { dialogs, accountId } instead of emitting StorageRecordDialogs as a side effect. The bootstrap call in afterConnectedEventHandler discards this return value, so dialogs fetched during initial account connection are never persisted to the database. The RPC handler in registerDialogEventHandlers does emit storageRecordDialogsEvent, but that handler isn't used for the bootstrap path — and it's registered after the bootstrap call completes.
Additional Locations (1)
| ctx.emitter.on(CoreEventType.TakeoutStatsFetch, async ({ chatId }) => { | ||
| await takeoutService.fetchChatSyncStats(chatId) | ||
| defineInvokeHandler(ctx.ctx, takeoutStatsFetchInvoke, async ({ chatId }) => { | ||
| return await takeoutService.fetchChatSyncStats(chatId) |
There was a problem hiding this comment.
RPC handler returns undefined on caught service error
Medium Severity
fetchChatSyncStats catches errors internally and returns undefined from the catch block. The new defineInvokeHandler wrapping this call will send undefined as the RPC response to the client. Previously, errors were silently swallowed with no event emitted. Now the client receives undefined instead of a ChatSyncStats object, likely causing destructuring errors on the receiving end. The catch block needs to either rethrow or return a default value.
Additional Locations (1)
| currentAccountId = undefined | ||
|
|
||
| logger.debug('CoreContext cleaned up') | ||
| useLogger('core:context').debug('CoreContext cleaned up') |
There was a problem hiding this comment.
Cleanup no longer removes event listeners causing memory leak
High Severity
The cleanup() function previously called emitter.removeAllListeners() to tear down all event subscriptions when an account logs out. That line was removed during the migration, and no Eventa equivalent was added. All onEvent, ctx.ctx.on, and defineInvokeHandler registrations remain active on the orphaned Eventa context after destroyCoreInstance runs, leaking memory. The destroyCoreInstance docstring even states it "Removes ALL event listeners from emitter" and "Disposes Eventa context listeners" but the implementation no longer does either.
Additional Locations (1)
2 participants
Summary
This PR migrates the core event system from
EventEmitter3to@moeru/eventa, a type-safe event framework. The migration introduces a cleaner separation between fire-and-forget events and RPC-style invoke handlers, with centralized event definitions and improved type safety across the codebase.Key Changes
Event System Overhaul: Replaced
EventEmitter3with@moeru/eventa'sEventContextanddefineEventa/defineInvokeEventaAPIsCoreEventTypeenum and replaced with individual typed event definitionsauth.ts,message.ts,storage.ts,dialog.ts, etc.) inpackages/core/src/events/dispatch.tswith centralized event mapping for wire protocol bridgingEvent Handler Refactoring: Updated all event handlers to use new APIs
ctx.emitter.on()toonEvent()helper ordefineInvokeHandler()for RPC handlersctx.withError()+ return to throwing errors in invoke handlersreturnresults instead of emitting separate eventsRPC vs Fire-and-Forget Separation: Clearly distinguished between two event patterns
storageFetchMessagesInvoke) usedefineInvokeHandler()and return responsesmessageFetchEvent) useonEvent()and emit without waitingrpcEventsandfireAndForgetEventsmaps for wire protocol dispatchServer-Side Event Dispatch: Created
apps/server/src/event-dispatch.tsto bridge JSON-over-WebSocket to EventaregisterCoreEventListeners()pattern withdispatchClientEvent()Client Adapter Updates: Updated
packages/client/src/adapters/core-bridge.tsto use new event mapsfireAndForgetEvents,notificationEvents, andrpcEventsUtility Helpers: Added
onEvent()andonceEvent()wrappers inpackages/core/src/utils/promise.tsbodypayload for cleaner handler signatureswaitForEvent()implementationTest Updates: Updated test files to use new event APIs
CoreEventTypereferences with imported event objectscreateContext()from EventaRemoved: Deleted
packages/core/src/utils/memory-leak-detector.ts(no longer needed with Eventa)Notable Implementation Details
packages/core/src/events/dispatch.tsfile serves as a single source of truth for wire protocol event mappinghttps://claude.ai/code/session_01JgqUCyTk9N37FSGY6nc97W
Note
High Risk
High risk because it rewires the core cross-cutting event bus (auth, storage, sync, takeout, message processing) and changes server/client WebSocket dispatch semantics, including new RPC-style invoke/return flows and error propagation via thrown exceptions.
Overview
Migrates the core event system from
EventEmitter3to@moeru/eventa, introducing a typedCoreContext.ctxand modular event definitions underpackages/core/src/events/*(plus sharedfireAndForgetEvents/rpcEvents/notificationEventsmaps).Refactors core handlers and services to use
onEvent/onceEventanddefineInvokeHandler, shifting several flows from “emit response events” to RPC-style invoke handlers that return results (notably storage and message summary/unread paths), and updates error handling in invokes to throw for unauthorized/failed operations.Updates the server WebSocket bridge to dispatch incoming messages via a new
event-dispatch.ts(RPC invokes awaited and replied to the calling peer; fire-and-forget emitted), and registers all core notification broadcasts upfront per account (deprecating the runtimeserver:event:registerlistener-registration behavior). Bot and client adapters are updated accordingly, tests are rewritten for Eventa contexts,@moeru/eventais added across packages, and the old memory leak detector is removed.Written by Cursor Bugbot for commit d2a5cb5. This will update automatically on new commits. Configure here.