Skip to content

fix(preload): one shared ipcRenderer listener for per-pane channels - #1039

Merged
Juliusolsson05 merged 3 commits into
mainfrom
fix/shared-ipc-listeners
Sep 19, 2026
Merged

Juliusolsson05 merged 3 commits into
mainfrom
fix/shared-ipc-listeners

Conversation

@Juliusolsson05

Copy link
Copy Markdown
Owner

Closes #1015.

Why

The dev app warned at every startup:

MaxListenersExceededWarning: 11 goal-loop:changed listeners added to [IpcRenderer] (and the same for dictation:stream-transcript).

It is not a leak. Every mounted GoalLoopPane and composer subscribes once and cleans up. But each subscription was its own ipcRenderer listener, so 11 panes crossed Node's default of 10. A permanent false alarm hides the next real leak. Raising MaxListeners would silence exactly that alarm.

Change

  • subscribeShared(channel, cb) in preload/api/ipc.ts:
    • one relay listener per channel, fanned out to a set of subscribers;
    • the relay is removed with the last subscriber;
    • each subscriber is isolated, so one pane that throws doesn't stop the others hearing the event (separate EventEmitter listeners didn't guarantee that either).
  • goal-loop:changed and dictation:stream-transcript use it.
  • LSP diagnostics keeps its own permanent listener on purpose: code blocks mount and unmount while scrolling, and its comment explains why it must not churn.

Tests

src/preload/api/sharedListeners.test.ts uses a real EventEmitter as ipcRenderer, for both channels:

  • 12 subscribers create 1 listener, and every subscriber gets every event;
  • the last unsubscribe removes the listener, and no MaxListeners warning is emitted;
  • a subscriber that throws doesn't starve the others.

All four cases fail on main. The goal-loop and dictation renderer suites pass (18), and npx tsc -b is clean.

🤖 Generated with Claude Code

Juliusolsson05 and others added 3 commits September 19, 2026 01:23
#1015. Against a real EventEmitter as ipcRenderer, 12 goal-loop:changed
or dictation:stream-transcript subscribers must create one listener,
deliver every event to all of them, remove the listener with the last
unsubscribe, and emit no MaxListeners warning. A subscriber that throws
must not starve the others.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
#1015. Every mounted GoalLoopPane and composer subscribed its own
ipcRenderer listener, so eleven panes crossed Node's MaxListeners of 10
and the dev app warned "Possible EventEmitter memory leak" at every
startup. It was not a leak, but a permanent false alarm hides the next
real one, and raising the limit would silence that alarm.

subscribeShared keeps one relay listener per channel, fanned out to a set
of subscribers. The relay is removed with the last subscriber, and each
subscriber is isolated from another's throw. goal-loop:changed and
dictation:stream-transcript use it. LSP diagnostics keeps its permanent
listener on purpose (see lsp.ts: code blocks mount and unmount while
scrolling).

Closes #1015

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… check real

From the #1039 review (CHANGES REQUESTED):
- MEDIUM: record-session:started and record-session:stopping are also
  subscribed per rendered pane (every Feed mounts a
  RenderShapeCaptureProvider), so 11 panes still warned at startup.
  tldr:changed and goal:changed do the same while a peek is up. All four
  use subscribeShared, and the test covers all six channels.
- LOW: Node emits MaxListenersExceededWarning on process.nextTick, so the
  test's warning spy must outlive a tick. Otherwise the check could never
  fail.
- NIT: the comment now says why LSP diagnostics keeps its own permanent
  listener.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@Juliusolsson05

Copy link
Copy Markdown
Owner Author

Review (CHANGES REQUESTED) addressed in ac0878c6:

  • MEDIUM: record-session:started, record-session:stopping, tldr:changed and goal:changed use subscribeShared. The shared-listener test now covers all six channels, and each fails on main with 12 listeners.
  • LOW: the warning spy now waits a tick (Node emits the warning on process.nextTick), so the check can fail.
  • NIT: the comment explains why LSP keeps its own permanent listener.

preload: 15/15, and tsc is clean.

@Juliusolsson05
Juliusolsson05 merged commit fe2b3bf into main Sep 19, 2026
2 checks passed
@Juliusolsson05
Juliusolsson05 deleted the fix/shared-ipc-listeners branch September 19, 2026 09:22
Juliusolsson05 added a commit that referenced this pull request Sep 19, 2026
…1047

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(renderer): goal-loop and dictation ipcRenderer listeners leak past MaxListeners threshold

1 participant